Bug 220767

Summary: Keyboard scroll snap does not animate in scrolling overflow
Product: WebKit Reporter: Martin Robinson <mrobinson>
Component: ScrollingAssignee: Martin Robinson <mrobinson>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: augus.dupin, changseok, esprehn+autocc, ews-watchlist, fred.wang, glenn, kondapallykalyan, pdr, raphael.schaad, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 218115    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch ews-feeder: commit-queue-

Description Martin Robinson 2021-01-20 07:34:26 PST
Steps to reproduce:

1. Visit https://codepen.io/edwinm/pen/BERXpG
2. Scroll horizontally with the keyboard

Actual results:

The viewport snaps between snap points without animating.

Expected results:

The viewport scroll position should animate between snap points
Comment 1 Martin Robinson 2021-01-21 05:30:39 PST
Created attachment 418031 [details]
Patch
Comment 2 Martin Robinson 2021-01-21 05:43:33 PST
Created attachment 418032 [details]
Patch
Comment 3 Martin Robinson 2021-01-21 05:44:31 PST
I would love some input on this change as I have a few doubts. On Mac, it doesn't seem like keyboard scrolling normally animates in overflow areas, but I found the experience of using the arrow keys to scroll pretty disorienting when snapping occurred. This change only animates when snapping will change the destination of a keyboard scroll. I'm not sure if that's the desired behavior.

The other question I have is about testing. I'm really unsure about how to test that animation is happening during snaps. I could write some code to have this run through ScrollAnimatorMock, but the substantive changes are in ScrollAnimatorMac. I'm not sure what the best approach is here.
Comment 4 Radar WebKit Bug Importer 2021-01-27 07:35:12 PST
<rdar://problem/73661634>
Comment 5 Martin Robinson 2021-02-18 23:57:55 PST
Comment on attachment 418032 [details]
Patch

After chatting with Simon, I think the best thing to do here is to enable the scroll animator for RenderLayers.
Comment 6 Martin Robinson 2021-02-24 05:09:28 PST
Created attachment 421399 [details]
Patch
Comment 7 Martin Robinson 2021-02-24 12:47:13 PST
I have uploaded a new patch here that turns on the ScrollAnimator for RenderLayers. In practice this means that scrolling with the keyboard in scrolling overflow will animate, when it didn't before. This also includes animations for scroll snapping.
Comment 8 Martin Robinson 2021-03-17 09:27:16 PDT
Created attachment 423495 [details]
Patch
Comment 9 Martin Robinson 2021-03-17 09:29:32 PDT
The new version of this change includes tests. I had to expose a new Internals API in order to override the system scroll animations setting. By default animations are disabled for scrolling on Mac using NSUserDefaults.
Comment 10 Martin Robinson 2021-03-18 06:44:10 PDT
Created attachment 423593 [details]
Patch
Comment 11 Martin Robinson 2021-03-25 05:39:56 PDT
Created attachment 424237 [details]
Patch
Comment 12 Martin Robinson 2021-03-26 02:05:59 PDT
Created attachment 424332 [details]
Patch
Comment 13 Raphael Schaad 2021-05-09 14:30:32 PDT
There's another minor bug in STP 124 that can be observed in the referenced codepen here: hit <space> to scroll down/<shift>+<space> to scroll up. Typically, Safari animates vertical scroll, but it doesn't for scroll snap containers.

This behavior was demonstrated as part of https://bugs.webkit.org/show_bug.cgi?id=223406. This also seems relevant: https://bugs.webkit.org/show_bug.cgi?id=223375
Comment 14 Martin Robinson 2021-11-30 01:50:46 PST

*** This bug has been marked as a duplicate of bug 233591 ***