[webkit-reviews] review granted: [Bug 103530] Web Inspector: Introduce new "Input" domain for emitting keyboard / mouse events. : [Attachment 176745] Patch

Thu Nov 29 10:22:10 PST 2012

Yury Semikhatsky <yurys at chromium.org> has granted Ken Kania
<kkania at chromium.org>'s request for review:
Bug 103530: Web Inspector: Introduce new "Input" domain for emitting keyboard /
mouse events.
https://bugs.webkit.org/show_bug.cgi?id=103530

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

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=176745&action=review

> Source/WebCore/inspector/Inspector.json:1942
> +		       { "name": "nodeId", "$ref": "DOM.NodeId" }

Please provide description for the parameter.

> Source/WebCore/inspector/Inspector.json:3261
> +		       { "name": "type", "type": "string", "enum": ["keyDown",
"keyUp", "rawKeyDown", "char"] },

Description is missing for this ans a few other parameters.

> Source/WebCore/inspector/InspectorInputAgent.h:17
> + * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY

This is a wrong copyright header. Google contributors use a bit different one.
See InspectorMemoryAgent.cpp for example.

> Source/WebCore/inspector/InspectorInputAgent.h:40
> +class InstrumentingAgents;

No need to declare it here as it is already provided by InspectorBaseAgent.

> LayoutTests/inspector-protocol/dom-focus.html:1
> +<html>

Let's move "Input" domain tests under LayoutTests/inspector-protocol/input. We
should eventually do the same for other tests under
LayoutTests/inspector-protocol/.

> LayoutTests/inspector-protocol/dom-focus.html:7
> +    document.querySelector("#second").addEventListener("focus", function() {

We usually use named functions even if they are used as callbacks. Same thing
with other anonymous functions in this patch.