From 8985e5a91dc8e9fecd9cd4c355486f68a568e3cd Mon Sep 17 00:00:00 2001 From: danfickle Date: Fri, 27 Nov 2020 22:42:57 +1100 Subject: [PATCH 1/2] #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. --- .../java/com/openhtmltopdf/layout/Layer.java | 4 ++++ .../java/com/openhtmltopdf/render/Box.java | 10 +++++++--- .../com/openhtmltopdf/render/LineBox.java | 20 ++++++++++++++++--- .../html/issue-594-content-repeated.html | 8 +++++++- .../NonVisualRegressionTest.java | 3 +-- 5 files changed, 36 insertions(+), 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 index e29a669b5..eb679c4b4 100644 --- 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 index 35c812c8b..cf9684953 100755 --- 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 index 2d83d4518..77aef3c40 100755 --- 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 index 18c0583bd..e718e1bee 100644 --- 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; + } @@ -17,7 +23,7 @@ One1 - Abcdefghij22 + Abcdefghij22 diff --git a/openhtmltopdf-examples/src/test/java/com/openhtmltopdf/nonvisualregressiontests/NonVisualRegressionTest.java b/openhtmltopdf-examples/src/test/java/com/openhtmltopdf/nonvisualregressiontests/NonVisualRegressionTest.java index 978d1d009..4544a6b0b 100644 --- a/openhtmltopdf-examples/src/test/java/com/openhtmltopdf/nonvisualregressiontests/NonVisualRegressionTest.java +++ b/openhtmltopdf-examples/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); } From d06ccc5d4d330761cd396783d3ce29bafc2bd06e Mon Sep 17 00:00:00 2001 From: danfickle Date: Sat, 28 Nov 2020 21:25:15 +1100 Subject: [PATCH 2/2] #594 #458 Test with font-size much larger than line height. With changes to get it working and test proof. --- .../java/com/openhtmltopdf/layout/Layer.java | 24 ++++++++++++++- .../com/openhtmltopdf/render/LineBox.java | 29 +++++++++++------- .../expected/pr-610-force-page-break-line.pdf | Bin 0 -> 2056 bytes .../html/pr-610-force-page-break-line.html | 17 ++++++++++ .../VisualRegressionTest.java | 9 ++++++ 5 files changed, 67 insertions(+), 12 deletions(-) create mode 100644 openhtmltopdf-examples/src/main/resources/visualtest/expected/pr-610-force-page-break-line.pdf create mode 100644 openhtmltopdf-examples/src/main/resources/visualtest/html/pr-610-force-page-break-line.html diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/Layer.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/Layer.java index eb679c4b4..923b7e501 100644 --- a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/Layer.java +++ b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/Layer.java @@ -1092,11 +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) { - return getPage(c, absY); + PageBox page = getPage(c, absY); + + if (page == null && absY < 0) { + List 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/LineBox.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/render/LineBox.java index 77aef3c40..776a27437 100755 --- 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 list, Layer layer) { for (int i = 0; i < getChildCount(); i++) { Box child = getChild(i); @@ -650,12 +664,12 @@ 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, @@ -663,14 +677,8 @@ public void checkPagePosition(LayoutContext c, boolean alwaysBreak) { // 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); + int greatestAbsY = getMaxPaintingBottom(); + int leastAbsY = getMinPaintingTop(); boolean needsPageBreak = alwaysBreak || greatestAbsY >= pageBox.getBottom() - c.getExtraSpaceBottom(); @@ -680,7 +688,6 @@ public void checkPagePosition(LayoutContext c, boolean alwaysBreak) { calcCanvasLocation(); } else if (pageBox.getTop() + c.getExtraSpaceTop() > getAbsY()) { int diff = pageBox.getTop() + c.getExtraSpaceTop() - getAbsY(); - setY(getY() + diff); calcCanvasLocation(); } diff --git a/openhtmltopdf-examples/src/main/resources/visualtest/expected/pr-610-force-page-break-line.pdf b/openhtmltopdf-examples/src/main/resources/visualtest/expected/pr-610-force-page-break-line.pdf new file mode 100644 index 0000000000000000000000000000000000000000..3aac2f57fd816c23fd423bc6103602521a354ffb GIT binary patch literal 2056 zcmd5-OHbQC5WbJZf0#=IsKV>@`jrYHK^{_D5NU|YA>v??O)!dgsqLt=f3}zYhMs!p z?50i~8^MJGj$(Po^ZaJ!b7#~a9&yKq&hKA;ega1q_3b?z9|N0we~`#}Ntvwl64)1+ z7pYdrxDx>zB}-YLN9toZIRUBW#!S@Ad$~+XGUz9z#Do5O&-Gl+z1ZXQ)P2YKzU%HE zlBZnH*E5;pfqszcuFO`YewZ(una&Ptu1%=j%xnt4KBe;lr@qMq1A7n6{RUW1uT_bn z=EP{VodCO#^E5frk2rN57tNnCbpnh6$0bLjB$woAQ!TkwB^?WlOdMciS?F~>BRFC! z_z#Ck=k>yMo8dU6x&yx{{Pp5-GYmWR@n*jb0#(3ssg~s(@+dNZ7bUen+XKQCLy(fWv-8^rsS8xZ{peTm<gbI_(X{hd8;1!2~|To|)xM_SICIwpLXRsrksw zoY&{xw5HP7;t;$k9DFLbiGwE`L2wut&Qm4vSrqKFRYh)KuuE54FFP@n$`%K^fNy3V zUcli}$>&k1^UwwbOFDHF>MUp`(DG4Tp-w|jD%6SNNrm>Nn_Pfht#3<9+lIV3u(PC) zHcK{1Gg%ysb(Sb#12xn0R4tKRr|MJ{X(gy-`IyTEZ4-~c-Ch_5fe3Ixd;Ra?l&ZN< zZDl;9b!f=kh?b}!^F&)F{I;zh5)n0QgNQI}$YS>iSvv-ILqc!YmJ{=|WWsCZ$3;lZ zZPfnAzvs2lYd6KSVZa=hRt%*}CrLYfbp>abm>_Z=?$Gv#i6aJ|e`e8qg syL?DchJ3(dF%ZMZ?VjMvD)b4vN`FYxOH0j3UfS;RAoQTq8C(wGFH%A0$^ZZW literal 0 HcmV?d00001 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 new file mode 100644 index 000000000..733224236 --- /dev/null +++ b/openhtmltopdf-examples/src/main/resources/visualtest/html/pr-610-force-page-break-line.html @@ -0,0 +1,17 @@ + + + + + +
SPACER
+
Line One
+ + diff --git a/openhtmltopdf-examples/src/test/java/com/openhtmltopdf/visualregressiontests/VisualRegressionTest.java b/openhtmltopdf-examples/src/test/java/com/openhtmltopdf/visualregressiontests/VisualRegressionTest.java index a9de79ee3..7c8e70362 100644 --- a/openhtmltopdf-examples/src/test/java/com/openhtmltopdf/visualregressiontests/VisualRegressionTest.java +++ b/openhtmltopdf-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)