Bug 81658 – Web Inspector: expose queryEventListeners() to console command line API

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Andrey Kosyakov]

This adds queryEventListeners() call to command line API in the inspector console. Only the JS listeners in current world are returned.
We primarily want this to be accessible to Web Inspector extensions, although this might be handy for having a closer look at the listeners to the interactive users as well.

[Andrey Kosyakov]

Created attachment 132815 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 132815 [details]
Patch

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

> Source/WebCore/bindings/js/JSInjectedScriptHostCustom.cpp:231
> +        eventListeners->putDirect(exec->globalData(), Identifier(exec, "node"), toJS(exec, globalObject, listenersArray[i].node));

Do we really want to return event listeners for ancestors?

> Source/WebCore/inspector/InjectedScriptHost.h:95
> +    void queryEventListenersImpl(Node*, Vector<EventListenerInfo>& listenersArray);

Why is this Impl suffix?

[Build Bot]

Comment on attachment 132815 [details]
Patch

Attachment 132815 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12005268

[Yury Semikhatsky]

Comment on attachment 132815 [details]
Patch

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

> LayoutTests/inspector/console/command-line-api-queryEventListeners.html:20
> +<div id="inner">

Please add a few attribute handlers as they will also be listed as the listeners.

[Andrey Kosyakov]

(In reply to comment #2)
> (From update of attachment 132815 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=132815&action=review
> 
> > Source/WebCore/bindings/js/JSInjectedScriptHostCustom.cpp:231
> > +        eventListeners->putDirect(exec->globalData(), Identifier(exec, "node"), toJS(exec, globalObject, listenersArray[i].node));
> 
> Do we really want to return event listeners for ancestors?

This is for consistency with what we do in the DOM agent/elements panel -- presumably, caller would be interested in the ancestors, given these may have interceptors or handlers for bubbling event. This could go under a boolean flag, perhaps.

> > Source/WebCore/inspector/InjectedScriptHost.h:95
> > +    void queryEventListenersImpl(Node*, Vector<EventListenerInfo>& listenersArray);
> 
> Why is this Impl suffix?

Just for consistency with inspectImpl(). I'm not particularly attached to it, though, so will gladly drop.

> Please add a few attribute handlers as they will also be listed as the listeners.

Fixed.

[Andrey Kosyakov]

Created attachment 132824 [details]
Patch

[Build Bot]

Comment on attachment 132824 [details]
Patch

Attachment 132824 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/11994322

[Andrey Kosyakov]

(In reply to comment #2)
> (From update of attachment 132815 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=132815&action=review
> 
> > Source/WebCore/bindings/js/JSInjectedScriptHostCustom.cpp:231
> > +        eventListeners->putDirect(exec->globalData(), Identifier(exec, "node"), toJS(exec, globalObject, listenersArray[i].node));
> 
> Do we really want to return event listeners for ancestors?

On a second thought, I'd let the caller do the work of traversing the ancestry -- this will make the interface simpler.

[Andrey Kosyakov]

Created attachment 132833 [details]
Patch

[Build Bot]

Comment on attachment 132833 [details]
Patch

Attachment 132833 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12001335

[Yury Semikhatsky]

> > > Source/WebCore/inspector/InjectedScriptHost.h:95
> > > +    void queryEventListenersImpl(Node*, Vector<EventListenerInfo>& listenersArray);
> > 
> > Why is this Impl suffix?
> 
> Just for consistency with inspectImpl(). I'm not particularly attached to it, though, so will gladly drop.
> 
Can we remove it from inspectImpl instead. Other methods seem not to use such suffix.

[Pavel Feldman]

(In reply to comment #11)
> > > > Source/WebCore/inspector/InjectedScriptHost.h:95
> > > > +    void queryEventListenersImpl(Node*, Vector<EventListenerInfo>& listenersArray);
> > > 
> > > Why is this Impl suffix?
> > 
> > Just for consistency with inspectImpl(). I'm not particularly attached to it, though, so will gladly drop.
> > 
> Can we remove it from inspectImpl instead. Other methods seem not to use such suffix.

If we drop Impl suffix, there will be no way to tell whether InjectedScriptHost::inspect is custom or not. We used this pattern for all the base implementations of custom members. I think it is useful.

[Pavel Feldman]

Comment on attachment 132833 [details]
Patch

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

> Source/WebCore/inspector/InjectedScriptSource.js:580
> +    "monitorEvents", "unmonitorEvents", "inspect", "copy", "clear", "queryEventListeners"

I'd call it getEventListeners, there is no query per se.

> Source/WebCore/inspector/InjectedScriptSource.js:690
> +        return InjectedScriptHost.queryEventListeners(node);

I'd add a request parameter determining whether caller is interested in self node's listeners or hierarchy ones as well.

[Andrey Kosyakov]

Committed r111529: <http://trac.webkit.org/changeset/111529>