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

Status: VERIFIED FIXED

(Core Graveyard :: GFX, defect, P3)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 1

[rickg]

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

Comment 2

[Peter Linss]

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.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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).

Comment 4

[troy]

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.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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).

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(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

[troy]

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.

Comment 8

[troy]

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

Comment 9

[Hixie (not reading bugmail)]

[And one more radar...]

Comment 10

[Patrick C. Beard]

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.

Comment 11

[chris hofmann]

m12

Comment 12

[Hixie (not reading bugmail)]

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...

Comment 13

[Patrick C. Beard]

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

[Christine Hoffman]

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. 

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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?

Comment 16

[Patrick C. Beard]

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.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

[leger]

Per input from rickg, putting on PDT- radar for beta1.

Comment 19

[Patrick C. Beard]

Reassigning all compositor bugs to kevin.

Comment 20

[Kevin McCluskey (gone)]

Moving to M16

Comment 21

[Kevin McCluskey (gone)]

Moving to M17

Comment 22

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 23

[Kevin McCluskey (gone)]

Moving to M18

Comment 24

[Kevin McCluskey (gone)]

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. 

Comment 25

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

That was me changing the status whiteboard to say that this needs input form the 
CSS WG.

Comment 26

[Hixie (not reading bugmail)]

As per meeting with ChrisD today, taking QA.

Comment 27

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 28

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 29

[Hixie (not reading bugmail)]

Reassigning to Robert, who has a fix pending review in bug 39621.

Comment 30

[Hixie (not reading bugmail)]

Rubber stamping [nsbeta3-], since Robert's fix is slated for the next release
do to high risk considerations.

Comment 31

[Hixie (not reading bugmail)]

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.

Comment 32

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yeah, we don't need WG input here anymore.

Comment 33

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The new view manager is checked in and enabled; this bug is now fixed.

Comment 34

[Hixie (not reading bugmail)]

VERIFIED that the tests pass modulo Robert's comments.

Very nice testcase. :-)