Fix/correct handling of wovn ignore (#164)

* Fixed comment insertion. * Bump up patch version. * Fixed test: new behavior for `restore()` makes that it always returns a valid HTML. * Re-implemented following WOVN.php's design. * Changed handling of <input>: only value is commented.
WOVNio · Nov 5, 2020 · 819a1fe · 819a1fe
1 parent 5d0116d
commit 819a1fe
Show file tree

Hide file tree

Showing 4 changed files with 78 additions and 46 deletions.
diff --git a/pom.xml b/pom.xml
@@ -7,7 +7,7 @@
     <groupId>com.github.wovnio</groupId>
     <artifactId>wovnjava</artifactId>
     <name>wovnjava</name>
-    <version>1.5.0</version>
+    <version>1.5.1</version>
     <url>https://github.com/WOVNio/wovnjava</url>
 
     <licenses>

diff --git a/src/main/java/com/github/wovnio/wovnjava/HtmlConverter.java b/src/main/java/com/github/wovnio/wovnjava/HtmlConverter.java
@@ -3,23 +3,24 @@
 import java.lang.StringBuilder;
 import java.util.Map;
 import java.util.HashMap;
-import java.util.ArrayList;
+
 import org.jsoup.Jsoup;
-import org.jsoup.nodes.Document;
+import org.jsoup.nodes.Attribute;
 import org.jsoup.nodes.Comment;
+import org.jsoup.nodes.Document;
 import org.jsoup.nodes.Element;
-import org.jsoup.parser.Tag;
 import org.jsoup.select.Elements;
+import org.jsoup.parser.Tag;
 
 class HtmlConverter {
-    private final ArrayList<String> markers = new ArrayList<String>();
-    private final String WOVN_MARKER_PREFIX = "wovn-marker-";
     private final Document doc;
     private final Settings settings;
     private final HashMap<String, String> hreflangMap;
+    private final HtmlReplaceMarker htmlReplaceMarker;
 
     HtmlConverter(Settings settings, Headers headers, String original) {
         this.settings = settings;
+        this.htmlReplaceMarker = new HtmlReplaceMarker();
         this.hreflangMap = headers.getHreflangUrlMap();
         doc = Jsoup.parse(original);
         doc.outputSettings().prettyPrint(false);
@@ -44,20 +45,7 @@ String convert(String lang) {
     }
 
     String restore(String html) {
-        StringBuilder sb = new StringBuilder();
-        String[] list = html.split("<!--" + WOVN_MARKER_PREFIX);
-
-        sb.append(list[0]);
-        for (int i=1; i<list.length; i++) {
-            String fragment = list[i];
-            String commentSuffix = "-->";
-            int suffixOffset = fragment.indexOf(commentSuffix);
-            String indexString = fragment.substring(0, suffixOffset);
-            int index = Integer.parseInt(indexString);
-            sb.append(markers.get(index));
-            sb.append(fragment.substring(suffixOffset + commentSuffix.length()));
-        }
-        return sb.toString();
+        return htmlReplaceMarker.revert(html);
     }
 
     private void removeHrefLangIfConflicts() {
@@ -119,7 +107,12 @@ private void removeForm() {
         for (Element element : elements) {
             String type = element.attr("type");
             if (type != null && type.toLowerCase().equals("hidden")) {
-                replaceNodeToMarkerComment(element);
+                if (element.hasAttr("value")) {
+                    String original = element.attr("value");
+                    String key = htmlReplaceMarker.generateKey();
+                    element.attr("value", key);
+                    htmlReplaceMarker.addValue(key, original);
+                }
             }
         }
     }
@@ -168,9 +161,9 @@ private void appendHrefLang() {
     }
 
     private void replaceNodeToMarkerComment(Element element) {
-        Comment comment = new Comment(WOVN_MARKER_PREFIX + String.valueOf(markers.size()));
-        element.replaceWith(comment);
-        markers.add(restore(element.outerHtml())); // restore original text if element has marker
+        String commentKey = htmlReplaceMarker.addCommentValue(htmlReplaceMarker.revert(element.html()));
+        element.html("");
+        element.appendChild(new Comment(commentKey));
     }
 
     private void replaceContentType() {

diff --git a/src/main/java/com/github/wovnio/wovnjava/HtmlReplaceMarker.java b/src/main/java/com/github/wovnio/wovnjava/HtmlReplaceMarker.java
@@ -0,0 +1,39 @@
+package com.github.wovnio.wovnjava;
+
+import java.util.HashMap;
+
+class HtmlReplaceMarker {
+    private final String WOVN_MARKER_PREFIX = "wovn-marker-";
+    private final HashMap<String, String> mappedValues;
+    private int keyCount;
+
+    HtmlReplaceMarker() {
+        this.mappedValues = new HashMap<String, String>();
+        this.keyCount = 0;
+    }
+
+    public String addCommentValue(String original) {
+        String key = generateKey();
+        String commentHtml = String.format("<!--%s-->", key);
+        mappedValues.put(commentHtml, original);
+        return key;
+    }
+
+    public void addValue(String key, String original) {
+        mappedValues.put(key, original);
+    }
+
+    public String revert(String markedHTML) {
+        for (String key: mappedValues.keySet()) {
+            String original = mappedValues.get(key);
+            markedHTML = markedHTML.replace(key, original);
+        }
+        return markedHTML;
+    }
+
+    public String generateKey() {
+        String key = WOVN_MARKER_PREFIX + String.valueOf(keyCount);
+        keyCount++;
+        return key;
+    }
+}
diff --git a/src/test/java/com/github/wovnio/wovnjava/HtmlConverterTest.java b/src/test/java/com/github/wovnio/wovnjava/HtmlConverterTest.java
@@ -36,7 +36,7 @@ public void testRemoveWovnSnippet() throws ConfigurationError {
 
     public void testRemoveScripts() throws ConfigurationError {
         String original = "<html><head><script>alert(1)</script></head><body>a <script>console.log(1)</script>b</body></html>";
-        String removedHtml = "<html><head><!--wovn-marker-0--></head><body>a <!--wovn-marker-1-->b</body></html>";
+        String removedHtml = "<html><head><script><!--wovn-marker-0--></script></head><body>a <script><!--wovn-marker-1--></script>b</body></html>";
         Settings settings = TestUtil.makeSettings(new HashMap<String, String>() {{ put("supportedLangs", "en,fr,ja"); }});
         HtmlConverter converter = this.createHtmlConverter(settings, location, original);
         String html = converter.strip();
@@ -56,7 +56,7 @@ public void testRemoveHrefLangIfConflicts() throws ConfigurationError {
 
     public void testRemoveWovnIgnore() throws ConfigurationError {
         String original = "<html><head></head><body><div>Hello <span wovn-ignore>Duke</span><span data-wovn-ignore>Silver</span>.</div></body></html>";
-        String removedHtml = "<html><head></head><body><div>Hello <!--wovn-marker-0--><!--wovn-marker-1-->.</div></body></html>";
+        String removedHtml = "<html><head></head><body><div>Hello <span wovn-ignore><!--wovn-marker-0--></span><span data-wovn-ignore><!--wovn-marker-1--></span>.</div></body></html>";
         Settings settings = TestUtil.makeSettings(new HashMap<String, String>() {{ put("supportedLangs", "en,fr,ja"); }});
         HtmlConverter converter = this.createHtmlConverter(settings, location, original);
         String html = converter.strip();
@@ -70,9 +70,9 @@ public void testRemoveClassIgnore() throws ConfigurationError {
           "<p class=\"ignore-me\">It's a fruit, <span class=\"name\">Louie</span>!</p>" +
           "</body></html>";
         String removedHtml = "<html><head></head><body>" +
-          "<p class=\"no-ignore\">The pizza needs <!--wovn-marker-0-->, <!--wovn-marker-1-->!</p>" +
-          "<!--wovn-marker-2-->" +
-          "</body></html>";
+        "<p class=\"no-ignore\">The pizza needs <b class=\"ingredient\"><!--wovn-marker-0--></b>, <span class=\"name\"><!--wovn-marker-1--></span>!</p>" +
+        "<p class=\"ignore-me\"><!--wovn-marker-2--></p>" +
+        "</body></html>";
         Settings settings = TestUtil.makeSettings(new HashMap<String, String>() {{
             put("supportedLangs", "en,fr,ja");
             put("ignoreClasses", "ignore-me,name,ingredient");
@@ -87,19 +87,19 @@ public void testRemoveClassIgnore() throws ConfigurationError {
 
     public void testRemoveForm() throws ConfigurationError {
         String original = "<html><head></head><body><form><input type=\"hidden\" name=\"csrf\" value=\"random\"><INPUT TYPE=\"HIDDEN\" NAME=\"CSRF_TOKEN\" VALUE=\"RANDOM\"></form></body></html>";
-        String removedHtml = "<html><head></head><body><form><!--wovn-marker-0--><!--wovn-marker-1--></form></body></html>";
+        String removedHtml = "<html><head></head><body><form><input type=\"hidden\" name=\"csrf\" value=\"wovn-marker-0\"><input TYPE=\"HIDDEN\" NAME=\"CSRF_TOKEN\" value=\"wovn-marker-1\"></form></body></html>";
         Settings settings = TestUtil.makeSettings(new HashMap<String, String>() {{ put("supportedLangs", "en,fr,ja"); }});
         HtmlConverter converter = this.createHtmlConverter(settings, location, original);
         String html = converter.strip();
 
         assertEquals(removedHtml, stripExtraSpaces(html));
         // jsoup make lower case tag name
-        assertEquals(original.replace("INPUT", "input"), stripExtraSpaces(converter.restore(html)));
+        assertEquals(original.replace("INPUT", "input").replace("VALUE", "value"), stripExtraSpaces(converter.restore(html)));
     }
 
     public void testNested() throws ConfigurationError {
-        String original = "<html><head></head><body><form wovn-ignore><script></script><input type=\"hidden\" name=\"csrf\" value=\"random\"><INPUT TYPE=\"HIDDEN\" NAME=\"CSRF_TOKEN\" VALUE=\"RANDOM\"></form></body></html>";
-        String removedHtml = "<html><head></head><body><!--wovn-marker-1--></body></html>";
+        String original = "<html><head></head><body><form wovn-ignore><script></script><input type=\"hidden\" name=\"csrf\" value=\"random\"><INPUT TYPE=\"HIDDEN\" NAME=\"CSRF_TOKEN\" value=\"RANDOM\"></form></body></html>";
+        String removedHtml = "<html><head></head><body><form wovn-ignore><!--wovn-marker-1--></form></body></html>";
         Settings settings = TestUtil.makeSettings(new HashMap<String, String>() {{ put("supportedLangs", "en,fr,ja"); }});
         HtmlConverter converter = this.createHtmlConverter(settings, location, original);
         String html = converter.strip();
@@ -192,23 +192,23 @@ public void testMixAllCase() throws ConfigurationError {
             "<script>10</script>" +
             "</body></html>";
         String removedHtml = "<html><head>" +
-            "<!--wovn-marker-0-->" +
+            "<script><!--wovn-marker-0--></script>" +
             "<link ref=\"altername\" hreflang=\"ar\" href=\"http://localhost:8080/ar/\">" +
             "</head><body>" +
-            "a <!--wovn-marker-1-->b" +
-            "<div>Hello <!--wovn-marker-9-->.</div>" +
-            "<form><!--wovn-marker-14--></form>" +
-            "<!--wovn-marker-2-->" +
-            "<!--wovn-marker-3-->" +
-            "<!--wovn-marker-4-->" +
-            "<!--wovn-marker-5-->" +
-            "<!--wovn-marker-6-->" +
+            "a <script><!--wovn-marker-1--></script>b" +
+            "<div>Hello <span wovn-ignore><!--wovn-marker-9--></span>.</div>" +
+            "<form><input type=\"hidden\" name=\"csrf\" value=\"wovn-marker-16\"></form>" +
+            "<script><!--wovn-marker-2--></script>" +
+            "<script><!--wovn-marker-3--></script>" +
+            "<script><!--wovn-marker-4--></script>" +
+            "<script><!--wovn-marker-5--></script>" +
+            "<script><!--wovn-marker-6--></script>" +
             "<div class=\"class-ignore-test\">" +
-            "<p class=\"no-ignore\">The pizza needs <!--wovn-marker-12-->, <!--wovn-marker-10-->!</p>" +
-            "<!--wovn-marker-13-->" +
+            "<p class=\"no-ignore\">The pizza needs <b class=\"ingredient\"><!--wovn-marker-12--></b>, <span class=\"name\" wovn-ignore><!--wovn-marker-13--></span>!</p>" +
+            "<p class=\"ignore-me\"><!--wovn-marker-14--></p>" +
             "</div>" +
-            "<!--wovn-marker-7-->" +
-            "<!--wovn-marker-8-->" +
+            "<script><!--wovn-marker-7--></script>" +
+            "<script><!--wovn-marker-8--></script>" +
             "</body></html>";
         Settings settings = TestUtil.makeSettings(new HashMap<String, String>() {{
             put("supportedLangs", "en,fr,ja");