Bug 106374 – REGRESSION(r136967): margin-top + overflow:hidden causes incorrect layout for internal floated elements

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Tony Chang]

This was reported at http://crbug.com/168511.

The test case is in the URL.  It looks like we're getting the wrong margin top for the second float?  In Firefox, the 3 white boxes are on the same horizontal line.

Robert, can you take a look?

[Robert Hogan]

Created attachment 181782 [details]
Patch

[Tony Chang]

Comment on attachment 181782 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=181782&action=review

Looks good to me, but might want to have hyatt take a quick peek.

> LayoutTests/fast/block/margin-collapse/self-collapsing-block-with-overflow-hidden-and-float-child.html:21
> +<div class="outside">
> +    <div class="inside"></div>
> +    <div class="inside"></div>
> +</div>

Nit: Can you either add some text saying what the expected output should be or change this to a js test (e.g., using check-layout.js) that will print PASS/FAIL?  Ref-tests are also slower than dumpAsText tests.

[Robert Hogan]

Created attachment 181952 [details]
Patch

[Dave Hyatt]

Comment on attachment 181952 [details]
Patch

This will break the case where the overflow:hidden self-collapsing block legitimately has to clear a float because it doesn't fit, etc. This is the reason we don't just check style()->clear(), since you can clear for other reasons besides just having the style set. The more interesting question to me is why this block is claiming that it has clearance when it obviously shouldn't.

[Robert Hogan]

Created attachment 181975 [details]
Patch

[Dave Hyatt]

Comment on attachment 181975 [details]
Patch

r=me

[WebKit Review Bot]

Comment on attachment 181975 [details]
Patch

Clearing flags on attachment: 181975

Committed r139337: <http://trac.webkit.org/changeset/139337>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.