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

Issue 835755 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-05-04
OS: Linux
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Backround: url() fixed within iframe scrolling in the wrong direction

Reported by pascal.p...@gmail.com, Apr 23 2018

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.117 Safari/537.36

Example URL:

Steps to reproduce the problem:
1. Have an iframe
2. Target site of that iframe has a background: url('background.jpg') fixed;
3. Scroll (horizontal or vertical)

What is the expected behavior?
Background image stays where it is, which it does in firefox, vivaldi, and internet explorer.

What went wrong?
The background image scrolls in the opposite direction as the content.

Does it occur on multiple sites: N/A

Is it a problem with a plugin? No 

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 66.0.3359.117  Channel: stable
OS Version: 16.04
Flash Version: 

We have found this problem on our production site, and I've thrown together a quick demonstration (see attached zip) to replicate the problem.

I currently do not know any other sites that have the same setup, but I suppose the same thing would happen there, too.
 
files.zip
1.1 MB Download

Comment 1 by woxxom@gmail.com, Apr 23 2018

Bisected to 96f85b68747a679ea1ac4cd05d6743ae5f7142b7 "Enable root layer scrolling"
Confirmed by disabling the feature via "--disable-blink-features=RootLayerScrolling" and observing the bug is gone.
Landed in 66.0.3345.0

* Underlying bug #1:
  Buggy scrolling

  Bisected via "--enable-blink-features=RootLayerScrolling":
  907b59e9ba819af5ef6802159dea8a40d114db40 "Support RLS low DPI fixed root backgrounds."
  Landed in 63.0.3230.0 but disabled by default

* Underlying  bug #2 :
  Image is not cropped to the iframe bounds

  Bisected via "--enable-blink-features=RootLayerScrolling":
  b5e200d9663137d83b9dd2c02b9cd2d13d8fc5d0 "[root layer scrolls] Fix paint chunk properties"
  Landed in 56.0.2917.0 but disabled by default

Both underlying bugs are still present in Chrome Canary.
Labels: Needs-Triage-M66
Cc: krajshree@chromium.org
Components: Blink>Image
Labels: Needs-Feedback Triaged-ET
Unable to reproduce the issue on ubuntu 17.10 using chrome reported version #66.0.3359.117.

Attached a screen cast for reference.

Following are the steps followed to reproduce the issue.
------------
1. Opened the attached files "iframe-src.html" and "index.html".
2. Observed that upon scrolling vertically and horizontally the background image stayed where it is as expected.

reporter@ - Could you please check the attached screen cast and please let us know if anything missed from our end.

Thanks...!!
835755.webm
5.0 MB View Download

Comment 4 by woxxom@gmail.com, Apr 24 2018

I've reproduced and bisected this in Windows 7, see comment#1.
First off, thanks for the response.

Second, you haven't missed anything in your screen cast. It's interesting that it's not happening to you. I've attached a screen cast too, showing the exact problem. My coworker has experienced the bug too, so it's interesting that your browser is behaving differently.
chrome-iframe-scroll.mp4
902 KB View Download
Project Member

Comment 6 by sheriffbot@chromium.org, Apr 24 2018

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: szager@chromium.org
Labels: -Pri-2 -Type-Compat Pri-1 Type-Bug-Regression
Owner: pdr@chromium.org
Status: Assigned (was: Unconfirmed)
pdr@, this is an RLS bug it seems. Could you verify the reproduction and maybe reassign?

Comment 8 by pdr@chromium.org, Apr 24 2018

Components: -Blink>Image Blink>Paint
Owner: skobes@chromium.org
Thanks for taking the time to file this with such a small testcase.

The reproducibility of this bug depends on whether we are on highdpi or not. To reproduce this on all systems, run content shell or chrome using:
--enable-blink-features=RootLayerScrolling --disable-prefer-compositing-to-lcd-text

I've attached a single-file repro case.

In terms of triage, most folks have an RLS bug or two at the moment. Steve, do you have time to look at this one? If not, please re-assign to szager.
index.html
234 bytes View Download

Comment 9 by skobes@chromium.org, Apr 24 2018

Cc: -szager@chromium.org skobes@chromium.org
Owner: szager@chromium.org
I am currently looking at two other RLS bugs so it will be a while before I can get to this one.  Stefan can you take a look if you have time?
I am OOO today, but I will take a look probably tomorrow.

Comment 11 by pdr@chromium.org, Apr 25 2018

Owner: pdr@chromium.org
I'll pick this one up.
Project Member

Comment 12 by bugdroid1@chromium.org, May 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/66da0224bbb29d288f3cee13d3a6d2f6cb33af0c

commit 66da0224bbb29d288f3cee13d3a6d2f6cb33af0c
Author: Philip Rogers <pdr@chromium.org>
Date: Thu May 03 18:03:43 2018

