1666774 - Can no longer stop mid-smooth scrolling by clicking

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect, P3)

Description

[Cristi Dima]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:81.0) Gecko/20100101 Firefox/81.0

Steps to reproduce:

1. Turn smooth scrolling on
2. Scroll on any page
3. During the scrolling animation left click anywhere on the current page

Actual results:

Scroll animation continues

Expected results:

Up until now (Firefox 81) the scrolling would stop mid-animation.

Comment 1

[Cristi Dima]

Adding this comment to explain why this feature was important to me. Imagine you're scrolling through a long page looking for something.

On a phone, you'd just tap the screen and the scrolling would stop right where you want it. This feature on Firefox desktop was exactly that. Found the thing you're looking for? Just click during scrolling and the page would stop right where you wanted it to.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

What mechanism were you using for scrolling? Scroll wheel/trackpad/keyboard? If on a trackpad I would imagine that touching the trackpad during a scroll animation should stop the scroll. If using a mousewheel then yeah this might have changed as a result of bug 1662013 but in that scenario I'm not sure it's something we would bring back.

Comment 4

[Cristi Dima]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #3)

What mechanism were you using for scrolling? Scroll wheel/trackpad/keyboard? If on a trackpad I would imagine that touching the trackpad during a scroll animation should stop the scroll. If using a mousewheel then yeah this might have changed as a result of bug 1662013 but in that scenario I'm not sure it's something we would bring back.

Yes, I am using a mousewheel.

Comment 5

[Botond Ballo [:botond]]

(In reply to Cristi Dima from comment #4)

If using a mousewheel then yeah this might have changed as a result of bug 1662013 but in that scenario I'm not sure it's something we would bring back.

Do we understand the mechanism by which bug 1662013 changed the behaviour here?

I personally find the duration of the smooth scroll animation created by a mousewheel to be sufficiently short (I think it's something like 200 ms?) that in the time it takes me to react and click the mouse button, the animation from the last wheel event has already stopped. In other words, I have a hard time noticing the difference between whether or not a click stops an in-progress animation.

However, it's possible that other similar scenarios that are more noticeable (e.g. a mouse click in the middle of a long scrollTo({behaviour: smooth})) were regressed by bug 1662013 as well, which may be more concerning.

Comment 6

[Botond Ballo [:botond]]

(Whoops, didn't actually mean to set the severity just yet.)

Comment 7

[Kartikaya Gupta (email:[e-mail])]

(In reply to Botond Ballo [:botond] from comment #5)

Do we understand the mechanism by which bug 1662013 changed the behaviour here?

No, I should look into that. needinfo'ing myself for it since I probably won't get to it today but can look next week.

In general we don't have any explicit code to stop scroll animations based on a click, so I imagine that the click was probably triggering a paint or such and that was interrupting the APZ scroll animation. But it should be easy to reproduce if I increase the mousewheel animation duration via prefs and see what's going on.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

I ran mozregression to find out what actually broke this. It's not bug 1662013, it's bug 1655238 that caused this. And while mozregressing it, I do agree the original behaviour was a bit more intuitive. It feels weird to click during an animation and not have it stop.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=608e2ec1d8bdee6b0929c42935fa2ba17c8a439a

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1666774 - Have mousedowns on non-scrollbar interrupt APZ animations. r?botond,tnikkel

When the user does a click outside the scrollbar, cancel any ongoing APZ
animations. This restores the behaviour we had prior to bug 1655238. Clicking
on the scrollbar track still leaves the animations ongoing so that bug 1655238
also remains fixed.

The test here is forked from helper_visual_scrollbars_pagescroll.html which was
added in the regressing bug, but is modified to exercise this new scenario.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7a939462bdb2
Have mousedowns on non-scrollbar interrupt APZ animations. r=botond

Comment 12

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/7a939462bdb2

Comment 13

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 9179682 [details]
Bug 1666774 - Have mousedowns on non-scrollbar interrupt APZ animations. r?botond,tnikkel

Beta/Release Uplift Approval Request

• User impact if declined: Clicking during a scroll animation no longer interrupts the scroll animation (regression).
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Scroll using scrollwheel and while the scroll animation is in progress, click on the page content. This should stop the scroll. It's easier to reproduce if the general.smoothScroll.mouseWheel.durationMinMS and general.smoothScroll.mouseWheel.durationMaxMS prefs are increased to a larger number like 1000. This slows down the animation and makes it easier to click during the animation.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Patch is small and in pretty well-understood code.
• String changes made/needed:

Comment 14

[Alin Ilea, Desktop QA]

Reproduced the initial issue in Beta 82.0b7 using Windows 10.
Verified - Fixed in latest Nightly 83.0a1 (build id: 20201006161706) using Windows 10 and Ubuntu 18.04. Tested with "general.smoothScroll.mouseWheel.durationMinMS" and "general.smoothScroll.mouseWheel.durationMaxMS" prefs set to 1000.

Comment 15

[Julien Cristau [:jcristau]]

Comment on attachment 9179682 [details]
Bug 1666774 - Have mousedowns on non-scrollbar interrupt APZ animations. r?botond,tnikkel

approved for 82.0b9

Comment 16

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/b2705612fbdd

Comment 17

[Alin Ilea, Desktop QA]

Verified - Fixed in latest Beta 82.0b9 (build id: 20201008183927) using Windows 10 and Ubuntu 18.04.