z-index of auto being treated as zero [POS]

VERIFIED FIXED in mozilla0.9

Status

Core Graveyard
GFX
P3
normal
VERIFIED FIXED
19 years ago
9 years ago

People

(Reporter: dbaron, Assigned: roc)

Tracking

({css2, testcase})

Trunk
mozilla0.9
css2, testcase

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: (py8ieh: check for the bug with negative z-index being behind the body vs behind the canvas), URL)

(Reporter)

Description

19 years ago
I think you're interpreting a z-index of 'auto' (which is the default)
incorrectly.  My understanding (check this, though) is that it should *not*
create a new local stacking context for an element with z-index of auto.
Therefore, the z-indexes of children of an element with z-index of auto are in
the same stacking context as that element, so the children can mix with the
siblings of the element.

This problem is seen in the test at the above URL.  It shows up most clearly in
the last three tests, where (incorrectly) the colored square with the number 3
is above (layer-wise) the one with the number 2 (henceforth 3/2), 5/4, and
7/6.   Also, in the 4th test in the page, 4/3.  In all the tests, you should
see 1/2, 2/3, 3/4, 4/5, ... 7/8, 1/8.

I see this problem in:
* June 4 linux source pull
* May 20 Windows 95 build.

Updated

18 years ago
Assignee: rickg → peterl

Comment 1

18 years ago
Peter -- can you look at this please? It appears correct to me.

Updated

18 years ago
Assignee: peterl → troy
Component: Layout → Views

Comment 2

18 years ago
The fourth container in this test is incorrect (in the markup), the third and
fourth Ps should be reversed (since -0 == +0).

Other than that, this looks like a valid problem in view ordering.
(Reporter)

Comment 3

18 years ago
Test fixed.  That's what I was trying to test - I just got mixed up (I think
when I reversed the stacking order on the whole test).

Updated

18 years ago
Status: NEW → ASSIGNED

Comment 4

18 years ago
Yes, what you're describing is what the spec says. The problem is the z-order
rules ('auto' not causing a new stacking context, and the fact that negative
z-index values cause a child to render below its parent) are not very intuitive
and they are very messy to implement.

I think that both the rule that 'auto' doesn't create a new stacking context and
the rule that negative z-index values cause a child to render below its parent
should be removed from the spec.

Cc'ing Peter to get his opinion as well.
(Reporter)

Comment 5

18 years ago
Using negative z-indexes is the only way to get stretched background images,
which people seem to want.  I think it works in IE5 (I'd have to check though).

I wouldn't think the auto thing would be that bad - and I don't think it makes
sense that every positioned element should have a new stacking context (although
IE does act that way, I admit).
(Reporter)

Comment 6

18 years ago
(I was going to bring this up as a separate bug after discussing it on
www-style, but it's relevant here...), but really (contrary to all
implementations so far) absolutely positioned elements without z-index (i.e.,
with 'auto') should be drawn mixed with other content (in document order), not
on top of that other content.

Comment 7

18 years ago
Yes, it looks like IE does render child boxes with negative z-index values below
their parent. Of course, all that means is that it was probably their idea to
add that to the spec. :-)

As far as every positioned element creating a new stacking context, that's the
traditional thing to do. It's also the most natural, because typically there's a
tree hierarchy formed where each parent defines a local coordinate space and a
local stacking (z) order. To have 'auto' not create a new stacking context, but
the parent still be a new coordinate space (i.e., child elements are positioned
relative to their parent) is a bit of a pain.

Updated

18 years ago
Assignee: troy → beard
Status: ASSIGNED → NEW
Component: Views → Compositor

Comment 8

18 years ago
Patrick, here's one of the two 'z-index' bugs that we need changes to the
compositor to support. I tried to weasel out of supporting this, but as you can
see I wasn't very successful.

For background on "layered presentation" in CSS2, read this:
file:///E:/css2/visuren.html#q30

The particular section that describes the 'z-index' property is here:
file:///E:/css2/visuren.html#z-index

The first thing that needs to be done is to change the API so there's a way for
layout to specify that the z-index valus is 'auto'. When you've done that let me
know, and I'll change layout to pass that information along

Once layout is making the correct call you can implement it and test to see that
it works.

The basic change to the compositor will be to modify how the display list is
built. Rather than just doing a postorder traversal of the view hierarchy, we'll
need to check for 'auto' z-index values and not establish a new local stacking
context

Updated

18 years ago
Status: NEW → ASSIGNED

Updated

18 years ago
Target Milestone: M10

Updated

18 years ago
Target Milestone: M10 → M11
Summary: z-index of auto being treated as zero → {css2} z-index of auto being treated as zero
[And one more radar...]

Comment 10

18 years ago
I've checked in changes to nsIViewManger, nsIView to enable setting this state.
The relevant methods are:

nsIViewManager::SetViewAutoZIndex(PRBool aAutoZIndex);
nsIVew::SetAutoZIndex(PRBool aAutoZIndex);
nsIView::GetAutoZIndex(PRBool &aAutoZIndex) const;

Let me know when you've made the appropriate layout changes.

Updated

18 years ago
QA Contact: petersen → chrisd

Comment 11

18 years ago
m12
Target Milestone: M11 → M12

