Bug 57050 – Untangle dependency between event ancestor chain computation and InspectorDOMAgent.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dimitri Glazkov (Google)]

Untangle dependency between event ancestor chain computation and InspectorDOMAgent.

[Dimitri Glazkov (Google)]

Created attachment 86826 [details]
Patch

[Darin Adler]

Comment on attachment 86826 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=86826&action=review

> Source/WebCore/ChangeLog:9
> +        Inspector's list of event listeners does not need to invoke Node::getEventListeners,
> +        because it simply needs a list of all ancestors of a node.

How did you know it only needed that?

> Source/WebCore/ChangeLog:16
> +        (WebCore::Node::dispatchGenericEvent): Move inDocument check out here from
> +            getEventAncestors.

Why?

> Source/WebCore/inspector/InspectorDOMAgent.cpp:748
>      // The Node's Event Ancestors (not including self)

I don’t think that calling these “Event Ancestors” is all that clear since it’s not necessarily the same objects you’d send a DOM event to.

[Dimitri Glazkov (Google)]

(In reply to comment #2)
> (From update of attachment 86826 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=86826&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        Inspector's list of event listeners does not need to invoke Node::getEventListeners,
> > +        because it simply needs a list of all ancestors of a node.
> 
> How did you know it only needed that?

The Inspector method just looks at all ancestors, outside the context of an event, and doesn't ever use the target/currentTarget parts of the EventContext.

I'll update the ChangeLog to clarify.

> 
> > Source/WebCore/ChangeLog:16
> > +        (WebCore::Node::dispatchGenericEvent): Move inDocument check out here from
> > +            getEventAncestors.
> 
> Why?

Ah, good catch. This is unnecessary, now that I am passing node in. This is a vestigial bit, I'll remove.

> 
> > Source/WebCore/inspector/InspectorDOMAgent.cpp:748
> >      // The Node's Event Ancestors (not including self)
> 
> I don’t think that calling these “Event Ancestors” is all that clear since it’s not necessarily the same objects you’d send a DOM event to.

Right. These are just node ancestors. I'll change the comment.

[Dimitri Glazkov (Google)]

Committed r81909: <http://trac.webkit.org/changeset/81909>