[webkit-reviews] review denied: [Bug 80394] CSS 2.1 failure: margin-collapse-clear-012 fails : [Attachment 160500] Patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Robert Hogan <robert at webkit.org>'s
request for review:
Bug 80394: CSS 2.1 failure: margin-collapse-clear-012 fails
https://bugs.webkit.org/show_bug.cgi?id=80394

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

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


Couple of comments:

> Source/WebCore/rendering/RenderBlock.cpp:2059
> +	   bool wouldCollapseMarginsWithParent = true;

Minor optimization, but it seems like you could initialize this to
canCollapseMarginAfterWithChildren. That way if the bit is already set to
false, you won't waste time doing the lookahead.

> Source/WebCore/rendering/RenderBlock.cpp:2089
> +    LayoutUnit logicalTop = yPos + heightIncrease;
> +    // After margin collapsing, one of our floats may now intrude into the
child.
> +    if (containsFloats() && child->isRenderBlock() &&
lowestFloatLogicalBottom() > logicalTop)
> +	   toRenderBlock(child)->addIntrudingFloats(this,
logicalLeftOffsetForContent(), logicalTop);

I'm not following this change. It doesn't seem like it should be necessary.