CSS containment: position: fixed children jitter on scrolling page |
||||||||||||||||||||||
Issue descriptionVersion: 53.0.2767.0 canary OS: Mac OSX What steps will reproduce the problem? (1) http://jsbin.com/beqexocoti/edit?html,output (2) Scroll the page slowly back and forth What is the expected output? The container has contain:content. The child span is position:fixed. I expect the "I'm child of the box" span not to jitter when the page is scrolled. What do you see instead? Scrolling the page (esp. slowly) up and down makes that <span> jitter.
,
Jun 14 2016
I can't repro this on Mac or linux. Could the testing team take a look in case I'm missing something?
,
Jun 14 2016
Make sure you're on a retina.
,
Jun 15 2016
I can't reproduce this on 53.0.2767.0 with Retina.
,
Jun 15 2016
I can reproduce.
,
Jun 15 2016
Unable to reproduce the issue on MacBook Pro (Retina) 10.11.5 chrome version 53.0.2767.0 and canary 53.0.2768.0 - observed that "I'm child of the box" stayed in the same location on scrolling up and down. Please find the screencast ericbidelman@, Could you please mention the Mac version on which you are facing the issue. Also please give a try with the new profile and update the thread.
,
Jun 15 2016
Re #6, please use my attached contain-paint.html.
,
Jun 15 2016
Thanks for taking the time to file this with a nice testcase. I can confirm this bug on 53.0.2765.0/canary on a retina mac and also 53.0.2763.0/dev on android. On mac this may require a trackpad instead of a mouse. Maybe we're scrolling the red box smoothly on the compositor but positioning/counter-scrolling the fixed text via the main thread. Another possibility is that we're applying the smooth scroll offset in the wrong place. Here's a jsbin: http://output.jsbin.com/raribu (Use this output link instead of the editor mode.)
,
Jun 15 2016
I think this is a layout issue.
,
Jun 16 2016
,
Jun 20 2016
Able to reproduce the issue on Mac Retina 10.11.5 using canary 53.0.2773.0 and its a regression issue broken in M52, not seen on Macbook Air,Win 7 and Ubuntu 14.04. Bisect info: =============== Good : 52.0.2740.0 Bad : 52.0.2741.0 Change Log: https://build.chromium.org/f/chromium/perf/dashboard/ui/changelog.html?url=/trunk/src&range=394580%3A394591 Suspect : https://codereview.chromium.org/1991563002 eae@ : Could you please take a look into this if its related to your change, else help us assigning to an appropriate owner for the same. Have added ReleaseBlock-Beta, please modify if you feel its not appropriate.
,
Jun 23 2016
M53 is branching soon and will be promoted to Beta in July.Your bug is labelled as Beta ReleaseBlock, pls make sure to land the fix ASAP. Thank you.
,
Jun 27 2016
eae@, Gentle Ping! could you please look into this change (https://codereview.chromium.org/1991563002) if possible? Thank you!
,
Jun 27 2016
Looks like a snapping problem with high-dpi. Possibly compositing related. (contain: paint is a new feature, this is not a regression)
,
Jun 28 2016
M53 is branching this week and will be promoted to Beta in July.Your bug is labelled as Beta ReleaseBlock, pls make sure to land the fix ASAP. Thank you.
,
Jun 28 2016
This is a new feature and likely should not be considered a beta blocker as it doesn't break any existing content. It's still a P1 however.
,
Jun 28 2016
Unable to reproduce using either trackpad or by dragging the scrollbar on retina Mac on latest canary. Also unable to reproduce on high-dpi Windows on latest canary. A slightly different problem occurs when using mouse-wheel scrolling on windows though where the text in the demo from comment 8 jumps with the full scroll offset before being re-positioned correctly. That looks like a scroll layer problem rather than a layout issue though. Grabbing the scrollbar or using the arrow keys to scroll works just fine, as does using a trackpad. Adding Blink>Scroll for further triage.
,
Jun 29 2016
,
Jun 30 2016
Over to scroll team for further triage based on current state as described in comment 17.
,
Jul 4 2016
Could anyone from the scroll team update the thread as per C#17.
,
Jul 11 2016
flackr@, could you triage this? I think it's an issue with the window scrolling on compositor while the position: fixed gets updated on the main thread. Disabling threaded scrolling fixes the issue. If we can't compositor scroll the element then we should make sure the scroll parent gets a main thread scrolling reason.
,
Jul 11 2016
,
Jul 11 2016
,
Jul 18 2016
@flackr: Gentle Ping!
,
Jul 18 2016
Seems like this only reproduces on high-dpi, in which case we should be able to promote the fixed position element to be able to scroll on the compositor. It seems if I add will-change: transform to the fixed pos element it no longer jitters.
,
Jul 18 2016
The issue seems to be that PaintLayer::scrollsWithViewport() returns false because ComputedStyle::canContainFixedPositionObjects() returns true for contain: paint. Are we sure that a contained fixed position element should actually scroll with the viewport? When we have a transform property on one of it's parents it no longer does (the other style property which can contain fixed).
,
Jul 20 2016
,
Jul 22 2016
,
Jul 25 2016
My fix should be ready to land this week, but I'm not sure how simple of a merge candidate it will be. We could alternately revert the change which made contain: paint contain fixed position descendants but then the behavior would be incorrect in a different way. As stated in comment #16 since this is a new feature perhaps we don't need to block release on this. In any event I'll try to get my fix landed ASAP.
,
Jul 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/139b50e91d0d86ad4ee31957a003dad264f38435 commit 139b50e91d0d86ad4ee31957a003dad264f38435 Author: flackr <flackr@chromium.org> Date: Wed Jul 27 01:11:42 2016 Ensure that we consistently check contains: paint for fixed position containment. contains: paint; should contain fixed position descendants, however we had only updated this check in one place while we have many other places in the code which simply checked for having transform related properties. This patch attempts to unify all of these code paths to call through ComputedStyle::canContainFixedPositionObjects. TEST=fast/css/containment/paint-containment-with-fixed-position-scrolled.html, LayoutGeometryMapTest.ContainsFixedPositionTest, MapCoordinatesTest.FixedPosInTransform, MapCoordinatesTest.FixedPosInContainPaint BUG= 619999 Review-Url: https://codereview.chromium.org/2173963002 Cr-Commit-Position: refs/heads/master@{#408002} [add] https://crrev.com/139b50e91d0d86ad4ee31957a003dad264f38435/third_party/WebKit/LayoutTests/fast/css/containment/paint-containment-with-fixed-position-scrolled-expected.html [add] https://crrev.com/139b50e91d0d86ad4ee31957a003dad264f38435/third_party/WebKit/LayoutTests/fast/css/containment/paint-containment-with-fixed-position-scrolled.html [modify] https://crrev.com/139b50e91d0d86ad4ee31957a003dad264f38435/third_party/WebKit/Source/core/layout/LayoutBox.cpp [modify] https://crrev.com/139b50e91d0d86ad4ee31957a003dad264f38435/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp [modify] https://crrev.com/139b50e91d0d86ad4ee31957a003dad264f38435/third_party/WebKit/Source/core/layout/LayoutGeometryMap.cpp [modify] https://crrev.com/139b50e91d0d86ad4ee31957a003dad264f38435/third_party/WebKit/Source/core/layout/LayoutGeometryMapStep.h [modify] https://crrev.com/139b50e91d0d86ad4ee31957a003dad264f38435/third_party/WebKit/Source/core/layout/LayoutView.cpp [modify] https://crrev.com/139b50e91d0d86ad4ee31957a003dad264f38435/third_party/WebKit/Source/core/layout/MapCoordinatesTest.cpp [modify] https://crrev.com/139b50e91d0d86ad4ee31957a003dad264f38435/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp [modify] https://crrev.com/139b50e91d0d86ad4ee31957a003dad264f38435/third_party/WebKit/Source/core/paint/PaintLayer.cpp [modify] https://crrev.com/139b50e91d0d86ad4ee31957a003dad264f38435/third_party/WebKit/Source/core/style/ComputedStyle.h [modify] https://crrev.com/139b50e91d0d86ad4ee31957a003dad264f38435/third_party/WebKit/Source/web/tests/LayoutGeometryMapTest.cpp [add] https://crrev.com/139b50e91d0d86ad4ee31957a003dad264f38435/third_party/WebKit/Source/web/tests/data/rgm_contains_fixed_position_test.html
,
Jul 29 2016
Tested this issue on Macbook Pro 10.11.5 using chrome latest canary M54-54.0.2811.0 by following steps mentioned in the original comment. Observed no jittery in scrolling the page as seen in comment #5. Hence adding TE-Verified label.
,
Jul 29 2016
,
Jul 29 2016
Your change meets the bar and is auto-approved for M53 (branch: 2785)
,
Jul 29 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/155dfcd5829342d0e52865e8dbd7c501f3ddeef1 commit 155dfcd5829342d0e52865e8dbd7c501f3ddeef1 Author: Robert Flack <flackr@chromium.org> Date: Fri Jul 29 14:39:00 2016 Ensure that we consistently check contains: paint for fixed position containment. contains: paint; should contain fixed position descendants, however we had only updated this check in one place while we have many other places in the code which simply checked for having transform related properties. This patch attempts to unify all of these code paths to call through ComputedStyle::canContainFixedPositionObjects. TEST=fast/css/containment/paint-containment-with-fixed-position-scrolled.html, LayoutGeometryMapTest.ContainsFixedPositionTest, MapCoordinatesTest.FixedPosInTransform, MapCoordinatesTest.FixedPosInContainPaint BUG= 619999 Review-Url: https://codereview.chromium.org/2173963002 Cr-Commit-Position: refs/heads/master@{#408002} (cherry picked from commit 139b50e91d0d86ad4ee31957a003dad264f38435) Review URL: https://codereview.chromium.org/2194913002 . Cr-Commit-Position: refs/branch-heads/2785@{#404} Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382} [add] https://crrev.com/155dfcd5829342d0e52865e8dbd7c501f3ddeef1/third_party/WebKit/LayoutTests/fast/css/containment/paint-containment-with-fixed-position-scrolled-expected.html [add] https://crrev.com/155dfcd5829342d0e52865e8dbd7c501f3ddeef1/third_party/WebKit/LayoutTests/fast/css/containment/paint-containment-with-fixed-position-scrolled.html [modify] https://crrev.com/155dfcd5829342d0e52865e8dbd7c501f3ddeef1/third_party/WebKit/Source/core/layout/LayoutBox.cpp [modify] https://crrev.com/155dfcd5829342d0e52865e8dbd7c501f3ddeef1/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp [modify] https://crrev.com/155dfcd5829342d0e52865e8dbd7c501f3ddeef1/third_party/WebKit/Source/core/layout/LayoutGeometryMap.cpp [modify] https://crrev.com/155dfcd5829342d0e52865e8dbd7c501f3ddeef1/third_party/WebKit/Source/core/layout/LayoutGeometryMapStep.h [modify] https://crrev.com/155dfcd5829342d0e52865e8dbd7c501f3ddeef1/third_party/WebKit/Source/core/layout/LayoutView.cpp [modify] https://crrev.com/155dfcd5829342d0e52865e8dbd7c501f3ddeef1/third_party/WebKit/Source/core/layout/MapCoordinatesTest.cpp [modify] https://crrev.com/155dfcd5829342d0e52865e8dbd7c501f3ddeef1/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp [modify] https://crrev.com/155dfcd5829342d0e52865e8dbd7c501f3ddeef1/third_party/WebKit/Source/core/paint/PaintLayer.cpp [modify] https://crrev.com/155dfcd5829342d0e52865e8dbd7c501f3ddeef1/third_party/WebKit/Source/core/style/ComputedStyle.h [modify] https://crrev.com/155dfcd5829342d0e52865e8dbd7c501f3ddeef1/third_party/WebKit/Source/web/tests/LayoutGeometryMapTest.cpp [add] https://crrev.com/155dfcd5829342d0e52865e8dbd7c501f3ddeef1/third_party/WebKit/Source/web/tests/data/rgm_contains_fixed_position_test.html
,
Jul 29 2016
This should now be fixed on 53 as well.
,
Aug 3 2016
Tested the issue on Mac 10.11.6 Mac(Retina) using chrome version 53.0.2785.45.Not observed any jittery while scrolling the page.Please find the attached screen cast for the same. Adding TE-Verified labels. |
||||||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||||
Comment 1 by ericbidelman@chromium.org
, Jun 14 2016