Bug 15562 – getComputedStyle returns "auto" for zIndex property even after it has been set, on non-positioned elements

• Component: CSS
• Status: RESOLVED FIXED

[Marcus Better]

If a style attribute is set in script, that should become the computed value for the attribute. The getComputedStyle function should therefore return the new value. However this is not the case with WebKit.

[Marcus Better]

Created attachment 16730 [details]
Test case

This checks the computed and inline zIndex style attributes for an element without an explicit style. Then the attribute is given a value with JavaScript. We then check if the computed style has been changed to reflect this.

[Marcus Better]

Test result on Firefox 2.0.0.7:

Before:
computed: auto
inline:
Setting z-index to 10...
computed: 10
inline: 10
SUCCESS!

[Marcus Better]

Test result on WebKit r26570 nightly:

Before:
computed: auto
inline: 
Setting z-index to 10...
computed: auto
inline: 10
FAILURE! Computed and inline style differ

[Mark Rowe (bdash)]

This is related to the fact that the div on which the z-index is set is unpositioned.  Adding "            e.style.position = "relative";" before retrieving the final computed style results in the expected value being returned.

[Mark Rowe (bdash)]

See http://bugs.webkit.org/show_bug.cgi?id=14321#c4 for why it returns auto when the element is not positioned.  Mitz, should we consider matching Firefox's behaviour here?

[Dave Hyatt]

I don't think we should.  This is a bug in Firefox I believe.  The computed result of z-index for an unpositioned element is auto, not the value specified in the stylesheet (since that value is going to be completely ignored when rendering).

cc'ing Hixie to see what he thinks.

[Ian 'Hixie' Hickson]

Actually the computed value of 'z-index' is, according to CSS2.1, always the specified value. The _used value_ is the one that depends on the other properties
in this case.

[Eric Seidel (no email)]

