Bug 96688 – Put position:fixed elements in their own layers and allow them to create a stacking context

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Beth Dakin]

Right now, pages with position:fixed elements are not able to be scrolled by the ScrollingCoordinator. We would like to change that. The first step to give position:fixed elements their own layers and to allow them to create a stacking context. 

<rdar://problem/11467961>

[Beth Dakin]

Created attachment 163948 [details]
Patch

[Beth Dakin]

Created attachment 163951 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 163951 [details]
Patch

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

> Source/WebKit/mac/WebView/WebView.mm:1553
> +    settings->setAcceleratedCompositingForFixedPositionEnabled(true);

I don' think we want to force compositing layers for position:fixed in WK1...

> Source/WebKit/mac/WebView/WebView.mm:1554
> +    settings->setFixedPositionCreatesStackingContext(true);

But we do want this one to avoid rendering differences.

[Beth Dakin]

Created attachment 163953 [details]
Patch with smfr's feedback

[Beth Dakin]

Created attachment 163977 [details]
Patch with updated test results

[Sam Weinig]

Comment on attachment 163977 [details]
Patch with updated test results

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

> LayoutTests/ChangeLog:12
> +        A few test result differences. These first three tests actually
> +        render differently now.

So they render incorrectly?  Seeing expected results which is a red-box is very confusing.

[WebKit Review Bot]

Comment on attachment 163977 [details]
Patch with updated test results

Attachment 163977 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13835866

New failing tests:
fast/block/positioning/016.html
fast/block/positioning/025.html
fast/block/positioning/fixed-position-stacking-context.html
compositing/geometry/fixed-position-composited-switch.html

[WebKit Review Bot]

Comment on attachment 163977 [details]
Patch with updated test results

Attachment 163977 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13855287

New failing tests:
fast/block/positioning/016.html
fast/block/positioning/025.html
fast/block/positioning/fixed-position-stacking-context.html
compositing/geometry/fixed-position-composited-switch.html

[Beth Dakin]

(In reply to comment #6)
> (From update of attachment 163977 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=163977&action=review
> 
> > LayoutTests/ChangeLog:12
> > +        A few test result differences. These first three tests actually
> > +        render differently now.
> 
> So they render incorrectly?  Seeing expected results which is a red-box is very confusing.

They don't render incorrectly; they render differently. The new results are expected given the change. These tests are explicitly doing things that test whether position:fixed creates a stacking context, so now that we are having it create a stacking context, they render differently.

I agree that the red boxes are confusing. However, this is going to necessarily render differently on ports that enable stacking context for fixed pos versus ports that do not. We could try to think of a way to change the tests that makes the differences less confusing.

If this is still confusing: yes, this change might break some web sites. According to jamesr, they have has this enabled in Chromium TOT for a bit now, and they found some bugs with Google sites and got the sites to change. They haven't found anything else major yet. We and they will be on the lookout for bugs.

[Beth Dakin]

Created attachment 164220 [details]
Patch

In this patch I added comments to the files that render red to hopefully provide some clarification whenever people come across these tests later.

I guess chromium bots are failing because they honor test results in the platform/mac directory before looking at the test directory. Not sure what the standard way to deal with that is.

[Beth Dakin]

Committed change with http://trac.webkit.org/changeset/128652

[Beth Dakin]

This caused some test failures on the WK2 bots, which I have fixed. See:

https://bugs.webkit.org/show_bug.cgi?id=96846
http://trac.webkit.org/changeset/128678

[Sam Weinig]

(In reply to comment #9)
> (In reply to comment #6)
> > (From update of attachment 163977 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=163977&action=review
> > 
> > > LayoutTests/ChangeLog:12
> > > +        A few test result differences. These first three tests actually
> > > +        render differently now.
> > 
> > So they render incorrectly?  Seeing expected results which is a red-box is very confusing.
> 
> They don't render incorrectly; they render differently. The new results are expected given the change. These tests are explicitly doing things that test whether position:fixed creates a stacking context, so now that we are having it create a stacking context, they render differently.
> 
> I agree that the red boxes are confusing. However, this is going to necessarily render differently on ports that enable stacking context for fixed pos versus ports that do not. We could try to think of a way to change the tests that makes the differences less confusing.
> 
> If this is still confusing: yes, this change might break some web sites. According to jamesr, they have has this enabled in Chromium TOT for a bit now, and they found some bugs with Google sites and got the sites to change. They haven't found anything else major yet. We and they will be on the lookout for bugs.

My suggestion then would be to change all ports to the new stacking mode, even when not using layers for fixed positioning.  And then change the test to show green.  I don't see real benefit in fracturing WebKit here.