Merge pull request #542 from syjer/478-li-style-decoration-positioning

#478 change list decoration placement logic
danfickle · Aug 31, 2020 · 3f42c9a · 3f42c9a
2 parents 2af0abb + 5600297
commit 3f42c9a
Show file tree

Hide file tree

Showing 7 changed files with 119 additions and 6 deletions.
diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/render/ListItemPainter.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/render/ListItemPainter.java
@@ -103,17 +103,25 @@ private static void drawGlyph(RenderingContext c, BlockBox box,
                 RenderingHints.VALUE_ANTIALIAS_ON);
 
         // calculations for bullets
-        StrutMetrics strutMetrics = box.getMarkerData().getStructMetrics();
-        MarkerData.GlyphMarker marker = box.getMarkerData().getGlyphMarker();
+        MarkerData markerData = box.getMarkerData();
+        StrutMetrics strutMetrics = markerData.getStructMetrics();
+        MarkerData.GlyphMarker marker = markerData.getGlyphMarker();
         int x = getReferenceX(c, box);
+        // see issue 478. To be noted, the X positioning does not consider the available padding space
+        // (like all the browsers it seems), so if the font is too big, the list decoration will be cut or outside
+        // the viewport.
         if (style.getDirection() == IdentValue.LTR) {
-            x += -marker.getLayoutWidth();
+            x += -marker.getLayoutWidth() + marker.getDiameter() * 1.1;
         }
         if (style.getDirection() == IdentValue.RTL){
-            x += box.getMarkerData().getReferenceLine().getWidth() + marker.getLayoutWidth();
+            x += markerData.getReferenceLine().getWidth() + marker.getDiameter() * 1.1;
         }
-        int y = getReferenceBaseline(c, box)
-            - (int)strutMetrics.getAscent() / 2 - marker.getDiameter() / 2;
+
+        // see issue https://github.com/danfickle/openhtmltopdf/issues/478#issuecomment-682066113
+        int bottomLine = getReferenceBaseline(c, box);
+        int top = bottomLine - (int) (strutMetrics.getAscent() / 1.5);
+
+        int y = bottomLine - (bottomLine-top) / 2 - marker.getDiameter() / 2 ;
         if (listStyle == IdentValue.DISC) {
             c.getOutputDevice().fillOval(x, y, marker.getDiameter(), marker.getDiameter());
         } else if (listStyle == IdentValue.SQUARE) {

diff --git a/...df-examples/src/main/resources/visualtest/expected/issue-478-list-decoration-position.pdf b/...df-examples/src/main/resources/visualtest/expected/issue-478-list-decoration-position.pdf
diff --git a/openhtmltopdf-examples/src/main/resources/visualtest/expected/replaced-plugin-latex.pdf b/openhtmltopdf-examples/src/main/resources/visualtest/expected/replaced-plugin-latex.pdf
diff --git a/openhtmltopdf-examples/src/main/resources/visualtest/expected/running-nested-list-items.pdf b/openhtmltopdf-examples/src/main/resources/visualtest/expected/running-nested-list-items.pdf
diff --git a/...ltopdf-examples/src/main/resources/visualtest/expected/text/columns-nested-unbalanced.pdf b/...ltopdf-examples/src/main/resources/visualtest/expected/text/columns-nested-unbalanced.pdf
diff --git a/...topdf-examples/src/main/resources/visualtest/html/issue-478-list-decoration-position.html b/...topdf-examples/src/main/resources/visualtest/html/issue-478-list-decoration-position.html
@@ -0,0 +1,95 @@
+<html>
+<head>
+    <style>
+
+@page {
+    size: 550px 450px;
+}
+
+body {
+    padding:0;
+    margin:0;
+}
+
+@font-face {
+  src: url(fonts/NotoNaskhArabic-Regular.ttf);
+  font-family: 'arabic';
+}
+
+@font-face {
+  src: url(fonts/SourceSansPro-Regular.ttf);
+  font-family: 'source-sans-pro';
+}
+
+@font-face {
+  src: url(fonts/Karla-Bold.ttf);
+  font-family: 'karla-bold';
+}
+
+.source-sans-pro {
+    font-family:source-sans-pro;
+}
+
+.karla-bold {
+    font-family:karla-bold;
+}
+
+.arabic {
+    font-family:arabic
+}
+
+.serif {
+    font-family:serif;
+}
+
+.sans-serif {
+    font-family:sans-serif;
+}
+
+li {
+    margin:0;
+    padding:0;
+}
+
+.e1 {
+ font-size: 40px;
+}
+.e2 {
+ font-size: 55px;
+}
+</style>
+</head>
+<body>
+
+<!--
+To be noted, like in chrome, if the font is _too big_ the decoration will be cut.
+Padding left/right can be adjusted if the current heuristic is not good enough.
+-->
+<ul >
+    <li class="serif" >A</li>
+    <li class="e1 serif">
+        <p>B</p>
+        <p>C</p>
+    </li>
+    <li class="e2 serif">D</li>
+    <li class="e3 serif">E</li>
+</ul>
+
+<ul class="arabic" style="direction:rtl; padding-right:40px;padding-left:0px">
+    <li>التنازلي  1234 بحق</li>
+    <li class="e1">التنازلي  1234 بحق</li> <!-- the decoration is partially cut-->
+    <li class="e2">التنازلي  1234 بحق</li> <!-- the decoration is nearly all cut-->
+</ul>
+
+<ul style="list-style-type:square">
+    <li class="karla-bold" >A</li>
+    <li class="e1 source-sans-pro">
+        <p>B</p>
+        <p>C</p>
+    </li>
+    <li class="e2 source-sans-pro">D</li> <!-- the decoration is partially cut-->
+    <li class="karla-bold">E</li>
+</ul>
+
+</body>
+</html>
diff --git a/...-examples/src/test/java/com/openhtmltopdf/visualregressiontests/VisualRegressionTest.java b/...-examples/src/test/java/com/openhtmltopdf/visualregressiontests/VisualRegressionTest.java
@@ -7,6 +7,8 @@
 import java.nio.charset.StandardCharsets;
 import java.util.Collections;
 
+import com.openhtmltopdf.bidi.support.ICUBidiReorderer;
+import com.openhtmltopdf.bidi.support.ICUBidiSplitter;
 import com.openhtmltopdf.extend.FSStream;
 import com.openhtmltopdf.objects.zxing.ZXingObjectDrawer;
 import org.junit.Before;
@@ -1240,6 +1242,14 @@ public void testBarcode() throws IOException {
 
     }
 
+    @Test
+    public void testIssue478ListDecorationPosition() throws IOException {
+        assertTrue(vt.runTest("issue-478-list-decoration-position", builder -> {
+            builder.useUnicodeBidiSplitter(new ICUBidiSplitter.ICUBidiSplitterFactory());
+            builder.useUnicodeBidiReorderer(new ICUBidiReorderer());
+        }));
+    }
+
     // TODO:
     // + Elements that appear just on generated overflow pages.
     // + content property (page counters, etc)