Updated

18 years ago
Target Milestone: M12 → M13
Keywords: css2
Migrating from {css2} to css2 keyword. The {css1}, {css2}, {css3} and {css-moz}
radars should now be considered deprecated in favour of keywords.
I am *really* sorry about the spam...

Updated

18 years ago
Target Milestone: M13 → M14

Comment 13

18 years ago
Display list processing is needed for this, but is temporarily not being used.
Either that, or the child views should really just become siblings. I'm guessing
keeping the parent/child relationship is the right answer, and have the display
list built using the right stacking context.

Comment 14

18 years ago
Adding 'beta1' keyword. Although to date, it is not clear whether z-index values 
are supported in beta1 (clarification has been requested), the property falls 
within the general CSS2 'layering' area, which should be supported by beta1. 
Keywords: beta1
(Reporter)

Comment 15

18 years ago
One question - why do absolutely positioned elements with 'z-index: auto' need
to have their own views?  Would it be easier if they didn't?

Updated

18 years ago
Summary: {css2} z-index of auto being treated as zero → z-index of auto being treated as zero

Comment 16

18 years ago
Good question. I've been scratching my head about how to implement this 
reasonably in the display list creation code, perhaps resorting to an explicit 
stacking context object to build the display list, but it sure complicates 
things. Vidur and I discussed this, and it has potentially serious problems. The 
CSS spec is a little muddy on exactly how this should be interpreted. If a simple 
tweak to the display list creation tree walk is all that's necessary, that's 
fine, but I fear that there's more involved here.
Target Milestone: M14 → M15
(Reporter)

Comment 17

18 years ago
I think a 'z-index' of 'auto' means just paint in document order (as I think
would happen if the elements didn't have their own views, if I understand things
correctly).  However, this would mess up some current pages, because previous
browsers have implemented 'auto' as '0'.  So it should probably work the old way
in quirks mode.

What are the other "potentially serious problems"?

Comment 18

18 years ago
Per input from rickg, putting on PDT- radar for beta1.
Whiteboard: [PDT-]

Comment 19

18 years ago
Reassigning all compositor bugs to kevin.
Assignee: beard → kmcclusk
Status: ASSIGNED → NEW
QA Contact: chrisd → petersen
Moving to M16
Status: NEW → ASSIGNED
Target Milestone: M15 → M16
Moving to M17
Target Milestone: M16 → M17
(Reporter)

Comment 22

18 years ago
Has this been discussed in the CSS WG or on www-style?  It should be brought up
if not.  I could come up with a simple proposal to change the spec - simply
change the definition of z-index: auto for absolutely positioned elements so
it's the same as 0 (and leave it as-is for relatively positioned elements).

I think it might be better to try and get the spec changed than work on this,
considering the weight of current practice.
Moving to M18
Target Milestone: M17 → M18
This bug is marked "future" because it is not critical for RTM (Release To 
Manufacturing). If anyone believes it is critical, please explain why in
this bug. 
Target Milestone: M18 → Future

Updated

17 years ago
Whiteboard: [PDT-] → [PDT-]need input from CSS WG
(Reporter)

Comment 25

17 years ago
That was me changing the status whiteboard to say that this needs input form the 
CSS WG.
As per meeting with ChrisD today, taking QA.
QA Contact: petersen → py8ieh=bugzilla
According to the spec, David's testcase should not show all 8 boxes in each 
case. In particular, the boxes with negative z-index don't show up because 
they're behind the BODY.
Also, Test H is invalid for two reasons: "5" should appear on top of "4", 
because it has the same z-index as "4" (== 0) but appears later in the content. 
Also, "7" should appear on top of "6" because "7" has z-index -2 but "6" is 
inside a container with z-index -3. (I've modified the testcase a little so I 
can actually see the elements with negative z-index :-).)

My overhauled nsViewManager2 passes all these tests.
Reassigning to Robert, who has a fix pending review in bug 39621.
Assignee: kmcclusk → roc+moz
Status: ASSIGNED → NEW
Depends on: 39621
Keywords: correctness, nsbeta3, patch, review, testcase
Target Milestone: Future → M18
Status: NEW → ASSIGNED
Summary: z-index of auto being treated as zero → z-index of auto being treated as zero [POS]
Rubber stamping [nsbeta3-], since Robert's fix is slated for the next release
do to high risk considerations.
Whiteboard: [PDT-]need input from CSS WG → [PDT-]need input from CSS WG [nsbeta3-]
Removing requirement for input from CSS WG since we don't need to change the 
spec any more since Robert thinks he implemented it correctly. Right?

Nominating for mozilla0.9/nsbeta1: we should enable this by default.
Keywords: beta1, nsbeta3 → mozilla0.9, nsbeta1
Whiteboard: [PDT-]need input from CSS WG [nsbeta3-]
Yeah, we don't need WG input here anymore.
Target Milestone: M18 → ---
Target Milestone: --- → mozilla0.9
The new view manager is checked in and enabled; this bug is now fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
VERIFIED that the tests pass modulo Robert's comments.

Very nice testcase. :-)
Status: RESOLVED → VERIFIED
Whiteboard: (py8ieh: check for the bug with negative z-index being behind the body vs behind the canvas)
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.