145148 2015-05-18 15:55:53 -0700 Scroll-snap animations should not start on axes with zero-delta 2015-05-19 14:27:39 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 bfulgham bfulgham bfulgham commit-queue simon.fraser webkit-bug-importer oldest_to_newest 1095716 0 bfulgham 2015-05-18 15:55:53 -0700 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. 1095717 1 253349 bfulgham 2015-05-18 15:58:29 -0700 Created attachment 253349 Patch 1095718 2 commit-queue 2015-05-18 16:00:29 -0700 Attachment 253349 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. 1095735 3 webkit-bug-importer 2015-05-18 16:33:33 -0700 <rdar://problem/21009187> 1095736 4 webkit-bug-importer 2015-05-18 16:33:35 -0700 <rdar://problem/21009192> 1095993 5 bfulgham 2015-05-19 12:40:14 -0700 I don't have a good non-flaky automatic test for this. It's pretty easy to test manually. 1096013 6 253349 dino 2015-05-19 13:14:25 -0700 Comment on attachment 253349 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 1096035 7 253349 bfulgham 2015-05-19 14:05:09 -0700 Comment on attachment 253349 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! 1096048 8 bfulgham 2015-05-19 14:27:39 -0700 Committed r184589: <http://trac.webkit.org/changeset/184589> 253349 2015-05-18 15:58:29 -0700 2015-05-19 13:14:25 -0700 Patch bug-145148-20150518155827.patch text/plain 1674 bfulgham SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE4NDUxNykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBACisyMDE1LTA1LTE4ICBCcmVudCBG dWxnaGFtICA8YmZ1bGdoYW1AYXBwbGUuY29tPgorCisgICAgICAgIFNjcm9sbC1zbmFwIGFuaW1h dGlvbnMgc2hvdWxkIG5vdCBzdGFydCBvbiBheGVzIHdpdGggemVyby1kZWx0YQorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTQ1MTQ4CisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTm8gbmV3IHRlc3RzIChPT1BT ISkuCisKKyAgICAgICAgRG9uJ3QgYmVnaW4gYSBzY3JvbGwgc25hcCBhbmltYXRpb24gb24gYW4g YXhpcyB0aGF0IGhhZCBubyBjaGFuZ2UgaW4gcG9zaXRpb24gKG9uIHRoYXQgYXhpcykKKyAgICAg ICAgZHVyaW5nIHRoZSBnZXN0dXJlLgorCisgICAgICAgICogcGxhdGZvcm0vY29jb2EvU2Nyb2xs Q29udHJvbGxlci5tbToKKyAgICAgICAgKFdlYkNvcmU6OlNjcm9sbENvbnRyb2xsZXI6OnByb2Nl c3NXaGVlbEV2ZW50Rm9yU2Nyb2xsU25hcE9uQXhpcyk6IERvbid0IGJlZ2luIGEgc2Nyb2xsIHNu CisKIDIwMTUtMDUtMTggIFNrYWNoa292IEFsZXhhbmRyICA8Z3NrYWNoa292QGdtYWlsLmNvbT4K IAogICAgICAgICBbRVM2XSBBcnJvdyBmdW5jdGlvbiBzeW50YXguIEZlYXR1cmUgZmxhZyBmb3Ig YXJyb3cgZnVuY3Rpb24KSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2NvY29hL1Njcm9s bENvbnRyb2xsZXIubW0KPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vY29j b2EvU2Nyb2xsQ29udHJvbGxlci5tbQkocmV2aXNpb24gMTg0NTEzKQorKysgU291cmNlL1dlYkNv cmUvcGxhdGZvcm0vY29jb2EvU2Nyb2xsQ29udHJvbGxlci5tbQkod29ya2luZyBjb3B5KQpAQCAt NTQ3LDcgKzU0Nyw3IEBAIHZvaWQgU2Nyb2xsQ29udHJvbGxlcjo6cHJvY2Vzc1doZWVsRXZlbnQK ICAgICAgICAgICAgIGlmIChzbmFwU3RhdGUubV9udW1XaGVlbERlbHRhc1RyYWNrZWQgPCBzbmFw U3RhdGUud2hlZWxEZWx0YVdpbmRvd1NpemUpCiAgICAgICAgICAgICAgICAgc25hcFN0YXRlLnB1 c2hJbml0aWFsV2hlZWxEZWx0YSh3aGVlbERlbHRhKTsKICAgICAgICAgICAgIAotICAgICAgICAg ICAgaWYgKHNuYXBTdGF0ZS5tX251bVdoZWVsRGVsdGFzVHJhY2tlZCA9PSBzbmFwU3RhdGUud2hl ZWxEZWx0YVdpbmRvd1NpemUpCisgICAgICAgICAgICBpZiAoKHNuYXBTdGF0ZS5tX251bVdoZWVs RGVsdGFzVHJhY2tlZCA9PSBzbmFwU3RhdGUud2hlZWxEZWx0YVdpbmRvd1NpemUpICYmIHNuYXBT dGF0ZS5hdmVyYWdlSW5pdGlhbFdoZWVsRGVsdGEoKSkKICAgICAgICAgICAgICAgICBiZWdpblNj cm9sbFNuYXBBbmltYXRpb24oYXhpcywgU2Nyb2xsU25hcFN0YXRlOjpHbGlkaW5nKTsKICAgICAg ICAgfQogICAgICAgICBicmVhazsK