[webkit-reviews] review denied: [Bug 118526] [CSS Regions] Propagate overflow from the flow thread to the first and last region : [Attachment 207254] Patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Andrei Bucur <abucur at adobe.com>'s
request for review:
Bug 118526: [CSS Regions] Propagate overflow from the flow thread to the first
and last region
https://bugs.webkit.org/show_bug.cgi?id=118526

Attachment 207254: Patch
https://bugs.webkit.org/attachment.cgi?id=207254&action=review

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


> Source/WebCore/rendering/RenderRegion.cpp:137
> +// FIXME: This is wrong for horizontal overflow because the overflow
rectangle doesn't take the region size into account.
> +LayoutRect RenderRegion::layoutOverflowRectForFlowThreadPortion(const
LayoutRect& flowThreadPortionRect, bool isFirstPortion, bool isLastPortion)
const

Is there no way to share more code with the other overflow function? There's a
lot of similar logic here.

> Source/WebCore/rendering/RenderRegion.cpp:375
> +    if (style()->writingMode() != m_flowThread->style()->writingMode()) {
> +	   if (m_flowThread->style()->writingMode() == RightToLeftWritingMode
|| style()->writingMode() == RightToLeftWritingMode)
> +	       layoutRect.setX(width() - layoutRect.maxX());
> +	   else if (m_flowThread->style()->writingMode() ==
BottomToTopWritingMode || style()->writingMode() == BottomToTopWritingMode)
> +	       layoutRect.setY(height() - layoutRect.maxY());
> +    }

This does not seem right to me.