Bug 148155

Summary: Scroll snapping should trigger when receiving a momentum end wheel event
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, bfulgham, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Mac   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 148326    
Attachments:
Description Flags
Patch ap: review+

Description Wenson Hsieh 2015-08-18 18:29:21 PDT
Scroll snapping should trigger when receiving a wheel event indicating momentum end. This will allow our failing scroll snapping layout tests to pass, since asynchronous momentum wheel events are currently being coalesced. However, in theory, if a user were to flick extremely lightly on the trackpad and generate fewer than 3 wheel momentum events before letting go, scroll snapping would not occur.
Comment 1 Wenson Hsieh 2015-08-18 18:29:55 PDT
<rdar://problem/22336880>
Comment 2 Wenson Hsieh 2015-08-18 23:06:25 PDT
Created attachment 259361 [details]
Patch
Comment 3 Alexey Proskuryakov 2015-08-19 08:53:47 PDT
Comment on attachment 259361 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=259361&action=review

> Source/WebCore/platform/cocoa/ScrollController.mm:571
> +        else {
> +            snapState.clearInitialWheelDeltaWindow();
> +            snapState.m_shouldOverrideWheelEvent = false;
> +        }

It's not obvious to me why this is inside an "else". beginScrollSnapAnimation does set these variables, but not in all code paths.
Comment 4 Wenson Hsieh 2015-08-19 09:05:12 PDT
Comment on attachment 259361 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=259361&action=review

Thanks for the review!

>> Source/WebCore/platform/cocoa/ScrollController.mm:571
>> +        }
> 
> It's not obvious to me why this is inside an "else". beginScrollSnapAnimation does set these variables, but not in all code paths.

Good point. I should be able to lift the logic to reset the delta window and override state out of the else clause, since we should be doing this anyway even if we don't trigger a glide here.
Comment 5 Wenson Hsieh 2015-08-19 11:14:00 PDT
Committed r188641: <http://trac.webkit.org/changeset/188641>