This is required by Acid3.  In fact, Hixie seems to have used this for 8 acid 3 tests. :(

[Eric Seidel (no email)]

Test 32: FAIL (underlying problems prevent this test from running properly)
Test 33: FAIL (underlying problems prevent this test from running properly)
Test 34: FAIL (underlying problems prevent this test from running properly)
Test 35: FAIL (underlying problems prevent this test from running properly)
Test 36: FAIL (underlying problems prevent this test from running properly)
Test 37: FAIL (underlying problems prevent this test from running properly)
Test 38: FAIL (underlying problems prevent this test from running properly)

I lied, it's only 7 Acid3 tests which depend on this bug.  It's kinda lame that 7 tests would exploit this bug, but oh well.

[Ian 'Hixie' Hickson]

Wow, that was totally accidental. I only used z-index because it was the only CSS2 property I could find that only took keywords and integers, which sidesteps many of the issues with getting stringified computed values. If you have any other suggestions for how to write that part of the test harness in a way that is justifiable using 2004-or-earlier CRs or RECs, I'm happy to oblige.

[Ian 'Hixie' Hickson]

Note that it's going to be 16 tests by the time I'm doing with this series of tests.

[Eric Seidel (no email)]

Acid3 no longer depends on this

[Michael Yagudaev]

Ok I believe I figured out the reason for this bug. 

If you read the css3 specification for z-index (http://www.w3.org/TR/CSS2/visuren.html#z-index) it says that the default value for a z-index is auto. Now, if an element is not applied with a position type that is non-static (i.e. position is relative, absolute or fixed), static is default for position so the z-index will have no effect on the element. So it seems webkit does not bother calculating this z-index and just returns the default, which is auto.

The work around to fix this therefore is
[selector]
{
   position: relative;
}

If you want to get the value of the container then use:
[selector]
{
   position: relative;
   z-index: inherit;
}

The above is really useful when implementing loading spinners for your content (e.g. a spinner that appears on top of a dialog, simply get the z-index and increment by one).

[Michael Yagudaev]

Posted a short article on how to get the z-index cross browsers, which also includes a jsfiddle example you can play around with:
http://www.yagudaev.com/programming/javascript/20-getting-reliable-z-index-cross-browser

(In reply to comment #13)
> Ok I believe I figured out the reason for this bug. 
> 
> If you read the css3 specification for z-index (http://www.w3.org/TR/CSS2/visuren.html#z-index) it says that the default value for a z-index is auto. Now, if an element is not applied with a position type that is non-static (i.e. position is relative, absolute or fixed), static is default for position so the z-index will have no effect on the element. So it seems webkit does not bother calculating this z-index and just returns the default, which is auto.
> 
> The work around to fix this therefore is
> [selector]
> {
>    position: relative;
> }
> 
> If you want to get the value of the container then use:
> [selector]
> {
>    position: relative;
>    z-index: inherit;
> }
> 
> The above is really useful when implementing loading spinners for your content (e.g. a spinner that appears on top of a dialog, simply get the z-index and increment by one).

[Simon Fraser (smfr)]

Created attachment 383942 [details]
Patch

[Antti Koivisto]

Comment on attachment 383942 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=383942&action=review

> Source/WebCore/ChangeLog:12
> +        Fix by storing the specified value in RenderStyle as "zIndex", and the used value as "effectiveZIndex", and
> +        converting all rendering code to use the "effective" variants. getComputedStyle reads "zIndex".

Are we going to end up with specified/effective values for many properties? Should we come up with some sort of generic solution to this problem, like generating a separate RenderStyle for getComputedStyle to read?

> Source/WebCore/rendering/style/StyleBoxData.h:57
>      int zIndex() const { return m_zIndex; }
>      bool hasAutoZIndex() const { return m_hasAutoZIndex; }

It might be good to rename these to 'original' (or 'specified'), similar to display properties.

> Source/WebCore/rendering/style/StyleBoxData.h:60
> +    bool effectiveZIndexIsAuto() const { return m_effectiveZIndexIsAuto; }

The usual naming for boolean properties is to put the verb first, 'hasEffectiveZIndexAuto'. In any case both these should follow the same naming pattern.

> Source/WebCore/style/StyleAdjuster.cpp:300
> +    if (style.hasAutoZIndex() || (style.position() == PositionType::Static && !m_parentBoxStyle.isDisplayFlexibleOrGridBox()))
> +        style.setEffectiveZIndexIsAuto();
> +    else
> +        style.setEffectiveZIndex(style.zIndex());

I suppose an alternative strategy would be to compute the effective value dynamically in render tree.

[Simon Fraser (smfr)]

(In reply to Antti Koivisto from comment #16)
> Comment on attachment 383942 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=383942&action=review
> 
> > Source/WebCore/ChangeLog:12
> > +        Fix by storing the specified value in RenderStyle as "zIndex", and the used value as "effectiveZIndex", and
> > +        converting all rendering code to use the "effective" variants. getComputedStyle reads "zIndex".
> 
> Are we going to end up with specified/effective values for many properties?
> Should we come up with some sort of generic solution to this problem, like
> generating a separate RenderStyle for getComputedStyle to read?

There probably will be more. When that happens we should figure out how to handle it.

> > Source/WebCore/rendering/style/StyleBoxData.h:57
> >      int zIndex() const { return m_zIndex; }
> >      bool hasAutoZIndex() const { return m_hasAutoZIndex; }
> 
> It might be good to rename these to 'original' (or 'specified'), similar to
> display properties.

I'll use "specified" and "used" to match CSS terminology.

> > Source/WebCore/rendering/style/StyleBoxData.h:60
> > +    bool effectiveZIndexIsAuto() const { return m_effectiveZIndexIsAuto; }
> 
> The usual naming for boolean properties is to put the verb first,
> 'hasEffectiveZIndexAuto'. In any case both these should follow the same
> naming pattern.

OK

> > Source/WebCore/style/StyleAdjuster.cpp:300
> > +    if (style.hasAutoZIndex() || (style.position() == PositionType::Static && !m_parentBoxStyle.isDisplayFlexibleOrGridBox()))
> > +        style.setEffectiveZIndexIsAuto();
> > +    else
> > +        style.setEffectiveZIndex(style.zIndex());
> 
> I suppose an alternative strategy would be to compute the effective value
> dynamically in render tree.

I considered that, but it's going to be slower, and in several places we set the used value to 0 to get stacking side-effects.

[Simon Fraser (smfr)]

Created attachment 383979 [details]
Patch

[EWS Watchlist]

Comment on attachment 383979 [details]
Patch

Attachment 383979 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/13269917

New failing tests:
js/dom/customAccessor-defineProperty.html

[EWS Watchlist]

Created attachment 383994 [details]
Archive of layout-test-results from ews212 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews212  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 383979 [details]:

media/modern-media-controls/compact-media-controls/compact-media-controls-constructor.html bug 193587 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 383979 [details]
Patch

Clearing flags on attachment: 383979

Committed r252724: <https://trac.webkit.org/changeset/252724>

[Simon Fraser (smfr)]

*** Bug 203390 has been marked as a duplicate of this bug. ***

[Radar WebKit Bug Importer]

<rdar://problem/58125677>