Skip to content

G
gitlab-ce
Commit 8614e0a9 authored by Brett Walker's avatar Brett Walker
Browse files
Options

Rework markdown footnote processing 

to fix parsing error

Changelog: changed
parent 6361a40d
Hide whitespace changes
Inline Side-by-side
Showing with 66 additions and 32 deletions
lib/banzai/filter/footnote_filter.rb
View file @ 8614e0a9
......@@ -21,9 +21,9 @@ module Banzai
FOOTNOTE_LI_REFERENCE_PATTERN = /\A#{FOOTNOTE_ID_PREFIX}.+\z/.freeze
FOOTNOTE_LINK_REFERENCE_PATTERN = /\A#{FOOTNOTE_LINK_ID_PREFIX}.+\z/.freeze
CSS_SECTION = "ol > li a[href^=\"\##{FOOTNOTE_LINK_ID_PREFIX}\"]"
CSS_SECTION = "section[data-footnotes]"
XPATH_SECTION = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_SECTION).freeze
CSS_FOOTNOTE = 'sup > a[id]'
CSS_FOOTNOTE = 'sup > a[data-footnote-ref]'
XPATH_FOOTNOTE = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_FOOTNOTE).freeze
# only needed when feature flag use_cmark_renderer is turned off
......@@ -37,20 +37,28 @@ module Banzai
XPATH_SECTION_OLD = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_SECTION_OLD).freeze
def call
xpath_section = Feature.enabled?(:use_cmark_renderer) ? XPATH_SECTION : XPATH_SECTION_OLD
return doc unless first_footnote = doc.at_xpath(xpath_section)
# Sanitization stripped off the section wrapper - add it back in
if Feature.enabled?(:use_cmark_renderer)
first_footnote.parent.parent.parent.wrap('<section class="footnotes" data-footnotes>')
# Sanitization stripped off the section class - add it back in
return doc unless section_node = doc.at_xpath(XPATH_SECTION)
section_node.append_class('footnotes')
else
return doc unless first_footnote = doc.at_xpath(XPATH_SECTION_OLD)
return doc unless first_footnote.parent
first_footnote.parent.wrap('<section class="footnotes">')
end
rand_suffix = "-#{random_number}"
modified_footnotes = {}
doc.xpath(XPATH_FOOTNOTE).each do |link_node|
xpath_footnote = if Feature.enabled?(:use_cmark_renderer)
XPATH_FOOTNOTE
else
Gitlab::Utils::Nokogiri.css_to_xpath('sup > a[id]')
end
doc.xpath(xpath_footnote).each do |link_node|
if Feature.enabled?(:use_cmark_renderer)
ref_num = link_node[:id].delete_prefix(FOOTNOTE_LINK_ID_PREFIX)
ref_num.gsub!(/[[:punct:]]/, '\\\\\&')
......@@ -58,7 +66,8 @@ module Banzai
ref_num = link_node[:id].delete_prefix(FOOTNOTE_LINK_ID_PREFIX_OLD)
end
node_xpath = Gitlab::Utils::Nokogiri.css_to_xpath("li[id=#{fn_id(ref_num)}]")
css = Feature.enabled?(:use_cmark_renderer) ? "section[data-footnotes] li[id=#{fn_id(ref_num)}]" : "li[id=#{fn_id(ref_num)}]"
node_xpath = Gitlab::Utils::Nokogiri.css_to_xpath(css)
footnote_node = doc.at_xpath(node_xpath)
if footnote_node || modified_footnotes[ref_num]
......@@ -69,7 +78,6 @@ module Banzai
# Sanitization stripped off class - add it back in
link_node.parent.append_class('footnote-ref')
link_node['data-footnote-ref'] = nil if Feature.enabled?(:use_cmark_renderer)
unless modified_footnotes[ref_num]
footnote_node[:id] += rand_suffix
......@@ -78,7 +86,6 @@ module Banzai
if backref_node
backref_node[:href] += rand_suffix
backref_node.append_class('footnote-backref')
backref_node['data-footnote-backref'] = nil if Feature.enabled?(:use_cmark_renderer)
end
modified_footnotes[ref_num] = true
......
lib/banzai/filter/sanitization_filter.rb
View file @ 8614e0a9
......@@ -28,6 +28,13 @@ module Banzai
allowlist[:attributes]['li'] = %w[id]
allowlist[:transformers].push(self.class.remove_non_footnote_ids)
if Feature.enabled?(:use_cmark_renderer)
# Allow section elements with data-footnotes attribute
allowlist[:elements].push('section')
allowlist[:attributes]['section'] = %w(data-footnotes)
allowlist[:attributes]['a'].push('data-footnote-ref', 'data-footnote-backref')
end
allowlist
end
......
spec/lib/banzai/filter/footnote_filter_spec.rb
View file @ 8614e0a9
......@@ -4,6 +4,7 @@ require 'spec_helper'
RSpec.describe Banzai::Filter::FootnoteFilter do
include FilterSpecHelper
using RSpec::Parameterized::TableSyntax
# rubocop:disable Style/AsciiComments
# first[^1] and second[^second] and third[^_😄_]
......@@ -13,16 +14,16 @@ RSpec.describe Banzai::Filter::FootnoteFilter do
# rubocop:enable Style/AsciiComments
let(:footnote) do
<<~EOF.strip_heredoc
<p>first<sup><a href="#fn-1" id="fnref-1">1</a></sup> and second<sup><a href="#fn-second" id="fnref-second">2</a></sup> and third<sup><a href="#fn-_%F0%9F%98%84_" id="fnref-_%F0%9F%98%84_">3</a></sup></p>
<p>first<sup><a href="#fn-1" id="fnref-1" data-footnote-ref>1</a></sup> and second<sup><a href="#fn-second" id="fnref-second" data-footnote-ref>2</a></sup> and third<sup><a href="#fn-_%F0%9F%98%84_" id="fnref-_%F0%9F%98%84_" data-footnote-ref>3</a></sup></p>
<section data-footnotes>
<ol>
<li id="fn-1">
<p>one <a href="#fnref-1" aria-label="Back to content">↩</a></p>
<p>one <a href="#fnref-1" aria-label="Back to content" data-footnote-backref>↩</a></p>
</li>
<li id="fn-second">
<p>two <a href="#fnref-second" aria-label="Back to content">↩</a></p>
<p>two <a href="#fnref-second" aria-label="Back to content" data-footnote-backref>↩</a></p>
</li>\n<li id="fn-_%F0%9F%98%84_">
<p>three <a href="#fnref-_%F0%9F%98%84_" aria-label="Back to content">↩</a></p>
<p>three <a href="#fnref-_%F0%9F%98%84_" aria-label="Back to content" data-footnote-backref>↩</a></p>
</li>
</ol>
EOF
......@@ -30,19 +31,20 @@ RSpec.describe Banzai::Filter::FootnoteFilter do
let(:filtered_footnote) do
<<~EOF.strip_heredoc
<p>first<sup class="footnote-ref"><a href="#fn-1-#{identifier}" id="fnref-1-#{identifier}" data-footnote-ref="">1</a></sup> and second<sup class="footnote-ref"><a href="#fn-second-#{identifier}" id="fnref-second-#{identifier}" data-footnote-ref="">2</a></sup> and third<sup class="footnote-ref"><a href="#fn-_%F0%9F%98%84_-#{identifier}" id="fnref-_%F0%9F%98%84_-#{identifier}" data-footnote-ref="">3</a></sup></p>
<section class=\"footnotes\" data-footnotes><ol>
<p>first<sup class="footnote-ref"><a href="#fn-1-#{identifier}" id="fnref-1-#{identifier}" data-footnote-ref>1</a></sup> and second<sup class="footnote-ref"><a href="#fn-second-#{identifier}" id="fnref-second-#{identifier}" data-footnote-ref>2</a></sup> and third<sup class="footnote-ref"><a href="#fn-_%F0%9F%98%84_-#{identifier}" id="fnref-_%F0%9F%98%84_-#{identifier}" data-footnote-ref>3</a></sup></p>
<section data-footnotes class=\"footnotes\">
<ol>
<li id="fn-1-#{identifier}">
<p>one <a href="#fnref-1-#{identifier}" aria-label="Back to content" class="footnote-backref" data-footnote-backref="">↩</a></p>
<p>one <a href="#fnref-1-#{identifier}" aria-label="Back to content" data-footnote-backref class="footnote-backref">↩</a></p>
</li>
<li id="fn-second-#{identifier}">
<p>two <a href="#fnref-second-#{identifier}" aria-label="Back to content" class="footnote-backref" data-footnote-backref="">↩</a></p>
<p>two <a href="#fnref-second-#{identifier}" aria-label="Back to content" data-footnote-backref class="footnote-backref">↩</a></p>
</li>
<li id="fn-_%F0%9F%98%84_-#{identifier}">
<p>three <a href="#fnref-_%F0%9F%98%84_-#{identifier}" aria-label="Back to content" class="footnote-backref" data-footnote-backref="">↩</a></p>
<p>three <a href="#fnref-_%F0%9F%98%84_-#{identifier}" aria-label="Back to content" data-footnote-backref class="footnote-backref">↩</a></p>
</li>
</ol></section>
</ol>
</section>
EOF
end
......@@ -52,7 +54,7 @@ RSpec.describe Banzai::Filter::FootnoteFilter do
let(:identifier) { link_node[:id].delete_prefix('fnref-1-') }
it 'properly adds the necessary ids and classes' do
expect(doc.to_html).to eq filtered_footnote
expect(doc.to_html).to eq filtered_footnote.strip
end
context 'using ruby-based HTML renderer' do
......@@ -101,4 +103,21 @@ RSpec.describe Banzai::Filter::FootnoteFilter do
end
end
end
context 'when detecting footnotes' do
where(:valid, :markdown) do
true | "1. one[^1]\n[^1]: AbC"
true | "1. one[^abc]\n[^abc]: AbC"
false | '1. [one](#fnref-abc)'
false | "1. one[^1]\n[^abc]: AbC"
end
with_them do
it 'detects valid footnotes' do
result = Banzai::Pipeline::FullPipeline.call(markdown, project: nil)
expect(result[:output].at_css('section.footnotes').present?).to eq(valid)
end
end
end
end
spec/lib/banzai/pipeline/full_pipeline_spec.rb
View file @ 8614e0a9
......@@ -43,26 +43,27 @@ RSpec.describe Banzai::Pipeline::FullPipeline do
let(:filtered_footnote) do
<<~EOF.strip_heredoc
<p dir="auto">first<sup class="footnote-ref"><a href="#fn-1-#{identifier}" id="fnref-1-#{identifier}" data-footnote-ref="">1</a></sup> and second<sup class="footnote-ref"><a href="#fn-%F0%9F%98%84second-#{identifier}" id="fnref-%F0%9F%98%84second-#{identifier}" data-footnote-ref="">2</a></sup> and twenty<sup class="footnote-ref"><a href="#fn-_twenty-#{identifier}" id="fnref-_twenty-#{identifier}" data-footnote-ref="">3</a></sup></p>
<section class="footnotes" data-footnotes><ol>
<p dir="auto">first<sup class="footnote-ref"><a href="#fn-1-#{identifier}" id="fnref-1-#{identifier}" data-footnote-ref>1</a></sup> and second<sup class="footnote-ref"><a href="#fn-%F0%9F%98%84second-#{identifier}" id="fnref-%F0%9F%98%84second-#{identifier}" data-footnote-ref>2</a></sup> and twenty<sup class="footnote-ref"><a href="#fn-_twenty-#{identifier}" id="fnref-_twenty-#{identifier}" data-footnote-ref>3</a></sup></p>
<section data-footnotes class="footnotes">
<ol>
<li id="fn-1-#{identifier}">
<p>one <a href="#fnref-1-#{identifier}" aria-label="Back to content" class="footnote-backref" data-footnote-backref=""><gl-emoji title="leftwards arrow with hook" data-name="leftwards_arrow_with_hook" data-unicode-version="1.1">↩</gl-emoji></a></p>
<p>one <a href="#fnref-1-#{identifier}" data-footnote-backref aria-label="Back to content" class="footnote-backref"><gl-emoji title="leftwards arrow with hook" data-name="leftwards_arrow_with_hook" data-unicode-version="1.1">↩</gl-emoji></a></p>
</li>
<li id="fn-%F0%9F%98%84second-#{identifier}">
<p>two <a href="#fnref-%F0%9F%98%84second-#{identifier}" aria-label="Back to content" class="footnote-backref" data-footnote-backref=""><gl-emoji title="leftwards arrow with hook" data-name="leftwards_arrow_with_hook" data-unicode-version="1.1">↩</gl-emoji></a></p>
<p>two <a href="#fnref-%F0%9F%98%84second-#{identifier}" data-footnote-backref aria-label="Back to content" class="footnote-backref"><gl-emoji title="leftwards arrow with hook" data-name="leftwards_arrow_with_hook" data-unicode-version="1.1">↩</gl-emoji></a></p>
</li>
<li id="fn-_twenty-#{identifier}">
<p>twenty <a href="#fnref-_twenty-#{identifier}" aria-label="Back to content" class="footnote-backref" data-footnote-backref=""><gl-emoji title="leftwards arrow with hook" data-name="leftwards_arrow_with_hook" data-unicode-version="1.1">↩</gl-emoji></a></p>
<p>twenty <a href="#fnref-_twenty-#{identifier}" data-footnote-backref aria-label="Back to content" class="footnote-backref"><gl-emoji title="leftwards arrow with hook" data-name="leftwards_arrow_with_hook" data-unicode-version="1.1">↩</gl-emoji></a></p>
</li>
</ol></section>
</ol>
</section>
EOF
end
it 'properly adds the necessary ids and classes' do
stub_commonmark_sourcepos_disabled
expect(html.lines.map(&:strip).join("\n")).to eq filtered_footnote
expect(html.lines.map(&:strip).join("\n")).to eq filtered_footnote.strip
end
context 'using ruby-based HTML renderer' do
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7