Revert "WebDriver supports non-BMP characters in SendKeys"

This reverts commit bb10ede. Reason for revert: Compound events must be a part of this patch, otherwise it breaks some functionalities. Original change's description: > WebDriver supports non-BMP characters in SendKeys > > We still don't do that in accordance with the standard. > Instead of splitting string on extended grapheme clusters > we just send all non-typeable characters in one batch > via Input.insertText method to Chrome. > > Bug: 2269 > Change-Id: Id486e527a3198f0e114b481c4a9a742b6b56ae9a > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3256792 > Commit-Queue: Vladimir Nechaev <nechaev@chromium.org> > Reviewed-by: Mathias Bynens <mathias@chromium.org> > Cr-Commit-Position: refs/heads/main@{#939071} (cherry picked from commit 0d2be63) (cherry picked from commit cc6cb4d) Bug: 1295243 Change-Id: Ie411a2bec946d89f6190b560b1c30aa121d0b404 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3450675 Reviewed-by: John Chen <johnchen@chromium.org> Commit-Queue: Vladimir Nechaev <nechaev@chromium.org> Cr-Commit-Position: refs/branch-heads/4758@{#1122} Cr-Branched-From: 4a2cf4b-refs/heads/main@{#950365}
chromium · Feb 9, 2022 · 97342fd · 97342fd
1 parent 068ee7a
commit 97342fd
Show file tree

Hide file tree

Showing 9 changed files with 43 additions and 153 deletions.
diff --git a/chrome/test/chromedriver/chrome/stub_web_view.cc b/chrome/test/chromedriver/chrome/stub_web_view.cc
@@ -152,10 +152,6 @@ Status StubWebView::DispatchKeyEvents(const std::vector<KeyEvent>& events,
   return Status(kOk);
 }
 
-Status StubWebView::InsertText(const std::string& text,
-                               bool async_dispatch_events) {
-  return Status(kOk);
-}
 
 Status StubWebView::GetCookies(std::unique_ptr<base::ListValue>* cookies,
                                const std::string& current_page_url) {

diff --git a/chrome/test/chromedriver/chrome/stub_web_view.h b/chrome/test/chromedriver/chrome/stub_web_view.h
@@ -75,8 +75,6 @@ class StubWebView : public WebView {
       bool async_dispatch_events = false) override;
   Status DispatchKeyEvents(const std::vector<KeyEvent>& events,
                            bool async_dispatch_events = false) override;
-  Status InsertText(const std::string& text,
-                    bool async_dispatch_events = false) override;
   Status GetCookies(std::unique_ptr<base::ListValue>* cookies,
                     const std::string& current_page_url) override;
   Status DeleteCookie(const std::string& name,

diff --git a/chrome/test/chromedriver/chrome/web_view.h b/chrome/test/chromedriver/chrome/web_view.h
@@ -165,16 +165,10 @@ class WebView {
   virtual Status DispatchTouchEventWithMultiPoints(
       const std::vector<TouchEvent>& events,
       bool async_dispatch_events) = 0;
-
   // Dispatch a sequence of key events.
   virtual Status DispatchKeyEvents(const std::vector<KeyEvent>& events,
                                    bool async_dispatch_events) = 0;
 
-  // Emulate inserting text that doesn't come from a key press,
-  // for example an emoji keyboard or an IME.
-  virtual Status InsertText(const std::string& text,
-                            bool async_dispatch_events) = 0;
-
   // Return all the cookies visible to the current page.
   virtual Status GetCookies(std::unique_ptr<base::ListValue>* cookies,
                             const std::string& current_page_url) = 0;

diff --git a/chrome/test/chromedriver/chrome/web_view_impl.cc b/chrome/test/chromedriver/chrome/web_view_impl.cc
@@ -788,19 +788,6 @@ Status WebViewImpl::DispatchKeyEvents(const std::vector<KeyEvent>& events,
   return status;
 }
 
-Status WebViewImpl::InsertText(const std::string& text,
-                               bool async_dispatch_events) {
-  Status status(kOk);
-  base::DictionaryValue params;
-  params.SetString("text", text);
-  if (async_dispatch_events) {
-    status = client_->SendCommandAndIgnoreResponse("Input.insertText", params);
-  } else {
-    status = client_->SendCommand("Input.insertText", params);
-  }
-  return status;
-}
-
 Status WebViewImpl::GetCookies(std::unique_ptr<base::ListValue>* cookies,
                                const std::string& current_page_url) {
   base::DictionaryValue params;

diff --git a/chrome/test/chromedriver/chrome/web_view_impl.h b/chrome/test/chromedriver/chrome/web_view_impl.h
@@ -123,8 +123,6 @@ class WebViewImpl : public WebView {
       bool async_dispatch_events = false) override;
   Status DispatchKeyEvents(const std::vector<KeyEvent>& events,
                            bool async_dispatch_events = false) override;
-  Status InsertText(const std::string& text,
-                    bool async_dispatch_events = false) override;
   Status GetCookies(std::unique_ptr<base::ListValue>* cookies,
                     const std::string& current_page_url) override;
   Status DeleteCookie(const std::string& name,

diff --git a/chrome/test/chromedriver/key_converter.cc b/chrome/test/chromedriver/key_converter.cc
@@ -11,10 +11,10 @@
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversion_utils.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/third_party/icu/icu_utf.h"
 #include "chrome/test/chromedriver/chrome/status.h"
 #include "chrome/test/chromedriver/chrome/ui_events.h"
 #include "chrome/test/chromedriver/keycode_text_conversion.h"
+
 namespace {
 
 struct ModifierMaskAndKeyCode {
@@ -320,12 +320,11 @@ const char* const kNormalisedKeyValue[] = {
 // * Replaced "OSLeft" and "OSRight" with "MetaLeft" and "MetaRight", to be
 //   compatible with Chrome.
 //   TODO(johnchen@chromium.org): Find a better way to handle this.
-const struct CodeForKey {
+const struct {
   char16_t key;
   char16_t alternate_key;
   std::string code;
 } kCodeForKey[] = {
-    // clang-format off
     {'`',    '~',    "Backquote"},
     {'\\',   '|',    "Backslash"},
     {0xE003, 0,      "Backspace"},
@@ -427,7 +426,6 @@ const struct CodeForKey {
     {0xE007, 0,      "NumpadEnter"},
     {0xE024, 0,      "NumpadMultiply"},
     {0xE027, 0,      "NumpadSubtract"},
-    // clang-format on
 };
 
 // The "key location for key" table from W3C spec
@@ -449,23 +447,6 @@ int GetKeyLocation(uint32_t code_point) {
 
 }  // namespace
 
-bool IsTypeableKey(char16_t key, std::string* code) {
-  if (!key)
-    return false;
-  auto* it = std::find_if(std::begin(kCodeForKey), std::end(kCodeForKey),
-                          [key](const CodeForKey& p) {
-                            return p.key == key || p.alternate_key == key;
-                          });
-
-  if (it != std::end(kCodeForKey)) {
-    if (code != nullptr) {
-      *code = it->code;
-    }
-    return true;
-  }
-  return false;
-}
-
 Status ConvertKeysToKeyEvents(const std::u16string& client_keys,
                               bool release_modifiers,
                               int* modifiers,
@@ -538,15 +519,18 @@ Status ConvertKeysToKeyEvents(const std::u16string& client_keys,
     int all_modifiers = sticky_modifiers;
 
     // Get the key code, text, and modifiers for the given key.
+    bool should_skip = false;
     bool is_special_key = KeyCodeFromSpecialWebDriverKey(key, &key_code);
     std::string error_msg;
-    if (is_special_key) {
+    if (is_special_key ||
+        KeyCodeFromShorthandKey(key, &key_code, &should_skip)) {
+      if (should_skip)
+        continue;
       if (key_code == ui::VKEY_UNKNOWN) {
-        return Status(
-            kUnknownError,
-            base::StringPrintf(
-                "unknown WebDriver key(%d) at string index (%" PRIuS ")",
-                static_cast<int>(key), i));
+        return Status(kUnknownError, base::StringPrintf(
+            "unknown WebDriver key(%d) at string index (%" PRIuS ")",
+            static_cast<int>(key),
+            i));
       }
       if (key_code == ui::VKEY_RETURN) {
         // For some reason Chrome expects a carriage return for the return key.
@@ -561,11 +545,12 @@ Status ConvertKeysToKeyEvents(const std::u16string& client_keys,
         int webdriver_modifiers = 0;
         if (key_code >= ui::VKEY_NUMPAD0 && key_code <= ui::VKEY_NUMPAD9)
           webdriver_modifiers = kNumLockKeyModifierMask;
-        if (!ConvertKeyCodeToText(key_code, webdriver_modifiers,
-                                  &unmodified_text, &error_msg))
+        if (!ConvertKeyCodeToText(
+            key_code, webdriver_modifiers, &unmodified_text, &error_msg))
           return Status(kUnknownError, error_msg);
-        if (!ConvertKeyCodeToText(key_code, all_modifiers | webdriver_modifiers,
-                                  &modified_text, &error_msg))
+        if (!ConvertKeyCodeToText(
+            key_code, all_modifiers | webdriver_modifiers, &modified_text,
+            &error_msg))
           return Status(kUnknownError, error_msg);
       }
     } else {
@@ -577,18 +562,18 @@ Status ConvertKeysToKeyEvents(const std::u16string& client_keys,
       if (key_code != ui::VKEY_UNKNOWN) {
         if (!ConvertKeyCodeToText(key_code, 0, &unmodified_text, &error_msg))
           return Status(kUnknownError, error_msg);
-        if (!ConvertKeyCodeToText(key_code, all_modifiers, &modified_text,
-                                  &error_msg))
+        if (!ConvertKeyCodeToText(
+            key_code, all_modifiers, &modified_text, &error_msg))
           return Status(kUnknownError, error_msg);
         if (unmodified_text.empty() || modified_text.empty()) {
           // To prevent char event for special cases like CTRL + x (cut).
           unmodified_text.clear();
           modified_text.clear();
         }
       } else {
-        // Non-typeable character must not be sent as KeyEvent
-        return Status(kUnknownError,
-                      "Cannot construct KeyEvent from non-typeable key");
+        // Do a best effort and use the raw key we were given.
+        unmodified_text = base::UTF16ToUTF8(keys.substr(i, 1));
+        modified_text = base::UTF16ToUTF8(keys.substr(i, 1));
       }
     }
 

diff --git a/chrome/test/chromedriver/key_converter.h b/chrome/test/chromedriver/key_converter.h
@@ -14,13 +14,6 @@
 struct KeyEvent;
 class Status;
 
-// Check if the character is typeable in accordance with WebDriver definition.
-// Quote: "An extended grapheme cluster is typeable
-//          if it consists of a single unicode code point
-//          and the code is not undefined."
-// See also: https://www.w3.org/TR/webdriver/#dfn-code
-bool IsTypeableKey(char16_t key, std::string* code = nullptr);
-
 // Converts keys into appropriate |KeyEvent|s. This will do a best effort
 // conversion. However, if the input is invalid it will return a status with
 // an error message. If |release_modifiers| is true, all modifiers would be

diff --git a/chrome/test/chromedriver/test/run_py_tests.py b/chrome/test/chromedriver/test/run_py_tests.py
@@ -1835,15 +1835,6 @@ def testSendKeysToNonTypeableInputElement(self):
     value = elem.GetProperty('value')
     self.assertEquals(input_value, value)
 
-  def testSendKeysNonBmp(self):
-    self._driver.Load(ChromeDriverTest.GetHttpUrlForFile(
-        '/chromedriver/two_inputs.html'))
-    elem = self._driver.FindElement('css selector', '#first')
-    expected = u'T\U0001f4a9XL\u0436'.encode('utf-8')
-    elem.SendKeys(expected)
-    actual = elem.GetProperty('value').encode('utf-8')
-    self.assertEquals(expected, actual)
-
   def testGetElementAttribute(self):
     self._driver.Load(self.GetHttpUrlForFile(
         '/chromedriver/attribute_colon_test.html'))
@@ -3955,25 +3946,6 @@ def testSendingTabKeyMovesToNextInputElement(self):
     self.assertEquals('prickly pete', self._driver.ExecuteScript(
         'return arguments[0].value;', second))
 
-  def testSendingTabKeyMovesToNextInputElementEscapedTab(self):
-    """This behavior is not specified by the WebDriver standard
-    but it is supported by us de facto.
-    According to this table https://www.w3.org/TR/webdriver/#keyboard-actions
-    the code point 0x09 (HT) must be sent to the browser via a CompositionEvent.
-    We however historically have been sending it as KeyEvent
-    with code = ui::VKEY_TAB which leads to focus change.
-    For the sake of contrast GeckoDriver and Firefox do not show this behavior.
-    If in the future it turns out that our current behavior is undesirable
-    we can remove this test.
-    """
-    self._driver.Load(self.GetHttpUrlForFile('/chromedriver/two_inputs.html'))
-    first = self._driver.FindElement('css selector', '#first')
-    second = self._driver.FindElement('css selector', '#second')
-    first.Click()
-    self._driver.SendKeys('snoopy\tprickly pete')
-    self.assertEquals('snoopy', first.GetProperty('value'))
-    self.assertEquals('prickly pete', second.GetProperty('value'))
-
   def testMobileEmulationDisabledByDefault(self):
     self.assertFalse(self._driver.capabilities['mobileEmulationEnabled'])
 

diff --git a/chrome/test/chromedriver/util.cc b/chrome/test/chromedriver/util.cc
@@ -7,7 +7,6 @@
 #include <stddef.h>
 #include <stdint.h>
 
-#include <algorithm>
 #include <string>
 
 #include "base/base64.h"
@@ -48,7 +47,18 @@ Status FlattenStringArray(const base::ListValue* src, std::u16string* dest) {
   for (const base::Value& i : src->GetList()) {
     if (!i.is_string())
       return Status(kUnknownError, "keys should be a string");
+
     std::u16string keys_list_part = base::UTF8ToUTF16(i.GetString());
+
+    for (char16_t ch : keys_list_part) {
+      if (CBU16_IS_SURROGATE(ch)) {
+        return Status(
+            kUnknownError,
+            base::StringPrintf("%s only supports characters in the BMP",
+                              kChromeDriverProductShortName));
+      }
+    }
+
     keys.append(keys_list_part);
   }
   *dest = keys;
@@ -57,67 +67,24 @@ Status FlattenStringArray(const base::ListValue* src, std::u16string* dest) {
 
 }  // namespace
 
-Status SendKeysOnWindow(WebView* web_view,
-                        const base::ListValue* key_list,
-                        bool release_modifiers,
-                        int* sticky_modifiers) {
+Status SendKeysOnWindow(
+    WebView* web_view,
+    const base::ListValue* key_list,
+    bool release_modifiers,
+    int* sticky_modifiers) {
   std::u16string keys;
   Status status = FlattenStringArray(key_list, &keys);
   if (status.IsError())
     return status;
-
-  // Replace shorthand keys with corresponding WebDriver special keys.
-  std::transform(keys.begin(), keys.end(), keys.begin(), [](char16_t ch) {
-    switch (ch) {
-      case u'\n':
-        return u'\uE006';
-      case u'\t':
-        return u'\uE004';
-      case u'\b':
-        return u'\uE003';
-      case u' ':
-        return u'\uE00D';
-      default:
-        return ch;
-    }
-  });
-
-  // \r goes together with \n on Windows.
-  // The last one is enough for identifying a line break.
-  keys.erase(std::remove(keys.begin(), keys.end(), u'\r'), keys.end());
-
+  std::vector<KeyEvent> events;
   int sticky_modifiers_tmp = *sticky_modifiers;
-
-  auto it2 = keys.begin();
-  for (auto it1 = keys.begin(); it1 != keys.end() && status.IsOk(); it1 = it2) {
-    bool is_typeable = IsTypeableKey(*it1);
-    it2 = std::find_if(next(it1), keys.end(), [is_typeable](char16_t ch) {
-      return is_typeable != IsTypeableKey(ch);
-    });
-    std::u16string block(it1, it2);
-
-    if (is_typeable) {
-      std::vector<KeyEvent> events;
-      status = ConvertKeysToKeyEvents(block, release_modifiers,
-                                      &sticky_modifiers_tmp, &events);
-      if (status.IsOk()) {
-        status = web_view->DispatchKeyEvents(events, false);
-      }
-    } else {
-      std::string block_utf8;
-      if (base::UTF16ToUTF8(block.c_str(), block.size(), &block_utf8)) {
-        status = web_view->InsertText(block_utf8, false);
-      } else {
-        // Malformed input, e.g. high surrogate not followed by a low surrogate.
-        status = Status(kUnknownError,
-                        "UTF16 to UTF8 conversion failed for the text");
-      }
-    }
-  }
-
+  status = ConvertKeysToKeyEvents(
+      keys, release_modifiers, &sticky_modifiers_tmp, &events);
+  if (status.IsError())
+    return status;
+  status = web_view->DispatchKeyEvents(events, false);
   if (status.IsOk())
     *sticky_modifiers = sticky_modifiers_tmp;
-
   return status;
 }