osr: win: Fix character events dropped for low byte 0x08/0x1B (fixes #4282) (#4283)

ui::KeyboardCode became a uint8_t on Windows in Chromium 148. A
character event carries its character in CefKeyEvent::windows_key_code,
so the ui::KeyEvent built from it keeps only the low byte in its key
code, and the one-argument NativeWebKeyboardEvent constructor copies
that key code into the web event. For characters whose low byte is 0x08
or 0x1B (e.g. U+041B Л, U+0408 Ј, U+011B ě) the web event then carries
VK_BACK/VK_ESCAPE, and WebFrameWidgetImpl::HandleCharEvent drops it
because WebKeyboardEvent::IsCharacterKey() is false for those codes.

Use the two-argument constructor for KEYEVENT_CHAR, as
RenderWidgetHostViewAura::InsertChar does for windowed browsers, so that
windows_key_code carries the full character.

libcef/browser/native/browser_platform_delegate_native_aura.cc

@@ -143,6 +143,16 @@ gfx::Point CefBrowserPlatformDelegateNativeAura::GetScreenPoint(
 input::NativeWebKeyboardEvent
 CefBrowserPlatformDelegateNativeAura::TranslateWebKeyEvent(
     const CefKeyEvent& key_event) const {
+  if (key_event.type == KEYEVENT_CHAR) {
+    // Pass the character rather than the key code: ui::KeyboardCode is a
+    // uint8_t on Windows, and a character whose low byte is 0x08 or 0x1B is
+    // then dropped by Blink as VK_BACK or VK_ESCAPE (see issue #4282).
+    // RenderWidgetHostViewAura::InsertChar does the same for windowed
+    // browsers.
+    return input::NativeWebKeyboardEvent(
+        TranslateUiKeyEvent(key_event),
+        static_cast<char16_t>(key_event.windows_key_code));
+  }
   return input::NativeWebKeyboardEvent(TranslateUiKeyEvent(key_event));
 }
 

tests/ceftests/os_rendering_unittest.cc

@@ -58,6 +58,22 @@ const int kExpectedSelectRectWidthVariance = 0;
 // word to be written into edit box
 const char kKeyTestWord[] = "done";
 
+// Characters whose low byte is VK_ESCAPE (0x1B) or VK_BACK (0x08). Blink does
+// not treat those key codes as character keys, so a character event that loses
+// its high byte on the way is dropped rather than inserted (see issue #4282).
+//   U+0410  control: low byte 0x10, shows input works when the rest is missing
+//   U+041B  Cyrillic, 0x1B
+//   U+0408  Cyrillic, 0x08
+//   U+011B  Latin Extended-A, 0x1B
+//   U+0E08  Thai, 0x08; Thai layouts deliver it as WM_CHAR without an IME
+//   U+FF1B  high byte 0xFF, which catches a sign extension as well as a drop
+const char16_t kCharTestWord[] = u"\u0410\u041B\u0408\u011B\u0E08\uFF1B";
+
+// What the page reports in OSR_TEST_CHAR_EVENTS: each keypress on the edit box
+// as "charCode:keyCode:key", then the edit box value.
+const char kKeypressPrefix[] = "osrkeypress:";
+const char kValuePrefix[] = "osrvalue:";
+
 constexpr uint32_t kAllTouchHandleFlags =
     (CEF_THS_FLAG_ENABLED | CEF_THS_FLAG_ORIENTATION | CEF_THS_FLAG_ORIGIN |
      CEF_THS_FLAG_ALPHA);
@@ -133,6 +149,9 @@ enum OSRTestType {
   OSR_TEST_INVALIDATE,
   // write into editbox LI08, click to navigate on LI09
   OSR_TEST_KEY_EVENTS,
+  // write non-ASCII characters into editbox LI08 with character events only,
+  // click on LI09 to report the keypresses and the value
+  OSR_TEST_CHAR_EVENTS,
   // mouse over LI10 will show a tooltip
   OSR_TEST_TOOLTIP,
   // mouse wheel will trigger a scroll event
@@ -388,6 +407,13 @@ class OSRTestHandler : public RoutingTestHandler,
                            browser->GetHost(), mouse_event, false),
             200);
       } break;
+      case OSR_TEST_CHAR_EVENTS:
+        // Have the page report keypresses and the value instead of
+        // navigating. Sent before the focus on the same channel, so it is in
+        // place before any input.
+        browser->GetMainFrame()->ExecuteJavaScript(
+            "window.reportInput = true;", browser->GetMainFrame()->GetURL(), 0);
+        [[fallthrough]];
       case OSR_TEST_KEY_EVENTS:
       case OSR_TEST_IME_COMMIT_TEXT:
       case OSR_TEST_IME_FINISH_COMPOSITION:
@@ -560,6 +586,19 @@ class OSRTestHandler : public RoutingTestHandler,
               100);
         }
         break;
+      case OSR_TEST_CHAR_EVENTS:
+        if (messageStr == "osrfocuseditbox") {
+          // Wait a bit after the focus change before continuing.
+          CefPostDelayedTask(
+              TID_UI, base::BindOnce(&OSRTestHandler::SendCharEvents, this),
+              100);
+        } else if (messageStr.rfind(kKeypressPrefix, 0) == 0) {
+          char_keypresses_.push_back(
+              messageStr.substr(sizeof(kKeypressPrefix) - 1));
+        } else if (messageStr.rfind(kValuePrefix, 0) == 0) {
+          FinishCharEvents(messageStr.substr(sizeof(kValuePrefix) - 1));
+        }
+        break;
       case OSR_TEST_IME_COMMIT_TEXT:
         if (messageStr == "osrfocuseditbox") {
           // Wait a bit after the focus change before continuing.
@@ -1669,6 +1708,56 @@ class OSRTestHandler : public RoutingTestHandler,
     ClickButtonToNavigate(browser);
   }
 
