diff --git a/lib/banzai/filter/markdown_engines/glfm_markdown.rb b/lib/banzai/filter/markdown_engines/glfm_markdown.rb index c382298844f7cf7d6ff3a782c68525281ef6d935..cc98f99ecf4d475d9c3b2780374ac088b6fbc17e 100644 --- a/lib/banzai/filter/markdown_engines/glfm_markdown.rb +++ b/lib/banzai/filter/markdown_engines/glfm_markdown.rb @@ -19,17 +19,20 @@ class GlfmMarkdown < Base github_pre_lang: true, hardbreaks: false, header_ids: Banzai::Renderer::USER_CONTENT_ID_PREFIX, + inapplicable_tasks: true, math_code: true, math_dollars: true, multiline_block_quotes: true, placeholder_detection: true, relaxed_autolinks: true, + relaxed_tasklist_character: true, sourcepos: true, smart: false, strikethrough: true, table: true, tagfilter: false, - tasklist: false, # still handled by a banzai filter/gem, + tasklist: true, + tasklist_classes: true, wikilinks_title_before_pipe: true, unsafe: true }.freeze diff --git a/lib/banzai/filter/sanitization_filter.rb b/lib/banzai/filter/sanitization_filter.rb index 3f681d12ac0ea72852da3868e382168b37aeb741..1bbc010181fd32b2de7cea4dccabf648afd18241 100644 --- a/lib/banzai/filter/sanitization_filter.rb +++ b/lib/banzai/filter/sanitization_filter.rb @@ -36,12 +36,21 @@ def customize_allowlist(allowlist) allowlist[:attributes]['li'] = %w[id] allowlist[:transformers].push(self.class.remove_id_attributes) + additional_allows(allowlist) + + allowlist + end + + def additional_allows(allowlist) # Remove any `class` property not required for these elements allowlist[:attributes]['a'].push('class') allowlist[:attributes]['div'] = %w[class] allowlist[:attributes]['p'] = %w[class] allowlist[:attributes]['span'].push('class') allowlist[:attributes]['code'].push('class') + allowlist[:attributes]['ul'] = %w[class] + allowlist[:attributes]['ol'] = %w[class] + allowlist[:attributes]['li'].push('class') allowlist[:transformers].push(self.class.remove_unsafe_classes) # Allow section elements with data-footnotes attribute @@ -49,7 +58,9 @@ def customize_allowlist(allowlist) allowlist[:attributes]['section'] = %w[data-footnotes] allowlist[:attributes]['a'].push('data-footnote-ref', 'data-footnote-backref', 'data-footnote-backref-idx') - allowlist + # Allow input for task list checkboxes + allowlist[:elements].push('input') + allowlist[:attributes]['input'] = %w[class type data-inapplicable] end class << self @@ -68,6 +79,7 @@ def remove_unsafe_table_style end end + # rubocop:disable Metrics/CyclomaticComplexity -- prefer using `case` for clarity and readability def remove_unsafe_classes ->(env) do node = env[:node] @@ -85,9 +97,16 @@ def remove_unsafe_classes node.remove_attribute('class') if remove_span_class?(node) when 'code' node.remove_attribute('class') if remove_code_class?(node) + when 'ul', 'ol' + node.remove_attribute('class') if remove_list_class?(node) + when 'li' + node.remove_attribute('class') if remove_list_item_class?(node) + when 'input' + node.remove_attribute('class') if remove_input_class?(node) end end end + # rubocop:enable Metrics/CyclomaticComplexity def remove_link_class?(node) node['class'] != 'anchor' @@ -115,6 +134,20 @@ def remove_code_class?(node) node['class'] != 'idiff' end + def remove_list_class?(node) + node['class'] != 'task-list' + end + + def remove_list_item_class?(node) + node['class'] != 'task-list-item' && + node['class'] != 'inapplicable task-list-item' && + node['class'] != 'task-list-item inapplicable' + end + + def remove_input_class?(node) + node['type'] != 'checkbox' || node['class'] != 'task-list-item-checkbox' + end + def remove_id_attributes ->(env) do node = env[:node] diff --git a/lib/banzai/filter/task_list_filter.rb b/lib/banzai/filter/task_list_filter.rb index f2fa74fa9fe99d5a811d23dc625a37443d241d20..89505b3f1258d826a54d610f13161b543745e7fc 100644 --- a/lib/banzai/filter/task_list_filter.rb +++ b/lib/banzai/filter/task_list_filter.rb @@ -1,105 +1,47 @@ # frozen_string_literal: true -require 'task_list/filter' - -# Generated HTML is transformed back to GFM by: -# - app/assets/javascripts/behaviors/markdown/nodes/ordered_task_list.js -# - app/assets/javascripts/behaviors/markdown/nodes/task_list.js -# - app/assets/javascripts/behaviors/markdown/nodes/task_list_item.js module Banzai module Filter - # TaskList filter replaces task list item markers (`[ ]`, `[x]`, and `[~]`) - # with checkboxes, marked up with metadata and behavior. - # - # This should be run on the HTML generated by the Markdown filter, after the - # SanitizationFilter. - # - # Syntax - # ------ - # - # Task list items must be in a list format: - # - # ``` - # - [ ] incomplete - # - [x] complete - # - [~] inapplicable - # ``` - # - # This class overrides TaskList::Filter in the `deckar01-task_list` gem - # to add support for inapplicable task items - class TaskListFilter < TaskList::Filter + class TaskListFilter < HTML::Pipeline::Filter prepend Concerns::PipelineTimingCheck - extend ::Gitlab::Utils::Override - XPATH = 'descendant-or-self::li[input[@data-inapplicable]] | descendant-or-self::li[p[input[@data-inapplicable]]]' - INAPPLICABLE = '[~]' - INAPPLICABLEPATTERN = /\[~\]/ + CSS = 'li.task-list-item' + XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze - # Pattern used to identify all task list items. - # Useful when you need iterate over all items. - NEWITEMPATTERN = / - \A - (?:\s*[-+*]|(?:\d+\.))? # optional list prefix - \s* # optional whitespace prefix - ( # checkbox - #{CompletePattern}| - #{IncompletePattern}| - #{INAPPLICABLEPATTERN} - ) - (?=\s) # followed by whitespace - /x + def call + doc.xpath(XPATH).each do |node| + # node is the `li` node, and the first child will be the `input` + stripped_source = node.children[0..(node.children.count - 1)].to_html + text = stripped_source.partition(/<(ol|ul)/) + source = ActionView::Base.full_sanitizer.sanitize(text[0]) + truncated_source = source.strip.truncate(100, separator: ' ', omission: '…') + aria_label = format(_('Check option: %{option}'), option: truncated_source) - # Force the gem's constant to use our new one - superclass.send(:remove_const, :ItemPattern) # rubocop: disable GitlabSecurity/PublicSend - superclass.const_set(:ItemPattern, NEWITEMPATTERN) + input_node = node.children.first + next unless input_node.name == 'input' - def inapplicable?(item) - !!(item.checkbox_text =~ INAPPLICABLEPATTERN) - end + input_node['aria-label'] = CGI.escapeHTML(aria_label) + node.prepend_child('') - override :render_item_checkbox - def render_item_checkbox(item) - stripped_source = item.source.sub(ItemPattern, '').strip - text = stripped_source.partition(/\<(ol|ul)/) - source = ActionView::Base.full_sanitizer.sanitize(text[0]) - truncated_source = source.truncate(100, separator: ' ', omission: '…') - aria_label = format(_('Check option: %{option}'), option: truncated_source) + next unless input_node['data-inapplicable'] - %() - end + start_node = node.children[1] - override :render_task_list_item - def render_task_list_item(item) - source = item.source + next unless start_node - if inapplicable?(item) - # Add a `` tag around the list item text. However because of the - # way tasks are built, the source can include an embedded sublist, like - # `[~] foobar\n
    ` should only be added to the main text. - source = source.partition("#{INAPPLICABLE} ") - text = source.last.partition(/\<(ol|ul)/) - text[0] = "#{text[0]}" - source[-1] = text.join - source = source.join - end + wrapper = Nokogiri::XML::Node.new('s', doc) + current = start_node.next_sibling - Nokogiri::HTML.fragment \ - source.sub(ItemPattern, render_item_checkbox(item)), 'utf-8' - end + start_node.add_next_sibling(wrapper) - override :call - def call - super + while current + next_node = current.next_sibling + + break if current.name == 'ol' || current.name == 'ul' - # add class to li for any inapplicable checkboxes - doc.xpath(XPATH).each do |li| - li.add_class('inapplicable') + wrapper.add_child(current) + current = next_node + end end doc diff --git a/spec/lib/banzai/filter/markdown_engines/glfm_markdown_spec.rb b/spec/lib/banzai/filter/markdown_engines/glfm_markdown_spec.rb index dfd1f92fecde462fc794365e11903def5b37b504..01115c32a0897d60f32a8312915c0f970f666f1a 100644 --- a/spec/lib/banzai/filter/markdown_engines/glfm_markdown_spec.rb +++ b/spec/lib/banzai/filter/markdown_engines/glfm_markdown_spec.rb @@ -12,6 +12,25 @@ expect(engine.render('# hi')).to eq expected end + it 'defaults to supporting task lists and inapplicable task items' do + engine = described_class.new({ no_sourcepos: true }) + markdown = <<~MARKDOWN + - [ ] one + - [x] two + - [~] three + MARKDOWN + + expected = <<~TEXT +
      +
    • one
    • +
    • two
    • +
    • three
    • +
    + TEXT + + expect(engine.render(markdown)).to eq expected + end + it 'turns off sourcepos' do engine = described_class.new({ no_sourcepos: true }) expected = <<~TEXT diff --git a/spec/lib/banzai/filter/sanitization_filter_spec.rb b/spec/lib/banzai/filter/sanitization_filter_spec.rb index aaa804079b2bde9ad6daee8516b059f920f41702..cfd354491ce284886b455d4f0686d94da55520f6 100644 --- a/spec/lib/banzai/filter/sanitization_filter_spec.rb +++ b/spec/lib/banzai/filter/sanitization_filter_spec.rb @@ -245,6 +245,25 @@ end end + describe 'tasklists' do + it 'allows class property on task list items' do + exp = <<~HTML +
      +
    • + +
    • +
    • + +
    • +
    + HTML + + act = filter(exp) + + expect(act.to_html).to eq exp + end + end + describe 'remove_unsafe_classes' do # rubocop:disable Layout/LineLength -- keep table rows on one line where(:html, :sanitized) do diff --git a/spec/lib/banzai/filter/task_list_filter_spec.rb b/spec/lib/banzai/filter/task_list_filter_spec.rb index 3e16c92046b11b84e6c88eb3f6f75b69f081c2ab..37053081542a8d29266e09e66e4b8c02eb0a747e 100644 --- a/spec/lib/banzai/filter/task_list_filter_spec.rb +++ b/spec/lib/banzai/filter/task_list_filter_spec.rb @@ -5,14 +5,51 @@ RSpec.describe Banzai::Filter::TaskListFilter, feature_category: :markdown do include FilterSpecHelper + def run_pipeline(text, context = { project: nil }) + stub_commonmark_sourcepos_disabled + + Nokogiri::HTML.parse(Banzai.render(text, context)) + end + it 'adds `` to every list item' do - doc = filter("
      \n
    • [ ] testing item 1
    • \n
    • [x] testing item 2
    • \n
    ") + markdown = <<~MARKDOWN + - [ ] testing item 1 + - [x] testing item 2 + - [~] testing item 3 + MARKDOWN + + doc = run_pipeline(markdown) + + expect(doc.xpath('.//li//task-button').count).to eq(3) + end + + it 'does not sanitize out the classes' do + markdown = <<~MARKDOWN + - [ ] testing item 1 + - [~] testing item 2 + MARKDOWN + + doc = run_pipeline(markdown) - expect(doc.xpath('.//li//task-button').count).to eq(2) + expect(doc.css('ul.task-list li.task-list-item input.task-list-item-checkbox').count).to eq(2) + expect(doc.css('ul.task-list li.task-list-item.inapplicable input.task-list-item-checkbox').count).to eq(1) end it 'adds `aria-label` to every checkbox in the list' do - doc = filter("
      \n
    • [ ] testing item 1
    • \n
    • [x] testing item 2
    • \n
    • [~] testing item 3
    • \n
    • [~] testing item 4 this is a very long label that should be truncated at some point but where does it truncate?
    • \n
    • [ ]
      suspicious item
    • \n
    • [ ]
      suspicious item 2
    • \n
    • [~]
    • \n
    • [~]
    • \n
    • [~]
    • \n
    • [~] " hijacking quotes \" a \' b ' c
    ") + markdown = <<~MARKDOWN + - [ ] testing item 1 + - [x] testing item 2 + - [~] testing item 3 + - [~] testing item 4 this is a very long label that should be truncated at some point but where does it truncate? + - [ ]
    suspicious item
    + - [ ]
    suspicious item 2
    + - [~] + - [~] + - [~] + - [~] " hijacking quotes \" a \' b ' c + MARKDOWN + + doc = run_pipeline(markdown) aria_labels = doc.xpath('.//li//input/@aria-label') @@ -26,55 +63,54 @@ expect(aria_labels[6].value).to eq('Check option: ') expect(aria_labels[7].value).to eq('Check option: ') expect(aria_labels[8].value).to eq('Check option: ') - expect(aria_labels[9].value).to eq("Check option: \" hijacking quotes \" a ' b ' c") + expect(aria_labels[9].value).to eq("Check option: " hijacking quotes " a ' b ' c") end it 'ignores checkbox on following line' do - doc = filter( - <<~HTML -
      -
    • one -
        -
      • foo - [ ] bar
      • -
      -
    • -
    - HTML - ) + markdown = <<~MARKDOWN + - one + - foo + [ ] bar + MARKDOWN + + doc = run_pipeline(markdown) expect(doc.xpath('.//li//input').count).to eq(0) end describe 'inapplicable list items' do - shared_examples 'a valid inapplicable task list item' do |html| - it "behaves correctly for `#{html}`" do - doc = filter("
    • #{html}
    ") + shared_examples 'a valid inapplicable task list item' do |markdown| + it "behaves correctly for `#{markdown}`" do + doc = run_pipeline(markdown) expect(doc.css('li.inapplicable input[data-inapplicable]').count).to eq(1) expect(doc.css('li.inapplicable > s').count).to eq(1) end end - shared_examples 'an invalid inapplicable task list item' do |html| - it "does nothing for `#{html}`" do - doc = filter("
    • #{html}
    ") + shared_examples 'an invalid inapplicable task list item' do |markdown| + it "does nothing for `#{markdown}`" do + doc = run_pipeline(markdown) expect(doc.css('li.inapplicable input[data-inapplicable]').count).to eq(0) end end - it_behaves_like 'a valid inapplicable task list item', '[~] foobar' - it_behaves_like 'a valid inapplicable task list item', '[~] foo bar' - it_behaves_like 'an invalid inapplicable task list item', '[ ] foobar' - it_behaves_like 'an invalid inapplicable task list item', '[x] foobar' - it_behaves_like 'an invalid inapplicable task list item', 'foo [~] bar' + it_behaves_like 'a valid inapplicable task list item', '- [~] foobar' + it_behaves_like 'a valid inapplicable task list item', '- [~] foo bar' + it_behaves_like 'an invalid inapplicable task list item', '- [ ] foobar' + it_behaves_like 'an invalid inapplicable task list item', '- [x] foobar' + it_behaves_like 'an invalid inapplicable task list item', '- foo [~] bar' it 'does not wrap a sublist with ' do - html = '[~] foo bar\n
    1. sublist
    ' - doc = filter("
    • #{html}
    ") - - expect(doc.to_html).to include('foo bar\n') + markdown = <<~MARKDOWN + - [~] foo bar + 1. sublist + MARKDOWN + doc = run_pipeline(markdown) + # doc = filter("
    • #{html}
    ") + + expect(doc.to_html).to include(" foo bar\n") expect(doc.css('li.inapplicable input[data-inapplicable]').count).to eq(1) expect(doc.css('li.inapplicable > s').count).to eq(1) end