New issue
Advanced search Search tips

Issue 898757 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Buggy scroll - Body element is shown during scroll

Reported by aka...@wix.com, Oct 25

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.67 Safari/537.36

Steps to reproduce the problem:
1. Open the site with Chrome 70 on Android
2. Scroll down / up

What is the expected behavior?
Smooth scroll

What went wrong?
The Body element is shown during scroll. I've marked the Body in Blue and the inner element in Red so it will be easier to identify the issue.

Did this work before? Yes Chrome 69

Does this work in other browsers? N/A

Chrome version: 70.0.3538.67  Channel: stable
OS Version: 8.0.0
Flash Version: 

I think is is somehow related to the meta viewport element.
 
index.html
4.1 KB View Download
WhatsApp Image 2018-10-25 at 08.39.48.jpeg
190 KB View Download
This is a Chrome 70.0.3538.64 bug on an ***Android Mobile Device***
Not sure how to edit this ticket properly.
Labels: -OS-Mac OS-Android
Status: Untriaged (was: Unconfirmed)
I managed to reproduce this on Chrome Dev 71, here is a screencast: https://drive.google.com/open?id=0B9LlSNwL2H8YbzRWeWJsSk92UUo0eU8yNjZmUDZ6UGg5RDJj.
I have hosted the bug creator's test file here: https://ritzy-green.glitch.me/.
Components: Blink>Paint
Labels: -Pri-2 Pri-1
Owner: bokan@chromium.org
Status: Assigned (was: Untriaged)
I can repro on 72.0.3591.0. This looks pretty bad, I'll take a look.
Cc: tsteiner@google.com
This bug is super critical as it is affecting all Wix website and a big portion of the site visitors traffic that reach those websites. 60% of Wix websites traffic comes from mobile devices and 50% our of mobile traffic comes from Chrome on Android, that means this but is affection 30% of Wix sites traffic!

As the Chrome 70 rollout is progressing our support is getting more and more complaints and it has become a major issue in our platform.
Status: Started (was: Assigned)
Looking at this today.
This bisected to my patch in https://chromium-review.googlesource.com/c/chromium/src/+/1173441/

Continuing to investigate a fix
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 2

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

commit f9cbcfa426bdf0cb830cd7d1ce7cc80845184d3e
Author: David Bokan <bokan@chromium.org>
Date: Fri Nov 02 15:33:39 2018

Fix URL bar clipping with min-scale > 1

In https://crrev.com/bf23241073ef806810f2aa90bbb153b639239c81, I rewrote
UpdateViewportContainerSizes to be compatible with Blink generated
property trees. As part of this, we now make use of the outer viewport's
clip node. However, unlike the layer's bounds, the clip node's bounds
do not include the resize we perform to match the content width of the
page.

In WebViewImpl::ResizeAfterLayout, we resize the FrameView (and thus the
LayoutView) after we do a layout. We do this so that the layout viewport
is sized according to the minimum possible page scale (so pos: fixed
Elements are in the correct locations when the page is fully zoomed
out).

However, this isn't accounted for correctly in the ClipNode. This CL
applies the minimum scale to the clip bounds directly when computing the
clip as a result of URL bar movement. We should really fix the place
where the ClipNode is computed but this CL will need to be merged so
this is a minimal fix.

Note also, the ClipNode is correctly calculated with blink-gen-property-
trees enabled. This fix is thus only required when that's turned off.

Bug:  898757 
Change-Id: I9c8ea517c2e98b6b2ac552b1ae64bd1675547614
Reviewed-on: https://chromium-review.googlesource.com/c/1313278
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604932}
[modify] https://crrev.com/f9cbcfa426bdf0cb830cd7d1ce7cc80845184d3e/cc/trees/layer_tree_host_impl.cc

Labels: Merge-Request-71
Confirmed fix in Canary 72.0.3602.0. Requesting merge to M71
Project Member

Comment 11 by sheriffbot@chromium.org, Nov 6

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-71 Merge-Approved-71
Approved for merge to 71, branch 3578.
Project Member

Comment 13 by bugdroid1@chromium.org, Nov 6

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/db06768cbeaa6b11039cbf62eaf9be8cb9a10392

commit db06768cbeaa6b11039cbf62eaf9be8cb9a10392
Author: David Bokan <bokan@chromium.org>
Date: Tue Nov 06 20:32:18 2018

Fix URL bar clipping with min-scale > 1

In https://crrev.com/bf23241073ef806810f2aa90bbb153b639239c81, I rewrote
UpdateViewportContainerSizes to be compatible with Blink generated
property trees. As part of this, we now make use of the outer viewport's
clip node. However, unlike the layer's bounds, the clip node's bounds
do not include the resize we perform to match the content width of the
page.

In WebViewImpl::ResizeAfterLayout, we resize the FrameView (and thus the
LayoutView) after we do a layout. We do this so that the layout viewport
is sized according to the minimum possible page scale (so pos: fixed
Elements are in the correct locations when the page is fully zoomed
out).

