Bug 228156 - macOS key-driven smooth scrolling does not work for pageUp/pageDown
Summary: macOS key-driven smooth scrolling does not work for pageUp/pageDown
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-07-21 12:31 PDT by Dana Estra
Modified: 2021-08-11 13:32 PDT (History)
5 users (show)

See Also:


Attachments
Patch (5.81 KB, patch)
2021-08-02 12:35 PDT, Dana Estra
no flags Details | Formatted Diff | Diff
Patch (5.80 KB, patch)
2021-08-02 12:38 PDT, Dana Estra
no flags Details | Formatted Diff | Diff
Patch (12.46 KB, patch)
2021-08-09 12:10 PDT, Dana Estra
no flags Details | Formatted Diff | Diff
Patch (15.13 KB, patch)
2021-08-10 15:44 PDT, Dana Estra
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dana Estra 2021-07-21 12:31:36 PDT
pageUp/fn+up-arrow and pageDown/fn+down-arrow are not smooth.
Comment 1 Radar WebKit Bug Importer 2021-07-21 12:37:59 PDT
<rdar://problem/80911788>
Comment 2 Dana Estra 2021-08-02 12:35:19 PDT
Created attachment 434784 [details]
Patch
Comment 3 Dana Estra 2021-08-02 12:38:40 PDT
Created attachment 434785 [details]
Patch
Comment 4 Tim Horton 2021-08-02 12:43:25 PDT
Comment on attachment 434785 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        No tests yet.

Maybe this is a good opportunity to start porting the iOS keyboard scrolling tests? I think there is one specifically about pageUp/Down

> Source/WebKit/UIProcess/API/mac/WKWebViewMac.mm:269
> +        [self.nextResponder tryToPerform:_cmd with:sender];

I think you need to return here, not say that you're not handling it (by passing it to the next responder) while ALSO handling it :)

> Source/WebKit/UIProcess/API/mac/WKWebViewMac.mm:277
> +        [self.nextResponder tryToPerform:_cmd with:sender];

(and here)
Comment 5 Dana Estra 2021-08-09 12:10:52 PDT
Created attachment 435197 [details]
Patch
Comment 6 Tim Horton 2021-08-09 12:29:43 PDT
Comment on attachment 435197 [details]
Patch

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

> LayoutTests/fast/scrolling/keyboard-scrolling-distance-pageDown.html:1
> +<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true EventHandlerDrivenSmoothKeyboardScrollingEnabled=true ] -->

Should wait and see if these actually pass on iOS (on the bots). I am hopeful but not certain :)

> LayoutTests/fast/scrolling/keyboard-scrolling-distance-pageDown.html:38
> +                dist: 0

WebKit style generally prefers not abbreviating things (I guess this is `distance`?)

> LayoutTests/fast/scrolling/keyboard-scrolling-distance-pageDown.html:42
> +                checkSuccessfulScroll(scrollObj)

missing semicolon (not required, just a good habit)
Comment 7 Dana Estra 2021-08-10 15:44:33 PDT
Created attachment 435310 [details]
Patch
Comment 8 EWS 2021-08-11 13:32:39 PDT
Committed r280928 (240446@main): <https://commits.webkit.org/240446@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 435310 [details].