Bug 217819 - WebDriver: add support for right variations of virtual keys
Summary: WebDriver: add support for right variations of virtual keys
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebDriver (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks: gtk-webdriver
  Show dependency treegraph
 
Reported: 2020-10-16 05:45 PDT by Carlos Garcia Campos
Modified: 2020-10-20 02:14 PDT (History)
7 users (show)

See Also:


Attachments
Patch (31.60 KB, patch)
2020-10-16 06:00 PDT, Carlos Garcia Campos
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (31.60 KB, patch)
2020-10-16 06:12 PDT, Carlos Garcia Campos
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (34.12 KB, patch)
2020-10-16 06:58 PDT, Carlos Garcia Campos
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (34.12 KB, patch)
2020-10-16 07:06 PDT, Carlos Garcia Campos
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (34.13 KB, patch)
2020-10-19 05:42 PDT, Carlos Garcia Campos
bburg: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (34.15 KB, patch)
2020-10-20 01:08 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>