Bug 165593 - DumpRenderTree EventSendingController must implement keyDown for "escape".
Summary: DumpRenderTree EventSendingController must implement keyDown for "escape".
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Mac Unspecified
: P2 Normal
Assignee: Jeremy Jones
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-12-07 21:43 PST by Jeremy Jones
Modified: 2016-12-12 09:30 PST (History)
6 users (show)

See Also:


Attachments
Patch (2.96 KB, patch)
2016-12-07 21:47 PST, Jeremy Jones
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Jones 2016-12-07 21:43:33 PST
DumpRenderTree EventSendingController must implement keyDown for "escape".
Comment 1 Jeremy Jones 2016-12-07 21:44:32 PST
rdar://problem/29568847
Comment 2 Jeremy Jones 2016-12-07 21:47:36 PST
Created attachment 296483 [details]
Patch
Comment 3 Alexey Proskuryakov 2016-12-07 23:08:05 PST
Comment on attachment 296483 [details]
Patch

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

> Tools/DumpRenderTree/mac/EventSendingController.mm:925
> +        const unichar ch = 0x1B;

This doesn't feel right to me. I believe that AppKit handles the Esc key internally, and its behavior is a lot more complicated than just sending a keyDown/keyUp to firstResponder. IIRC, the processing also includes sending a -performKeyEquivalent: and a -doCommandBySelector with both cancelOperation and cancel selectors.

Curiously, this may not be concern for WebKitTestRunner, because WK2 takes care of sending an even back to AppKit ifself.

I don't have any good suggestion for what to do here.

> LayoutTests/platform/mac-wk1/TestExpectations:-296
> -webkit.org/b/165589 pointer-lock/lock-lost-on-esc-in-fullscreen.html [ Skip ]

As fa as I can tell, you could just use this bug for the fix.
Comment 4 Darin Adler 2016-12-10 17:57:16 PST
Comment on attachment 296483 [details]
Patch

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

>> Tools/DumpRenderTree/mac/EventSendingController.mm:925
>> +        const unichar ch = 0x1B;
> 
> This doesn't feel right to me. I believe that AppKit handles the Esc key internally, and its behavior is a lot more complicated than just sending a keyDown/keyUp to firstResponder. IIRC, the processing also includes sending a -performKeyEquivalent: and a -doCommandBySelector with both cancelOperation and cancel selectors.
> 
> Curiously, this may not be concern for WebKitTestRunner, because WK2 takes care of sending an even back to AppKit ifself.
> 
> I don't have any good suggestion for what to do here.

I don’t think that’s quite right. I studied the relevant AppKit source code, and I am almost certain that the escape key event first passes through the responder chain as an argument to keyDown: and then the additional processing you are referring above done inside the -[NSWindow keyDown:] method. So I believe there is a good chance the change in this patch will work for testing escape key behavior.
Comment 5 Alexey Proskuryakov 2016-12-12 09:30:24 PST
My understanding is that with this patch, the event won't go through the responder chain, and will never reach -[NSWindow keyDown:]. So, the behavior of Esc is not accurately emulated.

If my understanding is correct, the behavior of eventSender.keyDown("escape") is very imprecise. For example, it will fail if the implementation ever changes to use -performKeyEquivalent:.

Then again, we already have a lot of special keys there, such as "leftArrow". I guess my concern is that Escape is the trickiest of them all.