FIX: proper details / summary excerpt (#30229)

It doesn't make much sense to have the content of a `<details>` in an excerpt so I replaced them with "▶ summary" instead.

That way, they can't be (ab)used in user cards for example.

Reference - https://meta.discourse.org/t/335094
This commit is contained in:
Régis Hanol 2024-12-12 09:09:49 +01:00 committed by GitHub
parent 7c00c52c36
commit 44cabc3569
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 13 additions and 78 deletions

View File

@ -24,8 +24,6 @@ class ExcerptParser < Nokogiri::XML::SAX::Document
@start_excerpt = false
@start_hashtag_icon = false
@in_details_depth = 0
@summary_contents = +""
@detail_contents = +""
end
def self.get_excerpt(html, length, options)
@ -127,12 +125,11 @@ class ExcerptParser < Nokogiri::XML::SAX::Document
include_tag(name, attributes)
end
when "details"
@detail_contents = +"" if @in_details_depth == 0
@in_details_depth += 1
when "summary"
if @in_details_depth == 1 && !@in_summary
@summary_contents = +""
@in_summary = true
characters("", truncate: false, count_it: false, encode: false)
end
when "svg"
attributes = Hash[*attributes.flatten]
@ -162,29 +159,6 @@ class ExcerptParser < Nokogiri::XML::SAX::Document
@in_quote = false
when "details"
@in_details_depth -= 1
if @in_details_depth == 0
@summary_contents = clean(@summary_contents)
@detail_contents = clean(@detail_contents)
if @current_length + @summary_contents.length >= @length
characters(
@summary_contents,
encode: false,
before_string: "<details class='disabled'><summary>",
after_string: "</summary></details>",
)
else
characters(
@summary_contents,
truncate: false,
encode: false,
before_string: "<details><summary>",
after_string: "</summary>",
)
characters(@detail_contents, encode: false, after_string: "</details>")
end
end
when "summary"
@in_summary = false if @in_details_depth == 1
when "div", "span"
@ -210,18 +184,10 @@ class ExcerptParser < Nokogiri::XML::SAX::Document
before_string: nil,
after_string: nil
)
return if @in_quote
return if @in_quote || @in_details_depth > 1 || (@in_details_depth == 1 && !@in_summary)
# we call length on this so might as well ensure we have a string
string = string.to_s
if @in_details_depth > 0
if @in_summary
@summary_contents << string
else
@detail_contents << string
end
return
end
@excerpt << before_string if before_string

View File

@ -6,43 +6,18 @@ RSpec.describe ExcerptParser do
it "handles nested <details> blocks" do
html = <<~HTML.strip
<details>
<summary>
FOO</summary>
<details>
<summary>
BAR</summary>
<p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Fusce ultrices, ex bibendum vestibulum vestibulum, mi velit pulvinar risus, sed consequat eros libero in eros. Fusce luctus mattis mauris, vitae semper lorem sodales quis. Donec pellentesque lacus ac ante aliquam, tincidunt iaculis risus interdum. In ullamcorper cursus massa ut lacinia. Donec quis diam finibus, rutrum odio eu, maximus leo. Nulla facilisi. Nullam suscipit quam et bibendum sagittis. Praesent sollicitudin neque at luctus ornare. Maecenas tristique dapibus risus, ac dictum ipsum gravida aliquam. Phasellus vehicula eu arcu sed imperdiet. Vestibulum ornare eros a nisi faucibus vehicula. Quisque congue placerat nulla, nec finibus nulla ultrices vitae. Quisque ac mi sem. Curabitur eu porttitor justo. Etiam dignissim in orci iaculis congue. Donec tempus cursus orci, a placerat elit varius nec.</p>
</details>
<summary>FOO</summary>
<details>
<summary>BAR</summary>
<p>Lorem ipsum dolor sit amet, consectetur adipiscing elit.</p>
</details>
</details>
HTML
expect(ExcerptParser.get_excerpt(html, 50, {})).to match_html <<~HTML
<details><summary>FOO</summary>BAR
Lorem ipsum dolor sit amet, consectetur adi&hellip;</details>
HTML
expect(ExcerptParser.get_excerpt(html, 6, {})).to match_html(
"<details><summary>FOO</summary>BAR&hellip;</details>",
)
expect(ExcerptParser.get_excerpt(html, 3, {})).to match_html(
'<details class="disabled"><summary>FOO</summary></details>',
)
end
it "respects length parameter for <details> block" do
html = "<details><summary>foo</summary><p>bar</p></details>"
expect(ExcerptParser.get_excerpt(html, 100, {})).to match_html(
"<details><summary>foo</summary>bar</details>",
)
expect(ExcerptParser.get_excerpt(html, 5, {})).to match_html(
"<details><summary>foo</summary>ba&hellip;</details>",
)
expect(ExcerptParser.get_excerpt(html, 3, {})).to match_html(
'<details class="disabled"><summary>foo</summary></details>',
)
expect(ExcerptParser.get_excerpt(html, 2, {})).to match_html(
'<details class="disabled"><summary>fo&hellip;</summary></details>',
)
expect(ExcerptParser.get_excerpt(html, 50, {})).to match_html "▶ FOO"
expect(ExcerptParser.get_excerpt(html, 6, {})).to match_html "▶ FOO"
expect(ExcerptParser.get_excerpt(html, 3, {})).to match_html "▶ FOO"
expect(ExcerptParser.get_excerpt(html, 2, {})).to match_html "▶ FO&hellip;"
end
it "allows <svg> with <use> inside for icons when keep_svg is true" do

View File

@ -923,16 +923,10 @@ RSpec.describe PrettyText do
).to eq("![car](http://cnn.com/a.gif)")
end
it "should keep details if too long" do
it "replaces details / summary with the summary" do
expect(
PrettyText.excerpt("<details><summary>expand</summary><p>hello</p></details>", 6),
).to match_html "<details class='disabled'><summary>expand</summary></details>"
end
it "doesn't disable details if short enough" do
expect(
PrettyText.excerpt("<details><summary>expand</summary><p>hello</p></details>", 60),
).to match_html "<details><summary>expand</summary>hello</details>"
).to match_html "▶ expand"
end
it "should remove meta information" do