Bug 85067 - [chromium] Put fixed-position elements in a stacking context so that they can be composited
Summary: [chromium] Put fixed-position elements in a stacking context so that they can...
Status: RESOLVED DUPLICATE of bug 87186
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Iain Merrick
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-27 08:01 PDT by Iain Merrick
Modified: 2012-07-08 19:29 PDT (History)
10 users (show)

See Also:


Attachments
Patch (8.49 KB, patch)
2012-04-27 08:21 PDT, Iain Merrick
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Iain Merrick 2012-04-27 08:01:54 PDT
[chromium] Put fixed-position elements in a stacking context so that they can be composited
Comment 1 Iain Merrick 2012-04-27 08:21:13 PDT
Created attachment 139207 [details]
Patch
Comment 2 Iain Merrick 2012-04-27 08:23:23 PDT
I still need to add a layout test for this, but I've uploaded this so we can start discussing this change.

With this setting enabled, fixed position elements won't be handled according to the spec -- the z-ordering of child elements with respect to the rest of the page could be different. However, most sites should be unaffected, and this is arguably becoming the de facto standard for mobile browsers.
Comment 3 WebKit Review Bot 2012-04-27 08:24:09 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 4 WebKit Review Bot 2012-04-27 08:24:32 PDT
Attachment 139207 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/ChangeLog:13:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Iain Merrick 2012-04-27 08:49:01 PDT
I'm looking for an existing layout test that covers a specific field in Settings, to see the best way of flipping it in Javascript, but I can't seem to find one. Can anyone point me at one? Thanks!
Comment 6 James Robinson 2012-04-27 13:08:40 PDT
Comment on attachment 139207 [details]
Patch

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

I think based on our experience from MobileSafari and Android we should try this out via WebKit nightlies / Chrome dev+canary to see if we can simply make this the new behavior, and if the compat issues aren't too bad propose it as an errata to CSS 2.1.  position:fixed behavior is just freakin' weird

> Source/WebCore/rendering/RenderLayer.h:383
> -    bool isStackingContext() const { return !hasAutoZIndex() || renderer()->isRenderView(); }
> +    bool isStackingContext() const;

I suspect isStackingContext() is fairly hot. If so, I think we should make this compile-time rather than runtime for performance reasons, even though that does make the testing situation a bit harder to deal with.
Comment 7 James Robinson 2012-04-27 13:09:23 PDT
Iain - to move this forward can you try to collect some data on whether isStackingContext() is a hot enough function for us to worry about out-of-lining it and doing a walk to Settings?
Comment 8 James Robinson 2012-04-27 13:38:09 PDT
It would also be interesting to run the current layout test suite with position:fixed always making a stacking context to see what's different.
Comment 9 Iain Merrick 2012-06-11 07:08:24 PDT

*** This bug has been marked as a duplicate of bug 87186 ***
Comment 10 Laszlo Gombos 2012-07-08 19:29:01 PDT
Comment on attachment 139207 [details]
Patch

It seems that this patch is obsolete as a duplicate is landed as r118263. Setting the obsolete flag to take the patch out from the review queue.