Closed Bug 999645 Opened 6 years ago Closed 6 years ago

KeyboardEvent.initKeyboardEvent requires too many arguments

Categories

(Core :: DOM: Events, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31
Tracking Status
firefox28 --- unaffected
firefox29 --- unaffected
firefox30 --- unaffected
firefox31 + fixed
firefox-esr24 --- unaffected

People

(Reporter: jryans, Assigned: masayuki)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file)

In bug 930893, support was added for the |initKeyboardEvent()| method.

However, it currently does not match other browsers with this support (like Chrome) because it requires all arguments (of which there are many) to be specified.  In Chrome's case at least, all arguments are optional.

I know of at least one thing that currently breaks because of this change, the 1Password 4 browser addon.  It uses feature detection to see if |initKeyboardEvent| is available.  If so, it will use it, but only supply the first 3 arguments.
Chrome is not following the WebIDL spec.
But perhaps because of Chrome being broken we'll need to add [optional] and default values for
various params.
(In reply to Olli Pettay [:smaug] from comment #1)
> Chrome is not following the WebIDL spec.
> But perhaps because of Chrome being broken we'll need to add [optional] and
> default values for
> various params.

It seems best to make this optional to match what's expected from Chrome, especially if Chrome has had this implemented for a long time.

But, I can also reach out to the addon developer if we don't want to make a change.

I would personally prefer optional params because in most cases, you only care to set a few of the fields, but I get that it seems to differ from the spec.
There may be other sites that make the same assumption as this addon... but it's hard to say for sure.
Yeah, that is the worrisome part if some sites are using init*Event in the odd way.
IE11 requires all arguments (even the "locale" extension!)
I think we should rather remove initKeyboardEvent which is not interoperable anyway. It will unbreak 1Password because it does not longer detect the initKeyboardEvent property.
I guess that feature detection of initKeyboardEvent isn't problem on public websites because according to comment 5, even if there are such websites, they are already broken.
(In reply to Masatoshi Kimura [:emk] from comment #5)
> IE11 requires all arguments (even the "locale" extension!)
> I think we should rather remove initKeyboardEvent which is not interoperable
> anyway. It will unbreak 1Password because it does not longer detect the
> initKeyboardEvent property.

But IE11 missing one ("detail", http://msdn.microsoft.com/en-us/library/ie/ff975297%28v=vs.85%29.aspx), so now only Firefox is compatible with D3E, but don't know if definition from D3E is correct (all browsers work differently), and potentially we can't set all properties for KeyboardEvent when using this method (like isComposing, code and locale). Chrome has a completely different behavior, but DOM3 Keyboard Events is mark as "No active development", so it is not surprise (http://www.chromestatus.com/features).
(In reply to spiritRKS1910 from comment #7)
> (In reply to Masatoshi Kimura [:emk] from comment #5)
> > IE11 requires all arguments (even the "locale" extension!)
> > I think we should rather remove initKeyboardEvent which is not interoperable
> > anyway. It will unbreak 1Password because it does not longer detect the
> > initKeyboardEvent property.
> 
> But IE11 missing one ("detail",
> http://msdn.microsoft.com/en-us/library/ie/ff975297%28v=vs.85%29.aspx)

Oh, I didn't realize this. Okay, the feature detection is really broken by our and D3E's initKeyboardEvent(). I agree with backing out the part.2 patch.

How about you, smaug?
Flags: needinfo?(bugs)
Yeah, let's do that
Flags: needinfo?(bugs)
Attachment #8412243 - Flags: review?(bugs) → review+
Maybe we should file a spec bug to move initKeyboardEvent() to the legacy section, just like keyCode and charCode.
I don't think so. The section is, "A.1 Legacy Event Initializer Interfaces" ;-)

But perhaps, I'll tell about this problem in next D3E meeting.
https://hg.mozilla.org/mozilla-central/rev/8df965877ac4
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.