[webkit-reviews] review granted: [Bug 191608] [iOS] Do not handle key events that are key commands : [Attachment 354992] Patch and layout tests

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Daniel Bates <dbates at webkit.org>'s
request for review:
Bug 191608: [iOS] Do not handle key events that are key commands
https://bugs.webkit.org/show_bug.cgi?id=191608

Attachment 354992: Patch and layout tests

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




--- Comment #7 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 354992
  --> https://bugs.webkit.org/attachment.cgi?id=354992
Patch and layout tests

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

> Source/WebCore/platform/ios/WebEvent.mm:238
> +- (WebEvent *)initWithKeyEventType:(WebEventType)type
> +			    timeStamp:(CFTimeInterval)timeStamp

Nit: I know other functions in this file does that but I don't think we're
supposed to align : like this in WebKit.

> LayoutTests/fast/events/ios/rich-edit-command+i-dispatches-keydown.html:30
> +var testElement = document.getElementById("test");
> +var event;
> +var ignoredFirstKeyDownEvent = false;

Use let?

> LayoutTests/fast/events/ios/rich-edit-command+i-dispatches-keydown.html:32
> +function shouldIgnoreKeyDownEvent(e)

Nit: Spell out event.

> LayoutTests/fast/events/ios/rich-edit-command+i-dispatches-keydown.html:47
> +    event = e;
> +

Why don't we just rely on window.event??

> LayoutTests/fast/events/ios/rich-edit-command+i.html:54
> +    await UIHelper.keyDown("i", ["meta"]);
> +}

Why can't we just dump the markup here? keydown is handled synchronously by
WebCore.