[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

Attachment 434249: Patch


--- Comment #15 from Tim Horton <thorton at apple.com> ---
Comment on attachment 434249
  --> https://bugs.webkit.org/attachment.cgi?id=434249

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