[webkit-reviews] review granted: [Bug 96188] inline-flex baseline is sometimes wrong : [Attachment 166972] Patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has granted Tony Chang <tony at chromium.org>'s
request for review:
Bug 96188: inline-flex baseline is sometimes wrong
https://bugs.webkit.org/show_bug.cgi?id=96188

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

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=166972&action=review


Just fix the marginBefore issue. The rest looks good.

For thoroughness sake, might want to add a test to verify the following text
from the spec:

When calculating the baseline according to the above rules, if the box
contributing a baseline has an ‘overflow’ value that allows scrolling, the box
must be treated as being in its initial scroll position for the purpose of
determining its baseline.

> Source/WebCore/rendering/RenderFlexibleBox.cpp:242
> +	   return baseline + marginBefore();

marginBefore is wrong here. For horizontal we need marginTop and for vertical
we need marginRight.

> Source/WebCore/rendering/RenderFlexibleBox.cpp:380
> +

Extra line break?