Generate bindings code for EventTarget.addEventListener() / removeEventListener(). Custom code is current hardcoded in the bindings generator, we need to clean this up.
Created attachment 279347 [details] Patch
Created attachment 279349 [details] Patch
Created attachment 279351 [details] Patch
Created attachment 279352 [details] Patch
Created attachment 279353 [details] Patch
Created attachment 279354 [details] Patch
Created attachment 279356 [details] Patch
Created attachment 279358 [details] Patch
Comment on attachment 279358 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=279358&action=review > Source/WebCore/dom/EventListenerMap.h:58 > + bool remove(const AtomicString& eventType, EventListener&, bool useCapture, size_t& indexOfRemovedListener); There’s no question that this is a step in the right direction. But really I think the remove functions should take a predicate (std::function, probably) telling them what to remove rather than an EventListener&. Expressing that predicate as a == function on an EventListener object is not the best way to do it. We should talk about this in person at some point. > Source/WebCore/dom/RegisteredEventListener.h:34 > + RegisteredEventListener(Ref<EventListener> listener, bool useCapture) Should this be Ref&& instead of just Ref? > LayoutTests/ChangeLog:11 > + Rebaseline existing test now that the 'length' property of those functions > + have changed. This is because the IDL has been updated to match our > + implementation (all parameters are optional). While this does not match > + the specification, the new length now matches our actual behavior. I don’t think this is good. I’d prefer that we still return length 2. Is there some easy way to do that?
Comment on attachment 279358 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=279358&action=review >> LayoutTests/ChangeLog:11 >> + the specification, the new length now matches our actual behavior. > > I don’t think this is good. I’d prefer that we still return length 2. Is there some easy way to do that? I don't think people actually rely on this. I do agree this is not great though. The length computation relies on arguments being marked as optional or not in the IDL. Besides adding a new IDL extended attribute, I am not quite sure how to maintain the length.
Created attachment 279453 [details] Patch
(In reply to comment #10) > Comment on attachment 279358 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=279358&action=review > > >> LayoutTests/ChangeLog:11 > >> + the specification, the new length now matches our actual behavior. > > > > I don’t think this is good. I’d prefer that we still return length 2. Is there some easy way to do that? > > I don't think people actually rely on this. I do agree this is not great > though. > The length computation relies on arguments being marked as optional or not > in the IDL. Besides adding a new IDL extended attribute, I am not quite sure > how to maintain the length. I have't found an easy way to fix this one. Do you think it is worth maintaining the previous length at the cost of adding some kind of IDL attribute, e.g. [Length=2])?
(In reply to comment #12) > (In reply to comment #10) > > Comment on attachment 279358 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=279358&action=review > > > > >> LayoutTests/ChangeLog:11 > > >> + the specification, the new length now matches our actual behavior. > > > > > > I don’t think this is good. I’d prefer that we still return length 2. Is there some easy way to do that? > > > > I don't think people actually rely on this. I do agree this is not great > > though. > > The length computation relies on arguments being marked as optional or not > > in the IDL. Besides adding a new IDL extended attribute, I am not quite sure > > how to maintain the length. > > I have't found an easy way to fix this one. Do you think it is worth > maintaining the previous length at the cost of adding some kind of IDL > attribute, e.g. [Length=2])? Alternatively, we could align our behavior with the spec (and with Firefox and Chrome) and mark the 2 first parameters as mandatory. This is a bit riskier but this is right thing.
Seems like we can add a temporary IDL extended attribute or temporary hard coded exception in the bindings script. Then we can remove it when we change behavior to not have the arguments be optional. No reason to temporarily return 0 since we have always returned 2 and we think that 2 is the right value long term.
Created attachment 279605 [details] Patch
Comment on attachment 279605 [details] Patch Clearing flags on attachment: 279605 Committed r201305: <http://trac.webkit.org/changeset/201305>
All reviewed patches have been landed. Closing bug.