Stop checking compositing in PaintLayer::GetBackgroundPaintLocation

PaintLayer::GetBackgroundPaintLocation returns a mask of where the
background should paint and is used for determining if composited
scrolling can occur at all through:
PaintLayerScrollableArea::ComputeNeedsCompositedScrolling.

If composited scrolling is not used, callsites handle not actually
painting onto the scrolling contents layer, see:
CompositedLayerMapping::UpdateBackgroundPaintsOntoScrollingContentsLayer

This patch breaks the dependency cycle introduced by [1]. Instead of
querying compositing state, this patch special-cases the root layer
so that kBackgroundPaintInScrollingContents can be returned even if
there is no overflow because the root layer creates scrolling contents
layers in this case.

[1] https://crrev.com/9e0775c12ad12

Bug:  835755 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I139f2f85c68d267de80c7b1e534a9fcef9af3983
Reviewed-on: https://chromium-review.googlesource.com/1042529
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555806}
[modify] https://crrev.com/66da0224bbb29d288f3cee13d3a6d2f6cb33af0c/third_party/blink/renderer/core/paint/paint_layer.cc

Project Member

Comment 13 by bugdroid1@chromium.org, May 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/410f7a0672f09bdf107a7e2c268990cc40b98982

commit 410f7a0672f09bdf107a7e2c268990cc40b98982
Author: Philip Rogers <pdr@chromium.org>
Date: Thu May 03 18:16:00 2018

[RLS] Correct non-composited fixed background position

We prefer to paint background-attachment: fixed into the scrolling
contents layer if text quality is important (at the cost of repainting
on every scroll). This approach had a bug for the LayoutView if there
turned out to be no scrolling layer (e.g., in a non-composited iframe).
The counterscroll logic in FixedAttachmentPositioningArea should
only apply if actually painting into a scrolling layer. This patch
queries the background compositing state before applying the counter-
scroll. This is safe because it occurs during paint which is after the
compositing decisions have been made.

Bug:  835755 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ieebffb50579b28488ef802736dd1210b8b27b26d
Reviewed-on: https://chromium-review.googlesource.com/1036941
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555814}
[add] https://crrev.com/410f7a0672f09bdf107a7e2c268990cc40b98982/third_party/WebKit/LayoutTests/paint/frames/iframe-with-scrolled-fixed-background-expected.html
[add] https://crrev.com/410f7a0672f09bdf107a7e2c268990cc40b98982/third_party/WebKit/LayoutTests/paint/frames/iframe-with-scrolled-fixed-background.html
[modify] https://crrev.com/410f7a0672f09bdf107a7e2c268990cc40b98982/third_party/blink/renderer/core/paint/background_image_geometry.cc

Project Member

Comment 14 by bugdroid1@chromium.org, May 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0985a37c57dac819058a36f96dd8c3a2362db162

commit 0985a37c57dac819058a36f96dd8c3a2362db162
Author: Philip Rogers <pdr@chromium.org>
Date: Thu May 03 18:45:32 2018

[RLS] Correct non-composited fixed background clip

We prefer to paint background-attachment: fixed into the scrolling
contents layer if text quality is important (at the cost of repainting
on every scroll). This approach had a bug for the LayoutView if there
turned out to be no scrolling layer (e.g., in a non-composited iframe).
Without a scrolling layer, a fixed background will not be clipped. This
patch ensures the background is clipped in ViewPainter.

Bug:  835755 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I6c19406fdacb64c5319709f9495e4fc8d608ae33
Reviewed-on: https://chromium-review.googlesource.com/1042266
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555827}
[add] https://crrev.com/0985a37c57dac819058a36f96dd8c3a2362db162/third_party/WebKit/LayoutTests/paint/frames/iframe-with-big-fixed-background-expected.html
[add] https://crrev.com/0985a37c57dac819058a36f96dd8c3a2362db162/third_party/WebKit/LayoutTests/paint/frames/iframe-with-big-fixed-background.html
[modify] https://crrev.com/0985a37c57dac819058a36f96dd8c3a2362db162/third_party/blink/renderer/core/paint/view_painter.cc

Comment 15 by pdr@chromium.org, May 3 2018

Labels: RegressedIn-66 Merge-Request-67
Requesting merge for both #14 and #13. Both of these are small changes.
NextAction: 2018-05-04
Pls update the bug with canary result tomorrow.

Also this is regressed in M66, how critical and safe to take these merges in for M67?
Labels: TE-Verified-M68 TE-Verified-68.0.3419.0
Able to reproduce the issue on ubuntu 17.10 using chrome reported version #66.0.3359.117 as per the comment #8.

Verified the fix on Ubuntu 17.10 using Chrome version #68.0.3419.0 as per the comment #8.
Attaching screen cast for reference.
Observed that background image stayed where it is on scrolling horizontally and vertically.
Hence, the fix is working as expected. 
Adding the verified labels.

