[webkit-reviews] review granted: [Bug 195281] [iOS] Should not scroll when checkbox, radio, submit, reset, or button is spacebar activated : [Attachment 363572] Patch and tests

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Daniel Bates
<dbates at webkit.org>'s request for review:
Bug 195281: [iOS] Should not scroll when checkbox, radio, submit, reset, or
button is spacebar activated
https://bugs.webkit.org/show_bug.cgi?id=195281

Attachment 363572: Patch and tests

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




--- Comment #8 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 363572
  --> https://bugs.webkit.org/attachment.cgi?id=363572
Patch and tests

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

> Source/WebCore/ChangeLog:17
> +	   WebCore implements spacebar activation on keydown for form controls.
For IE compatibility
> +	   WebCore does not mark such keydown events as handled so that a DOM
keypress event will
> +	   be subsequently dispatched. The current logic only skips calling the
base class's default

Do we still care about IE combat? What do other browsers do? Maybe HTML5
specifies this now?

> Source/WebCore/ChangeLog:21
> +	   event is not marked as handled, but WebCore actaully accounted for
this event then we need

actaully

> Source/WebCore/html/BaseCheckableInputType.cpp:67
> -void BaseCheckableInputType::handleKeydownEvent(KeyboardEvent& event)
> +auto BaseCheckableInputType::handleKeydownEvent(KeyboardEvent& event) ->
ShouldCallBaseEventHandler

I kinda feel like a bool return value would be as easily understood.

> Source/WebCore/html/BaseChooserOnlyDateAndTimeInputType.cpp:120
>      BaseClickableWithKeyInputType::handleKeydownEvent(*element(), event);
> +    return ShouldCallBaseEventHandler::Yes;

Why not return BaseClickableWithKeyInputType::handleKeydownEvent() ?