Bug 30318 – ScriptExecutionContext is not anymore needed to create an EventListener - remove old code.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Dmitry Titov]

Change http://trac.webkit.org/changeset/48884 removed the need to have ScriptExecutionContext to create an EventListener - rather it is pulled from EventTarget at the moment of event handling.
It seems we can remove the code in custom add/removeEventListener methods that retrieves the context which is no longer used. The potential change in behavior here is that in case the EventTarget has null context it wasn't adding a listener and now it would, which could later be invoked if the EventTarget will gain context, so it could change the behavior of some page, theoretically. But I don't think this scenario ever realizes. For example, it's impossible to create a Node w/o a pointer to Document and while a Node may change documents during its life, there is no code that can set the Node::m_document to null.

I see the original change did not remove this code, is there a reason to keep it?

[Dmitry Titov]

Created attachment 41076 [details]
Proposed fix.

[Alexey Proskuryakov]

Comment on attachment 41076 [details]
Proposed fix.

> I see the original change did not remove this code, is there a reason to keep
> it?

I just wanted to keep this potential behavior change separate form other changes in that patch. Thanks for following up on this!

> it's impossible to create a Node w/o a pointer to Document

That's not entirely accurate; see DOMImplementation::createDocumentType() for the one and only case when this happens. Maybe you could test for that before landing (and compare to Firefox), but it's unlikely that there is any harm in such a change.

r=me

[Dmitry Titov]

Created attachment 41141 [details]
Updated, with test

Verified that FF also allows to set an event listener on documentType - and will fire events on it once it is inserted into a live document tree.
Added test that verifies that.

[Alexey Proskuryakov]

Comment on attachment 41141 [details]
Updated, with test

r=me

[Dmitry Titov]

Landed: http://trac.webkit.org/changeset/49539