Bug 109284 – Web Inspector: for event listener provide handler function value in protocol and in UI

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

[Peter Rybin]

Event listener handler is typically a JavaScript function. A direct display of the function value is missing. Function value should be expandable as any JavaScript value.
This allows to see closure variables and function-as-object properties.

[Peter Rybin]

Created attachment 187293 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 187293 [details]
Patch

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

> Source/WebCore/bindings/v8/ScriptEventListener.cpp:112
> +    V8AbstractEventListener* v8Listener = static_cast<V8AbstractEventListener*>(listener);

Please extract common part with eventListenerHandlerBody into a separate function.

> Source/WebCore/inspector/Inspector.json:1748
> +                    { "name": "handler", "$ref": "Runtime.RemoteObject", "optional": true, "hidden": true, "description": "Event handler function value" }

The type is already hidden.

> Source/WebCore/inspector/Inspector.json:1870
> +                    { "name": "objectGroup", "type": "string", "optional": true, "description": "Symbolic group name for handler value." }

Please add a note on that handler won't be returned if the group is not specified.

> Source/WebCore/inspector/InspectorDOMAgent.cpp:1394
> +                InjectedScript injectedScript = m_injectedScriptManager->injectedScriptFor(mainWorldScriptState(frame));

You should use the world from event listener instead.

> Source/WebCore/inspector/front-end/EventListenersSidebarPane.js:123
> +            node.eventListeners("arabypaC", callback);

Please use WebInspector.EventListenersSidebarPane._objectGroupName

[Peter Rybin]

Created attachment 187603 [details]
Patch

[WebKit Review Bot]

Comment on attachment 187603 [details]
Patch

Attachment 187603 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16491317

New failing tests:
inspector/elements/event-listeners-about-blank.html

[Peter Rybin]

> > Source/WebCore/bindings/v8/ScriptEventListener.cpp:112
> > +    V8AbstractEventListener* v8Listener = static_cast<V8AbstractEventListener*>(listener);
> Please extract common part with eventListenerHandlerBody into a separate function.

Cannot do this thing. It turned out that final toWebCoreStringWithNullCheck statement depends on contextScope and breaks if the object doesn't exist.

> > Source/WebCore/inspector/Inspector.json:1748
> > +                    { "name": "handler", "$ref": "Runtime.RemoteObject", "optional": true, "hidden": true, "description": "Event handler function value" }
> The type is already hidden.

Done

> > Source/WebCore/inspector/Inspector.json:1870
> > +                    { "name": "objectGroup", "type": "string", "optional": true, "description": "Symbolic group name for handler value." }
> Please add a note on that handler won't be returned if the group is not specified.

Done

> > Source/WebCore/inspector/InspectorDOMAgent.cpp:1394
> > +                InjectedScript injectedScript = m_injectedScriptManager->injectedScriptFor(mainWorldScriptState(frame));
> You should use the world from event listener instead.

Done

> > Source/WebCore/inspector/front-end/EventListenersSidebarPane.js:123
> > +            node.eventListeners("arabypaC", callback);
> Please use WebInspector.EventListenersSidebarPane._objectGroupName

Done

[WebKit Review Bot]

Comment on attachment 187603 [details]
Patch

Attachment 187603 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16494309

New failing tests:
inspector/elements/event-listeners-about-blank.html

[Peter Rybin]

Created attachment 187641 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 187641 [details]
Patch

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

> Source/WebCore/inspector/Inspector.json:1759
> +                    { "name": "handler", "$ref": "Runtime.RemoteObject", "optional": true, "description": "Event handler function value" }

Missing . at the end of the sentence.

> Source/WebCore/inspector/InspectorDOMAgent.h:134
> +    virtual void getEventListenersForNode(ErrorString*, int nodeId, const WTF::String*, RefPtr<TypeBuilder::Array<TypeBuilder::DOM::EventListener> >& listenersArray);

Param name would make sense here, otherwise it is hard to guess that the string param is objectGroupId

[Peter Rybin]

Created attachment 187863 [details]
Patch

[WebKit Review Bot]

Comment on attachment 187863 [details]
Patch

Clearing flags on attachment: 187863

Committed r142627: <http://trac.webkit.org/changeset/142627>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Peter Rybin]

See duplicate bug with some context:
https://bugs.webkit.org/show_bug.cgi?id=107289