Bug 217819 – WebDriver: add support for right variations of virtual keys

• Status: RESOLVED
• Resolution: FIXED
• Component: WebDriver

[Carlos Garcia Campos]

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.

[Carlos Garcia Campos]

Created attachment 411564 [details]
Patch

[Carlos Garcia Campos]

Created attachment 411565 [details]
Patch

[Carlos Garcia Campos]

Created attachment 411569 [details]
Patch

[Carlos Garcia Campos]

Created attachment 411570 [details]
Patch

[Carlos Garcia Campos]

Created attachment 411740 [details]
Patch

I don't understand the wincairo failure

[Darin Adler]

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?

[Radar WebKit Bug Importer]

<rdar://problem/70457872>

[Blaze Burg]

(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;

[Blaze Burg]

Comment on attachment 411740 [details]
Patch

r=me

I think this needs to be rebased again.

[Fujii Hironori]

(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)

[Fujii Hironori]

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?

[Carlos Garcia Campos]

Created attachment 411845 [details]
Patch for landing

[Carlos Garcia Campos]

Committed r268717: <https://trac.webkit.org/changeset/268717>