However, this isn't accounted for correctly in the ClipNode. This CL
applies the minimum scale to the clip bounds directly when computing the
clip as a result of URL bar movement. We should really fix the place
where the ClipNode is computed but this CL will need to be merged so
this is a minimal fix.

Note also, the ClipNode is correctly calculated with blink-gen-property-
trees enabled. This fix is thus only required when that's turned off.

Bug:  898757 
Change-Id: I9c8ea517c2e98b6b2ac552b1ae64bd1675547614
Reviewed-on: https://chromium-review.googlesource.com/c/1313278
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#604932}(cherry picked from commit f9cbcfa426bdf0cb830cd7d1ce7cc80845184d3e)
Reviewed-on: https://chromium-review.googlesource.com/c/1320811
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#543}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/db06768cbeaa6b11039cbf62eaf9be8cb9a10392/cc/trees/layer_tree_host_impl.cc

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/db06768cbeaa6b11039cbf62eaf9be8cb9a10392

Commit: db06768cbeaa6b11039cbf62eaf9be8cb9a10392
Author: bokan@chromium.org
Commiter: bokan@chromium.org
Date: 2018-11-06 20:32:18 +0000 UTC

Fix URL bar clipping with min-scale > 1

In https://crrev.com/bf23241073ef806810f2aa90bbb153b639239c81, I rewrote
UpdateViewportContainerSizes to be compatible with Blink generated
property trees. As part of this, we now make use of the outer viewport's
clip node. However, unlike the layer's bounds, the clip node's bounds
do not include the resize we perform to match the content width of the
page.

In WebViewImpl::ResizeAfterLayout, we resize the FrameView (and thus the
LayoutView) after we do a layout. We do this so that the layout viewport
is sized according to the minimum possible page scale (so pos: fixed
Elements are in the correct locations when the page is fully zoomed
out).

However, this isn't accounted for correctly in the ClipNode. This CL
applies the minimum scale to the clip bounds directly when computing the
clip as a result of URL bar movement. We should really fix the place
where the ClipNode is computed but this CL will need to be merged so
this is a minimal fix.

Note also, the ClipNode is correctly calculated with blink-gen-property-
trees enabled. This fix is thus only required when that's turned off.

Bug:  898757 
Change-Id: I9c8ea517c2e98b6b2ac552b1ae64bd1675547614
Reviewed-on: https://chromium-review.googlesource.com/c/1313278
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#604932}(cherry picked from commit f9cbcfa426bdf0cb830cd7d1ce7cc80845184d3e)
Reviewed-on: https://chromium-review.googlesource.com/c/1320811
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#543}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Status: Fixed (was: Started)
Fix has been merged to M71 so this should go out in the next milestone.
Thank you so much for helping us!
Amit.

On Wed, Nov 7, 2018, 01:32 bo… via monorail <
monorail+v2.3285626211@chromium.org wrote:
Thanks as well from the Web DevRel side for the quick turn-around time, David! 
Status: Started (was: Fixed)
akaspi@: Have you been able to verify the fix on Wix in Canary or Dev channels? The fix should be making its way into beta channel soon - we'll try to merge it back to stable in M70 assuming everything goes well.
Bokan!: I'm akaspi's colleague, I have checked the fix on Canary channel and didn't see the bug happening again :)
Let us know when it is pushed to the Beta channel so we can check it on that as well.

Thanks
Scrolling is smooth, Works as per expected behavior.
Verfied on 71.0.3578.45 and 72.0.3604.0
Thanks, will give it the weekend in Beta to make sure nothing else goes wrong
Status: Fixed (was: Started)
Just to update: I looked into it but we're unfortunately not going to refresh stable channel with this fix. Updating stable channel is a big deal and carries its own risks so Chrome policy is to do this only exceptional circumstances (e.g. severe security bugs, frequent crashes). Unfortunately, this kind of site-compat issue doesn't fall into that category. Sorry.

The good news is M71 which does have the fix will be shipping in ~3 weeks. If that's too long, users can be advised to use Beta channel in the interim. Sites can also be fixed so they don't hit this bug. I realized I haven't described the issue or how to fix it here (shared privately off-bug) so I'll reproduce it here for anyone else following along:

The cause of this bug is using an unusual viewport <meta> tag:
<meta name="viewport" content="width=320, user-scalable=yes">

On a device that has a screen wider than 320px (which is many devices), this will make the content less wide than the device screen. In that case, the content has to be zoomed in and the minimum scale is > 1 (the bug is that Chrome isn't handling the minimum-scale>1 case correctly).

The more idiomatic meta tag is:
<meta name="viewport" content="width=device-width, user-scalable=yes">

Which will use the device's screen width to flow content into, meaning we won't cause any zooming and the bug won't occur. This will change the content in that it will appear the same size across all devices (currently, I expect the content would appear larger in proportion to the device's screen size). If that's the desired behavior, it can be achieved in other ways e.g. making font and other sizes proportional to screen size (e.g. vw and vh units).

I'll mark this as fixed but feel free to reply if you have questions or further issues.
Cc: benmason@chromium.org
Labels: Merge-Request-70
Status: Started (was: Fixed)
benmason@ says we might be doing a respin of M70 for other reasons so maybe we can piggy-back into that.

