Closed Bug 807897 Opened 7 years ago Closed 7 years ago
Setting "z-index" should convert a flex item into a stacking context (even for "position:static")
If a flex item has its z-index set (to anything other than "auto"), that needs to make it create a stacking context, even if it's "position:static". http://dev.w3.org/csswg/css3-flexbox/#painting I've got some code to do this already; patch coming soon. This blocks bug 783474, because that bug's patch stacks on top of this bug's patch.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #677676 - Flags: review?(dbaron)
7 years ago
Comment on attachment 677676 [details] [diff] [review] fix w/ reftest, v1 Maybe you should change nsIFrame::IsPseudoStackingContextFromStyle as well (to check IsFlexItem() && zindex != auto), though really that would just be for completeness, since it doesn't matter to any of the current callers. I'm going to mark this r=dbaron, though I should perhaps have transferred review to roc initially. I've at least cc:ed him.
Attachment #677676 - Flags: review?(dbaron) → review+
(In reply to David Baron [:dbaron] from comment #2) > Comment on attachment 677676 [details] [diff] [review] > fix w/ reftest, v1 > > Maybe you should change nsIFrame::IsPseudoStackingContextFromStyle as well > (to check IsFlexItem() && zindex != auto), though really that would just be > for completeness, since it doesn't matter to any of the current callers. I'm hesitant to do that, for 3 reasons: (a) as you say, it doesn't matter to the current callers. (it's only called in table-layout) (b) the documentation for that method says "looking only at style", whereas IsFlexItem() looks at the frame-tree. (though maybe that doesn't matter since it's cheap... I could easily change the documentation to say "and the frame tree" I suppose.) (c) IsPseudoStackingContextFromStyle() appears to already be incomplete -- it's missing e.g. checks for SVG effects, CSS transforms, and some extra fanciness for animated opacity that we check in nsIFrame::HasOpacity(). (All of those things end up toggling this line in BuildDisplayListForChild(): > 2205 // If you change this, also change IsPseudoStackingContextFromStyle() > 2206 pseudoStackingContext = true; http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsFrame.cpp?mark=2196-2206#2196 ...and per that comment at line 2205, I suspect those things *should* be checked in IsPseudoStackingContextFromStyle(), but they aren't currently). So -- are you OK with this landing as-is, without that change?
(In reply to Daniel Holbert [:dholbert] from comment #3) > So -- are you OK with this landing as-is, without that change? yes.
OS: Linux → All
Hardware: x86_64 → All
Status: ASSIGNED → RESOLVED
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.