[webkit-reviews] review denied: [Bug 107880] [CSS Regions] Region boxes should respect -shape-inside CSS property : [Attachment 189473] Updated patch, based on Mihnea's review + test fixed for cr-linux

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Zoltan Horvath <zoltan at webkit.org>'s
request for review:
Bug 107880: [CSS Regions] Region boxes should respect -shape-inside CSS
property
https://bugs.webkit.org/show_bug.cgi?id=107880

Attachment 189473: Updated patch, based on Mihnea's review + test fixed for
cr-linux
https://bugs.webkit.org/attachment.cgi?id=189473&action=review

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=189473&action=review


r=me

Remember that logicalHeightForLine is really just a heuristic that can be
wrong. You should make a more complex test case that actually has multiple
paragraphs inside the flow thread to illustrate this issue.

Basically just make sure to add in the offsetFromLogicalTopOfFirstPage and
you'll be fine, but let's get a test case that covers this.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:84
> +	   LayoutUnit offset = block->logicalHeight() +
logicalHeightForLine(block, false);

This offset isn't correct. It needs to be in the coordinates of the object that
you called regionAtBlockOffset on. That's the enclosingRenderFlowThread().