Bug 217819

Summary: WebDriver: add support for right variations of virtual keys
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebDriverAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aperez, Basuke.Suzuki, bburg, clopez, darin, Hironori.Fujii, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 166679    
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
bburg: review+, ews-feeder: commit-queue-
Patch for landing none

Description Carlos Garcia Campos 2020-10-16 05:45:59 PDT 
The browser is currently receiving the virtual keys already normalized, but WebDriver expects the key code to be the actual one, not the normalized one. We need to send the raw virtual keys to the browser and do the normalization there, but use the raw one when synthesizing the events.
Comment 1 Carlos Garcia Campos 2020-10-16 06:00:42 PDT 
Created attachment 411564 [details]
Patch
Comment 2 Carlos Garcia Campos 2020-10-16 06:12:41 PDT 
Created attachment 411565 [details]
Patch
Comment 3 Carlos Garcia Campos 2020-10-16 06:58:41 PDT 
Created attachment 411569 [details]
Patch
Comment 4 Carlos Garcia Campos 2020-10-16 07:06:35 PDT 
Created attachment 411570 [details]
Patch
Comment 5 Carlos Garcia Campos 2020-10-19 05:42:36 PDT 
Created attachment 411740 [details]
Patch

I don't understand the wincairo failure
Comment 6 Darin Adler 2020-10-19 12:51:32 PDT 
Comment on attachment 411740 [details]
Patch

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

> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:1865
> +static VirtualKey normalizedVirtualKey(VirtualKey key)

Isn’t the type named Inspector::Protocol::Automation::VirtualKey? What injects VirtualKey into the WebKit namespace? Why can we just write VirtualKey in the argument types but then need to write the whole type out in the function body?
Comment 7 Radar WebKit Bug Importer 2020-10-19 15:00:54 PDT 
<rdar://problem/70457872>
Comment 8 BJ Burg 2020-10-19 15:03:16 PDT 
(In reply to Darin Adler from comment #6)
> Comment on attachment 411740 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=411740&action=review
> 
> > Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:1865
> > +static VirtualKey normalizedVirtualKey(VirtualKey key)
> 
> Isn’t the type named Inspector::Protocol::Automation::VirtualKey? What
> injects VirtualKey into the WebKit namespace? Why can we just write
> VirtualKey in the argument types but then need to write the whole type out
> in the function body?

The enum names are shortened in SimulatedInputDispatcher.h in order to make the members more readable. This file includes that header.


using KeyboardInteraction = Inspector::Protocol::Automation::KeyboardInteractionType;
using VirtualKey = Inspector::Protocol::Automation::VirtualKey;
using VirtualKeySet = HashSet<VirtualKey, WTF::IntHash<VirtualKey>, WTF::StrongEnumHashTraits<VirtualKey>>;
using MouseButton = Inspector::Protocol::Automation::MouseButton;
using MouseInteraction = Inspector::Protocol::Automation::MouseInteraction;
using MouseMoveOrigin = Inspector::Protocol::Automation::MouseMoveOrigin;
Comment 9 BJ Burg 2020-10-19 15:03:34 PDT 
Comment on attachment 411740 [details]
Patch

r=me

I think this needs to be rebased again.
Comment 10 Fujii Hironori 2020-10-19 17:06:48 PDT 
(In reply to Carlos Garcia Campos from comment #5)
> I don't understand the wincairo failure

WinCairo EWS reported:

..\..\Source\WebKit\UIProcess\Automation\WebAutomationSession.cpp(1865): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int

VirtualKey is not defined, which is defined in WebKit/UIProcess/Automation/SimulatedInputDispatcher.h.
But, ENABLE_WEBDRIVER_ACTIONS_API is not true for WinCairo, which is defined in Source/WebKit/config.h.

WinCairo doesn't enable those features.

    WEBKIT_OPTION_DEFINE(ENABLE_WEBDRIVER_KEYBOARD_INTERACTIONS "Toggle WebDriver keyboard interactions" PRIVATE OFF)
    WEBKIT_OPTION_DEFINE(ENABLE_WEBDRIVER_MOUSE_INTERACTIONS "Toggle WebDriver mouse interactions" PRIVATE OFF)
    WEBKIT_OPTION_DEFINE(ENABLE_WEBDRIVER_TOUCH_INTERACTIONS "Toggle WebDriver touch interactions" PRIVATE OFF)
Comment 11 Fujii Hironori 2020-10-19 17:12:49 PDT 
Comment on attachment 411740 [details]
Patch

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

>>> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:1865
>>> +static VirtualKey normalizedVirtualKey(VirtualKey key)
>> 
>> Isn’t the type named Inspector::Protocol::Automation::VirtualKey? What injects VirtualKey into the WebKit namespace? Why can we just write VirtualKey in the argument types but then need to write the whole type out in the function body?
> 
> The enum names are shortened in SimulatedInputDispatcher.h in order to make the members more readable. This file includes that header.
> 
> 
> using KeyboardInteraction = Inspector::Protocol::Automation::KeyboardInteractionType;
> using VirtualKey = Inspector::Protocol::Automation::VirtualKey;
> using VirtualKeySet = HashSet<VirtualKey, WTF::IntHash<VirtualKey>, WTF::StrongEnumHashTraits<VirtualKey>>;
> using MouseButton = Inspector::Protocol::Automation::MouseButton;
> using MouseInteraction = Inspector::Protocol::Automation::MouseInteraction;
> using MouseMoveOrigin = Inspector::Protocol::Automation::MouseMoveOrigin;

I don't understand this change at all. Can you put this funciton in #if ENABLE(WEBDRIVER_ACTIONS_API), KaL?
Comment 12 Carlos Garcia Campos 2020-10-20 01:08:31 PDT 
Created attachment 411845 [details]
Patch for landing
Comment 13 Carlos Garcia Campos 2020-10-20 02:14:43 PDT 
Committed r268717: <https://trac.webkit.org/changeset/268717>