Bug 201913 – [Win][WebKitTestRunner] Implement EventSenderProxy::keyDown

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Fujii Hironori]

[Win][WebKitTestRunner] Implement EventSenderProxy::keyDown

[Fujii Hironori]

Created attachment 379030 [details]
WIP patch

[Fujii Hironori]

This WIP patch sends key events, but tests doesn't pass
expectedly because of asynchronous message passing from UI
process to Web process.

For example, fast/dom/fragment-activation-focuses-target.html has the following code.

>  eventSender.keyDown("\t");
>  shouldBe("document.activeElement", "document.getElementById('fragment3')");

This code expects the focus was moved imediately after calling keyDown.
But, key event message sent by UI process is still queued in that time.

How do other WebKit2 ports do?

[Fujii Hironori]

Comment 2 was completely wrong.
Web process processes messages while keyEvent (Bug 148326).

[Fujii Hironori]

Created attachment 379116 [details]
Patch

[Ross Kirsling]

Comment on attachment 379116 [details]
Patch

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

Seems reasonable based on DRT version.

> Tools/WebKitTestRunner/win/EventSenderProxyWin.cpp:230
> +        wchar_t buff[1];
> +        WKStringGetCharacters(keyRef, buff, _countof(buff));
> +        charCode = buff[0];

Is this better than the approach taken in WebKitBrowserWindow?
https://github.com/WebKit/webkit/blob/master/Tools/MiniBrowser/win/WebKitBrowserWindow.cpp#L46-L47

> Tools/WebKitTestRunner/win/EventSenderProxyWin.cpp:258
> +    keyData |= (KF_UP | KF_REPEAT) << 16;

I don't think I understand this line. Does this correspond to the PostMessage in the DRT implementation? Why auto-repeat?

[Fujii Hironori]

Comment on attachment 379116 [details]
Patch

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

>> Tools/WebKitTestRunner/win/EventSenderProxyWin.cpp:230
>> +        charCode = buff[0];
> 
> Is this better than the approach taken in WebKitBrowserWindow?
> https://github.com/WebKit/webkit/blob/master/Tools/MiniBrowser/win/WebKitBrowserWindow.cpp#L46-L47

Only the first character is needed. DRT also checks only the first character.

>> Tools/WebKitTestRunner/win/EventSenderProxyWin.cpp:258
>> +    keyData |= (KF_UP | KF_REPEAT) << 16;
> 
> I don't think I understand this line. Does this correspond to the PostMessage in the DRT implementation? Why auto-repeat?

Good catch.
I removed the PostMessage(WM_CHAR) part because I couldn't
understand why it is needed and I'm too lazy to study the reason from
the commit history.

It came from Bug 11449.
https://github.com/WebKit/webkit/commit/ca3369df7ca41a2199f9d2187642989488a94e2c

fast/forms/select-type-ahead-non-latin.html is the test case.
I need the PostMessage. Will fix.

[Fujii Hironori]

Comment on attachment 379116 [details]
Patch

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

>>> Tools/WebKitTestRunner/win/EventSenderProxyWin.cpp:258
>>> +    keyData |= (KF_UP | KF_REPEAT) << 16;
>> 
>> I don't think I understand this line. Does this correspond to the PostMessage in the DRT implementation? Why auto-repeat?
> 
> Good catch.
> I removed the PostMessage(WM_CHAR) part because I couldn't
> understand why it is needed and I'm too lazy to study the reason from
> the commit history.
> 
> It came from Bug 11449.
> https://github.com/WebKit/webkit/commit/ca3369df7ca41a2199f9d2187642989488a94e2c
> 
> fast/forms/select-type-ahead-non-latin.html is the test case.
> I need the PostMessage. Will fix.

You are right. DRT and my WTR implementations differ in this part.

I want to reuse existing EventSenderProxy::dispatchMessage in WTR if possible.
It calls TranslateMessage, this introduces the difference.
If I didn't turn those bits, TranslateMessage posts WM_CHAR even for WM_KEYUP.

According the spec, WM_KEYUP needs those bits.

https://docs.microsoft.com/en-us/windows/win32/inputdev/wm-keyup

> 30	The previous key state. The value is always 1 for a WM_KEYUP message.
> 31	The transition state. The value is always 1 for a WM_KEYUP message.

DRT doesn't need the those bits becasuse it doesn't use TranslateMessage for WM_KEYUP.

[Fujii Hironori]

Created attachment 379189 [details]
Patch

[Ross Kirsling]

Comment on attachment 379189 [details]
Patch

r=me, though it would be helpful to add some of that explanation to the ChangeLog too.

[Fujii Hironori]

Created attachment 379421 [details]
Patch for landing

[Fujii Hironori]

Committed r250279: <https://trac.webkit.org/changeset/250279>

[Radar WebKit Bug Importer]

<rdar://problem/55649032>