Regression : Unable to scroll chrome://settings page using mouse wheel.
Reported by
rp...@etouch.net,
Oct 26
|
||||||||||
Issue descriptionChrome Version: 72.0.3592.0 (Official Build)Revision 3274d2b27158c8f54008ac69629235202f28f306-refs/branch-heads/3592@{#1}(32/64-bit) OS: Win(10) What steps will reproduce the problem? 1. Freshly launch chrome, navigate to chrome://settings and try to scroll page using mouse wheel , observe Actual Result: Unable to scroll page using mouse wheel Expected Result: Should be able to scroll page using mouse wheel This is a regression issue, broken in M-72, and will soon update other info. Good Build:72.0.3591.0 (Revision:602532) Bad Build: 72.0.3592.0 (Revision:602946)
,
Oct 26
,
Oct 29
Adding release blocker label for this issue.Please reduce priority or remove if not the case. Thank You!
,
Oct 29
I've noticed that it happens to any element which have overflow:auto and the <body> or <html> of the document have overflow:hidden. If removed from these two, the scroll of the element or the body starts to work again.
,
Oct 29
Re#4, could you put up (attach/jsbin) a repro I can try it on?
,
Oct 29
This also breaks google docs scrolling.
,
Oct 29
Issue 899760 has been merged into this issue.
,
Oct 29
Noticed in https://www.sephora.com/, https://www.costco.com/. No error in console. bokan@ Please revert the CL ASAP, we are planning a Dev RC today.
,
Oct 29
Revert is in the CQ. I have a fix for what I think is related bug 899234 but I'll re-land with the fix included. I wasn't able to reproduce the issue on chrome://settings. In 899234, I could only repro broken scrolling on (for e.g.) apple.com when navigating to it from another domain. Navigating to it from a new tab page worked as expected. To those who can repro the issue on chrome://settings, can you check if it's a similar set of steps? Is there anything else missing in the repro steps above?
,
Oct 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a1da8cb36f3f3ede604c2ab578e5af5c3f06c4b0 commit a1da8cb36f3f3ede604c2ab578e5af5c3f06c4b0 Author: David Bokan <bokan@chromium.org> Date: Mon Oct 29 20:02:14 2018 Revert "Convert scrolling code to use Node" This reverts commit f51a68b073770244754b996f0d9e7599f1c09e99 in 3595 branch. TBR=skobes@chromium.org Bug: 899161 Change-Id: I84054ad57ae8aad412e3cb3137a112d7e90c47c4 Reviewed-on: https://chromium-review.googlesource.com/c/1305884 Reviewed-by: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/branch-heads/3595@{#3} Cr-Branched-From: 2afd2daef64f5ee3e111385180e487634adc4002-refs/heads/master@{#603394} [modify] https://crrev.com/a1da8cb36f3f3ede604c2ab578e5af5c3f06c4b0/third_party/blink/renderer/core/dom/node.cc [modify] https://crrev.com/a1da8cb36f3f3ede604c2ab578e5af5c3f06c4b0/third_party/blink/renderer/core/dom/node.h [modify] https://crrev.com/a1da8cb36f3f3ede604c2ab578e5af5c3f06c4b0/third_party/blink/renderer/core/frame/local_frame_view.cc [modify] https://crrev.com/a1da8cb36f3f3ede604c2ab578e5af5c3f06c4b0/third_party/blink/renderer/core/input/scroll_manager.cc [modify] https://crrev.com/a1da8cb36f3f3ede604c2ab578e5af5c3f06c4b0/third_party/blink/renderer/core/input/scroll_manager.h [modify] https://crrev.com/a1da8cb36f3f3ede604c2ab578e5af5c3f06c4b0/third_party/blink/renderer/core/page/scrolling/root_scroller_controller.cc [modify] https://crrev.com/a1da8cb36f3f3ede604c2ab578e5af5c3f06c4b0/third_party/blink/renderer/core/page/scrolling/root_scroller_controller.h [modify] https://crrev.com/a1da8cb36f3f3ede604c2ab578e5af5c3f06c4b0/third_party/blink/renderer/core/page/scrolling/root_scroller_test.cc [modify] https://crrev.com/a1da8cb36f3f3ede604c2ab578e5af5c3f06c4b0/third_party/blink/renderer/core/page/scrolling/root_scroller_util.cc [modify] https://crrev.com/a1da8cb36f3f3ede604c2ab578e5af5c3f06c4b0/third_party/blink/renderer/core/page/scrolling/root_scroller_util.h [modify] https://crrev.com/a1da8cb36f3f3ede604c2ab578e5af5c3f06c4b0/third_party/blink/renderer/core/page/scrolling/scroll_state.cc [modify] https://crrev.com/a1da8cb36f3f3ede604c2ab578e5af5c3f06c4b0/third_party/blink/renderer/core/page/scrolling/top_document_root_scroller_controller.cc [modify] https://crrev.com/a1da8cb36f3f3ede604c2ab578e5af5c3f06c4b0/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc
,
Oct 29
re#9: bokan@ I can reproduce it on the Settings Page when I open a NTP and navigate then to it by typing chrome://settings into the Omnibox. Reloading the Settings Page via CMD-R fixes it. (OS: MacOS 10.14)
,
Oct 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d4424640365874fbc63eeb94674841aa3f2487f2 commit d4424640365874fbc63eeb94674841aa3f2487f2 Author: David Bokan <bokan@chromium.org> Date: Mon Oct 29 21:08:34 2018 Revert "Convert scrolling code to use Node" This reverts commit f51a68b073770244754b996f0d9e7599f1c09e99. Reason for revert: breaks scrolling on some pages: https://crbug.com/899161 Original change's description: > Convert scrolling code to use Node > > In https://crrev.com/9f54d240af5d366b1d6a4d9b84438192a9f0a11c we moved > ScrollCustomization from using Elements to Node. This allows us to use > Nodes in the scrolling code which is much more natural since the > viewport scrolls using the LayoutView (document Node)'s scrollable area. > > The main change in this CL is letting the Document node be part of the > scroll chain, instead of using the documentElement in its place. This > removes some special-casing all the way through the scrolling pipeline. > > As a result the Global Root Scroller now sets the viewport scroll > callback on the document node. > > Bug: 897288 > Change-Id: I899e6007b676efb94675d94b3d97702c8f1f2a94 > Reviewed-on: https://chromium-review.googlesource.com/c/1299205 > Commit-Queue: David Bokan <bokan@chromium.org> > Reviewed-by: Steve Kobes <skobes@chromium.org> > Cr-Commit-Position: refs/heads/master@{#602935} TBR=bokan@chromium.org,skobes@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 897288 , 899161 Change-Id: Ic75c23a1bcde6d86e554c2b5bdb5548b37d57e5f Reviewed-on: https://chromium-review.googlesource.com/c/1305820 Commit-Queue: David Bokan <bokan@chromium.org> Reviewed-by: Steve Kobes <skobes@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#603626} [modify] https://crrev.com/d4424640365874fbc63eeb94674841aa3f2487f2/third_party/blink/renderer/core/dom/node.cc [modify] https://crrev.com/d4424640365874fbc63eeb94674841aa3f2487f2/third_party/blink/renderer/core/dom/node.h [modify] https://crrev.com/d4424640365874fbc63eeb94674841aa3f2487f2/third_party/blink/renderer/core/frame/local_frame_view.cc [modify] https://crrev.com/d4424640365874fbc63eeb94674841aa3f2487f2/third_party/blink/renderer/core/input/scroll_manager.cc [modify] https://crrev.com/d4424640365874fbc63eeb94674841aa3f2487f2/third_party/blink/renderer/core/input/scroll_manager.h [modify] https://crrev.com/d4424640365874fbc63eeb94674841aa3f2487f2/third_party/blink/renderer/core/page/scrolling/root_scroller_controller.cc [modify] https://crrev.com/d4424640365874fbc63eeb94674841aa3f2487f2/third_party/blink/renderer/core/page/scrolling/root_scroller_controller.h [modify] https://crrev.com/d4424640365874fbc63eeb94674841aa3f2487f2/third_party/blink/renderer/core/page/scrolling/root_scroller_test.cc [modify] https://crrev.com/d4424640365874fbc63eeb94674841aa3f2487f2/third_party/blink/renderer/core/page/scrolling/root_scroller_util.cc [modify] https://crrev.com/d4424640365874fbc63eeb94674841aa3f2487f2/third_party/blink/renderer/core/page/scrolling/root_scroller_util.h [modify] https://crrev.com/d4424640365874fbc63eeb94674841aa3f2487f2/third_party/blink/renderer/core/page/scrolling/scroll_state.cc [modify] https://crrev.com/d4424640365874fbc63eeb94674841aa3f2487f2/third_party/blink/renderer/core/page/scrolling/top_document_root_scroller_controller.cc [modify] https://crrev.com/d4424640365874fbc63eeb94674841aa3f2487f2/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc
,
Oct 29
Re#11: Hmm, is it a consistent repro or only occasional? I tried this on a MacBook pro and it works for me. When you see this, is it on a retina screen or low-DPI? Are you using a trackpad or a mouse with a physical wheel? In any case, this is now reverted in the branch and in ToT so I'll mark this as fixed. Will reland with fixes included.
,
Oct 29
,
Oct 29
re#13: It is a consistent repro. I am using a MacBook Air 11“ from Mid 2012 (low-DPI) with its build-in Trackpad. Thank you for the revert.
,
Oct 29
Issue 899939 has been merged into this issue.
,
Oct 30
,
Oct 30
Update : Rechecked the above issue on Windows (7,8,8.1,10),Linux (14.04 LTS),Mac OS X(10.13.1,10.13.6,10.14.1) using latest Canary build : 72.0.3596.0 and the issue is Fixed.Hence adding TE Verified Labels. Kindly refer the attached screen cast. Thank you..!!
,
Oct 30
Hi, Retested the above issue on (7,8,8.1,10),Mac(10.13.1, 10.13.6, 10.14.1) and Linux(14.04 LTS) OS using latest Dev build #72.0.3595.2 and issue is fixed. Now, able to scroll on chrome://settings page using mouse wheel. Kindly refer the attached screen-cast Thank You!
,
Oct 30
bokan, I'm surprised no tests were able to catch this regression, not even the top-chrome sliding tests.
,
Oct 30
It's unfortunate but we don't have great end-to-end coverage of top controls so regressions like this occasionally happen. In addition, this bug was a result of our complex loading behavior - it would reproduce only when navigating from one origin to another so even a basic "load some HTML, try to scroll, check if controls hid" test wouldn't have caught it. I'll make sure I land a test to at least catch this case in the future.
,
Oct 31
Issue 900442 has been merged into this issue.
,
Nov 2
FYI: I'm relanding the reverted CL with a fix. I've tried this out against all the reports received here so it shouldn't break (at least in the same way). Please let me know if you see anything similar.
,
Nov 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/469e7e4845955edfe85b5427914df43ae53268ba commit 469e7e4845955edfe85b5427914df43ae53268ba Author: David Bokan <bokan@chromium.org> Date: Mon Nov 05 14:50:29 2018 [Reland] Convert scrolling code to use Node In https://crrev.com/9f54d240af5d366b1d6a4d9b84438192a9f0a11c we moved ScrollCustomization from using Elements to Node. This allows us to use Nodes in the scrolling code which is much more natural since the viewport scrolls using the LayoutView (document Node)'s scrollable area. The main change in this CL is letting the Document node be part of the scroll chain, instead of using the documentElement in its place. This removes some special-casing all the way through the scrolling pipeline. As a result the Global Root Scroller now sets the viewport scroll callback on the document node. Reland node: This relands the original patch with the fix reviewed in https://crrev.com/c/1302856. Originally landed patch is uploaded in Patchset 1 for easy comparison of changes. TBR=dcheng@chromium.org,kenrb@chromium.org,lfg@chromium.org Bug: 897288 , 899161 Change-Id: Ib6b61eb3a193877abb61f896a9d1de4747496514 Reviewed-on: https://chromium-review.googlesource.com/c/1307843 Commit-Queue: David Bokan <bokan@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#605327} [modify] https://crrev.com/469e7e4845955edfe85b5427914df43ae53268ba/content/browser/site_per_process_browsertest.cc [modify] https://crrev.com/469e7e4845955edfe85b5427914df43ae53268ba/third_party/blink/renderer/core/dom/node.cc [modify] https://crrev.com/469e7e4845955edfe85b5427914df43ae53268ba/third_party/blink/renderer/core/dom/node.h [modify] https://crrev.com/469e7e4845955edfe85b5427914df43ae53268ba/third_party/blink/renderer/core/frame/local_frame_view.cc [modify] https://crrev.com/469e7e4845955edfe85b5427914df43ae53268ba/third_party/blink/renderer/core/input/scroll_manager.cc [modify] https://crrev.com/469e7e4845955edfe85b5427914df43ae53268ba/third_party/blink/renderer/core/input/scroll_manager.h [modify] https://crrev.com/469e7e4845955edfe85b5427914df43ae53268ba/third_party/blink/renderer/core/page/scrolling/root_scroller_controller.cc [modify] https://crrev.com/469e7e4845955edfe85b5427914df43ae53268ba/third_party/blink/renderer/core/page/scrolling/root_scroller_controller.h [modify] https://crrev.com/469e7e4845955edfe85b5427914df43ae53268ba/third_party/blink/renderer/core/page/scrolling/root_scroller_test.cc [modify] https://crrev.com/469e7e4845955edfe85b5427914df43ae53268ba/third_party/blink/renderer/core/page/scrolling/root_scroller_util.cc [modify] https://crrev.com/469e7e4845955edfe85b5427914df43ae53268ba/third_party/blink/renderer/core/page/scrolling/root_scroller_util.h [modify] https://crrev.com/469e7e4845955edfe85b5427914df43ae53268ba/third_party/blink/renderer/core/page/scrolling/scroll_state.cc [modify] https://crrev.com/469e7e4845955edfe85b5427914df43ae53268ba/third_party/blink/renderer/core/page/scrolling/top_document_root_scroller_controller.cc [modify] https://crrev.com/469e7e4845955edfe85b5427914df43ae53268ba/third_party/blink/renderer/core/page/scrolling/top_document_root_scroller_controller.h [modify] https://crrev.com/469e7e4845955edfe85b5427914df43ae53268ba/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by rp...@etouch.net
, Oct 26Owner: bokan@chromium.org
Status: Assigned (was: Unconfirmed)