Dom: Event not dispatched by the user agent should not fallback to legacy

Created on 31 Jan 2017  路  18Comments  路  Source: whatwg/dom

Currently, in the invoke with event algorithm, when no listener is found and the event has a legacy prefixed event type, the event is invoked with the legacy event type.

I think this should check whether the original event is dispatched by the user agent. That says, I think in the following case:

<script>
document.body.addEventListener('webkitTransitionEnd', evt => console.log(evt));
document.body.dispatchEvent(new TransitionEvent('transitionend'));
</script>

the listener shouldn't be triggered.

This proposal doesn't match what browsers currently do, but I suspect whether changing this could lead to web compatibility issues.

The main reason I think we should probably change is that, it would make it easier to write polyfill. Something like

elem.addEventListener('webkitTransitionEnd', function(evt) => {
  elem.dispatchEvent(new TransitionEvent('transitionend'));
});

would work in both new browsers and old browsers.

I raise this issue because there is a bug filed to Firefox about this for prefixed Fullscreen API that we are dispatching prefixed event when the script dispatches an unprefixed fullscreen event, which leads to an infinite recursive when there is no listener to unprefixed event.

Although fullscreen event isn't part of this spec (actually we don't spec any legacy event type for it currently), I think the same logic applies in both cases.

cc @smaug---- @annevk @miketaylr @foolip

Most helpful comment

(What a bad issue number...)

All 18 comments

(What a bad issue number...)

While the fullscreen events aren't in https://dom.spec.whatwg.org/#concept-event-listener-invoke, I think it might make sense to change this in general so that's it's possible to implement Fullscreen in the same way pending some conclusion on whether we need to spec prefixed Fullscreen (after successfully unprefixing it).

@upsuper, maybe you could test whether script-dispatched events of the type mentioned in the spec fall back to the prefixed events in various engines? I'm guessing not, and if so perhaps the fix is to make it depend on isTrusted?

It does fallback to prefixed events in Gecko, Blink, and WebKit. Edge doesn't seem to do so. I think we should match Edge's behavior here.

Can you write a web-platform-tests PR for this and poke someone from the Edge team on that? Perhaps they can tell us exactly what Edge's behavior is so that can be spec'd.

I think adding a check of isTrusted should be enough. I can write the test... but there are some other things in my queue which can take time...

Here's a few links to bugs and code that convinced us to add the legacy invoke hacks in Bug 1236979:

The biggest issue we know about is a map lib prioritized webkitTransitionEnd over transitionend when doing feature detection for prefixes, because some broken version of the Android browser had unprefixed relevant CSS properties, but did not unprefix events (i.e., would send prefixed events on an unprefixed transition or animation)

https://github.com/Leaflet/Leaflet/commit/64ca0af124b7145e8c2a746e2fb087b2298a4b72

+// webkitTransition comes first because some browser versions that drop vendor prefix don't do
+// the same for the transitionend event, in particular the Android 4.1 stock browser

Bing maps API usage breaking weather.com and strava, any other map using leaflet.js
https://bugzilla.mozilla.org/show_bug.cgi?id=1239342#c5
https://bugzilla.mozilla.org/show_bug.cgi?id=1236930

Possibly some versions of Modernizr (unconfirmed) had the same problem.
Also, bootstrap: https://github.com/twbs/bootstrap/blob/master/js/transition.js#L19

So, that said, for the known bustage, restricting invoking legacy events to trusted events should work. Of course, that doesn't guarantee anything for sites we don't know about that might be dispatching synthetic events. 馃槦

But we could totally stick this in Nightly and see what bustage reports we get, and do some targeted testing.

Submitted PR for the spec: #406.

@upsuper so this behavior is what Edge is doing here and what we want to implement in Firefox? @foolip would you align Chrome with this if it works so we can do fullscreen in a similar fashion?

Yes, as far as I can see, Edge is doing this. It passes all tests here. I want to do the same thing in Firefox since I think it is the most sensible way to fix bug 1332699.

Thanks, I'll give @foolip a chance to review before committing.

The change itself makes sense, but I had some questions in https://github.com/w3c/web-platform-tests/pull/4687 and would like to see if Edge passes the tests, to make sure there are no surprises. It looks like it'd be a one-liner to change in Blink, so I can give it a try once the test is imported if I don't forget.

@foolip would you align Chrome with this if it works so we can do fullscreen in a similar fashion?

This would also requiring changing the Fullscreen spec to fire events at the elements again. There are some silly things that happen if the element is removed, but I think it could be made fairly sane, so I could attempt to change the spec. Unfortunately, IIRC, Gecko fires the prefixed events at the document only, so if we try to standardize the prefixed APIs, someone is going to have to take on some risk in changing their model.

Strike the "again", I guess the spec has always followed Gecko's model of only targeting the document.

Fullscreen spec doesn't need any change at present (and probably neither in the future, as DOM spec may be the centric place for legacy event type). We haven't yet specced legacy fullscreen events, and we may or may not need to spec it. But I think it makes more sense to share the same mechanism even for unspecced things.

So I'm assuming you and @foolip will take care of bugs for Gecko and Chromium. Edge already follows the revised standard. What about WebKit?

Gecko's change has landed in Firefox 54, and may be uplifted to 53.

Can one of you file a bug on WebKit as well: https://bugs.webkit.org/enter_bug.cgi?product=WebKit? Just in case.

Was this page helpful?
0 / 5 - 0 ratings