+  void SendCharEvents() {
+    auto browser = GetBrowser();
+
+    // Character events only. The key down and key up around them take a
+    // different path, which OSR_TEST_KEY_EVENTS already covers.
+    for (const char16_t* c = kCharTestWord; *c; ++c) {
+      CefKeyEvent event;
+      event.type = KEYEVENT_CHAR;
+#if defined(OS_WIN)
+      // Windows clients pass on the WM_CHAR wParam, which is the character.
+      event.windows_key_code = *c;
+#endif
+      event.character = event.unmodified_character = *c;
+      browser->GetHost()->SendKeyEvent(event);
+    }
+
+    // Click with the mouse rather than from script. Mouse and keyboard events
+    // share one queue to the renderer, so the click cannot overtake the
+    // characters; script travels separately and can.
+    CefMouseEvent mouse_event;
+    const CefRect& button = GetElementBounds("btnnavigate");
+    mouse_event.x = MiddleX(button);
+    mouse_event.y = MiddleY(button);
+    SendMouseClickEvent(browser, mouse_event);
+  }
+
+  void FinishCharEvents(const std::string& value) {
+    const std::u16string word(kCharTestWord);
+    EXPECT_EQ(CefString(word).ToString(), value);
+
+    // A keypress for each character, carrying the whole code point. keyCode
+    // equals charCode for a keypress.
+    EXPECT_EQ(word.size(), char_keypresses_.size());
+    for (size_t i = 0; i < word.size() && i < char_keypresses_.size(); ++i) {
+      const std::string code = std::to_string(word[i]);
+      std::string expected = code + ":" + code + ":";
+      std::string actual = char_keypresses_[i];
+#if defined(OS_WIN)
+      // Only Windows builds the key from the character itself; Linux builds
+      // it from the key code, which a character event does not need to carry.
+      expected += CefString(word.substr(i, 1)).ToString();
+#else
+      actual = actual.substr(0, expected.size());
+#endif
+      EXPECT_EQ(expected, actual) << "character " << i;
+    }
+
+    DestroySucceededTestSoon();
+  }
+
   void SendIMECommitText() {
     auto browser = GetBrowser();
 
@@ -2025,6 +2114,9 @@ class OSRTestHandler : public RoutingTestHandler,
   typedef std::map<std::string, CefRect> ElementBoundsMap;
   ElementBoundsMap element_bounds_;
 
+  // OSR_TEST_CHAR_EVENTS: the page's keypress reports, in order.
+  std::vector<std::string> char_keypresses_;
+
   IMPLEMENT_REFCOUNTING(OSRTestHandler);
 };
 
@@ -2188,6 +2280,10 @@ OSR_TEST(Invalidate, OSR_TEST_INVALIDATE, 1.0f)
 OSR_TEST(Invalidate2x, OSR_TEST_INVALIDATE, 2.0f)
 OSR_TEST(KeyEvents, OSR_TEST_KEY_EVENTS, 1.0f)
 OSR_TEST(KeyEvents2x, OSR_TEST_KEY_EVENTS, 2.0f)
+#if defined(OS_WIN) || defined(OS_LINUX)
+// macOS builds character events from a synthetic NSEvent instead.
+OSR_TEST(CharEvents, OSR_TEST_CHAR_EVENTS, 1.0f)
+#endif
 OSR_TEST(Tooltip, OSR_TEST_TOOLTIP, 1.0f)
 OSR_TEST(Tooltip2x, OSR_TEST_TOOLTIP, 2.0f)
 OSR_TEST(Scrolling, OSR_TEST_SCROLLING, 1.0f)

tests/shared/resources/osr_test.html

@@ -38,7 +38,15 @@
   function getElement(id) { return document.getElementById(id); }
   function makeH1Red() { getElement('LI00').style.color='red'; }
   function makeH1Black() { getElement('LI00').style.color='black'; }
-  function navigate() { location.href='?k='+getElement('editbox').value; }
+  function navigate() {
+    // OSR_TEST_CHAR_EVENTS reports the value on the same channel as the
+    // keypresses instead of navigating, so that they arrive in order.
+    if (window.reportInput && window.testQuery) {
+      window.testQuery({request: "osrvalue:" + getElement('editbox').value});
+      return;
+    }
+    location.href='?k='+getElement('editbox').value;
+  }
   function load() {
     var elems = [];
     var param = { type: 'ElementBounds', elems: elems };
@@ -105,6 +113,13 @@
       window.testQuery({request: "osrfocus" + ev.target.id});
   }
 
+  function onKeypressTest(ev) {
+    if (window.reportInput && window.testQuery) {
+      window.testQuery({request: "osrkeypress:" + ev.charCode + ":" +
+                                 ev.keyCode + ":" + ev.key});
+    }
+  }
+
   function allowDrop(ev) {
     ev.preventDefault();
   }
@@ -174,7 +189,7 @@
       </li>
   <li id='LI07'>Invalidate should trigger OnPaint once</li>
   <li id='LI08'>Click and write here with SendKeyEvent to trigger repaints:
-      <input id='editbox' type='text' value='' size="5" onfocus="onFocusTest(event)"></li>
+      <input id='editbox' type='text' value='' size="5" onfocus="onFocusTest(event)" onkeypress="onKeypressTest(event)"></li>
   <li id='LI09'>Click here with SendMouseClickEvent to navigate:
       <input id='btnnavigate' type='button' onclick='navigate()'
       value='Click here to navigate' /></li>