Closed Bug 821426 Opened 7 years ago Closed 7 years ago

{inc} Incremental layout of flexbox w/ stretched child leaves grandchild un-stretched


(Core :: Layout, defect)

Not set





(Reporter: dholbert, Assigned: dholbert)




(2 files, 1 obsolete file)

 1. Load attached testcase.
 2. Hover / un-hover the green box.

EXPECTED RESULTS: Box gets a border, and then loses the border when you un-hover. No red visible.

ACTUAL RESULTS: Red becomes visible when you hover; stays visible after you un-hover.

This happens because our flex container reflows its children twice; once to measure their height, and once as an "actual" reflow with stretched widths. When we're doing an incremental reflow, the flex container's first "measuring" reflow ends up clearing the dirty bits on grandchildren and fixing them at their new sizes, but we don't call FinishReflowChild on the actual flex items themselves, so _they_ don't get new sizes. (this is an optimization, because we know this reflow's sizes are temporary anyway).  This ends up causing us trouble, because the container's second "real" reflow checks the new size against the old size, sees that they're the same, and hence doesn't end up fully reflowing the grandchildren, because it doesn't look like they need it.

To fix this, I think we need to call FinishReflowChild() on the flex items after all, after the "measuring" reflow, even though we know that the reflow won't give us their final positions.  We need to do this so that their _actual_ reflow will end up visiting descendants and blowing away any positioning/sizing that was set during the "measuring" reflow.

(See also bug 820111 comment 6, where I also explained this.)
Attached file testcase 1
Here's the testcase. It gives EXPECTED RESULTS in chrome (dev channel) & opera.
Assignee: nobody → dholbert
OS: Linux → All
Hardware: x86_64 → All
Attached patch fix v1 (obsolete) — Splinter Review
Attachment #691947 - Flags: review?(dbaron)
Attached patch fix v1aSplinter Review
(Tweaked patch to name its reftest "-1a" instead of "-1", since bug 821449's patch will add another test "-1b" that will share the same reference case.)
Attachment #691947 - Attachment is obsolete: true
Attachment #691947 - Flags: review?(dbaron)
Attachment #692209 - Flags: review?(dbaron)
Comment on attachment 692209 [details] [diff] [review]
fix v1a

Attachment #692209 - Flags: review?(dbaron) → review+
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
You need to log in before you can comment on or make changes to this bug.