Summary: | getComputedStyle returns "auto" for zIndex property even after it has been set, on non-positioned elements | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Marcus Better <marcus> | ||||||||||
Component: | CSS | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | arv, bdakin, cdumez, commit-queue, dbates, dino, dstockwell, esprehn+autocc, ews-watchlist, fred.wang, glenn, gyuyoung.kim, ian, jfernandez, joepeck, koivisto, kondapallykalyan, macpherson, menard, m.goleb+bugzilla, michaelyagudaev, mifenton, mitz, mrowe, pdr, rego, sam, simon.fraser, svillar, tkent, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | 523.x (Safari 3) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | OS X 10.4 | ||||||||||||
Attachments: |
|
Description
Marcus Better
2007-10-19 07:27:10 PDT
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.
Test result on Firefox 2.0.0.7: Before: computed: auto inline: Setting z-index to 10... computed: 10 inline: 10 SUCCESS! 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 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. 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? 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. 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. This is required by Acid3. In fact, Hixie seems to have used this for 8 acid 3 tests. :( 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. 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. Note that it's going to be 16 tests by the time I'm doing with this series of tests. Acid3 no longer depends on this 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). 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). Created attachment 383942 [details]
Patch
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. (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. Created attachment 383979 [details]
Patch
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 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
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: graouts@apple.com) The commit-queue is continuing to process your patch. Comment on attachment 383979 [details] Patch Clearing flags on attachment: 383979 Committed r252724: <https://trac.webkit.org/changeset/252724> *** Bug 203390 has been marked as a duplicate of this bug. *** |