Closed Bug 406568 Opened 15 years ago Closed 15 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: phiw2, Assigned: roc)

References

()

Details

(Keywords: regression, testcase)

Attachments

(4 files)

Attached image 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.
Attached file Testcase #1
Keywords: testcase
OS: Mac OS X → All
Hardware: Macintosh → All
Summary: ElPais.com incorrect positioned link boxes after nested floats → {inc} ElPais.com incorrect positioned link boxes after nested floats
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...
Assignee: nobody → roc
Status: NEW → ASSIGNED
Attached patch fixSplinter Review
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.
Attachment #291369 - Flags: superreview?(dbaron)
Attachment #291369 - Flags: review?(dbaron)
This should be a P1 blocker since it's a very visible layout regression on at least one big site.
Flags: blocking1.9?
Priority: -- → P1
Plusing based on comment 5
Flags: blocking1.9? → blocking1.9+
Whiteboard: [needs review]
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
Attachment #291369 - Flags: superreview?(dbaron)
Attachment #291369 - Flags: superreview+
Attachment #291369 - Flags: review?(dbaron)
Attachment #291369 - Flags: review+
Checked in
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Flags: in-testsuite+
Whiteboard: [needs review]
No longer depends on: 453486
Depends on: 442542
You need to log in before you can comment on or make changes to this bug.