[webkit-reviews] review granted: [Bug 229733] REGRESSION(r280928) The smooth keyboard scrolling is enabled only for PageUp and PageDown keys : [Attachment 436938] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Fujii Hironori
<Hironori.Fujii at sony.com>'s request for review:
Bug 229733: REGRESSION(r280928) The smooth keyboard scrolling is enabled only
for PageUp and PageDown keys
https://bugs.webkit.org/show_bug.cgi?id=229733

Attachment 436938: Patch

https://bugs.webkit.org/attachment.cgi?id=436938&action=review




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

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

> Source/WebCore/ChangeLog:3
> +	   REGRESSION(r280928) The smooth keyboard scrolling is enabled only
for PageUp and PageDown keys

>From the wording of the title ("enabled only for") I assumed the fix would be
"enable it for more things". But actually it is fixing the fact that it was
accidentally enabled unconditionally for pageup/down. Current title is fine,
but a rewording could make it more clear. Regardless, this seems fine.

>>> Source/WebCore/page/EventHandler.cpp:3812
>>>		 startKeyboardScrolling(event);
>> 
>> This looks like it will make PageUp and PageDown not scroll at all? Is that
right?
> 
> This change restores the previous behavior before r280928.
> If the key event isn't consumed by WebCore's handleKeyEvent, WebKit's
WebPage::performDefaultBehaviorForKeyEvent handles it.

Possible we should just move the setting check /inside/ startKeyboardScrolling?