67312 2011-08-31 13:30:59 -0700 [meta] EventTarget should be in the prototype chain 2023-05-12 23:40:28 -0700 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) Unspecified Unspecified RESOLVED CONFIGURATION CHANGED P2 Normal --- 67329 67516 88120 88232 1 dominicc dominicc annevk ap arv benjamin cdumez chrisguttandin laszlo.gombos mjs oren.me rniwa sam oldest_to_newest 459857 0 dominicc 2011-08-31 13:30:59 -0700 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. 466147 1 ap 2011-09-12 22:56:46 -0700 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. 466376 2 dominicc 2011-09-13 10:33:24 -0700 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. 466396 3 ap 2011-09-13 10:56:51 -0700 This is not nearly what I said. 466404 4 arv 2011-09-13 11:10:54 -0700 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); }; } 466426 5 ap 2011-09-13 11:41:58 -0700 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. 606102 6 ojan 2012-04-19 16:18:54 -0700 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? 606104 7 ojan 2012-04-19 16:19:51 -0700 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? 907359 8 cdumez 2013-07-10 06:46:20 -0700 (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. 907375 9 ap 2013-07-10 08:32:09 -0700 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"). 907379 10 arv 2013-07-10 08:45:33 -0700 (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) 907407 11 sam 2013-07-10 10:57:12 -0700 (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. 1955194 12 annevk 2023-05-12 23:40:28 -0700 In retrospect I think you all were right that this change was not worth it, but alas, it happened and WebKit has been changed.