Document assumptions made in TrailingObjects::updateMidpointsForTrailingBoxes
Created attachment 227366 [details] Patch
Comment on attachment 227366 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227366&action=review > Source/WebCore/rendering/line/TrailingObjects.cpp:56 > + // There is something subtle going on here. This function gets called in two cases: > + // 1) In BreakingContext::handleEndOfLine(). The offset below can be 0 > + // in this case (meaning that the code below sets the offset to 4B), but that is okay > + // because BreakingContext::handleEndOfLine() immediately fixes up the offset by calling > + // increment() on the iterator, which detects offsets that are beyond the end of its node > + // and moves the iterator to the next node. > + // 2) When BreakingContext::handleText() detects multiple whitespace characters in a row. In > + // that case, the only way that the offset can be 0 is if the first two characters are > + // whitespace. However, LineBreaker::nextSegmentBreak() calls > + // LineBreaker::skipLeadingWhitespace() to skip over such whitespace before calling > + // BreakingContext::handleText(). > + // Therefore, this -1 wrapping is benign (albeit brittle). I think it would be better to make the code more self-documenting (and safe) than having to write a comment like this. > Source/WebCore/rendering/line/TrailingObjects.cpp:57 > lineMidpointState.midpoints()[trailingSpaceMidpoint].setOffset(lineMidpointState.midpoints()[trailingSpaceMidpoint].offset() -1); You should add a space before the 1
Comment on attachment 227366 [details] Patch Rather than documenting bad design, I should fix the bad design.
*** This bug has been marked as a duplicate of bug 130540 ***