Merge pull request #610 from danfickle/fix_594_repeated_content

Fixes #594, fixes #458 Fix for repeating content and PDF/UA crash.
danfickle · Nov 28, 2020 · a9ba3af · a9ba3af
2 parents 03c31d4 + d06ccc5
commit a9ba3af
Show file tree

Hide file tree

Showing 8 changed files with 94 additions and 12 deletions.
diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/Layer.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/Layer.java
@@ -1092,7 +1092,33 @@ public void addPage(CssContext c) {
         pages.add(pageBox);
     }
 
+    /**
+     * Returns the page box for a Y position.
+     * If the y position is less than 0 then the first page will
+     * be returned if available.
+     * Returns null if there are no pages available or absY
+     * is past the last page.
+     */
+    public PageBox getFirstPage(CssContext c, int absY) {
+        PageBox page = getPage(c, absY);
+
+        if (page == null && absY < 0) {
+            List<PageBox> pages = getPages();
+
+            if (!pages.isEmpty()) {
+                return pages.get(0);
+            }
+        }
+
+        return page;
+    }
+
     public PageBox getFirstPage(CssContext c, Box box) {
+        if (box instanceof LineBox) {
+            LineBox lb = (LineBox) box;
+            return getPage(c, lb.getMinPaintingTop());
+        }
+
         return getPage(c, box.getAbsY());
     }
 

diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/render/Box.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/render/Box.java
@@ -756,13 +756,17 @@ public void calcChildLocations() {
     }
 
     public int forcePageBreakBefore(LayoutContext c, IdentValue pageBreakValue, boolean pendingPageName) {
-        PageBox page = c.getRootLayer().getFirstPage(c, this);
+        return forcePageBreakBefore(c, pageBreakValue, pendingPageName, getAbsY());
+    }
+
+    public int forcePageBreakBefore(LayoutContext c, IdentValue pageBreakValue, boolean pendingPageName, int absY) {
+        PageBox page = c.getRootLayer().getFirstPage(c, absY);
         if (page == null) {
             XRLog.log(Level.WARNING, LogMessageId.LogMessageId0Param.LAYOUT_BOX_HAS_NO_PAGE);
             return 0;
         } else {
             int pageBreakCount = 1;
-            if (page.getTop() == getAbsY()) {
+            if (page.getTop() == absY) {
                 pageBreakCount--;
                 if (pendingPageName && page == c.getRootLayer().getLastPage()) {
                     c.getRootLayer().removeLastPage();
@@ -783,7 +787,7 @@ public int forcePageBreakBefore(LayoutContext c, IdentValue pageBreakValue, bool
                 c.setPageName(c.getPendingPageName());
             }
 
-            int delta = page.getBottom() + c.getExtraSpaceTop() - getAbsY();
+            int delta = page.getBottom() + c.getExtraSpaceTop() - absY;
             if (page == c.getRootLayer().getLastPage()) {
                 c.getRootLayer().addPage(c);
             }

diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/render/LineBox.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/render/LineBox.java
@@ -419,6 +419,20 @@ public void setPaintingTop(int paintingTop) {
         _paintingTop = paintingTop;
     }
 
+    public int getMinPaintingTop() {
+        int paintingAbsTop = getAbsY() + getPaintingTop();
+        int lineAbsTop = getAbsY();
+
+        return Math.min(lineAbsTop, paintingAbsTop);
+    }
+
+    public int getMaxPaintingBottom() {
+        int paintingAbsBottom = getAbsY() + getPaintingTop() + getPaintingHeight();
+        int lineAbsBottom = getAbsY() + getHeight();
+
+        return Math.max(paintingAbsBottom, lineAbsBottom);
+    }
+
     public void addAllChildren(List<? super Box> list, Layer layer) {
         for (int i = 0; i < getChildCount(); i++) {
             Box child = getChild(i);
@@ -650,23 +664,30 @@ public void analyzePageBreaks(LayoutContext c, ContentLimitContainer container)
         container.updateTop(c, getAbsY());
         container.updateBottom(c, getAbsY() + getHeight());
     }
-    
+
     public void checkPagePosition(LayoutContext c, boolean alwaysBreak) {
         if (! c.isPageBreaksAllowed()) {
             return;
         }
-        
+
         PageBox pageBox = c.getRootLayer().getFirstPage(c, this);
         if (pageBox != null) {
+            // We need to force a page break if any of our content goes over a page break,
+            // otherwise we will get repeated content in page margins (because content is
+            // printed on both pages).
+
+            // Painting top and bottom take account of line-height other than 1.
+            int greatestAbsY = getMaxPaintingBottom();
+            int leastAbsY = getMinPaintingTop();
+
             boolean needsPageBreak = 
-                alwaysBreak || getAbsY() + getHeight() >= pageBox.getBottom() - c.getExtraSpaceBottom();
-                
+                alwaysBreak || greatestAbsY >= pageBox.getBottom() - c.getExtraSpaceBottom();
+
            if (needsPageBreak) {
-               forcePageBreakBefore(c, IdentValue.ALWAYS, false);
+               forcePageBreakBefore(c, IdentValue.ALWAYS, false, leastAbsY);
                calcCanvasLocation();
            } else if (pageBox.getTop() + c.getExtraSpaceTop() > getAbsY()) {
                int diff = pageBox.getTop() + c.getExtraSpaceTop() - getAbsY();
-
                setY(getY() + diff);
                calcCanvasLocation();
            }

diff --git a/...tmltopdf-examples/src/main/resources/visualtest/expected/pr-610-force-page-break-line.pdf b/...tmltopdf-examples/src/main/resources/visualtest/expected/pr-610-force-page-break-line.pdf
diff --git a/openhtmltopdf-examples/src/main/resources/visualtest/html/issue-594-content-repeated.html b/openhtmltopdf-examples/src/main/resources/visualtest/html/issue-594-content-repeated.html
@@ -8,6 +8,12 @@
     body {
         margin: 0;
     }
+    td {
+      background-color: red;
+    }
+    span {
+      background-color: blue;
+    }
 </style>
 </head>
     <body style="line-height: 0.8;">
@@ -17,7 +23,7 @@
               <td>One</td><td>1</td>
           </tr>
           <tr>
-              <td>Abcdefghij2</td><td>2</td>
+              <td><span>Abcdefghij2</span></td><td>2</td>
           </tr>
         </table>
     </body>

diff --git a/openhtmltopdf-examples/src/main/resources/visualtest/html/pr-610-force-page-break-line.html b/openhtmltopdf-examples/src/main/resources/visualtest/html/pr-610-force-page-break-line.html
@@ -0,0 +1,17 @@
+<html>
+<head>
+<style>
+    @page {
+        size: 150px 100px;
+        margin: 15px;
+    }
+    body {
+        margin: 0;
+    }
+</style>
+</head>
+  <body>
+    <div style="height: 20px;">SPACER</div>
+    <div style="line-height: 20px; font-size: 60px;">Line One</div>
+  </body>
+</html>
diff --git a/...les/src/test/java/com/openhtmltopdf/nonvisualregressiontests/NonVisualRegressionTest.java b/...les/src/test/java/com/openhtmltopdf/nonvisualregressiontests/NonVisualRegressionTest.java
@@ -819,12 +819,11 @@ public void testIssue458PageContentRepeatedInMargin() throws IOException {
      * Table row repeating on two pages. See issue 594.
      */
     @Test
-    @Ignore // The second row is repeating on both pages.
     public void testIssue594RepeatingContentTableRow() throws IOException {
         try (PDDocument doc = run("issue-594-content-repeated")) {
             PDFTextStripper stripper = new PDFTextStripper();
             String text = stripper.getText(doc).replaceAll("(\\r|\\n)", "");
-            String expected = "One" + "Abcdefghij2";
+            String expected = "One 1" + "Abcdefghij2 2";
 
             assertEquals(expected, text);
         }

diff --git a/...-examples/src/test/java/com/openhtmltopdf/visualregressiontests/VisualRegressionTest.java b/...-examples/src/test/java/com/openhtmltopdf/visualregressiontests/VisualRegressionTest.java
@@ -1286,6 +1286,15 @@ public void testIssue599TrimLeadingSpaceException() throws IOException {
         assertTrue(vt.runTest("issue-599-trim-leading-space-exception"));
     }
 
+    /**
+     * Tests auto page break before line-box with content
+     * much larger than line-height.
+     */
+    @Test
+    public void testPr610ForcePageBreakLine() throws IOException {
+        assertTrue(vt.runTest("pr-610-force-page-break-line"));
+    }
+
     // TODO:
     // + Elements that appear just on generated overflow pages.
     // + content property (page counters, etc)