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 <[email protected]> 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 <[email protected]>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 >> [email protected] >> https://lists.webkit.org/mailman/listinfo/webkit-dev >> >> >
_______________________________________________ webkit-dev mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-dev
