Bug 57050 - Untangle dependency between event ancestor chain computation and InspectorDOMAgent.
Summary: Untangle dependency between event ancestor chain computation and InspectorDOM...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Dimitri Glazkov (Google)
URL:
Keywords:
Depends on:
Blocks: 55515
  Show dependency treegraph
 
Reported: 2011-03-24 13:50 PDT by Dimitri Glazkov (Google)
Modified: 2011-03-24 15:20 PDT (History)
1 user (show)

See Also:


Attachments
Patch (5.13 KB, patch)
2011-03-24 13:58 PDT, Dimitri Glazkov (Google)
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitri Glazkov (Google) 2011-03-24 13:50:14 PDT
Untangle dependency between event ancestor chain computation and InspectorDOMAgent.
Comment 1 Dimitri Glazkov (Google) 2011-03-24 13:58:25 PDT
Created attachment 86826 [details]
Patch
Comment 2 Darin Adler 2011-03-24 15:02:58 PDT
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.
Comment 3 Dimitri Glazkov (Google) 2011-03-24 15:06:49 PDT
(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.
Comment 4 Dimitri Glazkov (Google) 2011-03-24 15:20:26 PDT
Committed r81909: <http://trac.webkit.org/changeset/81909>