Summary: | Web Inspector: Introduce new "Input" domain for emitting keyboard / mouse events. | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Pavel Feldman <pfeldman> | ||||||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Ken Kania <kkania> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | apavlov, dglazkov, gyuyoung.kim, keishi, kkania, loislo, pfeldman, pmuellr, rakuco, vsevik, web-inspector-bugs, webkit-ews, webkit.review.bot, yurys | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | All | ||||||||||||||||
OS: | All | ||||||||||||||||
Attachments: |
|
Description
Pavel Feldman
2012-11-28 09:09:55 PST
Created attachment 176591 [details]
Patch
Comment on attachment 176591 [details] Patch Attachment 176591 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15013645 Comment on attachment 176591 [details] Patch Attachment 176591 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/15010692 Comment on attachment 176591 [details] Patch Attachment 176591 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/15018629 Comment on attachment 176591 [details] Patch Attachment 176591 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15015646 Comment on attachment 176591 [details] Patch Attachment 176591 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/15029554 Created attachment 176745 [details]
Patch
Comment on attachment 176745 [details] Patch 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. (In reply to comment #8) > Let's move "Input" domain tests under LayoutTests/inspector-protocol/input. We should eventually do the same for other tests under LayoutTests/inspector-protocol/. > Ok, this particular one seems to be from DOM domain. Comment on attachment 176745 [details] Patch Attachment 176745 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15044447 Created attachment 177003 [details]
Patch
Comment on attachment 177003 [details] Patch Attachment 177003 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15083080 (In reply to comment #12) > (From update of attachment 177003 [details]) > Attachment 177003 [details] did not pass mac-ews (mac): > Output: http://queues.webkit.org/results/15083080 /Volumes/Data/EWS/WebKit/Source/WebCore/inspector/InspectorInputAgent.cpp:58:57: error: unused parameter 'error' [-Werror,-Wunused-parameter] Just omit the name of the parameter if it is unused. Created attachment 177048 [details]
Patch
Comment on attachment 177048 [details] Patch Attachment 177048 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15061550 New failing tests: inspector-protocol/input/dispatchKeyEvent.html Created attachment 177056 [details]
Patch
Comment on attachment 177056 [details] Patch Attachment 177056 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15086085 New failing tests: inspector-protocol/input/dispatchKeyEvent.html Created attachment 177360 [details]
Patch
I'm wondering why it crashed on Mac and passed on other platforms? Comment on attachment 177360 [details]
Patch
Should this be formulated such that it can asynchronou? E.g. what's the execution order of
page.dispatchKeyEvent('x')
runtime.Evaluate('window.didGetX');
for some page that sets didGetX in a keyHandler on the main page...
If I push them onto the websocket right away?
I think this is less important for keys, but for mouse events, its definitely important that we think through the ordering of things.
Comment on attachment 177360 [details] Patch Attachment 177360 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15121591 New failing tests: inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html Nat, The dispatchKeyEvent command processes the key event synchronously, although some effects may be asynchronous (such as if the key stroke submitted a form which later causes a navigation). For your example, as long as the commands are sent in order, things will behave as you expect. (In reply to comment #21) > (From update of attachment 177360 [details]) > Attachment 177360 [details] did not pass chromium-ews (chromium-xvfb): > Output: http://queues.webkit.org/results/15121591 > > New failing tests: > inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html Looks like a flakiness unrelated to the change. Comment on attachment 177360 [details] Patch Clearing flags on attachment: 177360 Committed r136521: <http://trac.webkit.org/changeset/136521> All reviewed patches have been landed. Closing bug. |