FIX: Calculation of text length for <details> in excerpt was wrong

lib/excerpt_parser.rb

@@ -49,7 +49,8 @@ class ExcerptParser < Nokogiri::XML::SAX::Document
   end
 
   def include_tag(name, attributes)
-    characters("<#{name} #{attributes.map { |k, v| "#{k}=\"#{escape_attribute(v)}\"" }.join(' ')}>", false, false, false)
+    characters("<#{name} #{attributes.map { |k, v| "#{k}=\"#{escape_attribute(v)}\"" }.join(' ')}>",
+               truncate: false, count_it: false, encode: false)
   end
 
   def start_element(name, attributes = [])
@@ -130,12 +131,12 @@ class ExcerptParser < Nokogiri::XML::SAX::Document
     case name
     when "a"
       unless @strip_links
-        characters("</a>", false, false, false)
+        characters("</a>", truncate: false, count_it: false, encode: false)
         @in_a = false
       end
     when "p", "br"
       if @keep_newlines
-        characters("<br>", false, false, false)
+        characters("<br>", truncate: false, count_it: false, encode: false)
       else
         characters(" ")
       end
@@ -144,18 +145,31 @@ class ExcerptParser < Nokogiri::XML::SAX::Document
     when "details"
       @in_details_depth -= 1
       if @in_details_depth == 0
-        full = "<details><summary>#{clean(@summary_contents)}</summary>#{clean(@detail_contents)}</details>"
-        if @current_length + full.length > @length
-          @excerpt << "<details class='disabled'><summary>#{@summary_contents[0..@length]}</summary></details>"
+        @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
-          @excerpt << full
+          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"
       throw :done if @start_excerpt
-      characters("</span>", false, false, false) if @in_spoiler
+      characters("</span>", truncate: false, count_it: false, encode: false) if @in_spoiler
       @in_spoiler = false
     end
   end
@@ -164,7 +178,7 @@ class ExcerptParser < Nokogiri::XML::SAX::Document
     ERB::Util.html_escape(str.strip)
   end
 
-  def characters(string, truncate = true, count_it = true, encode = true)
+  def characters(string, truncate: true, count_it: true, encode: true, before_string: nil, after_string: nil)
     return if @in_quote
 
     # we call length on this so might as well ensure we have a string
@@ -178,15 +192,20 @@ class ExcerptParser < Nokogiri::XML::SAX::Document
       return
     end
 
+    @excerpt << before_string if before_string
+
     encode = encode ? lambda { |s| ERB::Util.html_escape(s) } : lambda { |s| s }
     if count_it && @current_length + string.length > @length
       length = [0, @length - @current_length - 1].max
       @excerpt << encode.call(string[0..length]) if truncate
       @excerpt << (@text_entities ? "..." : "&hellip;")
       @excerpt << "</a>" if @in_a
+      @excerpt << after_string if after_string
       throw :done
     end
+
     @excerpt << encode.call(string)
+    @excerpt << after_string if after_string
     @current_length += string.length if count_it
   end
 end

spec/components/excerpt_parser_spec.rb

@@ -4,17 +4,32 @@ require "excerpt_parser"
 describe ExcerptParser do
 
   it "handles nested <details> blocks" do
-    html = <<~HTML
+    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. 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>
       </details>
     HTML
 
-    expect(ExcerptParser.get_excerpt(html, 200, strip_links: true)).to eq(%{<details class='disabled'><summary>FOO</summary></details>})
+    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>')
+  end
 end

spec/components/pretty_text_spec.rb

@@ -389,7 +389,7 @@ describe PrettyText do
       end
 
       it "should keep details if too long" do
-        expect(PrettyText.excerpt("<details><summary>expand</summary><p>hello</p></details>", 30)).to match_html "<details class='disabled'><summary>expand</summary></details>"
+        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