Skip to content

Commit

Permalink
FIX: proper details / summary excerpt (discourse#30229)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ZogStriP authored Dec 12, 2024
1 parent 7c00c52 commit 44cabc3
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 78 deletions.
38 changes: 2 additions & 36 deletions lib/excerpt_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ def initialize(length, options = nil)
@start_excerpt = false
@start_hashtag_icon = false
@in_details_depth = 0
@summary_contents = +""
@detail_contents = +""
end

def self.get_excerpt(html, length, options)
Expand Down Expand Up @@ -127,12 +125,11 @@ def start_element(name, attributes = [])
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]
Expand Down Expand Up @@ -162,29 +159,6 @@ def end_element(name)
@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"
Expand All @@ -210,18 +184,10 @@ def characters(
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

Expand Down
43 changes: 9 additions & 34 deletions spec/lib/excerpt_parser_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,43 +6,18 @@
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
Expand Down
10 changes: 2 additions & 8 deletions spec/lib/pretty_text_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -923,16 +923,10 @@ def cook(*args)
).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
Expand Down

0 comments on commit 44cabc3

Please sign in to comment.