If the user clicks in the scrollbar track (but not on the thumb) we cancel all animations when we create the new drag input block in InputQueue::ReceiveMouseInput. So this cancels an in progress smooth scroll. It doesn't actually cancel the animation, it clear mAnimation and clears the state on the controller but the animation still runs to completion. Should we file another bug for that?

If the user clicks in the scrollbar track (but not on the thumb) we cancel all animations when we create the new drag input block in InputQueue::ReceiveMouseInput. So this cancels an in progress smooth scroll. It doesn't actually cancel the animation, it clear mAnimation and clears the state on the controller but the animation still runs to completion. Should we file another bug for that?

Can you provide specific STR for the problem this bug is attempting to fix, including prefs that need to be set? I'd like to understand it a bit better.

Set apz.force_disable_desktop_zooming_scrollbars to false, apz.allow_zooming to true. Load a page with a good amount of vertical scroll. Click 3 times in the scrollbar track to do 3 page scrolls down, do it slowly so each animation has time to finish. Observe the scroll position. Scroll back to the top. Now do the 3 clicks quickly. Observe the scroll position, it will not have scrolled as far as the first time.

This happens because we cancel the inprogress smooth scroll animations. So the code added in bug 1655242 cannot find an in progress animation, and we start a new relative smooth scroll from where ever the scroll offset happens to be.

Bug 1655237 fixed another problem that prevents the code added in bug 1655242 from finding an in progress animation.

Thanks, that helps.

So the goal here is to only stop existing APZ animations when the user mouses down on the scrollthumb. Right now we start a drag input block on any mousedown, and so we stop the animations on any mousedown (including on content area). This doesn't happen if we use the main-thread mechanism for scrollbar paging, because that doesn't happen via an APZ animation.

APZ does know when the user clicks on a scrollthumb vs scrollbar vs neither, but that information just doesn't get plumbed all the way through to the InputQueue. So if we plumb it in, we can make better decisions about when to cancel animations. I'll try and write something.

For completeness I should also mention there's the case where a scrollbar animation is going, and then you alt+click on a part of the scrollbar track (alt modifier on macOS, might be different on other platforms). This makes the scrollthumb jump to your cursor and puts you into a scrollthumb dragging state. AFAIK we don't handle this kind of scrollbar dragging in APZ, so we can do the easy thing on the APZ side, and not cancel the existing APZ animation (this is easy because it is consistent with the rule of "only cancel animations when the user clicks on the thumb"). Then the main thread will take care of jumping into a dragging state and kill any other animation activity.

Instead of setting the mTargetConfirmed flag to false in APZCTreeManager when
potentially starting a drag block, let the input block creation do it a little
bit later. This change ensures that the InputQueue ReciveInputEvent methods
have access to the mHitScrollthumb flag, and so can make better decisions.

Depends on D85452

I'm attaching the WIPs I wrote. This seems to accomplish the goal of not canceling animations unless the user mouses down on the scrollthumb. However it doesn't fix the STR in comment 3, there might be additional places that trigger cancellation of the animation. It might also be because the code I'm running on is older; I had to manually apply bug 1655237. I would investigate more but it would involve inserting printfs into FrameMetrics.h and I don't feel like waiting for another build right now...

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #8)

I'm attaching the WIPs I wrote. This seems to accomplish the goal of not canceling animations unless the user mouses down on the scrollthumb. However it doesn't fix the STR in comment 3, there might be additional places that trigger cancellation of the animation. It might also be because the code I'm running on is older; I had to manually apply bug 1655237. I would investigate more but it would involve inserting printfs into FrameMetrics.h and I don't feel like waiting for another build right now...

If you didn't have bug 1655237 in your tree then you probably didn't have bug 1655242, which is also necessary to fix the STR. I can try your patches today.

(In reply to Timothy Nikkel (:tnikkel) from comment #9)

If you didn't have bug 1655237 in your tree then you probably didn't have bug 1655242, which is also necessary to fix the STR. I can try your patches today.

Yeah I don't have that either. I guess I should update my tree.

Your two WIP patches do fix the bug for me.

I wrote a test too. Will put that patch up assuming the try push is green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3f0198aa8c6324d8a8b126ed381da1fa70e1e259

Ugh, behaviour is different on linux when clicking on the scrollbar track. I'll have to tweak the test a bit.

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

Now with ui.scrollToClick=0, which fixes it for me locally on Linux.

Depends on D85453

https://hg.mozilla.org/mozilla-central/rev/d74e81817c7e
https://hg.mozilla.org/mozilla-central/rev/20c5d5880939
https://hg.mozilla.org/mozilla-central/rev/9dc94550a9db