[webkit-reviews] review granted: [Bug 69808] Implement -webkit-flex-align for cross axis alignment in flex-flow: row : [Attachment 110749] Patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has granted Tony Chang <tony at chromium.org>'s
request for review:
Bug 69808: Implement -webkit-flex-align for cross axis alignment in flex-flow:
row
https://bugs.webkit.org/show_bug.cgi?id=69808

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

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


r=me

> Source/WebCore/rendering/RenderFlexibleBox.cpp:573
> +	       Length height = isHorizontalFlow() ? child->style()->height() :
child->style()->width();

RenderStyle does have helpers for margins but not for logical height. If you
wanted to add a logicalHeightUsing method to RenderStyle helper so you could
say child->style()->logcialHeightUsing(style()) I would be fine with that. Not
required though.

Also note you don't handle maxHeight clamping here. Not sure if you have to or
not. I'd put a FIXME: in asking and not worry about it for now.