Issue metadata
Sign in to add a comment
|
Buggy scroll - Body element is shown during scroll
Reported by
aka...@wix.com,
Oct 25
|
||||||||||||||||||||||
Issue descriptionUserAgent: 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.
,
Oct 25
I managed to reproduce this on Chrome Dev 71, here is a screencast: https://drive.google.com/open?id=0B9LlSNwL2H8YbzRWeWJsSk92UUo0eU8yNjZmUDZ6UGg5RDJj.
,
Oct 25
I have hosted the bug creator's test file here: https://ritzy-green.glitch.me/.
,
Oct 25
I can repro on 72.0.3591.0. This looks pretty bad, I'll take a look.
,
Oct 26
,
Oct 28
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.
,
Oct 31
Looking at this today.
,
Oct 31
This bisected to my patch in https://chromium-review.googlesource.com/c/chromium/src/+/1173441/ Continuing to investigate a fix
,
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
,
Nov 6
Confirmed fix in Canary 72.0.3602.0. Requesting merge to M71
,
Nov 6
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
,
Nov 6
Approved for merge to 71, branch 3578.
,
Nov 6
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
,
Nov 6
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}
,
Nov 6
Fix has been merged to M71 so this should go out in the next milestone.
,
Nov 7
Thank you so much for helping us! Amit. On Wed, Nov 7, 2018, 01:32 bo… via monorail < monorail+v2.3285626211@chromium.org wrote:
,
Nov 7
Thanks as well from the Web DevRel side for the quick turn-around time, David!
,
Nov 7
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.
,
Nov 7
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
,
Nov 8
Scrolling is smooth, Works as per expected behavior. Verfied on 71.0.3578.45 and 72.0.3604.0
,
Nov 9
Thanks, will give it the weekend in Beta to make sure nothing else goes wrong
,
Nov 13
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.
,
Nov 14
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.
,
Nov 14
,
Nov 14
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}
,
Nov 14
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
,
Nov 14
The fix is merged into M70 - if we do decide to update stable channel is will include this fix.
,
Nov 14
Great news! thank you guys for pushing this into M70! we appreciate that.
,
Nov 16
Scrolling is smooth, Works as per expected behavior. Verfied on 70.0.3538.110
,
Nov 16
Thank you guys so much for helping us! On Fri, Nov 16, 2018, 18:17 kgna… via monorail < monorail+v2.3659348243@chromium.org wrote:
,
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 |
|||||||||||||||||||||||
Comment 1 by aka...@wix.com
, Oct 25