[chromedriver] Fix bugs in SendKeys.

chrome/test/chromedriver/key_converter.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/test/chromedriver/key_converter.h"
 
 #include "base/format_macros.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/test/chromedriver/chrome/status.h"
@@ skipping 79 matching lines @@
     ui::VKEY_F10,
     ui::VKEY_F11,
     ui::VKEY_F12};
 
 const char16 kWebDriverNullKey = 0xE000U;
 const char16 kWebDriverShiftKey = 0xE008U;
 const char16 kWebDriverControlKey = 0xE009U;
 const char16 kWebDriverAltKey = 0xE00AU;
 const char16 kWebDriverCommandKey = 0xE03DU;
 
+// Returns whether the given key code has a corresponding printable char.
+// Notice: The given key code should be a special WebDriver key code.
+bool IsPrintable(ui::KeyboardCode key_code) {

[kkania, 2013/04/15 21:35:35]

How about rename this IsSpecialKeyPrintable?

[chrisgao (Use stgao instead), 2013/04/15 23:57:01]

Done.

+  return key_code == ui::VKEY_TAB || key_code == ui::VKEY_SPACE ||
+      key_code == ui::VKEY_OEM_1 || key_code == ui::VKEY_OEM_PLUS ||
+      key_code == ui::VKEY_OEM_COMMA ||
+      (key_code >= ui::VKEY_NUMPAD0 && key_code <= ui::VKEY_DIVIDE);
+}
+
 // Returns whether the given key is a WebDriver key modifier.
 bool IsModifierKey(char16 key) {
   switch (key) {
     case kWebDriverShiftKey:
     case kWebDriverControlKey:
     case kWebDriverAltKey:
     case kWebDriverCommandKey:
       return true;
     default:
       return false;
@@ skipping 125 matching lines @@
         key_events.push_back(CreateKeyUpEvent(key_code, sticky_modifiers));
       continue;
     }
 
     ui::KeyboardCode key_code = ui::VKEY_UNKNOWN;
     std::string unmodified_text, modified_text;
     int all_modifiers = sticky_modifiers;
 
     // Get the key code, text, and modifiers for the given key.
     bool should_skip = false;
-    if (KeyCodeFromSpecialWebDriverKey(key, &key_code) ||
+    bool is_special_key = KeyCodeFromSpecialWebDriverKey(key, &key_code);
+    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));
       }
       if (key_code == ui::VKEY_RETURN) {
         // For some reason Chrome expects a carriage return for the return key.
         modified_text = unmodified_text = "\r";
+      } else if (is_special_key && !IsPrintable(key_code)) {
+        modified_text = unmodified_text = "";

[kkania, 2013/04/15 21:35:35]

Explain why this is needed, perhaps give an example.  We probably want to remove
this once we route the event through the browser instead of the renderer, since
this code isn't actually correct...

[chrisgao (Use stgao instead), 2013/04/15 23:57:01]

Done.

       } else {
         // WebDriver assumes a numpad key should translate to the number,
         // which requires NumLock to be on with some platforms. This isn't
         // formally in the spec, but is expected by their tests.
         int webdriver_modifiers = 0;
         if (key_code >= ui::VKEY_NUMPAD0 && key_code <= ui::VKEY_NUMPAD9)
           webdriver_modifiers = kNumLockKeyModifierMask;
         unmodified_text = ConvertKeyCodeToText(key_code, webdriver_modifiers);
         modified_text = ConvertKeyCodeToText(
             key_code,
             all_modifiers | webdriver_modifiers);
       }
     } else {
       int necessary_modifiers = 0;
       ConvertCharToKeyCode(key, &key_code, &necessary_modifiers);
       all_modifiers |= necessary_modifiers;
       if (key_code != ui::VKEY_UNKNOWN) {
         unmodified_text = ConvertKeyCodeToText(key_code, 0);
         modified_text = ConvertKeyCodeToText(key_code, all_modifiers);
-      }
-
-      if (unmodified_text.empty() || modified_text.empty()) {
+        if (unmodified_text.empty() || modified_text.empty()) {
+          unmodified_text.clear();

[kkania, 2013/04/15 21:35:35]

explain why this is needed, perhaps give an example

[chrisgao (Use stgao instead), 2013/04/15 23:57:01]

Done.

+          modified_text.clear();
+        }
+      } else {
         // Do a best effort and use the raw key we were given.
-        if (unmodified_text.empty())
-          unmodified_text = UTF16ToUTF8(keys.substr(i, 1));
-        if (modified_text.empty())
-          modified_text = UTF16ToUTF8(keys.substr(i, 1));
+        unmodified_text = UTF16ToUTF8(keys.substr(i, 1));
+        modified_text = UTF16ToUTF8(keys.substr(i, 1));
       }
     }
 
     // Create the key events.
     bool necessary_modifiers[3];
     for (int i = 0; i < 3; ++i) {
       necessary_modifiers[i] =
           all_modifiers & kModifiers[i].mask &&
           !(sticky_modifiers & kModifiers[i].mask);
       if (necessary_modifiers[i]) {
@@ skipping 13 matching lines @@
       if (necessary_modifiers[i]) {
         key_events.push_back(
             CreateKeyUpEvent(kModifiers[i].key_code, sticky_modifiers));
       }
     }
   }
   client_key_events->swap(key_events);
   *modifiers = sticky_modifiers;
   return Status(kOk);
 }