Skip to content

Commit

Permalink
#429 #482 #483 #491 Safety valve on unbreakable word reattempts on ne…
Browse files Browse the repository at this point in the history
…w 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.
  • Loading branch information
danfickle committed Jun 7, 2020
1 parent f251520 commit 990012c
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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!");
}
}
}
}
Expand Down Expand Up @@ -368,14 +389,20 @@ 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.
* Otherwise, if there are floats and the current line is otherwise empty, moves below float and trys again.
* 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) {
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -436,17 +463,18 @@ 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.
// This will be done on a new line as lbContext.isNeedsNewLine is true (see context
// 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,
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<html>
<head>
<style>
@page {
size: 160px 300px;
margin: 0;
}
body {
margin: 10px 30px;
border: 1px solid red;
word-wrap: break-word;
}
</style>
</head>
<body>
This isaverylotof words someofwhichwill fitontheline others whichwillnot all
intheoneparagraph. Soletscelebratelongwords and short words thatfitsomewhere.
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 990012c

Please sign in to comment.