Adding request merge for 70. The merge is up at https://crrev.com/c/1322970. It's already been merged back to M71 and has been baking in Canary for almost 2 weeks.
Labels: -Merge-Request-70 Merge-Approved-70
Labels: -Merge-Approved-70 Merge-Merged-70-3538
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/ac3be36af32f0edd12f4c237d2abef8515a6260f

Commit: ac3be36af32f0edd12f4c237d2abef8515a6260f
Author: bokan@chromium.org
Commiter: bokan@chromium.org
Date: 2018-11-14 21:23:59 +0000 UTC

Fix URL bar clipping with min-scale > 1

In https://crrev.com/bf23241073ef806810f2aa90bbb153b639239c81, I rewrote
UpdateViewportContainerSizes to be compatible with Blink generated
property trees. As part of this, we now make use of the outer viewport's
clip node. However, unlike the layer's bounds, the clip node's bounds
do not include the resize we perform to match the content width of the
page.

In WebViewImpl::ResizeAfterLayout, we resize the FrameView (and thus the
LayoutView) after we do a layout. We do this so that the layout viewport
is sized according to the minimum possible page scale (so pos: fixed
Elements are in the correct locations when the page is fully zoomed
out).

However, this isn't accounted for correctly in the ClipNode. This CL
applies the minimum scale to the clip bounds directly when computing the
clip as a result of URL bar movement. We should really fix the place
where the ClipNode is computed but this CL will need to be merged so
this is a minimal fix.

Note also, the ClipNode is correctly calculated with blink-gen-property-
trees enabled. This fix is thus only required when that's turned off.

(cherry picked from commit f9cbcfa426bdf0cb830cd7d1ce7cc80845184d3e)

Bug:  898757 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Icd362c4518c506a40cbb507ff664cbb2d677d564
Reviewed-on: https://chromium-review.googlesource.com/c/1322970
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#1087}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
Project Member

Comment 26 by bugdroid1@chromium.org, Nov 14

Labels: merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ac3be36af32f0edd12f4c237d2abef8515a6260f

commit ac3be36af32f0edd12f4c237d2abef8515a6260f
Author: David Bokan <bokan@chromium.org>
Date: Wed Nov 14 21:23:59 2018

Fix URL bar clipping with min-scale > 1

In https://crrev.com/bf23241073ef806810f2aa90bbb153b639239c81, I rewrote
UpdateViewportContainerSizes to be compatible with Blink generated
property trees. As part of this, we now make use of the outer viewport's
clip node. However, unlike the layer's bounds, the clip node's bounds
do not include the resize we perform to match the content width of the
page.

In WebViewImpl::ResizeAfterLayout, we resize the FrameView (and thus the
LayoutView) after we do a layout. We do this so that the layout viewport
is sized according to the minimum possible page scale (so pos: fixed
Elements are in the correct locations when the page is fully zoomed
out).

However, this isn't accounted for correctly in the ClipNode. This CL
applies the minimum scale to the clip bounds directly when computing the
clip as a result of URL bar movement. We should really fix the place
where the ClipNode is computed but this CL will need to be merged so
this is a minimal fix.

Note also, the ClipNode is correctly calculated with blink-gen-property-
trees enabled. This fix is thus only required when that's turned off.

(cherry picked from commit f9cbcfa426bdf0cb830cd7d1ce7cc80845184d3e)

Bug:  898757 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Icd362c4518c506a40cbb507ff664cbb2d677d564
Reviewed-on: https://chromium-review.googlesource.com/c/1322970
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#1087}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/ac3be36af32f0edd12f4c237d2abef8515a6260f/cc/trees/layer_tree_host_impl.cc

Status: Fixed (was: Started)
The fix is merged into M70 - if we do decide to update stable channel is will include this fix.
Great news! thank you guys for pushing this into M70! we appreciate that.
Scrolling is smooth, Works as per expected behavior.
Verfied on 70.0.3538.110
Thank you guys so much for helping us!

On Fri, Nov 16, 2018, 18:17 kgna… via monorail <
monorail+v2.3659348243@chromium.org wrote:
Project Member

Comment 31 by bugdroid1@chromium.org, Nov 16

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

commit a6426e398c74b4f61ff9a62bdc22d2b66b9fff0f
Author: David Bokan <bokan@chromium.org>
Date: Fri Nov 16 19:04:33 2018

Add test for viewport clipping issue

In https://crrev.com/f9cbcfa426bdf0cb830cd7d1ce7cc80845184d3e I landed
a fix to  https://crbug.com/898757 . This fix was needed ASAP since it
needed to be merged. I landed a fix without a test at the time. This CL
adds a test.

Bug: 901083, 898757 
Change-Id: I0b3dc44a7e94e33f5897f7d9f19159523807dc56
Reviewed-on: https://chromium-review.googlesource.com/c/1336786
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608880}
[modify] https://crrev.com/a6426e398c74b4f61ff9a62bdc22d2b66b9fff0f/cc/trees/layer_tree_host_impl_unittest.cc

Sign in to add a comment