Allow attachment images to be used within block elements

Image attachments can be embedded inside HTML block level elements to
different degrees of validity eg inside a <p> that's invalid whereas
inside a <td> it's valid.

However the way kramdown handles these scenarios is not desirable in
either instance. It will convert the block level HTML elements into
their HTML entity equivalent - thus an end user would see <div
class="img"> on their screen.

The way I've fixed this isn't pretty, we try output the image in a
single line - which resolves some issues - and then we try convert back
elements we expect to have been converted back to HTML.

lib/govspeak.rb

@@ -215,14 +215,24 @@ def insert_strong_inside_p(body, parser=Govspeak::Document)
       render_image(attachment.url, attachment.title.tr("\n", " "), nil, attachment.id)
     end
 
+    # As of version 1.12.0 of Kramdown the block elements (div & figcaption)
+    # inside this html block will have it's < > converted into HTML Entities
+    # when ever this code is used inside block level elements.
+    #
+    # To resolve this we have a post-processing task that will convert this
+    # back into HTML (I know - it's ugly). The way we could resolve this
+    # without ugliness would be to output only inline elements which rules
+    # out div and figcaption
+    #
+    # This issue is not considered a bug by kramdown: https://github.com/gettalong/kramdown/issues/191
     def render_image(url, alt_text, caption = nil, id = nil)
       id_attr = id ? %{ id="attachment_#{id}"} : ""
       lines = []
       lines << %{<figure#{id_attr} class="image embedded">}
-      lines << %Q{  <div class="img"><img src="#{encode(url)}" alt="#{encode(alt_text)}"></div>}
-      lines << %Q{  <figcaption>#{encode(caption.strip)}</figcaption>} if caption && !caption.strip.empty?
+      lines << %Q{<div class="img"><img src="#{encode(url)}" alt="#{encode(alt_text)}"></div>}
+      lines << %Q{<figcaption>#{caption.strip}</figcaption>} if caption && !caption.strip.empty?
       lines << '</figure>'
-      lines.join "\n"
+      lines.join
     end
 
     wrap_with_div('summary', '$!')

lib/govspeak/post_processor.rb

@@ -38,5 +38,31 @@ def output
         el[:class] = "last-child"
       end
     end
+
+    # This "fix" here is tied into the rendering of images as one of the
+    # pre-processor tasks. As images can be created inside block level elements
+    # it's possible that their block level elements can be HTML entity escaped
+    # to produce "valid" HTML.
+    #
+    # This sucks for us as we spit the user out HTML elements.
+    #
+    # This fix reverses this, and of course, totally sucks because it's tightly
+    # coupled to the `render_image` code and it really isn't cool to undo HTML
+    # entity encoding.
+    extension("fix image attachment escaping") do |document|
+      document.css("figure.image").map do |el|
+        xml = el.children.to_s
+        next unless xml =~ /<div class="img">|<figcaption>/
+        el.children = xml
+          .gsub(
+            %r{<(div class="img")>(.*?)<(/div)>},
+            "<\\1>\\2<\\3>"
+          )
+          .gsub(
+            %r{&lt;(figcaption)&gt;(.*?)&lt;(/figcaption&)gt;},
+            "<\\1>\\2<\\3>"
+          )
+      end
+    end
   end
 end

test/govspeak_attachments_image_test.rb

@@ -68,4 +68,15 @@ def compress_html(html)
     }
     assert_match(compress_html(expected_html_output), compress_html(rendered))
   end
+
+  # test inserted because divs can be stripped inside a table
+  test "can be rendered inside a table" do
+    rendered = render_govspeak(
+      "| [embed:attachments:image:1fe8] |",
+      [build_attachment(content_id: "1fe8", id: nil)]
+    )
+
+    regex = %r{<td><figure class="image embedded"><div class="img">(.*?)</div></figure></td>}
+    assert_match(regex, rendered)
+  end
 end

test/govspeak_test.rb

@@ -634,22 +634,22 @@ class GovspeakTest < Minitest::Test
   test "can reference attached images using !!n" do
     images = [OpenStruct.new(alt_text: 'my alt', url: "http://example.com/image.jpg")]
     given_govspeak "!!1", images do
-      assert_html_output %Q{
-          <figure class="image embedded">
-            <div class="img"><img alt="my alt" src="http://example.com/image.jpg"></div>
-          </figure>
-      }
+      assert_html_output(
+        %{<figure class="image embedded">} +
+        %{<div class="img"><img src="http://example.com/image.jpg" alt="my alt"></div>} +
+        %{</figure>}
+      )
   end
 end
 
 test "alt text of referenced images is escaped" do
   images = [OpenStruct.new(alt_text: %Q{my alt '&"<>}, url: "http://example.com/image.jpg")]
   given_govspeak "!!1", images do
-    assert_html_output %Q{
-        <figure class="image embedded">
-          <div class="img"><img alt="my alt '&amp;&quot;&lt;&gt;" src="http://example.com/image.jpg"></div>
-        </figure>
-        }
+    assert_html_output(
+      %{<figure class="image embedded">} +
+      %{<div class="img"><img src="http://example.com/image.jpg" alt="my alt '&amp;&quot;&lt;&gt;"></div>} +
+      %{</figure>}
+    )
     end
   end
 
@@ -663,23 +663,23 @@ class GovspeakTest < Minitest::Test
   test "adds image caption if given" do
     images = [OpenStruct.new(alt_text: "my alt", url: "http://example.com/image.jpg", caption: 'My Caption & so on')]
     given_govspeak "!!1", images do
-      assert_html_output %Q{
-          <figure class="image embedded">
-            <div class="img"><img alt="my alt" src="http://example.com/image.jpg"></div>
-            <figcaption>My Caption &amp; so on</figcaption>
-          </figure>
-        }
+      assert_html_output(
+        %{<figure class="image embedded">} +
+        %{<div class="img"><img src="http://example.com/image.jpg" alt="my alt"></div>\n} +
+        %{<figcaption>My Caption &amp; so on</figcaption>} +
+        %{</figure>}
+      )
     end
   end
 
   test "ignores a blank caption" do
     images = [OpenStruct.new(alt_text: "my alt", url: "http://example.com/image.jpg", caption: '  ')]
     given_govspeak "!!1", images do
-      assert_html_output %Q{
-          <figure class="image embedded">
-            <div class="img"><img alt="my alt" src="http://example.com/image.jpg"></div>
-          </figure>
-        }
+      assert_html_output(
+        %{<figure class="image embedded">} +
+        %{<div class="img"><img src="http://example.com/image.jpg" alt="my alt"></div>} +
+        %{</figure>}
+      )
     end
   end