Bug 67312 - [meta] EventTarget should be in the prototype chain
Summary: [meta] EventTarget should be in the prototype chain
Status: RESOLVED CONFIGURATION CHANGED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dominic Cooney
URL:
Keywords:
Depends on: 67329 67516 88120 88232
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-31 13:30 PDT by Dominic Cooney
Modified: 2023-05-12 23:40 PDT (History)
11 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dominic Cooney 2011-08-31 13:30:59 PDT
Per recent drafts of DOM Core, EventTarget should have an interface object (ie is no longer NoInterfaceObject) <http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#interface-eventtarget> and in every relevant spec, with the possible exception of FileReader, EventTarget is specified as a parent interface. For more context, see the thread on webkit-dev in June <https://lists.webkit.org/pipermail/webkit-dev/2011-June/thread.html#17024> and July <https://lists.webkit.org/pipermail/webkit-dev/2011-August/017685.html>.

This is a metabug to track introducing an EventTarget interface object and changing the JavaScript bindings to put EventTarget on the prototype chain instead of mixing it into every event target.
Comment 1 Alexey Proskuryakov 2011-09-12 22:56:46 PDT
For some context: this idea has gained lukewarm acceptance (at best) on webkit-dev, see <https://lists.webkit.org/pipermail/webkit-dev/2011-June/017041.html>.

Since making this change doesn't seem to benefit anyone, but comes with all early adopter costs and risks, it's not clear to me why WebKit should be the first engine to make and ship it.
Comment 2 Dominic Cooney 2011-09-13 10:33:24 PDT
So… I guess we should start rolling back features in WebKit that are not in IE and Firefox. This early adopter risk is killing us.
Comment 3 Alexey Proskuryakov 2011-09-13 10:56:51 PDT
This is not nearly what I said.
Comment 4 Erik Arvidsson 2011-09-13 11:10:54 PDT
By moving EventTarget to become the root class of XHR, Window, Node etc we get a better prototype chain, allowing monkey patching events:

if (DEBUG) {
  var ael = EventTarget.prototype.addEventListener;
  EventTarget.prototype.addEventListener = function(type, listener, capture) {
    // Ensure no typos
    assert(isValidEventType(type))
    return ael.call(this, type, listener, capture);
  };
}
Comment 5 Alexey Proskuryakov 2011-09-13 11:41:58 PDT
Thanks, that's the rationale that I've seen, too. I doubt that defining a function on all these completely unrelated objects is of any use in practice though - you'll need a switch inside of such function to do anything meaningful anyway (e.g. isValidEventType() should better return different results for XHRs, windows and nodes).

But this ship has sailed, and it's definitely not useful to discuss whether this change is good or not in this bug. My suggestion is to give a thought to what the best timing to make the change is.
Comment 6 Ojan Vafai 2012-04-19 16:18:54 PDT
Gecko has moved EventTarget to being the superclass of XHR in FF14 and is moving other interfaces over piecemeal as part of their DOM bindings refactor.

The web compatibility and performance risks seem low here and most (all?) of the relevant specs have changed to put EventTarget on the prototype change.  I don't see a reason to hold back on this longer. Alexey, were there other concerns you had with being an early adopter? It seems really unlikely to me that this will break pages. Maybe you disagree on that point?
Comment 7 Ojan Vafai 2012-04-19 16:19:51 PDT
The last webkit-dev thread on this was https://lists.webkit.org/pipermail/webkit-dev/2011-August/017685.html, which got no responses. The thread was asking if there were any objections. Silence == no objections?
Comment 8 Chris Dumez 2013-07-10 06:46:20 PDT
(In reply to comment #5)
> Thanks, that's the rationale that I've seen, too. I doubt that defining a function on all these completely unrelated objects is of any use in practice though - you'll need a switch inside of such function to do anything meaningful anyway (e.g. isValidEventType() should better return different results for XHRs, windows and nodes).
> 
> But this ship has sailed, and it's definitely not useful to discuss whether this change is good or not in this bug. My suggestion is to give a thought to what the best timing to make the change is.

I have been working on this on Blink side:
http://code.google.com/p/chromium/issues/detail?id=257583

One good side-effect was that there is a lot less code duplication in the generated bindings as the EventTarget API is only implemented in one place. This resulted in a smaller binary size (~90k less in release and ~450k in debug).

Alexey, the previous discussion is a bit old and one of your issues seemed to be about the timing. How do you feel about this change nowadays? If this is fine with you, I would be happy making similar changes in WebKit.

Sorry to start this again, I'm just wondering if it is worth for me porting my work over.
Comment 9 Alexey Proskuryakov 2013-07-10 08:32:09 PDT
Has this change shipped in Chrome and Firefox? Did it break any sites? My objection was only about paying early adopter cost for something we don't care about.

On the other hand, Sam had an objection in principle ("I don't think we should do this").
Comment 10 Erik Arvidsson 2013-07-10 08:45:33 PDT
(In reply to comment #9)
> Has this change shipped in Chrome and Firefox? Did it break any sites? My objection was only about paying early adopter cost for something we don't care about.

This has been shipping in IE since v9.

It has been in Firefox for a few stable releases (I don't know which exact version)
Comment 11 Sam Weinig 2013-07-10 10:57:12 PDT
(In reply to comment #9)
> Has this change shipped in Chrome and Firefox? Did it break any sites? My objection was only about paying early adopter cost for something we don't care about.
> 
> On the other hand, Sam had an objection in principle ("I don't think we should do this").

I still think will be of limited utility, but the standards bodies and implementations have done it anyway. It is probably time to bow to tides of change.
Comment 12 Anne van Kesteren 2023-05-12 23:40:28 PDT
In retrospect I think you all were right that this change was not worth it, but alas, it happened and WebKit has been changed.