Thanks...!!
835755@linux.webm
3.5 MB View Download
The NextAction date has arrived: 2018-05-04

Comment 19 by pdr@chromium.org, May 4 2018

I also confirmed this works locally. Note that this only reproduced with --disable-prefer-compositing-to-lcd-text (or a low-dpi device).

This patch is important to merge because fixed backgrounds are unusable on low-dpi devices. The two fixes are straightforward. I have not seen other reports of this, but I suspect it is because the people likely to file bugs are on mid-to-high end machines where this doesn't reproduce.

Requesting merge for both #14 and #13
Labels: -Merge-Request-67 Merge-Approved-67
Approving merge for CLs listed at #13 and #14 to M67 branch 3396 based on comment #17 and #19. Pls merge ASAP. Thank you.
Project Member

Comment 21 by bugdroid1@chromium.org, May 4 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e140c310c7430ec01d8abc87c52d729ceef44198

commit e140c310c7430ec01d8abc87c52d729ceef44198
Author: Philip Rogers <pdr@chromium.org>
Date: Fri May 04 17:55:17 2018

[RLS] Correct non-composited fixed background clip

We prefer to paint background-attachment: fixed into the scrolling
contents layer if text quality is important (at the cost of repainting
on every scroll). This approach had a bug for the LayoutView if there
turned out to be no scrolling layer (e.g., in a non-composited iframe).
Without a scrolling layer, a fixed background will not be clipped. This
patch ensures the background is clipped in ViewPainter.

Bug:  835755 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I6c19406fdacb64c5319709f9495e4fc8d608ae33
Reviewed-on: https://chromium-review.googlesource.com/1042266
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#555827}(cherry picked from commit 0985a37c57dac819058a36f96dd8c3a2362db162)
Reviewed-on: https://chromium-review.googlesource.com/1044571
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#483}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[add] https://crrev.com/e140c310c7430ec01d8abc87c52d729ceef44198/third_party/WebKit/LayoutTests/paint/frames/iframe-with-big-fixed-background-expected.html
[add] https://crrev.com/e140c310c7430ec01d8abc87c52d729ceef44198/third_party/WebKit/LayoutTests/paint/frames/iframe-with-big-fixed-background.html
[modify] https://crrev.com/e140c310c7430ec01d8abc87c52d729ceef44198/third_party/blink/renderer/core/paint/view_painter.cc

Project Member

Comment 22 by bugdroid1@chromium.org, May 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/40e3f980a317442a9b61da3e5d9f253fbe55b695

commit 40e3f980a317442a9b61da3e5d9f253fbe55b695
Author: Philip Rogers <pdr@chromium.org>
Date: Fri May 04 17:56:16 2018

[RLS] Correct non-composited fixed background position

We prefer to paint background-attachment: fixed into the scrolling
contents layer if text quality is important (at the cost of repainting
on every scroll). This approach had a bug for the LayoutView if there
turned out to be no scrolling layer (e.g., in a non-composited iframe).
The counterscroll logic in FixedAttachmentPositioningArea should
only apply if actually painting into a scrolling layer. This patch
queries the background compositing state before applying the counter-
scroll. This is safe because it occurs during paint which is after the
compositing decisions have been made.

Bug:  835755 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ieebffb50579b28488ef802736dd1210b8b27b26d
Reviewed-on: https://chromium-review.googlesource.com/1036941
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#555814}(cherry picked from commit 410f7a0672f09bdf107a7e2c268990cc40b98982)
Reviewed-on: https://chromium-review.googlesource.com/1044726
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#484}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[add] https://crrev.com/40e3f980a317442a9b61da3e5d9f253fbe55b695/third_party/WebKit/LayoutTests/paint/frames/iframe-with-scrolled-fixed-background-expected.html
[add] https://crrev.com/40e3f980a317442a9b61da3e5d9f253fbe55b695/third_party/WebKit/LayoutTests/paint/frames/iframe-with-scrolled-fixed-background.html
[modify] https://crrev.com/40e3f980a317442a9b61da3e5d9f253fbe55b695/third_party/blink/renderer/core/paint/background_image_geometry.cc

Comment 23 by pdr@chromium.org, May 9 2018

Status: Fixed (was: Assigned)
 Issue 844562  has been merged into this issue.
So will this be fixed in a release today (Chrome 67) as specified on the release calendar?
Releases roll out in a staged fashion to make sure we haven't broken anything serious for a large number of users. End of the week is a reasonable timeframe to expect to see M-67 as stable for everyone, assuming we indeed have not horribly broken anything.

Comment 27 by pdr@chromium.org, Jun 7 2018

Cc: pdr@chromium.org pbomm...@chromium.org
 Issue 848208  has been merged into this issue.

Sign in to add a comment