Summary: | Web Inspector: for event listener provide handler function value in protocol and in UI | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Peter Rybin <prybin> | ||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | abarth, apavlov, dglazkov, graouts, haraken, japhet, joepeck, keishi, loislo, pfeldman, pmuellr, timothy, vsevik, web-inspector-bugs, webkit.review.bot, yurys | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Peter Rybin
2013-02-08 04:18:18 PST
Created attachment 187293 [details]
Patch
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 Created attachment 187603 [details]
Patch
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 > > 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 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 Created attachment 187641 [details]
Patch
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 Created attachment 187863 [details]
Patch
Comment on attachment 187863 [details] Patch Clearing flags on attachment: 187863 Committed r142627: <http://trac.webkit.org/changeset/142627> All reviewed patches have been landed. Closing bug. See duplicate bug with some context: https://bugs.webkit.org/show_bug.cgi?id=107289 |