Bug 87186

Created attachment 143407 [details] Patch

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 on attachment 143407 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143407&action=review > Source/WebKit/chromium/public/WebSettings.h:153 > + virtual void setFixedPositionCreatesStackingContext(bool) = 0; Do you think we should alphabetize this list?

Comment on attachment 143407 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143407&action=review >> Source/WebKit/chromium/public/WebSettings.h:153 >> + virtual void setFixedPositionCreatesStackingContext(bool) = 0; > > Do you think we should alphabetize this list? I don't know, I couldn't discern any existing organization in this list so I just added this to the end.

Ok. I can post an alphabetization patch after you land this one.

Committed r118095: <http://trac.webkit.org/changeset/118095>

Re-opened since this is blocked by 87201

Comment on attachment 143407 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143407&action=review > Source/WebCore/css/StyleResolver.cpp:2052 > #ifdef FIXED_POSITION_CREATES_STACKING_CONTEXT > || style->position() == FixedPosition > +#else > + || (style->position() == FixedPosition && e->document()->page()->settings()->fixedPositionCreatesStackingContext()) > #endif Why not just turn this on for Qt? Yael?

(In reply to comment #8) > (From update of attachment 143407 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=143407&action=review > > > Source/WebCore/css/StyleResolver.cpp:2052 > > #ifdef FIXED_POSITION_CREATES_STACKING_CONTEXT > > || style->position() == FixedPosition > > +#else > > + || (style->position() == FixedPosition && e->document()->page()->settings()->fixedPositionCreatesStackingContext()) > > #endif > > Why not just turn this on for Qt? Yael? I mean I believe we are the only ones using FIXED_POSIT... but I guess we can use the setting instead

(In reply to comment #9) > (In reply to comment #8) > > (From update of attachment 143407 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=143407&action=review > > > > > Source/WebCore/css/StyleResolver.cpp:2052 > > > #ifdef FIXED_POSITION_CREATES_STACKING_CONTEXT > > > || style->position() == FixedPosition > > > +#else > > > + || (style->position() == FixedPosition && e->document()->page()->settings()->fixedPositionCreatesStackingContext()) > > > #endif > > > > Why not just turn this on for Qt? Yael? > > I mean I believe we are the only ones using FIXED_POSIT... but I guess we can use the setting instead The BlackBerry port also opts in to FIXED_POSIT... these days. But we'd also be happy to use the setting instead.

I think it'd be a good idea for other ports to move over to the Setting, but I'll leave that to decision up to them.

(In reply to comment #8) > (From update of attachment 143407 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=143407&action=review > > > Source/WebCore/css/StyleResolver.cpp:2052 > > #ifdef FIXED_POSITION_CREATES_STACKING_CONTEXT > > || style->position() == FixedPosition > > +#else > > + || (style->position() == FixedPosition && e->document()->page()->settings()->fixedPositionCreatesStackingContext()) > > #endif > > Why not just turn this on for Qt? Yael? Please see https://bugs.webkit.org/show_bug.cgi?id=83980#c3

In chromium we need to have this be a setting to verify compatibility issues. I think Simon's other comment is out of date, requiresCompositingForPosition() is aware of the acceleratedCompositingForFixedPositionEnabled compositing trigger which should cover this.

Created attachment 143649 [details] Patch for landing

Comment on attachment 143649 [details] Patch for landing Clearing flags on attachment: 143649 Committed r118263: <http://trac.webkit.org/changeset/118263>

All reviewed patches have been landed. Closing bug.

This test fails on Qt. * fast/block/positioning/fixed-position-stacking-context.html Diff: --- /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/fast/block/positioning/fixed-position-stacking-context-expected.txt +++ /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/fast/block/positioning/fixed-position-stacking-context-actual.txt @@ -8,6 +8,6 @@ layer at (0,0) size 100x100 RenderBlock (positioned) zI: 1 {DIV} at (0,0) size 100x100 [bgcolor=#FF0000] layer at (0,0) size 100x100 + RenderBlock (positioned) zI: 2 {DIV} at (0,0) size 100x100 [bgcolor=#008000] +layer at (0,0) size 100x100 RenderBlock (positioned) zI: 1 {DIV} at (0,0) size 100x100 [bgcolor=#FF0000] -layer at (0,0) size 100x100 - RenderBlock (positioned) zI: 2 {DIV} at (0,0) size 100x100 [bgcolor=#008000] We got a red rectangle on Qt platform. Could you check it, please?

I skipped the test in r118346: <http://trac.webkit.org/changeset/118346> to paint the bot green.

(In reply to comment #17) > This test fails on Qt. > > * fast/block/positioning/fixed-position-stacking-context.html > > Diff: > > --- /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/fast/block/positioning/fixed-position-stacking-context-expected.txt > +++ /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/fast/block/positioning/fixed-position-stacking-context-actual.txt > @@ -8,6 +8,6 @@ > layer at (0,0) size 100x100 > RenderBlock (positioned) zI: 1 {DIV} at (0,0) size 100x100 [bgcolor=#FF0000] > layer at (0,0) size 100x100 > + RenderBlock (positioned) zI: 2 {DIV} at (0,0) size 100x100 [bgcolor=#008000] > +layer at (0,0) size 100x100 > RenderBlock (positioned) zI: 1 {DIV} at (0,0) size 100x100 [bgcolor=#FF0000] > -layer at (0,0) size 100x100 > - RenderBlock (positioned) zI: 2 {DIV} at (0,0) size 100x100 [bgcolor=#008000] > > We got a red rectangle on Qt platform. Could you check it, please? Since you are using the #define to control this behavior instead of the Setting, you'll need Qt-specific baselines for this test.

*** Bug 85067 has been marked as a duplicate of this bug. ***