Make AttributeProvider work for Image nodes (#31)

Instead of disabling tags in the writer and using the same visitor for
rendering the alt text, just use a separate visitor.

This allows us to get rid of some complicated logic too. The only
downside is that we now have to build up the whole alt text in memory.

Author: robinst

commonmark/src/main/java/org/commonmark/html/HtmlRenderer.java

@@ -304,20 +304,20 @@ public void visit(OrderedList orderedList) {
 
         @Override
         public void visit(Image image) {
-            if (html.isTagAllowed()) {
-                String url = optionallyPercentEncodeUrl(image.getDestination());
-                html.raw("<img src=\"" + escape(url, true) +
-                        "\" alt=\"");
-            }
-            html.disableTags();
-            visitChildren(image);
-            html.enableTags();
-            if (html.isTagAllowed()) {
-                if (image.getTitle() != null) {
-                    html.raw("\" title=\"" + escape(image.getTitle(), true));
-                }
-                html.raw("\" />");
+            String url = optionallyPercentEncodeUrl(image.getDestination());
+
+            AltTextVisitor altTextVisitor = new AltTextVisitor();
+            image.accept(altTextVisitor);
+            String altText = altTextVisitor.getAltText();
+
+            Map<String, String> attrs = new LinkedHashMap<>();
+            attrs.put("src", url);
+            attrs.put("alt", altText);
+            if (image.getTitle() != null) {
+                attrs.put("title", image.getTitle());
             }
+
+            html.tag("img", getAttrs(image, attrs), true);
         }
 
         @Override
@@ -434,4 +434,28 @@ private void setCustomAttributes(Node node, Map<String, String> attrs) {
             }
         }
     }
+
+    private static class AltTextVisitor extends AbstractVisitor {
+
+        private final StringBuilder sb = new StringBuilder();
+
+        String getAltText() {
+            return sb.toString();
+        }
+
+        @Override
+        public void visit(Text text) {
+            sb.append(text.getLiteral());
+        }
+
+        @Override
+        public void visit(SoftLineBreak softLineBreak) {
+            sb.append('\n');
+        }
+
+        @Override
+        public void visit(HardLineBreak hardLineBreak) {
+            sb.append('\n');
+        }
+    }
 }

commonmark/src/main/java/org/commonmark/html/HtmlWriter.java

@@ -5,39 +5,20 @@
 import java.io.IOException;
 import java.util.Collections;
 import java.util.Map;
-import java.util.regex.Pattern;
 
 public class HtmlWriter {
 
     private static final Map<String, String> NO_ATTRIBUTES = Collections.emptyMap();
-    private static final Pattern HTML_TAG_PATTERN = Pattern.compile("<[^>]*>");
 
     private final Appendable buffer;
-    private int nesting = 0;
     private char lastChar = 0;
 
     public HtmlWriter(Appendable out) {
         this.buffer = out;
     }
 
     public void raw(String s) {
-        if (isTagAllowed()) {
-            append(s);
-        } else {
-            append(HTML_TAG_PATTERN.matcher(s).replaceAll(""));
-        }
-    }
-
-    public boolean isTagAllowed() {
-        return nesting == 0;
-    }
-
-    public void disableTags() {
-        nesting++;
-    }
-
-    public void enableTags() {
-        nesting--;
+        append(s);
     }
 
     public void tag(String name) {
@@ -50,10 +31,6 @@ public void tag(String name, Map<String, String> attrs) {
 
     // Helper function to produce an HTML tag.
     public void tag(String name, Map<String, String> attrs, boolean voidElement) {
-        if (!isTagAllowed()) {
-            return;
-        }
-
         append("<");
         append(name);
         if (attrs != null && !attrs.isEmpty()) {

commonmark/src/test/java/org/commonmark/test/HtmlRendererTest.java

@@ -3,6 +3,7 @@
 import org.commonmark.html.AttributeProvider;
 import org.commonmark.html.HtmlRenderer;
 import org.commonmark.node.FencedCodeBlock;
+import org.commonmark.node.Image;
 import org.commonmark.node.Node;
 import org.commonmark.parser.Parser;
 import org.junit.Test;
@@ -77,7 +78,7 @@ public void percentEncodeUrl() {
     }
 
     @Test
-    public void attributeProvider() {
+    public void attributeProviderForCodeBlock() {
         AttributeProvider custom = new AttributeProvider() {
             @Override
             public void setAttributes(Node node, Map<String, String> attributes) {
@@ -99,11 +100,46 @@ public void setAttributes(Node node, Map<String, String> attributes) {
         assertEquals("<pre><code data-custom=\"evil&quot;\">content\n</code></pre>\n", rendered2);
     }
 
+    @Test
+    public void attributeProviderForImage() {
+        AttributeProvider custom = new AttributeProvider() {
+            @Override
+            public void setAttributes(Node node, Map<String, String> attributes) {
+                if (node instanceof Image) {
+                    attributes.remove("alt");
+                    attributes.put("test", "hey");
+                }
+            }
+        };
+
+        HtmlRenderer renderer = HtmlRenderer.builder().attributeProvider(custom).build();
+        String rendered = renderer.render(parse("![foo](/url)\n"));
+        assertEquals("<p><img src=\"/url\" test=\"hey\" /></p>\n", rendered);
+    }
+
     @Test
     public void orderedListStartZero() {
         assertEquals("<ol start=\"0\">\n<li>Test</li>\n</ol>\n", defaultRenderer().render(parse("0. Test\n")));
     }
 
+    @Test
+    public void imageAltTextWithSoftLineBreak() {
+        assertEquals("<p><img src=\"/url\" alt=\"foo\nbar\" /></p>\n",
+                defaultRenderer().render(parse("![foo\nbar](/url)\n")));
+    }
+
+    @Test
+    public void imageAltTextWithHardLineBreak() {
+        assertEquals("<p><img src=\"/url\" alt=\"foo\nbar\" /></p>\n",
+                defaultRenderer().render(parse("![foo  \nbar](/url)\n")));
+    }
+
+    @Test
+    public void imageAltTextWithEntities() {
+        assertEquals("<p><img src=\"/url\" alt=\"foo \u00E4\" /></p>\n",
+                defaultRenderer().render(parse("![foo &auml;](/url)\n")));
+    }
+
     private static HtmlRenderer defaultRenderer() {
         return HtmlRenderer.builder().build();
     }