Re: [webkit-dev] RenderLayer::isStackingContext() confusion
Thanks for the explanation. I submitted a patch, just in case reviewers agree that a comment is appropriate there. But I won't argue strongly either way. https://bugs.webkit.org/show_bug.cgi?id=113909 On Wed, Apr 3, 2013 at 1:18 PM, Elliott Sprehn wrote: > The function does what it says, it's just not obvious why: > > > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/WebCore/css/StyleResolver.cpp&sq=package:chromium&type=cs&q=adjustRenderStyle&l=1469 > > First we set the z-index to be auto for static position elements. Next we > set the z-index to be 0 for elements based on the big set of cases in > adjustRenderStyle, and then you can tell in RenderLayer if you're a > stacking context based on if you have z-index != auto. > > I'd add a comment explaining. The alternative is to check that big set of > cases every time someone calls isStackingContext but that might be slower. > Changing the name of the method would make the code more confusing, since > it is checking the right thing just through some complicated process. > > > On Wed, Apr 3, 2013 at 3:48 PM, Shawn Singh wrote: > >> Background: >> >> According to CSS 2.1 spec, an element becomes a stacking context when: >> - it is positioned (relative, absolute, or fixed) >> - and it has a non-auto z-index >> >> Spec also mentions that there's room for expanding this definition; the >> examples I know about are fixed-position elements and elements with >> transforms. >> >> >> Questions: >> >> The implementation of RenderLayer::isStackingContext() looks fishy. It >> only checks for a non-auto z-index. It does not check for the positioning >> style of the layer. There is no indication that it checks for any more >> recent special cases such as fixed-position or transforms. >> >> Is this function name a misnomer? should it actually be renamed to >> hasNonAutoZIndex() ? >> >> If the additional criteria is not being checked inside this helper >> function, then where is it? I see some code in collectLayers that looks >> vaguely similar to checking whether an element is positioned, but what >> about all the other uses of isStackingContext everywhere? >> >> Thanks! >> Shawn >> >> ___ >> webkit-dev mailing list >> webkit-dev@lists.webkit.org >> https://lists.webkit.org/mailman/listinfo/webkit-dev >> >> > ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] RenderLayer::isStackingContext() confusion
The function does what it says, it's just not obvious why: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/WebCore/css/StyleResolver.cpp&sq=package:chromium&type=cs&q=adjustRenderStyle&l=1469 First we set the z-index to be auto for static position elements. Next we set the z-index to be 0 for elements based on the big set of cases in adjustRenderStyle, and then you can tell in RenderLayer if you're a stacking context based on if you have z-index != auto. I'd add a comment explaining. The alternative is to check that big set of cases every time someone calls isStackingContext but that might be slower. Changing the name of the method would make the code more confusing, since it is checking the right thing just through some complicated process. On Wed, Apr 3, 2013 at 3:48 PM, Shawn Singh wrote: > Background: > > According to CSS 2.1 spec, an element becomes a stacking context when: > - it is positioned (relative, absolute, or fixed) > - and it has a non-auto z-index > > Spec also mentions that there's room for expanding this definition; the > examples I know about are fixed-position elements and elements with > transforms. > > > Questions: > > The implementation of RenderLayer::isStackingContext() looks fishy. It > only checks for a non-auto z-index. It does not check for the positioning > style of the layer. There is no indication that it checks for any more > recent special cases such as fixed-position or transforms. > > Is this function name a misnomer? should it actually be renamed to > hasNonAutoZIndex() ? > > If the additional criteria is not being checked inside this helper > function, then where is it? I see some code in collectLayers that looks > vaguely similar to checking whether an element is positioned, but what > about all the other uses of isStackingContext everywhere? > > Thanks! > Shawn > > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > https://lists.webkit.org/mailman/listinfo/webkit-dev > > ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] RenderLayer::isStackingContext() confusion
Background: According to CSS 2.1 spec, an element becomes a stacking context when: - it is positioned (relative, absolute, or fixed) - and it has a non-auto z-index Spec also mentions that there's room for expanding this definition; the examples I know about are fixed-position elements and elements with transforms. Questions: The implementation of RenderLayer::isStackingContext() looks fishy. It only checks for a non-auto z-index. It does not check for the positioning style of the layer. There is no indication that it checks for any more recent special cases such as fixed-position or transforms. Is this function name a misnomer? should it actually be renamed to hasNonAutoZIndex() ? If the additional criteria is not being checked inside this helper function, then where is it? I see some code in collectLayers that looks vaguely similar to checking whether an element is positioned, but what about all the other uses of isStackingContext everywhere? Thanks! Shawn ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
