From 990012c64ddbe06d6b5bb3ad37d77f82c7b5e4d8 Mon Sep 17 00:00:00 2001 From: danfickle Date: Sun, 7 Jun 2020 22:48:39 +1000 Subject: [PATCH] #429 #482 #483 #491 Safety valve on unbreakable word reattempts on new line. With test to prove that we don't trigger safety valve with lots of unbreakable words. Safety valve should only be triggered if there is still a bug in our code. --- .../openhtmltopdf/layout/InlineBoxing.java | 48 ++++++++++++++---- ...-429-break-word-lots-of-short-and-long.pdf | Bin 0 -> 2021 bytes ...429-break-word-lots-of-short-and-long.html | 19 +++++++ .../VisualRegressionTest.java | 9 ++++ 4 files changed, 66 insertions(+), 10 deletions(-) create mode 100644 openhtmltopdf-examples/src/main/resources/visualtest/expected/issue-429-break-word-lots-of-short-and-long.pdf create mode 100644 openhtmltopdf-examples/src/main/resources/visualtest/html/issue-429-break-word-lots-of-short-and-long.html diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/InlineBoxing.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/InlineBoxing.java index d53fb752c..6ef6cad7d 100644 --- a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/InlineBoxing.java +++ b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/InlineBoxing.java @@ -24,6 +24,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.logging.Level; import org.w3c.dom.Element; @@ -47,6 +48,7 @@ import com.openhtmltopdf.render.MarkerData; import com.openhtmltopdf.render.StrutMetrics; import com.openhtmltopdf.render.TextDecoration; +import com.openhtmltopdf.util.XRLog; /** * This class is responsible for flowing inline content into lines. Block @@ -155,7 +157,9 @@ public static void layoutContent(LayoutContext c, BlockBox box, int initialY, in } boolean inCharBreakingMode = false; - + int troublesomeStartPosition = -1; + int troublesomeAttemptCount = 0; + do { lbContext.reset(); @@ -183,14 +187,31 @@ public static void layoutContent(LayoutContext c, BlockBox box, int initialY, in needFirstLetter = false; } else { if (style.getWordWrap() != IdentValue.BREAK_WORD) { - if (!startInlineText(c, lbContext, inlineBox, space, current, fit, trimmedLeadingSpace, false)) { + StartInlineTextResult result = startInlineText(c, lbContext, inlineBox, space, current, fit, trimmedLeadingSpace, false); + if (result == StartInlineTextResult.RECONSUME_BELOW_FLOATS) { continue; } } else { - boolean shouldContinue = !startInlineText(c, lbContext, inlineBox, space, current, fit, trimmedLeadingSpace, inCharBreakingMode); + StartInlineTextResult result = startInlineText(c, lbContext, inlineBox, space, current, fit, trimmedLeadingSpace, inCharBreakingMode); inCharBreakingMode = lbContext.isFinishedInCharBreakingMode(); - if (shouldContinue) { + + if (result == StartInlineTextResult.RECONSUME_BELOW_FLOATS) { continue; + } else if (result == StartInlineTextResult.RECONSUME_UNBREAKABLE_ON_NEW_LINE) { + if (troublesomeStartPosition == lbContext.getStart()) { + troublesomeAttemptCount++; + } else { + troublesomeStartPosition = lbContext.getStart(); + troublesomeAttemptCount = 1; + } + + if (troublesomeAttemptCount > 5) { + XRLog.general(Level.SEVERE, + "A fatal infinite loop bug was detected in the line breaking " + + "algorithm for break-word! Start-substring=[" + lbContext.getStartSubstring() + "], " + + "end=" + lbContext.getEnd()); + throw new RuntimeException("Infinite loop bug in break-word line breaking algorithm!"); + } } } } @@ -368,6 +389,12 @@ private static void startNewInlineLine(LayoutContext c, BlockBox box, int breakA space.remainingWidth -= c.getBlockFormattingContext().getFloatDistance(c, current.line, space.remainingWidth); } + private enum StartInlineTextResult { + RECONSUME_BELOW_FLOATS, + RECONSUME_UNBREAKABLE_ON_NEW_LINE, + LINE_FINISHED + } + /** * Trys to consume the text in lbContext. If successful it creates an InlineText and adds it to the current inline * layout box. @@ -375,7 +402,7 @@ private static void startNewInlineLine(LayoutContext c, BlockBox box, int breakA * Otherwise, trys again on a new line. * @return true if the line is finished, false if we must continue */ - private static boolean startInlineText( + private static StartInlineTextResult startInlineText( LayoutContext c, LineBreakContext lbContext, InlineBox inlineBox, SpaceVariables space, StateVariables current, int fit, boolean trimmedLeadingSpace, boolean tryToBreakAnywhere) { @@ -406,9 +433,9 @@ private static boolean startInlineText( // Go back to before the troublesome unbreakable content. lbContext.resetEnd(); - // Return false so that we continue with line breaking with the new remaining width. + // Return appropriate ret val so that we continue with line breaking with the new remaining width. // The InlineText we produced above is not used. - return false; + return StartInlineTextResult.RECONSUME_BELOW_FLOATS; } } @@ -436,6 +463,9 @@ private static boolean startInlineText( space.pendingLeftMBP -= marginBorderPadding; space.remainingWidth -= marginBorderPadding; } + + // Line is finsished, consume content afterward on new line. + return StartInlineTextResult.LINE_FINISHED; } else { // We could not fit this text on the current line and it was unbreakable. // So rewind to reconsume the troublesome text. @@ -443,10 +473,8 @@ private static boolean startInlineText( // of this method in layoutContent). // The inline text object is not consumed. lbContext.resetEnd(); + return StartInlineTextResult.RECONSUME_UNBREAKABLE_ON_NEW_LINE; } - - // We should go ahead and create a new line if needed, after this method. - return true; } private static void startFirstLetterInlineLayoutBox(LayoutContext c, SpaceVariables space, StateVariables current, diff --git a/openhtmltopdf-examples/src/main/resources/visualtest/expected/issue-429-break-word-lots-of-short-and-long.pdf b/openhtmltopdf-examples/src/main/resources/visualtest/expected/issue-429-break-word-lots-of-short-and-long.pdf new file mode 100644 index 0000000000000000000000000000000000000000..ead86ce0b5e96da4abe7e5d8010901b4efc0fd90 GIT binary patch literal 2021 zcma)7OK;;g5Wdf^m`kt;ils>ET?7H*7tqI|X<~E_!5$QwwiHODOEK%+{@Gso8+z)Y zL)mg=7icBJk~o}i9^X(iR}Ztrd%+^O`ty&!eg}aZ&C4s?-a@eax>FcT3s=-;3&AhS zww2Ks2$n+dP;6C;lGGpJ?hce*dssMtPpv8(0cM3$cs=`8a>;qZQz@fR$ZrIH$N9S( zvSiKrZKW(;o1N07Yieh9>kV6(=4N2pyE>1o3ju;3s&$Lck&omP1XJ^-9f|?a@BAAG z!3VXjii!Dx&zy1ee>^P(h2_Iz$^6r zepGtvN)#fBeMGGzP8uLDVH)EeBz_MZUqNvE0MDNYxQ1UzLLp!S6i)Y6cW)gi%CV;X z;k(zZp@fg1yXtYYTac2)C`C&526F#6ENaLTMy{pk9yQ325&pC+VRKNA=t!hdo)f&Y zbz1k3NHQuUDfA`>0&nFUb+pn4apCo_yf83``k2BXhC>zu#6?;@0-}k2M8;$Z!9q}V zak)8CB=8slk6E4(A6D2n=y`)n5BYPKY+_>Rqm_09_2QsdSz764!Wrv9p_Tjl~$wBVaBpPV?#hY zjwt$)qEoJYxZ1c;K80jl99b?X%gRS=wAvMR1Xp>+;#0oG))u>x;ghMAYgZ%SS)SfD( zSP%Ode49S@pku@K&8u%#3px{+l3>`;{raRJFuU0H>wfz%FT529p59(uH$J>jKrks< z)!7f0RioPXkESU!1arMI>q>7i_*m(C-B!I|(7(S}e`02tVj>8y9e7poTU4NnO zMIy;BMLeZw&$Xl^0?tKYc&R1gkXSg=5^+k+5V}@B>npgpKrw?yN@ReF&ClCqIo11^Z9g= + + + + +This isaverylotof words someofwhichwill fitontheline others whichwillnot all +intheoneparagraph. Soletscelebratelongwords and short words thatfitsomewhere. + + 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 c7814d76b..4e1139d84 100644 --- a/openhtmltopdf-examples/src/test/java/com/openhtmltopdf/visualregressiontests/VisualRegressionTest.java +++ b/openhtmltopdf-examples/src/test/java/com/openhtmltopdf/visualregressiontests/VisualRegressionTest.java @@ -1177,6 +1177,15 @@ public void testTooLongWordsFallBelowFloats() throws IOException { assertTrue(vt.runTest("too-long-words-fall-below-floats")); } + /** + * Test for break-word with lots of unreakable words to make sure we don't + * trigger the safety valve which is part of the fix for 482. + */ + @Test + public void testIssue429BreakWordLotsOfShortAndLong() throws IOException { + assertTrue(vt.runTest("issue-429-break-word-lots-of-short-and-long")); + } + // TODO: // + Elements that appear just on generated overflow pages. // + content property (page counters, etc)