#594 #458 Force page break before line going accross two pages.

This fixes repeating content in page margins when line-height is other than one. It also fixes the PDF UA crash caused by the repeating content. However, it is a behavior changing fix. Documents with text split over two pages (usually undesired) will now get a forced page break before the split text.
danfickle · Nov 27, 2020 · 8985e5a · 8985e5a
1 parent 58b1fa8
commit 8985e5a
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 9 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,6 +1092,10 @@ public void addPage(CssContext c) {
         pages.add(pageBox);
     }
 
+    public PageBox getFirstPage(CssContext c, int absY) {
+        return getPage(c, absY);
+    }
+
     public PageBox getFirstPage(CssContext c, Box box) {
         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
@@ -658,11 +658,25 @@ public void checkPagePosition(LayoutContext c, boolean alwaysBreak) {
 
         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 paintingAbsTop = getAbsY() + getPaintingTop();
+            int paintingAbsBottom = paintingAbsTop + getPaintingHeight();
+
+            int lineAbsTop = getAbsY();
+            int lineAbsBottom = lineAbsTop + getHeight();
+
+            int leastAbsY = Math.min(paintingAbsTop, lineAbsTop);
+            int greatestAbsY = Math.max(paintingAbsBottom, lineAbsBottom);
+
             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();

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/...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);
         }