RESOLVED DUPLICATE of bug 130540 130552
Document assumptions made in TrailingObjects::updateMidpointsForTrailingBoxes
https://bugs.webkit.org/show_bug.cgi?id=130552
Summary Document assumptions made in TrailingObjects::updateMidpointsForTrailingBoxes
Myles C. Maxfield
Reported 2014-03-20 18:02:33 PDT
Document assumptions made in TrailingObjects::updateMidpointsForTrailingBoxes
Attachments
Patch (2.90 KB, patch)
2014-03-20 18:05 PDT, Myles C. Maxfield
mmaxfield: review-
Myles C. Maxfield
Comment 1 2014-03-20 18:05:06 PDT
Simon Fraser (smfr)
Comment 2 2014-03-20 18:09:56 PDT
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
Myles C. Maxfield
Comment 3 2014-03-20 18:21:07 PDT
Comment on attachment 227366 [details] Patch Rather than documenting bad design, I should fix the bad design.
Myles C. Maxfield
Comment 4 2014-03-25 12:48:03 PDT
*** This bug has been marked as a duplicate of bug 130540 ***
Note You need to log in before you can comment on or make changes to this bug.