Fix event handler processing algorithm special cases #2428
Conversation
This is a follow-up to 7163372, which forgot about the case of new ErrorEvent("click", ...), as noted in #2398 (comment).
Fwiw, I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1345996 already for Gecko. |
|
This looks good for the error case. I still think it would be better to make similar changes for the "beforeunload" case, because I'm not very happy relying on no one adding a BeforeUnloadEvent constructor. Actually, I wonder what browser behavior is around non-BeforeUnloadEvent objects with type "beforeunloadevent". In this case, the handler will be a BeforeUnloadEventHandler, so return values will be converted to string-or-null. So if the event is not a BeforeUnloadEvent (e.g. |
|
For the case of |
That won't help. The handler type is BeforeUnloadEventHandler, since the handler type matches the event type. |
|
As in, it sounds like non-Gecko engines do not have OnBeforeUnloadEventHandlerNonNull at all and instead hand-roll something on top of EventHandlerNonNull or something. |
|
Yes, that was the conclusion I was coming to as well. I think I am OK going with Gecko on this one even though it's in the minority, because it is a worthwhile goal to have these be implementable in Web IDL. |
Follows whatwg/html#2428. This also fixes beforeunload-canceling.html to use promise_test instead of async_test to avoid running two tests concurrently.
Follows whatwg/html#2428. This also fixes beforeunload-canceling.html to use promise_test instead of async_test to avoid running two tests concurrently.
domenic
changed the title
|
OK, PTAL @bzbarsky. I've updated the OP and also added tests. |
Follows whatwg/html#2428. This also fixes beforeunload-canceling.html to use promise_test instead of async_test to avoid running two tests concurrently.
This is a follow-up to 7163372, which
forgot about the case of ErrorEvents whose type is not "error", and
BeforeUnloadEvents whose type is not "beforeunload", as noted in #2398.
/cc @bzbarsky
Tests: web-platform-tests/wpt#5109
After this gets merged I need to file all the browser bugs for #2398.