Scroll snapping and scroll momentum interact poorly

VERIFIED FIXED in Firefox 46

Status

()

Core
Panning and Zooming
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: sicking, Assigned: kats)

Tracking

unspecified
mozilla46
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

Steps to reproduce:

1. Load https://webkit.org/demos/scroll-snap/index.html#scroll-normal
2. Use mac touchpad to "flick" the numbers sideways. I.e. give the scrollable
   area a little bit of scroll speed and then quickly remove fingers from
   touchpad.
3. Notice the numbers keeps scrolling for a bit after fingers are removed.
4. Load https://webkit.org/demos/scroll-snap/index.html#scroll-snap-1
5. Use mac touchpad to "flick" the numbers sideways.

Actual behavior:
The scrolling numbers abruptly change scroll speed when the fingers are removed from the touchpad.

It feels like the scrollable area is immediately halted as soon as the fingers are removed, and then slowly animated to get to the closest scroll snapping position. No regard is made for the speed that the area had when it was "let go".

Expected behavior:
The scrollable area should have the same "feel" of momentum. When the fingers are removed from the touchpad in step 5 above, we should never abruptly change the scroll speed.

It should even be possible to flick the scrollable area past multiple scroll-snap positions. I.e. if I start scrolling at the 1, but give it a good flick, it should be possible for the scrollable area to scroll all the way over to the 4 before it comes to a stop. Even if I removed my fingers before we had scrolled much pasted the 1.

Or if I give it a more gentle flick, it should be possible to get to the 3.


In general, it seems like there are other bad interactions between scroll snapping and momentum scrolling. Unless I make sure that there is no momentum when I release the fingers off the touchpad, we often don't end up scrolling exactly to a scroll-snap position, but end up a little bit off of it.

I'm not sure if this problem is specific to OSX and touchpads, or if it also happens on touchscreens like Android and B2G.
The "In general..." part seems to be bug 1233413.
See Also: → bug 1233413
Blocks: 1178298
So when I tried reproducing this I saw even worse problems, where the scrollable can get stuck between snap points. That seems to be a regression from bug 1229039. The way it works is the main-thread will preventDefault wheel events that are targetted at scrollframes with snapping, and do the snapping itself. However the changes in bug 1229039 cause subsequent pan gesture events to start new input blocks, which cancel the smooth scroll animation triggered by the main thread.

If I undo that change then I see the thing Jonas filed this bug about, where the momentum-based destination is not used, only the touch-end destination is used for fling snapping purposes.
Blocks: 1229039
Assignee: nobody → bugmail.mozilla
I think the right fix here is to special-case OS X trackpad-based wheel events so that they don't do main-thread snapping but use the same code that we use for touch in APZC.
Created attachment 8703195 [details]
MozReview Request: Bug 1235994 - Extract a helper function to request a snap to the predicted destination. r?botond

Review commit: https://reviewboard.mozilla.org/r/29311/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/29311/
Attachment #8703195 - Flags: review?(botond)
Created attachment 8703196 [details]
MozReview Request: Bug 1235994 - Add a flag to wheel events to track if they may have momentum following them. r?mstange

Review commit: https://reviewboard.mozilla.org/r/29313/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/29313/
Attachment #8703196 - Flags: review?(mstange)
Created attachment 8703197 [details]
MozReview Request: Bug 1235994 - For wheel events which may have momentum following them, handle scroll snapping in APZ. r?mstange

Review commit: https://reviewboard.mozilla.org/r/29315/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/29315/
Attachment #8703197 - Flags: review?(mstange)
Try push at [1]. I also verified this doesn't regress bug 1141884.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=349067717188
Comment on attachment 8703195 [details]
MozReview Request: Bug 1235994 - Extract a helper function to request a snap to the predicted destination. r?botond

https://reviewboard.mozilla.org/r/29311/#review26089
Attachment #8703195 - Flags: review?(botond) → review+
(Assignee)

Comment 9

2 years ago
greentry
No idea why the try push in comment 7 was so orange, it doesn't seem related to my changes. I did a new try push rebased to latest m-c at https://treeherder.mozilla.org/#/jobs?repo=try&revision=5fe140591228
Huh, I thought I had submitted my reviews in this bug already. Sorry for the delay.
Attachment #8703197 - Flags: review?(mstange) → review+
Comment on attachment 8703197 [details]
MozReview Request: Bug 1235994 - For wheel events which may have momentum following them, handle scroll snapping in APZ. r?mstange

https://reviewboard.mozilla.org/r/29315/#review26679
Comment on attachment 8703196 [details]
MozReview Request: Bug 1235994 - Add a flag to wheel events to track if they may have momentum following them. r?mstange

https://reviewboard.mozilla.org/r/29313/#review26677
Attachment #8703196 - Flags: review?(mstange) → review+

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5c18d14659cb
https://hg.mozilla.org/mozilla-central/rev/cdda106011de
https://hg.mozilla.org/mozilla-central/rev/d476ed635a0a
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Works great! Thanks!!
Status: RESOLVED → VERIFIED

Updated

2 years ago
Duplicate of this bug: 1233413
You need to log in before you can comment on or make changes to this bug.