RESOLVED FIXED 145148
Scroll-snap animations should not start on axes with zero-delta 
https://bugs.webkit.org/show_bug.cgi?id=145148
Summary Scroll-snap animations should not start on axes with zero-delta
Brent Fulgham
Reported 2015-05-18 15:55:53 PDT
I noticed a bug in two-dimensional scroll snap regions as follows: 1. Begin a vertical scroll-snap gesture. 2. Scroll region being a vertical scroll animation. 3. Interrupt the vertical scroll with a horizontal gesture. 4. A horizontal scroll will begin. 5. Suddenly, the horizontal scroll will end and a vertical scroll will start. 6. The remainder of the original vertical scroll completes. This is happening because of a flaw in the gesture handling code. a. Vertical gestures are tracked, and a vertical target is selected. b. Horizontal gesture interrupts animation, clearing the wheel tracking state. c. Horizontal gesture is tracked, causing non-zero X delta values to be tracked, as well as an equivalent set of zero Y delta values. d. When the horizontal gesture enters the inertial scrolling phase, WebKit activates the scroll snap logic. e. WebKit first checks for vertical scrolling. It sees that it has a valid count of wheel events, and triggers the start of a vertical scroll snap animation. f. The vertical scroll animation begins overriding wheel events. g. Later, WebKit checks for horizontal scrolling. However, it sees that wheel events are being overridden, and stops. h. Consequently, the horizontal snap is "converted" to a vertical snap. The fix is to block starting the scroll snap animation on an axis if the user gesture involved zero delta in the direction of the axis.
Attachments
Patch (1.63 KB, patch)
2015-05-18 15:58 PDT, Brent Fulgham 		dino: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Brent Fulgham
Comment 1 2015-05-18 15:58:29 PDT
Created attachment 253349 [details] Patch
WebKit Commit Bot
Comment 2 2015-05-18 16:00:29 PDT
Attachment 253349 [details] did not pass style-queue: ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Radar WebKit Bug Importer
Comment 3 2015-05-18 16:33:33 PDT
<rdar://problem/21009187>
Radar WebKit Bug Importer
Comment 4 2015-05-18 16:33:35 PDT
<rdar://problem/21009192>
Brent Fulgham
Comment 5 2015-05-19 12:40:14 PDT
I don't have a good non-flaky automatic test for this. It's pretty easy to test manually.
Dean Jackson
Comment 6 2015-05-19 13:14:25 PDT
Comment on attachment 253349 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253349&action=review > Source/WebCore/ChangeLog:8 > + No new tests (OOPS!). Oops. > Source/WebCore/ChangeLog:14 > + * platform/cocoa/ScrollController.mm: > + (WebCore::ScrollController::processWheelEventForScrollSnapOnAxis): Don't begin a scroll sn This sentence does not e
Brent Fulgham
Comment 7 2015-05-19 14:05:09 PDT
Comment on attachment 253349 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253349&action=review >> Source/WebCore/ChangeLog:8 >> + No new tests (OOPS!). > > Oops. Oops! >> Source/WebCore/ChangeLog:14 >> + (WebCore::ScrollController::processWheelEventForScrollSnapOnAxis): Don't begin a scroll sn > > This sentence does not e Oops!
Brent Fulgham
Comment 8 2015-05-19 14:27:39 PDT
Committed r184589: <http://trac.webkit.org/changeset/184589>
Note You need to log in before you can comment on or make changes to this bug.