1387828 - Reinstate support for initKeyboardEvent with optional arguments

Status: RESOLVED FIXED

(Core :: DOM: Events, enhancement)

Description

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Historically we supported a KeyEvent type.  Eventually the standard said to support KeyboardEvent, so we did.  It's supposed to have an initKeyboardEvent initializer, which was added in bug 930893, but it was removed in bug 999645 because it broke a browser add-on.  It seems the add-on in question assumed that some of the arguments were optional, which Chrome does, and our implementation had all arguments required.

The problem is that other browsers do support this method and probably can't drop support, so for interop we should support it.  See discussion here:

https://github.com/w3c/web-platform-tests/pull/4117
https://github.com/w3c/uievents/issues/112

smaug was in favor of trying to re-add it with optional arguments.

Comment 1

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: Bug 1387828 - Reinstate support for initKeyboardEvent;

https://github.com/w3c/uievents/issues/112

This is supported by all other UAs.  In the past we had compatibility
problems when trying to add support, but it seems these might be fixed
if we make all arguments optional beyond the first.

The interface chosen for the method is from the spec, which seems to
match documentation for IE, except that I made all arguments optional
beyond the first.  Chrome supports a significantly different argument
list after the first several.  Hopefully it doesn't matter much for now.
I supported IE's "locale" argument, but it has no effect.

Review commit: https://reviewboard.mozilla.org/r/165304/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/165304/

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8894219 [details]
Bug 1387828 - Reinstate support for initKeyboardEvent;

https://reviewboard.mozilla.org/r/165304/#review170588

::: dom/webidl/KeyboardEvent.webidl:32
(Diff revision 1)
>    readonly attribute boolean       isComposing;
>  
>    readonly attribute DOMString key;
>    readonly attribute DOMString code;
>  
> +  [Throws]

I don't understand why these params. Why for example no code?
Could you please give a link to some spec which also other browser vendors agree.

Comment 3

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Comment on attachment 8894219 [details]
Bug 1387828 - Reinstate support for initKeyboardEvent;

https://reviewboard.mozilla.org/r/165304/#review170588

> I don't understand why these params. Why for example no code?
> Could you please give a link to some spec which also other browser vendors agree.

Microsoft docs: https://msdn.microsoft.com/en-us/library/ff975297(v=vs.85).aspx
Blink source: https://src.chromium.org/viewvc/blink/trunk/Source/core/events/KeyboardEvent.idl#l43
WebKit source: https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/dom/KeyboardEvent.idl#L52
Spec: https://w3c.github.io/uievents/#dom-keyboardeveng-initkeyboardevent

Note that they all agree in the names of the first six parameters, but Microsoft/spec differ from Blink/WebKit for parameters after that.  I don't know if the semantics agree for the first six.  Hopefully it won't matter for web compat?  Probably the Blink/WebKit behavior is more compatible, so if you want we can ask for the spec to change instead of matching it.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

In the spec the method doesn't take any param o_O

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8894219 [details]
Bug 1387828 - Reinstate support for initKeyboardEvent;

https://reviewboard.mozilla.org/r/165304/#review170594

I think we need the spec clarified before making the change. Browser vendors should agree on this. And if not, and given that this is deprecated method, we should probably follow blink/webkit.

Comment 6

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Comment on attachment 8894219 [details]
Bug 1387828 - Reinstate support for initKeyboardEvent;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/165304/diff/1-2/

Comment 7

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Spec updated to match Chrome: https://github.com/w3c/uievents/pull/153

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8894219 [details]
Bug 1387828 - Reinstate support for initKeyboardEvent;

https://reviewboard.mozilla.org/r/165304/#review173644

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8ee32af87bd4
Reinstate support for initKeyboardEvent; r=smaug

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8ee32af87bd4