Bug 68791
Summary: | [meta] Remove init*Event methods | ||
---|---|---|---|
Product: | WebKit | Reporter: | Anne van Kesteren <annevk> |
Component: | DOM | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | abarth, ap, cmarcelo, dglazkov, dominicc, haraken, haraken, ian, laszlo.gombos, luchellesheldon, sam |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Bug Depends on: | 71338, 71340, 71344, 71374, 71636, 71687, 71689, 71691, 71698, 71701 | ||
Bug Blocks: |
Anne van Kesteren
Now we have event constructors the init*Event methods on recently introduced Event interfaces can be removed. It is no longer correct to support them for CustomEvent, ErrorEvent, ProgressEvent, etc.
Attachments | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |
Adam Barth
Is the idea here that no one has shipped these APIs yet so author aren't using them?
Dimitri Glazkov (Google)
We can't really remove them, they are already used widely in the wild :(
Dimitri Glazkov (Google)
http://codesearch.google.com/codesearch#search/&q=init(Custom%7CError%7CMouse%7CProgress)Event%20lang:js&type=cs
Adam Barth
This search comes up clean:
http://codesearch.google.com/codesearch#search/&q=init(Error%7CProgress)Event%20lang:js&type=cs
Dimitri Glazkov (Google)
(In reply to comment #4)
> This search comes up clean:
>
> http://codesearch.google.com/codesearch#search/&q=init(Error%7CProgress)Event%20lang:js&type=cs
We can certainly try to kill them :)
Ian 'Hixie' Hickson
Obviously the ancient ones like initMouseEvent aren't going anywhere.
But many of the new ones haven't even shipped in multiple browsers yet. Those are the ones that should go, e.g. almost all the ones in the HTML spec.
Dimitri Glazkov (Google)
(In reply to comment #6)
> Obviously the ancient ones like initMouseEvent aren't going anywhere.
>
> But many of the new ones haven't even shipped in multiple browsers yet. Those are the ones that should go, e.g. almost all the ones in the HTML spec.
SGTM.
Sam Weinig
Purging initErrorEvent and initProgressEvent sounds great (probably can't with Custom, since I think some YUI stuff use it). There are probably some other ones we can kill as well (perhaps some of the ones we have added in the last few months, Composition?)
Sam Weinig
CloseEvent? MediaSteamEvent? WebGLContextEvent? PopStateEvent? PageTransisionEvent? The webkit version of MessageEvent?
Dominic Cooney
Assuming we want to remove separate init*Event methods in separate patches, to make it easier to roll individual methods back in if it turns out they are used in the wild.
Dominic Cooney
Should these events be removed from EventFactory.in at the same time? createEvent('ProgressEvent') does not seem useful without initProgressEvent, for example.
Adam Barth
> Should these events be removed from EventFactory.in at the same time? createEvent('ProgressEvent') does not seem useful without initProgressEvent, for example.
That will effect the JS wrapper selection as well. When the IDL says Event, the bindings actually return a wrapper for the actual type of the Event. EventFactory.in provides an enumeration of all the event interfaces.
IMHO, there isn't anything to be gained by preventing these objects from being created by createEvent.
Dominic Cooney
(In reply to comment #12)
> That will effect the JS wrapper selection as well. When the IDL says Event, the bindings actually return a wrapper for the actual type of the Event. EventFactory.in provides an enumeration of all the event interfaces.
>
> IMHO, there isn't anything to be gained by preventing these objects from being created by createEvent.
Got it. I will leave createEvent alone.
Dominic Cooney
I have taken a look at all of the events in WebKit, the relevant specs, and whether the events are used (using Code Search.) Here is what I think we should do. For convenience I am just going to mention the event interface name, but I mean init*Event:
These are ones we already removed:
ErrorEvent (bug 71338), MediaStreamEvent (bug 71344), TrackEvent (never implemented)
I think we can remove these:
BeforeLoadEvent (bug 71636), CloseEvent (bug 71374), OverflowEvent, PageTransitionEvent, PopStateEvent, ProgressEvent (bug 71340), WebKitAnimationEvent, WebKitTransitionEvent
We *might* be able to remove these:
StorageEvent—used by Unhosted; HashChangeEvent—used by Ample SDK.
I think we can remove these, but we need to file bugs against the specs to get the specs updated:
AudioProcessingEvent, DeviceMotionEvent, DeviceOrientationEvent, WebGLContextEvent
And for completeness, these are the untouchables—we can’t remove these, it seems hordes of people use them:
CompositionEvent, CustomEvent (YUI), Event, KeyboardEvent, MessageEvent, MouseEvent, MutationEvent, TextEvent, TouchEvent, UIEvent, WheelEvent
Dominic Cooney
On more careful inspection I am not sure whether YUI uses initCustomEvent. It has a method of its own, initCustomEvent*s*, that is unrelated but shows up in casual searches.
However initCustomEvent is used in smaller frameworks such as Ample, JSDOM and "XBL" <http://codesearch.google.com/#search/&q=initCustomEvent%5B%5Es%5D%20lang:js&type=cs>
Anne van Kesteren
Fixed per dependencies.