New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 619999 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

CSS containment: position: fixed children jitter on scrolling page

Project Member Reported by ericbidelman@chromium.org, Jun 14 2016

Issue description

Version:  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.


 
Cc: vollick@chromium.org
Labels: Needs-TestConfirmation
Status: Unconfirmed (was: Untriaged)
I can't repro this on Mac or linux. Could the testing team take a look in case I'm missing something?
Make sure you're on a retina.
I can't reproduce this on 53.0.2767.0 with Retina.
I can reproduce.
contain-paint.html
751 bytes View Download
contain-paint.mp4
457 KB View Download
Cc: tkonch...@chromium.org
Labels: Needs-Feedback
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.
619999.mov
7.2 MB Download
Re #6, please use my attached contain-paint.html.

Comment 8 by pdr@chromium.org, Jun 15 2016

Components: Blink>Compositing
Labels: -Pri-3 -Needs-Feedback OS-Android Pri-2
Status: Available (was: Unconfirmed)
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.)
Components: -Blink>Compositing -Blink>Paint Blink>Layout
Status: Untriaged (was: Available)
I think this is a layout issue.
Labels: -Type-Bug -Pri-2 -Needs-TestConfirmation ReleaseBlock-Beta has-Bisect M-53 Pri-1 Type-Bug-Regression
Owner: e...@chromium.org
Status: Assigned (was: Untriaged)
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.
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. 
Components: Blink>CSS
eae@, Gentle Ping! could you please look into this change (https://codereview.chromium.org/1991563002) if possible?

Thank you!

Comment 14 by e...@chromium.org, Jun 27 2016

Cc: osh...@chromium.org
Labels: -OS-Android -Type-Bug-Regression -OS-Mac OS-All Type-Bug
Looks like a snapping problem with high-dpi. Possibly compositing related.

(contain: paint is a new feature, this is not a regression)
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.

Comment 16 by e...@chromium.org, 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.

Comment 17 by e...@chromium.org, Jun 28 2016

Components: Blink>Scroll
Owner: ----
Status: Untriaged (was: Assigned)
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.

Comment 18 by suzyh@chromium.org, Jun 29 2016

Components: -Blink>CSS

Comment 19 by e...@chromium.org, Jun 30 2016

Components: -Blink>Layout
Labels: -ReleaseBlock-Beta ReleaseBlock-Stable
Over to scroll team for further triage based on current state as described in comment 17.

Comment 20 by ajha@chromium.org, Jul 4 2016

Could anyone from the scroll team update the thread as per C#17.

Comment 21 by bokan@chromium.org, Jul 11 2016

Owner: flackr@chromium.org
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.

Comment 22 by bokan@chromium.org, Jul 11 2016

Cc: bokan@chromium.org
Status: Assigned (was: Untriaged)
@flackr: Gentle Ping!
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.
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).
Status: Started (was: Assigned)
Cc: chrishtr@chromium.org e...@chromium.org
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.
Project Member

Comment 30 by bugdroid1@chromium.org, 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

Labels: TE-Verified-54.0.2811.0 TE-Verified-M54
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.

ScrollPage.mp4
602 KB View Download
Labels: Merge-Request-53

Comment 33 by dimu@chromium.org, Jul 29 2016

Labels: -Merge-Request-53 Merge-Approved-53 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M53 (branch: 2785)
Project Member

Comment 34 by bugdroid1@chromium.org, Jul 29 2016

Labels: -merge-approved-53 merge-merged-2785
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

Status: Fixed (was: Started)
This should now be fixed on 53 as well.
Labels: TE-Verified-M53 TE-Verified-53.0.2785.45
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.
619999.mp4
707 KB View Download

Sign in to add a comment