[webkit-reviews] review denied: [Bug 68129] change RenderFlexibleBox to act on logical coordinates : [Attachment 107899] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Sep 20 11:41:07 PDT 2011

Dave Hyatt <hyatt at apple.com> has denied Ojan Vafai <ojan at chromium.org>'s
request for review:
Bug 68129: change RenderFlexibleBox to act on logical coordinates

Attachment 107899: Patch

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

Switch all the queries of writing mode to use isHorizontalWritingMode() instead
of style()->isHorizontalWritingMode().

The one other observation I have is make your helper functions do as much work
as possible to avoid switching on writing mode too much. For example if you
only ever look at border and padding together, then combine those into a single
helper rather than calling two functions, one for border and one for padding.

Things mostly look correct to me except for the orthogonal case mentioned

> Source/WebCore/rendering/RenderFlexibleBox.cpp:155
> +	   return child->maxPreferredLogicalWidth() -
logicalBorderWidthForChild(child) - logicalScrollbarHeightForChild(child) -

You can't use maxPreferredLogicalWidth if your writing mode is orthogonal to
your parent's. I'm not sure what you should be using, since I can't remember
what the writing-mode spec said. It's either 0 or the height of the viewport or
something. I can't recall, but should be in the spec.

More information about the webkit-reviews mailing list