406568 - {inc} ElPais.com incorrect positioned link boxes after nested floats

Status: RESOLVED FIXED

(Core :: Layout: Floats, defect, P1)

Description

[philippe (part-time)]

Attachment: screenshot

Midway down the page, an horizontal block contains multiple (nested) floats and some small blocks of links 'Ver más vídeos', 'Anterior', 'siguiente', and a larger list of 'sports' links on the right. The boxes have a blue background. Those small links are place on top of the main box where they should be placed at the bottom (with position:absolute; bottom:xxpx)

regression: (hourly builds, test with OS X)
20071129_1803 build fails
20071129_1431 build works

http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1196375460&maxdate=1196388179

--> Bug 320378.

Comment 1

[Mats Palmgren (inactive)]

Attachment: Testcase #1

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

So I guess here we're reflowing everything assuming no clearance and positioning the abs-pos element based on that, then we're reflowing again with clearance but skipping reflow of the abs-pos element because it's not dirty. I'm not quite sure how this worked before or 320378 regressed it though...

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: further reduced testcase

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Basically the problem is that we reflow abs-pos children when the computed block size may be bogus because we're going to reflow again so we skipped a bunch of lines. This shouldn't be a problem --- the children should be reflowed again when the size changes back to the real size on the second reflow pass. But it turns out that when we're bailing out due to a clearance change we never get around to doing FinishReflowChild on the blocks that will be reflowed again, so we never set the block's frame size to the bogus value. So in this testcase the block has height 200px, then it computes bogus height 0 and reflows abs-pos kids assuming that height, *then* it computes the correct size of 200px again but it still thinks the old size is 200px so it doesn't reflow the abs-pos kids again.

Calling FinishReflowChild on blocks that will be reflowed again doesn't seem like a great idea, and it also seems tricky and possibly risky, so I'm taking the easy way out and just avoiding reflow of abs-pos kids when the block will be reflowed again and the block height can't be trusted. This now depends on *not* setting the block frame size until the real reflow has finished.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This should be a P1 blocker since it's a very visible layout regression on at least one big site.

Comment 6

[Mike Schroepfer]

Plusing based on comment 5

Comment 7

[David Baron :dbaron:]

Comment on attachment 291369 [details] [diff] [review]
fix

So if WillReflowAgainForClearance is true, that means we'll reflow again without setting the size?  It might be worth adding a comment saying so.

r+sr=dbaron

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Checked in