[webkit-reviews] review granted: [Bug 109956] -webkit-margin-collapse: separate doesn't work correctly for before margins : [Attachment 189523] Patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has granted Andrei Bucur <abucur at adobe.com>'s
request for review:
Bug 109956: -webkit-margin-collapse: separate doesn't work correctly for before
margins
https://bugs.webkit.org/show_bug.cgi?id=109956

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

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


r=me

> Source/WebCore/rendering/RenderBlock.cpp:2079
> +	       if (marginInfo.canCollapseWithMarginBefore())
> +		   setLogicalHeight(logicalHeight() +
marginBeforeForChild(child));
> +	       else
> +		   setLogicalHeight(logicalHeight() + marginInfo.margin() +
marginBeforeForChild(child));

I think this would a bit better as:

LayoutUnit separateMargin = !marginInfo.canCollapseWithMarginBefore() ?
marginInfo.margin() : 0;
setLogicalHeight(logicalHeight() + marginBeforeForChild(child) +
separateMargin);