[webkit-reviews] review granted: [Bug 228009] Add key-driven smooth scrolling to macOS : [Attachment 434249] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Jul 27 15:15:33 PDT 2021
Tim Horton <thorton at apple.com> has granted Dana Estra <destra at apple.com>'s
request for review:
Bug 228009: Add key-driven smooth scrolling to macOS
https://bugs.webkit.org/show_bug.cgi?id=228009
Attachment 434249: Patch
https://bugs.webkit.org/attachment.cgi?id=434249&action=review
--- Comment #15 from Tim Horton <thorton at apple.com> ---
Comment on attachment 434249
--> https://bugs.webkit.org/attachment.cgi?id=434249
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=434249&action=review
> Source/WebCore/page/EventHandler.cpp:3807
> + if (event.type() == eventNames().keyupEvent)
> + stopKeyboardScrolling();
It is odd that this one doesn't follow the pattern of the others (passing it to
editor first, bailing if it ate the event). On the other hand, we know that we
never got here with keyup before, so I assume editor's handleKeyboardEvent
doesn't do anything with keyup? (please check) But in that case, maybe it's
best to follow the pattern anyway to minimize surprise later. Would be
interested to hear other people's opinions.
> Source/WebCore/platform/KeyboardScrollingAnimator.h:42
> + void handleKeyUpEventWhileScrolling();
The caller doesn't actually know that this is "while scrolling", right, they're
just plumbing us every keyup? So maybe drop to just "handleKeyUpEvent". The
class name indicates who's handling it and why sufficiently.
More information about the webkit-reviews
mailing list