1345044 - Make init*Event() arguments optional except the first

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[Anne (:annevk)]

Change to standard: https://github.com/whatwg/dom/pull/417 and https://github.com/whatwg/html/pull/2410.

Tests: https://github.com/w3c/web-platform-tests/pull/5043 and https://github.com/w3c/web-platform-tests/pull/5044.

Comment 1

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

I think we should really make _all_ init*Events to follow the same pattern. The spec changes covered only parts.

Comment 2

[Anne (:annevk)]

Indeed, follow-up issues were filed for the UI Events specification, but they're simply not fixed yet there. All init*Event() methods should work the same though.

Comment 3

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

Attachment: patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c281e4d16fc716502e6fb91b0e6b42a6739be97e

Comment 4

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

Comment on attachment 8844701 [details] [diff] [review]
patch

This is rather mechanical change.
I explicitly didn't want to change argument names or anything, so the coding style in different webidl files vary.


There might be some failing tests which need minor tweaking.

Comment 5

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

And tests should be coming from w3C wpt

Comment 6

[Ming-Chou Shih [:stone]]

Comment on attachment 8844701 [details] [diff] [review]
patch

Should CompositionEvent also be covered? Others are good to me. Thanks.

Comment 7

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

oh, fun. I do have the changes to CompositionEvent still in my editor, but apparently didn't click save.

Comment 8

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

And good catch!

Comment 9

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

Attachment: with composition event

Comment 10

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

Plenty of wpt failures.
I think better to wait for the wpt merge and then fix unexpected passes.

Comment 11

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

Do you want me to deal with the test failures for you?

Comment 12

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

Oh, bug 1345490 landed. I was hoping jgraham would have landed also this at the same time.
But if you can fix the unexpected passes, great.

Comment 13

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

I have a green try run <https://treeherder.mozilla.org/#/jobs?repo=try&revision=2869a57520517517bdb5b372abeb13d3d4cade70&selectedJob=88325633>, with one failure that's been fixed locally.  Should I just push this, or do you want to review the test changes?

Comment 14

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

Why the additions to /testing/web-platform/meta/html/dom/interfaces.html.ini ?

Comment 15

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

Because it wasn't updated for the spec change, so if I don't update it to match the new spec, we have new incorrect unexpected fails.

Comment 16

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

What spec change? How is stuff like
"Element interface: document.createElement("noscript") must inherit property "slot" with the proper type " related to this all?
Why is
+[NoInterfaceObject,
+ Exposed=Window]
+interface Slotable {
+  readonly attribute HTMLSlotElement? assignedSlot;
+};
+Element implements Slotable;
+Text implements Slotable;
added in this bug?

Comment 17

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

I explained in the commit message:

> The change to
> untested-interfaces.idl is basically just copying updates from
> dom/interfaces.html, which created four new expected failures that are
> unrelated to the code changes here. (I didn't think the noise was
> enough to bother separating into a different commit.)

The test IDL change is not related to this bug, it's just updating the test to a more recent IDL from the spec.  I could put that in a separate commit if you want.  Then I'd have to add some expected failures and remove them in the next commit.  I didn't do it because it was slightly more work and didn't seem to make a difference, but I can do it that way if you prefer.

Comment 18

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

What I would expect to see here is no idl changes, only changes to .ini files by removing the annotations that some tests fail.
Please don't merge random changes to a single changeset.

Comment 19

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

Attachment: 0001-Bug-1345044-Make-init-Event-arguments-optional-excep.patch

Okay.  How about this?

Comment 20

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

(The test updates are in a separate patch reviewed by jgraham.)

Comment 21

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

Comment on attachment 8854421 [details] [diff] [review]
0001-Bug-1345044-Make-init-Event-arguments-optional-excep.patch

thanks

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9f385861a95e
Update an IDL file for spec change; r=jgraham
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b25d7259b4f
Make init*Event() arguments optional except the first; r=stone

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9f385861a95e
https://hg.mozilla.org/mozilla-central/rev/5b25d7259b4f