Commit 1ee5d1e6 authored by John Chen's avatar John Chen Committed by Commit Bot

[ChromeDriver] Fix issues in keyboard actions

Fixing various issues in the implementation of keyboard actions, including:
* Proper handling of modifier keys.
* Use W3C spec compliant codes and names for keys.
* Setting key location property.
* Keeping track of key presses between multiple actions.

Most of the logic is in ConvertKeyActionToKeyEvent function, which was
based on ConvertKeysToKeyEvents function for legacy (non action based)
keyboard support.

ChromeDriver now passes most WPT test cases for keyboard actions
(webdriver/tests/perform_actions/key*.py), except:
* 4 failures in key_special_keys.py, because Unicode characters
  above U+FFFF and grapheme clusters are not yet supported.
* 7 failures in key_events.py, due to inconsistency in "code for key"
  table in W3C spec (https://github.com/w3c/webdriver/pull/1384).
* 4 failures in key_events.py, because Chrome refers to the Meta keys
  as MetaLeft and MetaRight, while WPT names them OSLeft and OSRight.

Bug: chromedriver:1897
Change-Id: Icaf9fff81c7e8127a849699f90871d8d24af4dfa
Reviewed-on: https://chromium-review.googlesource.com/c/1392056Reviewed-by: default avatarLan Wei <lanwei@chromium.org>
Reviewed-by: default avatarCaleb Rouleau <crouleau@chromium.org>
Commit-Queue: John Chen <johnchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619480}
parent 6a964885
...@@ -43,7 +43,10 @@ KeyEvent::KeyEvent() ...@@ -43,7 +43,10 @@ KeyEvent::KeyEvent()
modified_text(std::string()), modified_text(std::string()),
unmodified_text(std::string()), unmodified_text(std::string()),
key(std::string()), key(std::string()),
key_code(ui::VKEY_UNKNOWN) {} key_code(ui::VKEY_UNKNOWN),
location(0),
code(),
is_from_action(false) {}
KeyEvent::KeyEvent(const KeyEvent& that) KeyEvent::KeyEvent(const KeyEvent& that)
: type(that.type), : type(that.type),
...@@ -51,7 +54,10 @@ KeyEvent::KeyEvent(const KeyEvent& that) ...@@ -51,7 +54,10 @@ KeyEvent::KeyEvent(const KeyEvent& that)
modified_text(that.modified_text), modified_text(that.modified_text),
unmodified_text(that.unmodified_text), unmodified_text(that.unmodified_text),
key(that.key), key(that.key),
key_code(that.key_code) {} key_code(that.key_code),
location(that.location),
code(that.code),
is_from_action(that.is_from_action) {}
KeyEvent::~KeyEvent() {} KeyEvent::~KeyEvent() {}
...@@ -89,6 +95,27 @@ KeyEventBuilder* KeyEventBuilder::SetKeyCode(ui::KeyboardCode key_code) { ...@@ -89,6 +95,27 @@ KeyEventBuilder* KeyEventBuilder::SetKeyCode(ui::KeyboardCode key_code) {
return this; return this;
} }
KeyEventBuilder* KeyEventBuilder::SetLocation(int location) {
key_event_.location = location;
return this;
}
KeyEventBuilder* KeyEventBuilder::SetDefaultKey(const std::string& key) {
if (key_event_.key.size() == 0)
key_event_.key = key;
return this;
}
KeyEventBuilder* KeyEventBuilder::SetCode(const std::string& code) {
key_event_.code = code;
return this;
}
KeyEventBuilder* KeyEventBuilder::SetIsFromAction() {
key_event_.is_from_action = true;
return this;
}
KeyEvent KeyEventBuilder::Build() { KeyEvent KeyEventBuilder::Build() {
DCHECK(key_event_.type != kInvalidEventType); DCHECK(key_event_.type != kInvalidEventType);
return key_event_; return key_event_;
......
...@@ -104,6 +104,9 @@ struct KeyEvent { ...@@ -104,6 +104,9 @@ struct KeyEvent {
std::string unmodified_text; std::string unmodified_text;
std::string key; std::string key;
ui::KeyboardCode key_code; ui::KeyboardCode key_code;
int location;
std::string code;
bool is_from_action;
}; };
class KeyEventBuilder { class KeyEventBuilder {
...@@ -117,6 +120,10 @@ class KeyEventBuilder { ...@@ -117,6 +120,10 @@ class KeyEventBuilder {
KeyEventBuilder* SetText(const std::string& unmodified_text, KeyEventBuilder* SetText(const std::string& unmodified_text,
const std::string& modified_text); const std::string& modified_text);
KeyEventBuilder* SetKeyCode(ui::KeyboardCode key_code); KeyEventBuilder* SetKeyCode(ui::KeyboardCode key_code);
KeyEventBuilder* SetLocation(int location);
KeyEventBuilder* SetDefaultKey(const std::string& key);
KeyEventBuilder* SetCode(const std::string& key);
KeyEventBuilder* SetIsFromAction();
KeyEvent Build(); KeyEvent Build();
void Generate(std::list<KeyEvent>* key_events); void Generate(std::list<KeyEvent>* key_events);
......
...@@ -519,12 +519,28 @@ Status WebViewImpl::DispatchKeyEvents(const std::list<KeyEvent>& events) { ...@@ -519,12 +519,28 @@ Status WebViewImpl::DispatchKeyEvents(const std::list<KeyEvent>& events) {
params.SetString("text", it->modified_text); params.SetString("text", it->modified_text);
params.SetString("unmodifiedText", it->unmodified_text); params.SetString("unmodifiedText", it->unmodified_text);
params.SetInteger("windowsVirtualKeyCode", it->key_code); params.SetInteger("windowsVirtualKeyCode", it->key_code);
ui::DomCode dom_code = ui::UsLayoutKeyboardCodeToDomCode(it->key_code); std::string code;
std::string code = ui::KeycodeConverter::DomCodeToCodeString(dom_code); if (it->is_from_action) {
code = it->code;
} else {
ui::DomCode dom_code = ui::UsLayoutKeyboardCodeToDomCode(it->key_code);
code = ui::KeycodeConverter::DomCodeToCodeString(dom_code);
}
if (!code.empty()) if (!code.empty())
params.SetString("code", code); params.SetString("code", code);
if (!it->key.empty()) if (!it->key.empty())
params.SetString("key", it->key); params.SetString("key", it->key);
else if (it->is_from_action)
params.SetString("key", it->modified_text);
if (it->location != 0) {
// The |location| parameter in DevTools protocol only accepts 1 (left
// modifiers) and 2 (right modifiers). For location 3 (numeric keypad),
// it is necessary to set the |isKeypad| parameter.
if (it->location == 3)
params.SetBoolean("isKeypad", true);
else
params.SetInteger("location", it->location);
}
Status status = client_->SendCommand("Input.dispatchKeyEvent", params); Status status = client_->SendCommand("Input.dispatchKeyEvent", params);
if (status.IsError()) if (status.IsError())
return status; return status;
......
This diff is collapsed.
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include <string> #include <string>
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "base/values.h"
#include "ui/events/keycodes/keyboard_codes.h" #include "ui/events/keycodes/keyboard_codes.h"
struct KeyEvent; struct KeyEvent;
...@@ -24,4 +25,9 @@ Status ConvertKeysToKeyEvents(const base::string16& keys, ...@@ -24,4 +25,9 @@ Status ConvertKeysToKeyEvents(const base::string16& keys,
int* modifiers, int* modifiers,
std::list<KeyEvent>* key_events); std::list<KeyEvent>* key_events);
Status ConvertKeyActionToKeyEvent(const base::DictionaryValue* action_object,
base::DictionaryValue* input_state,
bool is_key_down,
std::vector<KeyEvent>* client_key_events);
#endif // CHROME_TEST_CHROMEDRIVER_KEY_CONVERTER_H_ #endif // CHROME_TEST_CHROMEDRIVER_KEY_CONVERTER_H_
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "base/callback.h" #include "base/callback.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversion_utils.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/threading/platform_thread.h" #include "base/threading/platform_thread.h"
#include "base/time/time.h" #include "base/time/time.h"
...@@ -1003,17 +1004,14 @@ Status ProcessInputActionSequence( ...@@ -1003,17 +1004,14 @@ Status ProcessInputActionSequence(
base::DictionaryValue tmp_state; base::DictionaryValue tmp_state;
tmp_state.SetString("id", id); tmp_state.SetString("id", id);
if (type == "key") { if (type == "key") {
std::unique_ptr<base::ListValue> pressed(new base::ListValue); // Initialize a key input state object
bool alt = false; // (https://w3c.github.io/webdriver/#dfn-key-input-state).
bool shift = false; tmp_state.SetDictionary("pressed",
bool ctrl = false; std::make_unique<base::DictionaryValue>());
bool meta = false; // For convenience, we use one integer property to encode four Boolean
// properties (alt, shift, ctrl, meta) from the spec, using values from
tmp_state.SetList("pressed", std::move(pressed)); // enum KeyModifierMask.
tmp_state.SetBoolean("alt", alt); tmp_state.SetInteger("modifiers", 0);
tmp_state.SetBoolean("shift", shift);
tmp_state.SetBoolean("ctrl", ctrl);
tmp_state.SetBoolean("meta", meta);
} else if (type == "pointer") { } else if (type == "pointer") {
std::unique_ptr<base::ListValue> pressed(new base::ListValue); std::unique_ptr<base::ListValue> pressed(new base::ListValue);
int x = 0; int x = 0;
...@@ -1073,11 +1071,19 @@ Status ProcessInputActionSequence( ...@@ -1073,11 +1071,19 @@ Status ProcessInputActionSequence(
return status; return status;
} else { } else {
std::string key; std::string key;
// TODO: check if key is a single unicode code point bool valid = action_item->GetString("value", &key);
if (!action_item->GetString("value", &key)) { if (valid) {
return Status(kInvalidArgument, // check if key is a single unicode code point
"'value' must be a single unicode point"); int32_t char_index = 0;
uint32_t code_point;
valid =
base::ReadUnicodeCharacter(key.c_str(), key.size(), &char_index,
&code_point) &&
static_cast<std::string::size_type>(char_index + 1) == key.size();
} }
if (!valid)
return Status(kInvalidArgument,
"'value' must be a single Unicode code point");
action->SetString("value", key); action->SetString("value", key);
} }
} else if (type == "pointer") { } else if (type == "pointer") {
...@@ -1219,36 +1225,19 @@ Status ExecutePerformActions(Session* session, ...@@ -1219,36 +1225,19 @@ Status ExecutePerformActions(Session* session,
action->GetString("subtype", &subtype); action->GetString("subtype", &subtype);
std::string id; std::string id;
action->GetString("id", &id); action->GetString("id", &id);
base::DictionaryValue* input_state;
if (!session->input_state_table.GetDictionary(id, &input_state))
return Status(kUnknownError, "missing input state");
if (subtype == "pause") { if (subtype == "pause") {
key_events.push_back(builder.SetType(kPauseEventType)->Build()); key_events.push_back(builder.SetType(kPauseEventType)->Build());
} else { } else {
base::DictionaryValue dispatch_params; Status status = ConvertKeyActionToKeyEvent(
base::string16 raw_key; action, input_state, subtype == "keyDown", &key_events);
action->GetString("value", &raw_key);
base::char16 key = raw_key[0]; if (status.IsError())
// TODO: understand necessary_modifiers return status;
int necessary_modifiers = 0;
ui::KeyboardCode key_code = ui::VKEY_UNKNOWN;
std::string error_msg;
ConvertCharToKeyCode(key, &key_code, &necessary_modifiers,
&error_msg);
if (!error_msg.empty())
return Status(kUnknownError, error_msg);
if (subtype == "keyDown")
key_events.push_back(builder.SetType(kKeyDownEventType)
->SetText(base::UTF16ToUTF8(raw_key),
base::UTF16ToUTF8(raw_key))
->SetKeyCode(key_code)
->SetModifiers(0)
->Build());
else if (subtype == "keyUp")
key_events.push_back(builder.SetType(kKeyUpEventType)
->SetText(base::UTF16ToUTF8(raw_key),
base::UTF16ToUTF8(raw_key))
->SetKeyCode(key_code)
->SetModifiers(0)
->Build());
} }
} }
longest_key_list_size = longest_key_list_size =
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment