diff --git a/CHANGELOG.md b/CHANGELOG.md index 97c17b083..de593e20c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## CHANGELOG ### head - 1.0.6-SNAPSHOT +**IMPORTANT:** [#615](https://github.com/danfickle/openhtmltopdf/issues/615) This is a bug fix release for an endless loop issue when using break-word with floating elements with a top/bottom margin. + See commit log. diff --git a/README.md b/README.md index d95ed903a..e33da6554 100644 --- a/README.md +++ b/README.md @@ -68,6 +68,7 @@ from ````/openhtmltopdf-examples/src/main/java/com/openhtmltopdf/testcases/Testc ## CHANGELOG ### head - 1.0.6-SNAPSHOT +**IMPORTANT:** [#615](https://github.com/danfickle/openhtmltopdf/issues/615) This is a bug fix release for an endless loop issue when using break-word with floating elements with a top/bottom margin. + See commit log. diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/BlockFormattingContext.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/BlockFormattingContext.java index ebe747b5e..65f4c0e2a 100755 --- a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/BlockFormattingContext.java +++ b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/BlockFormattingContext.java @@ -84,6 +84,7 @@ public void clear(LayoutContext c, Box current) { getFloatManager().clear(c, this, current); } + @Override public String toString() { return "BlockFormattingContext: (" + _x + "," + _y + ")"; } diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/Breaker.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/Breaker.java index 6efa09e55..95b1c9702 100644 --- a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/Breaker.java +++ b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/Breaker.java @@ -72,41 +72,93 @@ private static int getFirstLetterEnd(String text, int start) { return end; } - public static LineBreakResult breakText(LayoutContext c, - LineBreakContext context, int avail, - CalculatedStyle style, boolean tryToBreakAnywhere, int lineWidth) { - + public enum BreakTextResult { + /** + * Has completely consumed the string. + */ + FINISHED, + + /** + * In char breaking mode, need a newline before continuing. + * At least one character has been consumed. + */ + CONTINUE_CHAR_BREAKING_ON_NL, + + /** + * In word breaking mode, need a newline before continuing. + * At least one word has been consumed. + */ + CONTINUE_WORD_BREAKING_ON_NL, + + /** + * Not a single char fitted, but we consumed one character anyway. + */ + CHAR_UNBREAKABLE_BUT_CONSUMED, + + /** + * Not a single word fitted, but we consumed a word anyway. + * Only returned when word-wrap is not break-word. + */ + WORD_UNBREAKABLE_BUT_CONSUMED, + + /** + * DANGER: Last char did not fit and we are directing the + * parent method to reconsume char on a newline. + * Example, below floats. + */ + DANGER_RECONSUME_CHAR_ON_NL, + + /** + * DANGER: Last word did not fit and we are directing the + * parent method to reconsume word on a newline. + * Example, below floats. + */ + DANGER_RECONSUME_WORD_ON_NL; + } + + public static BreakTextResult breakText( + LayoutContext c, + LineBreakContext context, + int avail, + CalculatedStyle style, + boolean tryToBreakAnywhere, + int lineWidth, + boolean forceOutput) { + FSFont font = style.getFSFont(c); IdentValue whitespace = style.getWhitespace(); - float letterSpacing = style.hasLetterSpacing() ? + float letterSpacing = style.hasLetterSpacing() ? style.getFloatPropertyProportionalWidth(CSSName.LETTER_SPACING, 0, c) : 0f; // ====== handle nowrap if (whitespace == IdentValue.NOWRAP) { int width = Breaker.getTextWidthWithLetterSpacing(c, font, context.getMaster(), letterSpacing); - if (width <= avail) { + if (width <= avail || forceOutput) { c.setLineBreakedBecauseOfNoWrap(false); context.setEnd(context.getLast()); context.setWidth(width); - return LineBreakResult.WORD_BREAKING_FINISHED; + context.setNeedsNewLine(false); + return BreakTextResult.FINISHED; } else if (!c.isLineBreakedBecauseOfNoWrap()) { c.setLineBreakedBecauseOfNoWrap(true); context.setEnd(context.getStart()); context.setWidth(0); context.setNeedsNewLine(true); - return LineBreakResult.WORD_BREAKING_NEED_NEW_LINE; + context.setUnbreakable(true); + return BreakTextResult.DANGER_RECONSUME_WORD_ON_NL; } else { c.setLineBreakedBecauseOfNoWrap(false); context.setEnd(context.getLast()); context.setWidth(width); - return LineBreakResult.WORD_BREAKING_FINISHED; + context.setNeedsNewLine(false); + return BreakTextResult.FINISHED; } } - //check if we should break on the next newline + // Check if we should break on the next newline if (whitespace == IdentValue.PRE || - whitespace == IdentValue.PRE_WRAP || - whitespace == IdentValue.PRE_LINE) { + whitespace == IdentValue.PRE_WRAP || + whitespace == IdentValue.PRE_LINE) { int n = context.getStartSubstring().indexOf(WhitespaceStripper.EOL); if (n > -1) { context.setEnd(context.getStart() + n + 1); @@ -114,52 +166,66 @@ public static LineBreakResult breakText(LayoutContext c, context.setNeedsNewLine(true); context.setEndsOnNL(true); } else if (whitespace == IdentValue.PRE) { - context.setEnd(context.getLast()); + context.setEnd(context.getLast()); context.setWidth(Breaker.getTextWidthWithLetterSpacing(c, font, context.getCalculatedSubstring(), letterSpacing)); + context.setNeedsNewLine(false); } } - //check if we may wrap + // Check if we may wrap if (whitespace == IdentValue.PRE || - (context.isNeedsNewLine() && context.getWidth() <= avail)) { + (context.isNeedsNewLine() && context.getWidth() <= avail)) { return context.isNeedsNewLine() ? - LineBreakResult.WORD_BREAKING_NEED_NEW_LINE : - LineBreakResult.WORD_BREAKING_FINISHED; + BreakTextResult.CONTINUE_WORD_BREAKING_ON_NL : + BreakTextResult.FINISHED; } context.setEndsOnNL(false); - + if (style.getWordWrap() != IdentValue.BREAK_WORD) { // Ordinary old word wrap which will overflow too long unbreakable words. - return doBreakText(c, context, avail, style, tryToBreakAnywhere); + return toBreakTextResult( + doBreakText(c, context, avail, style, tryToBreakAnywhere)); } else { int originalStart = context.getStart(); int totalWidth = 0; // The idea is we only break a word if it will not fit on a line by itself. - + LineBreakResult result; + BreakTextResult breakResult; + LOOP: while (true) { int savedEnd = context.getEnd(); result = doBreakText(c, context, avail, style, tryToBreakAnywhere); switch (result) { - case WORD_BREAKING_FINISHED: + case WORD_BREAKING_FINISHED: /* Fallthru */ case CHAR_BREAKING_FINISHED: + totalWidth += context.getWidth(); + breakResult = BreakTextResult.FINISHED; + break LOOP; + case CHAR_BREAKING_NEED_NEW_LINE: totalWidth += context.getWidth(); + breakResult = BreakTextResult.CONTINUE_CHAR_BREAKING_ON_NL; break LOOP; case CHAR_BREAKING_UNBREAKABLE: - if (totalWidth == 0 && - avail == lineWidth) { + if ((totalWidth == 0 && avail == lineWidth) || + forceOutput) { // We are at the start of the line but could not fit a single character! totalWidth += context.getWidth(); + breakResult = BreakTextResult.CHAR_UNBREAKABLE_BUT_CONSUMED; break LOOP; } else { // We may be at the end of the line, so pick up at next line. + // FIXME: This is very dangerous and has led to infinite + // loops. Needs review. context.setEnd(savedEnd); + context.setNeedsNewLine(true); + breakResult = BreakTextResult.DANGER_RECONSUME_CHAR_ON_NL; break LOOP; } @@ -177,12 +243,14 @@ public static LineBreakResult breakText(LayoutContext c, } else { // Else, finish so it can be put on a new line. totalWidth += context.getWidth(); + breakResult = BreakTextResult.CONTINUE_WORD_BREAKING_ON_NL; break LOOP; } } case WORD_BREAKING_UNBREAKABLE: { if (context.getWidth() >= lineWidth || - context.isFirstCharInLine()) { + context.isFirstCharInLine() || + forceOutput) { // If the word is too long to fit on a line by itself or // if we are at the start of a line, // retry in character breaking mode. @@ -194,11 +262,12 @@ public static LineBreakResult breakText(LayoutContext c, // FIXME: This is very dangerous and has led to infinite // loops. Needs review. context.setEnd(savedEnd); + breakResult = BreakTextResult.DANGER_RECONSUME_WORD_ON_NL; break LOOP; } } } - + context.setStart(context.getEnd()); avail -= context.getWidth(); totalWidth += context.getWidth(); @@ -206,16 +275,44 @@ public static LineBreakResult breakText(LayoutContext c, context.setStart(originalStart); context.setWidth(totalWidth); - + // We need to know this for the next line. context.setFinishedInCharBreakingMode(tryToBreakAnywhere); - return result; + return breakResult; } } - - private static LineBreakResult doBreakText(LayoutContext c, - LineBreakContext context, int avail, CalculatedStyle style, + + /** + * Converts a LineBreakResult returned from doBreakText in + * word-wrapping mode to a BreakTextResult. + * + * Throws a runtime exception if unexpected result found. + */ + private static BreakTextResult toBreakTextResult(LineBreakResult res) { + switch (res) { + case WORD_BREAKING_FINISHED: + return BreakTextResult.FINISHED; + case WORD_BREAKING_NEED_NEW_LINE: + return BreakTextResult.CONTINUE_WORD_BREAKING_ON_NL; + case WORD_BREAKING_UNBREAKABLE: + return BreakTextResult.WORD_UNBREAKABLE_BUT_CONSUMED; + + case CHAR_BREAKING_FINISHED: // Fall-thru + case CHAR_BREAKING_FOUND_WORD_BREAK: // Fall-thru + case CHAR_BREAKING_NEED_NEW_LINE: // Fall-thru + case CHAR_BREAKING_UNBREAKABLE: // Fall-thru + default: + throw new RuntimeException("PROGRAMMER ERROR: Unexpected LineBreakResult from word wrap"); + } + } + + private static LineBreakResult doBreakText( + LayoutContext c, + LineBreakContext context, + int avail, + CalculatedStyle style, boolean tryToBreakAnywhere) { + if (!tryToBreakAnywhere) { return doBreakText(c, context, avail, style, STANDARD_LINE_BREAKER); } else { @@ -227,15 +324,15 @@ private static LineBreakResult doBreakText(LayoutContext c, ToIntFunction measurer = (str) -> c.getTextRenderer().getWidth(c.getFontContext(), font, str); - + String currentString = context.getStartSubstring(); FSTextBreaker lineIterator = STANDARD_LINE_BREAKER.getBreaker(currentString, c.getSharedContext()); - FSTextBreaker charIterator = STANDARD_CHARACTER_BREAKER.getBreaker(currentString, c.getSharedContext()); - + FSTextBreaker charIterator = STANDARD_CHARACTER_BREAKER.getBreaker(currentString, c.getSharedContext()); + return doBreakCharacters(currentString, lineIterator, charIterator, context, avail, letterSpacing, measurer); } } - + /** * Breaks at most one word (until the next word break) going character by character to see * what will fit in. @@ -303,11 +400,12 @@ static LineBreakResult doBreakCharacters( graphicsLength > 0) { // Exact fit.. boolean needNewLine = currentString.length() > left; - + context.setNeedsNewLine(needNewLine); context.setEnd(left + context.getStart()); context.setWidth(graphicsLength); - + context.setUnbreakable(false); + if (left >= currentString.length()) { return LineBreakResult.CHAR_BREAKING_FINISHED; } else if (left >= nextWordBreak) { @@ -340,6 +438,7 @@ static LineBreakResult doBreakCharacters( context.setWidth(graphicsLength); context.setEnd(nextCharBreak + context.getStart()); context.setEndsOnWordBreak(nextCharBreak == nextWordBreak); + context.setUnbreakable(false); if (nextCharBreak >= currentString.length()) { return LineBreakResult.CHAR_BREAKING_FINISHED; @@ -358,8 +457,10 @@ static LineBreakResult doBreakCharacters( context.setWidth(lastGoodGraphicsLength); context.setEnd(lastGoodWrap + context.getStart()); context.setEndsOnWordBreak(lastGoodWrap == nextWordBreak); + context.setUnbreakable(false); if (lastGoodWrap >= currentString.length()) { + context.setNeedsNewLine(false); return LineBreakResult.CHAR_BREAKING_FINISHED; } else if (lastGoodWrap >= nextWordBreak) { return LineBreakResult.CHAR_BREAKING_FOUND_WORD_BREAK; @@ -376,13 +477,15 @@ static LineBreakResult doBreakCharacters( context.setEnd(end + context.getStart()); context.setEndsOnWordBreak(end == nextWordBreak); context.setWidth(splitWidth); - + context.setNeedsNewLine(end < currentString.length()); + return LineBreakResult.CHAR_BREAKING_UNBREAKABLE; } else { // Empty string. context.setEnd(context.getStart()); context.setWidth(0); context.setNeedsNewLine(false); + context.setUnbreakable(false); return LineBreakResult.CHAR_BREAKING_FINISHED; } @@ -498,6 +601,7 @@ static LineBreakResult doBreakTextWords( if (current.graphicsLength <= avail) { context.setWidth(current.graphicsLength); context.setEnd(context.getMaster().length()); + context.setNeedsNewLine(false); // It all fit! return LineBreakResult.WORD_BREAKING_FINISHED; } diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/FloatManager.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/FloatManager.java index 4faace32f..e6745a4ba 100644 --- a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/FloatManager.java +++ b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/FloatManager.java @@ -25,8 +25,10 @@ import java.util.Collections; import java.util.Iterator; import java.util.List; +import java.util.stream.Stream; import com.openhtmltopdf.css.style.CssContext; +import com.openhtmltopdf.css.style.derived.RectPropertySet; import com.openhtmltopdf.render.BlockBox; import com.openhtmltopdf.render.Box; import com.openhtmltopdf.render.LineBox; @@ -37,55 +39,63 @@ * non-floated (block) boxes. */ public class FloatManager { - public static final int LEFT = 1; - public static final int RIGHT = 2; + public enum FloatDirection { + LEFT, + RIGHT; + } /* Lazily created for performance. */ private List _leftFloats = Collections.emptyList(); private List _rightFloats = Collections.emptyList(); - + private final Box _master; public FloatManager(Box master) { this._master = master; } - - private List getAddableFloats(int direction) { + + private List getAddableFloats(FloatDirection direction) { if (getFloats(direction).isEmpty()) { setFloats(direction, new ArrayList<>()); } return getFloats(direction); } - - private void setFloats(int direction, List list) { - if (direction == LEFT) { + + private void setFloats(FloatDirection direction, List list) { + if (direction == FloatDirection.LEFT) { _leftFloats = list; } else { + assert direction == FloatDirection.RIGHT; _rightFloats = list; } } - + public void floatBox(LayoutContext c, Layer layer, BlockFormattingContext bfc, BlockBox box) { if (box.getStyle().isFloatedLeft()) { - position(c, bfc, box, LEFT); - save(box, layer, bfc, LEFT); + position(c, bfc, box, FloatDirection.LEFT); + save(box, layer, bfc, FloatDirection.LEFT); } else if (box.getStyle().isFloatedRight()) { - position(c, bfc, box, RIGHT); - save(box, layer, bfc, RIGHT); + position(c, bfc, box, FloatDirection.RIGHT); + save(box, layer, bfc, FloatDirection.RIGHT); } } public void clear(CssContext cssCtx, BlockFormattingContext bfc, Box box) { if (box.getStyle().isClearLeft()) { - moveClear(cssCtx, bfc, box, getFloats(LEFT)); + moveClear(cssCtx, bfc, box, getFloats(FloatDirection.LEFT)); } if (box.getStyle().isClearRight()) { - moveClear(cssCtx, bfc, box, getFloats(RIGHT)); + moveClear(cssCtx, bfc, box, getFloats(FloatDirection.RIGHT)); } } - private void save(BlockBox current, Layer layer, BlockFormattingContext bfc, int direction) { + private void save( + BlockBox current, + Layer layer, + BlockFormattingContext bfc, + FloatDirection direction) { + Point p = bfc.getOffset(); getAddableFloats(direction).add(new BoxOffset(current, p.x, p.y)); layer.addFloat(current, bfc); @@ -96,7 +106,7 @@ private void save(BlockBox current, Layer layer, BlockFormattingContext bfc, int } private void position(CssContext cssCtx, BlockFormattingContext bfc, - BlockBox current, int direction) { + BlockBox current, FloatDirection direction) { moveAllTheWayOver(current, direction); alignToLastOpposingFloat(cssCtx, bfc, current, direction); @@ -115,25 +125,32 @@ private void position(CssContext cssCtx, BlockFormattingContext bfc, } if (current.getStyle().isCleared()) { - if (current.getStyle().isClearLeft() && direction == LEFT) { - moveAllTheWayOver(current, LEFT); - } else if (current.getStyle().isClearRight() && direction == RIGHT) { - moveAllTheWayOver(current, RIGHT); + if (current.getStyle().isClearLeft() && direction == FloatDirection.LEFT) { + moveAllTheWayOver(current, FloatDirection.LEFT); + } else if (current.getStyle().isClearRight() && direction == FloatDirection.RIGHT) { + moveAllTheWayOver(current, FloatDirection.RIGHT); } moveFloatBelow(cssCtx, bfc, current, getFloats(direction)); } } - - public List getFloats(int direction) { - return direction == LEFT ? _leftFloats : _rightFloats; + + public List getFloats(FloatDirection direction) { + return direction == FloatDirection.LEFT ? _leftFloats : _rightFloats; } - private List getOpposingFloats(int direction) { - return direction == LEFT ? _rightFloats : _leftFloats; + public Stream getFloatStream(FloatDirection direction) { + return getFloats(direction).stream(); } - private void alignToLastFloat(CssContext cssCtx, - BlockFormattingContext bfc, BlockBox current, int direction) { + private List getOpposingFloats(FloatDirection direction) { + return direction == FloatDirection.LEFT ? _rightFloats : _leftFloats; + } + + private void alignToLastFloat( + CssContext cssCtx, + BlockFormattingContext bfc, + BlockBox current, + FloatDirection direction) { List floats = getFloats(direction); if (floats.size() > 0) { @@ -157,9 +174,9 @@ private void alignToLastFloat(CssContext cssCtx, } if (moveOver) { - if (direction == LEFT) { + if (direction == FloatDirection.LEFT) { currentBounds.x = lastBounds.x + last.getWidth(); - } else if (direction == RIGHT) { + } else if (direction == FloatDirection.RIGHT) { currentBounds.x = lastBounds.x - current.getWidth(); } @@ -171,11 +188,12 @@ private void alignToLastFloat(CssContext cssCtx, } } - private void alignToLastOpposingFloat(CssContext cssCtx, - BlockFormattingContext bfc, BlockBox current, int direction) { + private void alignToLastOpposingFloat( + CssContext cssCtx, + BlockFormattingContext bfc, BlockBox current, FloatDirection direction) { List floats = getOpposingFloats(direction); - if (floats.size() > 0) { + if (!floats.isEmpty()) { Point offset = bfc.getOffset(); BoxOffset lastOffset = floats.get(floats.size() - 1); @@ -194,10 +212,10 @@ private void alignToLastOpposingFloat(CssContext cssCtx, } } - private void moveAllTheWayOver(BlockBox current, int direction) { - if (direction == LEFT) { + private void moveAllTheWayOver(BlockBox current, FloatDirection direction) { + if (direction == FloatDirection.LEFT) { current.setX(0); - } else if (direction == RIGHT) { + } else if (direction == FloatDirection.RIGHT) { current.setX(current.getContainingBlock().getContentWidth() - current.getWidth()); } } @@ -208,22 +226,17 @@ private boolean fitsInContainingBlock(BlockBox current) { } private int findLowestY(CssContext cssCtx, List floats) { - int result = 0; - - for (BoxOffset floater : floats) { - Rectangle bounds = floater.getBox().getMarginEdge( - cssCtx, -floater.getX(), -floater.getY()); - if (bounds.y + bounds.height > result) { - result = bounds.y + bounds.height; - } - } - - return result; + return floats + .stream() + .map(floater -> floater.getBox().getMarginEdge(cssCtx, -floater.getX(), -floater.getY())) + .mapToInt(bounds -> bounds.y + bounds.height) + .max() + .orElse(0); } public int getClearDelta(CssContext cssCtx, int bfcRelativeY) { - int lowestLeftY = findLowestY(cssCtx, getFloats(LEFT)); - int lowestRightY = findLowestY(cssCtx, getFloats(RIGHT)); + int lowestLeftY = findLowestY(cssCtx, getFloats(FloatDirection.LEFT)); + int lowestRightY = findLowestY(cssCtx, getFloats(FloatDirection.RIGHT)); int lowestY = Math.max(lowestLeftY, lowestRightY); @@ -235,21 +248,15 @@ private boolean overlaps(CssContext cssCtx, BlockFormattingContext bfc, Point offset = bfc.getOffset(); Rectangle bounds = current.getMarginEdge(cssCtx, -offset.x, -offset.y); - for (BoxOffset floater : floats) { - Rectangle floaterBounds = floater.getBox().getMarginEdge(cssCtx, - -floater.getX(), -floater.getY()); - - if (floaterBounds.intersects(bounds)) { - return true; - } - } - - return false; + return floats + .stream() + .map(floater -> floater.getBox().getMarginEdge(cssCtx, -floater.getX(), -floater.getY())) + .anyMatch(floaterBounds -> floaterBounds.intersects(bounds)); } private void moveFloatBelow(CssContext cssCtx, BlockFormattingContext bfc, Box current, List floats) { - if (floats.size() == 0) { + if (floats.isEmpty()) { return; } @@ -264,7 +271,7 @@ private void moveFloatBelow(CssContext cssCtx, BlockFormattingContext bfc, private void moveClear(CssContext cssCtx, BlockFormattingContext bfc, Box current, List floats) { - if (floats.size() == 0) { + if (floats.isEmpty()) { return; } @@ -287,8 +294,8 @@ private void moveClear(CssContext cssCtx, BlockFormattingContext bfc, } public void removeFloat(BlockBox floater) { - removeFloat(floater, getFloats(LEFT)); - removeFloat(floater, getFloats(RIGHT)); + removeFloat(floater, getFloats(FloatDirection.LEFT)); + removeFloat(floater, getFloats(FloatDirection.RIGHT)); } private void removeFloat(BlockBox floater, List floats) { @@ -302,8 +309,8 @@ private void removeFloat(BlockBox floater, List floats) { } public void calcFloatLocations() { - calcFloatLocations(getFloats(LEFT)); - calcFloatLocations(getFloats(RIGHT)); + calcFloatLocations(getFloats(FloatDirection.LEFT)); + calcFloatLocations(getFloats(FloatDirection.RIGHT)); } private void calcFloatLocations(List floats) { @@ -324,48 +331,47 @@ private void applyLineHeightHack(CssContext cssCtx, Box line, Rectangle bounds) public int getNextLineBoxDelta(CssContext cssCtx, BlockFormattingContext bfc, LineBox line, int containingBlockContentWidth) { - BoxDistance left = getFloatDistance(cssCtx, bfc, line, containingBlockContentWidth, getFloats(LEFT), LEFT); - BoxDistance right = getFloatDistance(cssCtx, bfc, line, containingBlockContentWidth, getFloats(RIGHT), RIGHT); - - int leftDelta; - int rightDelta; - - if (left.getBox() != null) { - leftDelta = calcDelta(cssCtx, line, left); - } else { - leftDelta = 0; - } + BoxDistance left = getFloatDistance(cssCtx, bfc, line, containingBlockContentWidth, getFloats(FloatDirection.LEFT), FloatDirection.LEFT); + BoxDistance right = getFloatDistance(cssCtx, bfc, line, containingBlockContentWidth, getFloats(FloatDirection.RIGHT), FloatDirection.RIGHT); - if (right.getBox() != null) { - rightDelta = calcDelta(cssCtx, line, right); - } else { - rightDelta = 0; - } + int leftDelta = left.getBox() != null ? calcDelta(cssCtx, line, left) : 0; + int rightDelta = right.getBox() != null ? calcDelta(cssCtx, line, right) : 0; return Math.max(leftDelta, rightDelta); } private int calcDelta(CssContext cssCtx, LineBox line, BoxDistance boxDistance) { BlockBox floated = boxDistance.getBox(); + Rectangle rect = floated.getBorderEdge(floated.getAbsX(), floated.getAbsY(), cssCtx); - int bottom = rect.y + rect.height; + RectPropertySet margin = floated.getMargin(cssCtx); + + // NOTE: This was not taking account of margins and thus was not clearing properly. + // See: https://github.com/danfickle/openhtmltopdf/pull/618 + int bottom = (int) Math.ceil(rect.y + rect.height + margin.bottom()); + return bottom - line.getAbsY(); } public int getLeftFloatDistance(CssContext cssCtx, BlockFormattingContext bfc, LineBox line, int containingBlockContentWidth) { - return getFloatDistance(cssCtx, bfc, line, containingBlockContentWidth, getFloats(LEFT), LEFT).getDistance(); + return getFloatDistance(cssCtx, bfc, line, containingBlockContentWidth, getFloats(FloatDirection.LEFT), FloatDirection.LEFT).getDistance(); } public int getRightFloatDistance(CssContext cssCtx, BlockFormattingContext bfc, LineBox line, int containingBlockContentWidth) { - return getFloatDistance(cssCtx, bfc, line, containingBlockContentWidth, getFloats(RIGHT), RIGHT).getDistance(); + return getFloatDistance(cssCtx, bfc, line, containingBlockContentWidth, getFloats(FloatDirection.RIGHT), FloatDirection.RIGHT).getDistance(); } - private BoxDistance getFloatDistance(CssContext cssCtx, BlockFormattingContext bfc, - LineBox line, int containingBlockContentWidth, - List floatsList, int direction) { - if (floatsList.size() == 0) { + private BoxDistance getFloatDistance( + CssContext cssCtx, + BlockFormattingContext bfc, + LineBox line, + int containingBlockContentWidth, + List floatsList, + FloatDirection direction) { + + if (floatsList.isEmpty()) { return new BoxDistance(null, 0); } @@ -373,27 +379,30 @@ private BoxDistance getFloatDistance(CssContext cssCtx, BlockFormattingContext b Rectangle lineBounds = line.getMarginEdge(cssCtx, -offset.x, -offset.y); lineBounds.width = containingBlockContentWidth; - int farthestOver = direction == LEFT ? lineBounds.x : lineBounds.x + lineBounds.width; + int farthestOver = direction == FloatDirection.LEFT ? lineBounds.x : lineBounds.x + lineBounds.width; applyLineHeightHack(cssCtx, line, lineBounds); + BlockBox farthestOverBox = null; - for (int i = 0; i < floatsList.size(); i++) { - BoxOffset floater = floatsList.get(i); + + for (BoxOffset floater : floatsList) { Rectangle fr = floater.getBox().getMarginEdge(cssCtx, -floater.getX(), -floater.getY()); + if (lineBounds.intersects(fr)) { - if (direction == LEFT && fr.x + fr.width > farthestOver) { + if (direction == FloatDirection.LEFT && fr.x + fr.width > farthestOver) { farthestOver = fr.x + fr.width; - } else if (direction == RIGHT && fr.x < farthestOver) { + } else if (direction == FloatDirection.RIGHT && fr.x < farthestOver) { farthestOver = fr.x; } + farthestOverBox = floater.getBox(); } } - if (direction == LEFT) { + if (direction == FloatDirection.LEFT) { return new BoxDistance(farthestOverBox, farthestOver - lineBounds.x); } else { - return new BoxDistance(farthestOverBox,lineBounds.x + lineBounds.width - farthestOver); + return new BoxDistance(farthestOverBox, lineBounds.x + lineBounds.width - farthestOver); } } @@ -403,20 +412,15 @@ public Box getMaster() { public Point getOffset(BlockBox floater) { // FIXME inefficient (but probably doesn't matter) - return getOffset(floater, - floater.getStyle().isFloatedLeft() ? getFloats(LEFT) : getFloats(RIGHT)); + return getOffset(floater, + getFloatStream(floater.getStyle().isFloatedLeft() ? FloatDirection.LEFT : FloatDirection.RIGHT)); } - private Point getOffset(BlockBox floater, List floats) { - for (BoxOffset boxOffset : floats) { - BlockBox box = boxOffset.getBox(); - - if (box.equals(floater)) { - return new Point(boxOffset.getX(), boxOffset.getY()); - } - } - - return null; + private Point getOffset(BlockBox floater, Stream floats) { + return floats.filter(boxOffset -> boxOffset.getBox().equals(floater)) + .findFirst() + .map(boxOffset -> new Point(boxOffset.getX(), boxOffset.getY())) + .orElse(null); } private void performFloatOperation(FloatOperation op, List floats) { @@ -431,8 +435,8 @@ private void performFloatOperation(FloatOperation op, List floats) { } public void performFloatOperation(FloatOperation op) { - performFloatOperation(op, getFloats(LEFT)); - performFloatOperation(op, getFloats(RIGHT)); + performFloatOperation(op, getFloats(FloatDirection.LEFT)); + performFloatOperation(op, getFloats(FloatDirection.RIGHT)); } public static class BoxOffset { @@ -475,8 +479,14 @@ BlockBox getBox() { int getDistance() { return _distance; } + + @Override + public String toString() { + return "BoxDistance [_box=" + _box + ", _distance=" + _distance + "]"; + } } + @FunctionalInterface public interface FloatOperation { public void operate(Box floater); } 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 fddd8893b..9abbc31a6 100644 --- a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/InlineBoxing.java +++ b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/InlineBoxing.java @@ -37,6 +37,7 @@ import com.openhtmltopdf.css.style.CssContext; import com.openhtmltopdf.css.style.derived.BorderPropertySet; import com.openhtmltopdf.css.style.derived.RectPropertySet; +import com.openhtmltopdf.layout.Breaker.BreakTextResult; import com.openhtmltopdf.render.AnonymousBlockBox; import com.openhtmltopdf.render.BlockBox; import com.openhtmltopdf.render.Box; @@ -158,8 +159,6 @@ public static void layoutContent(LayoutContext c, BlockBox box, int initialY, in } boolean inCharBreakingMode = false; - int troublesomeStartPosition = -1; - int troublesomeAttemptCount = 0; do { lbContext.reset(); @@ -190,34 +189,24 @@ public static void layoutContent(LayoutContext c, BlockBox box, int initialY, in needFirstLetter = false; } else { if (style.getWordWrap() != IdentValue.BREAK_WORD) { - StartInlineTextResult result = startInlineText(c, lbContext, inlineBox, space, current, fit, trimmedLeadingSpace, false); + StartInlineTextResult result = startInlineText(c, lbContext, inlineBox, space, current, fit, trimmedLeadingSpace, false, lbContext.possibleEndlessLoop()); if (result == StartInlineTextResult.RECONSUME_BELOW_FLOATS) { + lbContext.newLine(); continue; } } else { - StartInlineTextResult result = startInlineText(c, lbContext, inlineBox, space, current, fit, trimmedLeadingSpace, inCharBreakingMode); + StartInlineTextResult result = startInlineText(c, lbContext, inlineBox, space, current, fit, trimmedLeadingSpace, inCharBreakingMode, lbContext.possibleEndlessLoop()); inCharBreakingMode = lbContext.isFinishedInCharBreakingMode(); if (result == StartInlineTextResult.RECONSUME_BELOW_FLOATS) { + lbContext.newLine(); 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.log(Level.SEVERE, LogMessageId.LogMessageId2Param.GENERAL_FATAL_INFINITE_LOOP_BUG_IN_LINE_BREAKING_ALGO, - lbContext.getStartSubstring(), lbContext.getEnd()); - throw new RuntimeException("Infinite loop bug in break-word line breaking algorithm!"); - } - } - } + } } } if (lbContext.isNeedsNewLine()) { + lbContext.newLine(); + startNewInlineLine(c, box, breakAtLine, blockLayoutDirection, space, current, previous, contentStart, openInlineBoxes, iBMap, minimumLineHeight, markerData, pendingFloats, hasFirstLinePEs, pendingInlineLayers, lineOffset, inlineBox, lbContext); @@ -401,12 +390,17 @@ private enum StartInlineTextResult { * 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 StartInlineTextResult startInlineText( - LayoutContext c, LineBreakContext lbContext, InlineBox inlineBox, - SpaceVariables space, StateVariables current, int fit, - boolean trimmedLeadingSpace, boolean tryToBreakAnywhere) { + LayoutContext c, + LineBreakContext lbContext, + InlineBox inlineBox, + SpaceVariables space, + StateVariables current, + int fit, + boolean trimmedLeadingSpace, + boolean tryToBreakAnywhere, + boolean forceOutput) { lbContext.saveEnd(); CalculatedStyle style = inlineBox.getStyle(); @@ -414,13 +408,16 @@ private static StartInlineTextResult startInlineText( // Layout the text into the remaining width on this line. Will only go to the end of the line (at most) // and will produce one InlineText object. InlineText inlineText = layoutText( - c, style, space.remainingWidth - fit, lbContext, false, inlineBox.getTextDirection(), tryToBreakAnywhere, space.maxAvailableWidth - fit); + c, style, space.remainingWidth - fit, lbContext, false, inlineBox.getTextDirection(), tryToBreakAnywhere, space.maxAvailableWidth - fit, forceOutput); if (style.hasLetterSpacing()) { inlineText.setLetterSpacing(style.getFloatPropertyProportionalWidth(CSSName.LETTER_SPACING, 0, c)); } - - if (lbContext.isUnbreakable() && !current.line.isContainsContent()) { + + if (lbContext.isUnbreakable() && + !current.line.isContainsContent() && + !forceOutput) { + int delta = c.getBlockFormattingContext().getNextLineBoxDelta(c, current.line, space.maxAvailableWidth); if (delta > 0) { @@ -441,7 +438,15 @@ private static StartInlineTextResult startInlineText( } if (!lbContext.isUnbreakable() || - (lbContext.isUnbreakable() && !current.line.isContainsContent())) { + (lbContext.isUnbreakable() && + !current.line.isContainsContent() && + lbContext.getEnd() > lbContext.getStart()) || + forceOutput) { + + if (forceOutput) { + XRLog.log(Level.SEVERE, LogMessageId.LogMessageId1Param.GENERAL_FORCED_OUTPUT_TO_AVOID_INFINITE_LOOP, lbContext.getCalculatedSubstring()); + } + // We can use the inline text by adding it to the current inline layout box. // We also mark the text as consumed by the line break context and reduce the width // we have remaining on this line. @@ -553,8 +558,8 @@ private static InlineLayoutBox addFirstLetterBox(LayoutContext c, LineBox curren currentIB.addInlineChild(c, iB); current.setContainsContent(true); - InlineText text = layoutText(c, iB.getStyle(), remainingWidth, lbContext, true, textDirection, true, maxAvailableWidth); - + InlineText text = layoutText(c, iB.getStyle(), remainingWidth, lbContext, true, textDirection, true, maxAvailableWidth, false); + if (iB.getStyle().hasLetterSpacing()) { text.setLetterSpacing(iB.getStyle().getFloatPropertyProportionalWidth(CSSName.LETTER_SPACING, 0, c)); } @@ -1098,18 +1103,28 @@ private static void finishPendingInlineLayers(LayoutContext c, List layer } } - private static InlineText layoutText(LayoutContext c, CalculatedStyle style, int remainingWidth, - LineBreakContext lbContext, boolean needFirstLetter, - byte textDirection, boolean tryToBreakAnywhere, int lineWidth) { + private static InlineText layoutText( + LayoutContext c, + CalculatedStyle style, + int remainingWidth, + LineBreakContext lbContext, + boolean needFirstLetter, + byte textDirection, + boolean tryToBreakAnywhere, + int lineWidth, + boolean forceOutput) { + InlineText result = new InlineText(); String masterText = lbContext.getMaster(); - + if (needFirstLetter) { masterText = TextUtil.transformFirstLetterText(masterText, style); lbContext.setMaster(masterText); Breaker.breakFirstLetter(c, lbContext, remainingWidth, style); } else { - Breaker.breakText(c, lbContext, remainingWidth, style, tryToBreakAnywhere, lineWidth); + BreakTextResult breakResult = + Breaker.breakText(c, lbContext, remainingWidth, style, tryToBreakAnywhere, lineWidth, forceOutput); + lbContext.checkConsistency(breakResult); } result.setMasterText(masterText); @@ -1117,7 +1132,7 @@ private static InlineText layoutText(LayoutContext c, CalculatedStyle style, int result.setWidth(lbContext.getWidth()); result.setTextDirection(textDirection); result.setEndsOnSoftHyphen(lbContext.isEndsOnSoftHyphen()); - + return result; } diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/LineBreakContext.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/LineBreakContext.java index 6cf733859..352228219 100644 --- a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/LineBreakContext.java +++ b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/LineBreakContext.java @@ -19,6 +19,8 @@ */ package com.openhtmltopdf.layout; +import com.openhtmltopdf.layout.Breaker.BreakTextResult; + /** * A bean which serves as a way for the layout code to pass information to the * line breaking code and for the line breaking code to pass instructions back @@ -37,8 +39,7 @@ public enum LineBreakResult { CHAR_BREAKING_FINISHED, WORD_BREAKING_FINISHED; } - - + private String _master; private int _start; private int _end; @@ -52,20 +53,53 @@ public enum LineBreakResult { private boolean _endsOnWordBreak; private boolean _finishedInCharBreakingMode; private boolean _isFirstChar; - - public int getLast() { - return _master.length(); - } - + + // These keep track of our attempts to move to a newline + // before outputting the same content. + // Due to several infinite loop bugs, we only allow + // a defined number of attempts before forcing the output of + // text (with a log message). + private int _lastNewLineStartPosition; + private int _newlineAttemptsForLastStartPosition; + public void reset() { _width = 0; _unbreakable = false; _needsNewLine = false; + _endsOnNL = false; _endsOnSoftHyphen = false; _nextWidth = 0; _endsOnWordBreak = false; + _finishedInCharBreakingMode = false; + _isFirstChar = false; } - + + /** + * Record a newline attempt. This should be called + * at each newline and then possibleEndlessLoop method can + * be called to check if excessive attempts have been made to + * output the same character on newlines. + */ + public void newLine() { + if (this.getStart() == _lastNewLineStartPosition) { + _newlineAttemptsForLastStartPosition++; + } else { + _lastNewLineStartPosition = this.getStart(); + _newlineAttemptsForLastStartPosition = 1; + } + } + + /** + * @see {@link #newLine()} + */ + public boolean possibleEndlessLoop() { + return _newlineAttemptsForLastStartPosition > 5; + } + + public int getLast() { + return _master.length(); + } + public int getEnd() { return _end; } @@ -206,4 +240,48 @@ public boolean isFirstCharInLine() { public void setFirstCharInLine(boolean isFirstChar) { _isFirstChar = isFirstChar; } + + /** + * Given the result of text breaking, makes some sanity preserving + * asserts to check the state of this object. + */ + public void checkConsistency( + BreakTextResult breakResult) { + + switch (breakResult) { + case CONTINUE_CHAR_BREAKING_ON_NL: + assert this.getStart() < this.getEnd(); + assert !this.isUnbreakable(); + assert this.isNeedsNewLine(); + break; + case CONTINUE_WORD_BREAKING_ON_NL: + assert this.getStart() < this.getEnd(); + assert !this.isUnbreakable(); + assert this.isNeedsNewLine(); + break; + case DANGER_RECONSUME_CHAR_ON_NL: + assert this.isUnbreakable(); + assert this.getStart() <= this.getEnd(); + break; + case DANGER_RECONSUME_WORD_ON_NL: + assert this.isUnbreakable(); + assert this.getStart() <= this.getEnd(); + break; + case FINISHED: + assert this.isFinished(); + assert this.getStart() < this.getEnd(); + assert !this.isUnbreakable(); + assert !this.isNeedsNewLine(); + break; + case WORD_UNBREAKABLE_BUT_CONSUMED: + assert this.isUnbreakable(); + assert this.getStart() < this.getEnd(); + break; + case CHAR_UNBREAKABLE_BUT_CONSUMED: + assert this.isUnbreakable(); + assert this.getStart() < this.getEnd(); + break; + } + } + } diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/render/FlowingColumnContainerBox.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/render/FlowingColumnContainerBox.java index 88c12f824..a5b829935 100644 --- a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/render/FlowingColumnContainerBox.java +++ b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/render/FlowingColumnContainerBox.java @@ -136,8 +136,8 @@ private void layoutFloats(TreeMap columns, List columnMap, PersistentBFC bfc, int columnCount, int colWidth, int colGap) { - List floatsL = this.getPersistentBFC().getFloatManager().getFloats(FloatManager.LEFT); - List floatsR = this.getPersistentBFC().getFloatManager().getFloats(FloatManager.RIGHT); + List floatsL = this.getPersistentBFC().getFloatManager().getFloats(FloatManager.FloatDirection.LEFT); + List floatsR = this.getPersistentBFC().getFloatManager().getFloats(FloatManager.FloatDirection.RIGHT); layoutFloats(columnMap, floatsL, columnCount, colWidth, colGap); layoutFloats(columnMap, floatsR, columnCount, colWidth, colGap); @@ -173,8 +173,8 @@ private int adjustUnbalanced(LayoutContext c, Box child, int colGap, int colWidt final List pages = c.getRootLayer().getPages(); final boolean haveFloats = - !this.getPersistentBFC().getFloatManager().getFloats(FloatManager.LEFT).isEmpty() || - !this.getPersistentBFC().getFloatManager().getFloats(FloatManager.RIGHT).isEmpty(); + !this.getPersistentBFC().getFloatManager().getFloats(FloatManager.FloatDirection.LEFT).isEmpty() || + !this.getPersistentBFC().getFloatManager().getFloats(FloatManager.FloatDirection.RIGHT).isEmpty(); // We only need the tree map if we have floats. final TreeMap columnMap = haveFloats ? new TreeMap<>() : null; diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/util/LogMessageId.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/util/LogMessageId.java index 85d247454..32985bfc4 100644 --- a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/util/LogMessageId.java +++ b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/util/LogMessageId.java @@ -144,6 +144,7 @@ enum LogMessageId1Param implements LogMessageId { GENERAL_PDF_URI_IN_HREF_IS_NOT_A_VALID_URI(XRLog.GENERAL, "'{}' in href is not a valid URI, will be skipped"), GENERAL_PDF_FOUND_FORM_CONTROL_WITH_NO_ENCLOSING_FORM(XRLog.GENERAL, "Found form control ({}) with no enclosing form. Ignoring."), GENERAL_PDF_A_ELEMENT_DOES_NOT_HAVE_OPTION_CHILDREN(XRLog.GENERAL, "A <{}> element does not have