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

[bugzilla-daemon at webkit.org]

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
https://bugs.webkit.org/show_bug.cgi?id=68129

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

------- 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
below.

> Source/WebCore/rendering/RenderFlexibleBox.cpp:155
> +	   return child->maxPreferredLogicalWidth() -
logicalBorderWidthForChild(child) - logicalScrollbarHeightForChild(child) -
logicalPaddingWidthForChild(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.