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

Issue 746922 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression : Page autoscrolling issue is observed when middle clicked on a vertical scrollbar.

Reported by avsha...@etouch.net, Jul 20 2017

Issue description

Chrome version : 61.0.3162.0  (Official Build) 46d636d25e054da0b0d8270d525c84f39146ce30-refs/heads/master@{#488073} 32/64 bit
OS : Windows (7,8,10)

What steps will reproduce the problem?
1. Launch chrome and open NTP or any other page having multi-page contents.
2. Middle click on a vertical scroll bar seen on NTP and drag mouse downwards to start autoscrolling.
3. Observe.

Actual Result : Page does not autoscroll when middle clicked on a vertical scroll bar and mouse is dragged down.
(Note : Even the mouse pointer changes into autoscroll icon but page does not autoscroll on dragging).

Expected Result : Page should autoscroll when middle clicking on vertical scroll bar and mouse is being dragged.

This is a regression issue broken in ‘M-61’, below is the Manual Regression range and will soon update other info.
Good build : 61.0.3141.0
Bad build : 61.0.3142.0

Note : Above issue is not reproducible in Mac(10.11.6, 10.12.3) and Linux(14.04 LTS) OS.
 
Actual_Result.mp4
1.6 MB View Download
Expected_Result.mp4
9.1 MB View Download
Labels: hasbisect-per-revision
Owner: sahel@chromium.org
Status: Assigned (was: Unconfirmed)
Using the per-revision bisect providing the bisect results,
Good build:61.0.3141.0(Revision:482153).
Bad build:61.0.3142.0(Revision:482491).

You are probably looking for a change made after 482415 (known good), but no later than 482416 (first known bad).

CHANGE-LOG URL:
---------------
https://chromium.googlesource.com/chromium/src/+log/2daeff3cbf72a9d38a08df1a3093a88ef2151e67..60b41cdfe22237cb667050017349375ca9272ccf

From the CL above, assigning the issue to the concern owner

@sahel : Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.

Review-Url:https://codereview.chromium.org/2907053004
Note : Windows specific issue and Able to reproduce in latest Canary #61.0.3162.0

Comment 2 by sahel@chromium.org, Jul 31 2017

Status: Started (was: Assigned)

Comment 3 by sahel@chromium.org, Aug 2 2017

Cc: bokan@chromium.org sahel@chromium.org
 Issue 746209  has been merged into this issue.

Comment 4 by sahel@chromium.org, Aug 2 2017

 Issue 750396  has been merged into this issue.
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 4 2017

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

commit a7bd4b63f512849d1107eb4c319835e6fffbd938
Author: Sahel Sharify <sahel@chromium.org>
Date: Fri Aug 04 19:12:51 2017

Regression fixed: Autoscrolling on vertical scrollbar works.

Before landing the cl below scrollChain used to get re-computed whenever
needed, including when it wasn't computed for a GSB and needed to get
computed for its following GSU. With the change below now it gets
computed only for GSB events and if it is empty while handling a GSU,
the GSU gets ignored.
https://codereview.chromium.org/2907053004

With this cl GSB events in case of touchpad or auto scrolling are not
handled by scrollbars and the scrollchain is computed while handling
them. Then their following GSU events won't get suppressed.

All/ParameterizedWebFrameTest.scrollbarIsNotHandlingTouchpadScroll/*

Bug:  746922 
Test: fast/events/autoscroll-over-scrollbar.html,
Change-Id: I5620d64044afb3fd0f596267be141f9751626827
Reviewed-on: https://chromium-review.googlesource.com/598999
Reviewed-by: Rick Byers <rbyers@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: Sahel Sharifymoghaddam <sahel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492095}
[add] https://crrev.com/a7bd4b63f512849d1107eb4c319835e6fffbd938/third_party/WebKit/LayoutTests/fast/events/autoscroll-over-scrollbar.html
[modify] https://crrev.com/a7bd4b63f512849d1107eb4c319835e6fffbd938/third_party/WebKit/Source/core/exported/WebFrameTest.cpp
[modify] https://crrev.com/a7bd4b63f512849d1107eb4c319835e6fffbd938/third_party/WebKit/Source/core/input/ScrollManager.cpp
[modify] https://crrev.com/a7bd4b63f512849d1107eb4c319835e6fffbd938/third_party/WebKit/Source/platform/scroll/Scrollbar.cpp

Comment 6 by sahel@chromium.org, Aug 4 2017

Status: Fixed (was: Started)

Comment 7 by pdr@chromium.org, Aug 4 2017

Thanks for the quick fix!

M62 has branched. Should this be considered for a merge?

Comment 8 by sahel@chromium.org, Aug 11 2017

Cc: kkaluri@chromium.org
 Issue 703213  has been merged into this issue.

Comment 9 by bokan@chromium.org, Aug 11 2017

Labels: Merge-Request-61
Status: Started (was: Fixed)
Sahel, the fix has been baking in trunk for ~1 week, I think it's safe to merge to 61. 
Project Member

Comment 10 by sheriffbot@chromium.org, Aug 11 2017

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

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Per comment #9, change is baked in Canary and safe to merge. 
But just to be extra cautious, could you please answer followings:
* Does this change having enough automation tests coverage?
* Any other important details to justify the merge.

Please note M61 is already in Beta, so merge bar is VERY high. Thank you.

Comment 12 by bokan@chromium.org, Aug 11 2017

Re: tests, wheel and scrollbar scrolling have extensive layout tests in Blink so mainline scrolling use cases should be well protected.

I'd say the more compelling case for merging is  issue 746209  which is dup'd into this. That is, scrolls starting over a subscroller's scrollbar won't bubble up to the main page. This can be annoying to users as their scrolls can stop as they slowly scroll down a page.
Project Member

Comment 13 by bugdroid1@chromium.org, Aug 11 2017

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

commit a57a8b2195e2853a143999a0004278d1e4bd3a32
Author: Sahel Sharify <sahel@chromium.org>
Date: Fri Aug 11 20:53:22 2017

Removed the old comment on scrollbars handling GSB's with wheel source.

This is a follow up for https://chromium-review.googlesource.com/598999

With the above change scrollbars don't handle gesture scroll events
while wheel/trackpad/auto scrolling. Scrolling on a scrollbar won't be
any different from scrolling on its corresponding element.

Bug:  746922 
Change-Id: Iefd29840d41e748a73845d73dc72641a137980f3
Reviewed-on: https://chromium-review.googlesource.com/612423
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: Sahel Sharifymoghaddam <sahel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493846}
[modify] https://crrev.com/a57a8b2195e2853a143999a0004278d1e4bd3a32/third_party/WebKit/Source/core/input/ScrollManager.cpp

Labels: TE-Verified-M62 TE-Verified-62.0.3185.0
Tested the issue on Windows-7 using chrome latest Canary M62-62.0.3185.0 by following steps mentioned in the original comment. Observed that auto scroll when middle clicking on vertical scroll bar with mouse working as expected. Hence adding TE-Verified label.

Please fine the attachment for reference.

Thank you!
746922.mp4
9.5 MB View Download
Cc: abdulsyed@chromium.org
Labels: -Merge-Review-61 Merge-Approved-61
Approving merge to M61 branch 3163 based on comments #9, #12 and #14. Please merge ASAP. Thank you.

Comment 17 by sahel@chromium.org, Aug 14 2017

Status: Fixed (was: Started)

Sign in to add a comment