Closed Bug 858332 Opened 7 years ago Closed 7 years ago

Make flex items pseudo-stacking contexts

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Per http://krijnhoetmer.nl/irc-logs/css/20130403#l-455 , the CSSWG has resolved that flex items should be painted as pseudo-stacking contexts.

Filing this bug on implementing that change.
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: x86_64 → All
(I filed bug 858333 on the same change for table cells. Ideally, this bug and that bug should land around the same time, so that we're consistent on this behavior.)
Attached patch fix v1 (obsolete) — Splinter Review
This patch changes the paint flags we use for flex items, so that they'll be pseudo-stacking-contexts, and it adds a reftest to test for that behavior.

This patch also marks one test as failing -- the test compares a flex container (in the testcase) vs. a table (in the reference case), and there's some overlap between adjacent flex items / table-cells.  Until bug 858333 is fixed, the overlap will render differently between tables vs. flex items.
Attachment #734015 - Flags: review?(dbaron)
Attached patch fix v1bSplinter Review
No new code changes in this patch (this is a one-liner, for actual code changes).  Just test changes -- I tweaked the initial reftest a bit, and added a second reftest to explicitly ensure that we're making flex items *pseudo* stacking contexts, rather than "full" stacking contexts, by setting interleaving "z-index" values on children inside adjacent flex items. (The children overflow their parents (the flex items), so that they can visibly overlap & interleave).
Attachment #734015 - Attachment is obsolete: true
Attachment #734015 - Flags: review?(dbaron)
Attachment #735469 - Flags: review?(dbaron)
Comment on attachment 735469 [details] [diff] [review]
fix v1b

dbaron's out this week and traveling next week -- mats, if you happen to get to this before he does and feel comfortable reviewing, that'd be great!
Attachment #735469 - Flags: review?
Attachment #735469 - Flags: review? → review?(matspal)
Comment on attachment 735469 [details] [diff] [review]
fix v1b

The code change looks fine as such, but perhaps you should wait with this
until the corresponding table cell issues in bug 858333 are resolved?
Attachment #735469 - Flags: review?(matspal) → review+
Agreed. Looking at that bug now.
Attachment #735469 - Flags: review?(dbaron)
(In reply to Mats Palmgren [:mats] from comment #6)
> The code change looks fine as such, but perhaps you should wait with this
> until the corresponding table cell issues in bug 858333 are resolved?

dbaron says we probably don't want to gate this on bug 858333 -- that one turned out to be trickier, due to the complex way that tables are painted, and it's still not even clear what the right behavior should be.

This bug, in comparison, is clearly-defined and simple to implement.  Plus, it'd be best to get this in ASAP, before too many people start developing complex flexbox-based web apps that this change might potentially break.

So: I'm going to go ahead and land this.
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/cf3a2de4af83
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
[fantasai, do you know if this made it into the spec? Right now http://dev.w3.org/csswg/css-flexbox/#painting starts with "Flex items paint exactly the same as inline blocks", which doesn't seem right.]
Flags: needinfo?(fantasai.bugs)
Sorry, disregard Comment 11; I'd forgotten that inline blocks themselves are pseudo-stacking contexts. (They are, per http://lists.w3.org/Archives/Public/www-style/2012Jul/0546.html )

So, looks like this did make it into the spec, via the text quoted in comment 11. Hooray!
Flags: needinfo?(fantasai.bugs)
You need to log in before you can comment on or make changes to this bug.