[webkit-reviews] review denied: [Bug 109284] Web Inspector: for event listener provide handler function value in protocol and in UI : [Attachment 187293] Patch

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has denied Peter Rybin
<prybin at chromium.org>'s request for review:
Bug 109284: Web Inspector: for event listener provide handler function value in
protocol and in UI
https://bugs.webkit.org/show_bug.cgi?id=109284

Attachment 187293: Patch
https://bugs.webkit.org/attachment.cgi?id=187293&action=review

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
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