Bug 1529467 - Make TextInputHandler::HandleCommand() dispatch eKeyDown event r=m_kato a=lizzard
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 25 Feb 2019 04:13:20 +0000
changeset 516163 af029ffaf0ff1fa524d3b0dac01e3e40ecd106b2
parent 516162 4d2851ed4896e486b4b00ffaea383d988f81713c
child 516164 cf5c156b86ceeda2be61619c3cb3aa26503876e5
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato, lizzard
Bug 1529467 - Make TextInputHandler::HandleCommand() dispatch eKeyDown event r=m_kato a=lizzard Currently, when IME sends a command with committing composition, TextInputHandler::HandleCommand() dispatches a *fake* `keypress` event for making default event handler (e.g., focused editor) handle the event. However, we stopped dispatching `keypress` events for non-printable keys. Therefore, web apps cannot do that like us. On macOS, simple conversion IMEs like Korean 2-set IME, behave differently if active application is changed. E.g., on Safari, some of them may never use composition string, but not so on Chrome and Firefox. So, this is what the case we need to emulate Safari's behavior. Dispatching a fake `keydown` event for this purpose does **not** conform to UI Events because `keydown` events should notify web apps of **physical** key state changes. However, Chrome dispatches fake `keydown` events intentionally. Therefore, we should follow this hacky behavior for user experience. Differential Revision: https://phabricator.services.mozilla.com/D20644
--- a/widget/cocoa/TextInputHandler.mm
+++ b/widget/cocoa/TextInputHandler.mm
@@ -2598,29 +2598,31 @@ bool TextInputHandler::HandleCommand(Com
   //       the key press in editor.
   // If it's handling actual key event and hasn't cause any composition
   // events nor other key events, we should expose actual modifier state.
   // Otherwise, we should adjust Control, Option and Command state since
   // editor may behave differently if some of them are active.
   bool dispatchFakeKeyPress = !(currentKeyEvent && currentKeyEvent->IsProperKeyEvent(aCommand));
+  WidgetKeyboardEvent keydownEvent(true, eKeyDown, widget);
   WidgetKeyboardEvent keypressEvent(true, eKeyPress, widget);
   if (!dispatchFakeKeyPress) {
     // If we're acutally handling a key press, we should dispatch
     // the keypress event as-is.
+    currentKeyEvent->InitKeyEvent(this, keydownEvent, false);
     currentKeyEvent->InitKeyEvent(this, keypressEvent, false);
   } else {
     // Otherwise, we should dispatch "fake" keypress event.
     // However, for making it possible to compute edit commands, we need to
     // set current native key event to the fake keyboard event even if it's
     // not same as what we expect since the native keyboard event caused
     // this command.
     NSEvent* keyEvent = currentKeyEvent ? currentKeyEvent->mKeyEvent : nullptr;
-    keypressEvent.mNativeKeyEvent = keyEvent;
+    keydownEvent.mNativeKeyEvent = keypressEvent.mNativeKeyEvent = keyEvent;
                          "Without native key event, NativeKeyBindings cannot compute aCommand");
     switch (aCommand) {
       case CommandInsertLineBreak:
       case CommandInsertParagraph: {
         // Although, Shift+Enter and Enter are work differently in HTML
         // editor, we should expose actual Shift state if it's caused by
         // Enter key for compatibility with Chromium.  Chromium breaks
@@ -2785,19 +2787,46 @@ bool TextInputHandler::HandleCommand(Com
         if (aCommand == CommandComplete) {
           keypressEvent.mModifiers |= MODIFIER_ALT;
         return false;
-  }
+    nsCocoaUtils::InitInputEvent(keydownEvent, keyEvent);
+    keydownEvent.mKeyCode = keypressEvent.mKeyCode;
+    keydownEvent.mKeyNameIndex = keypressEvent.mKeyNameIndex;
+    keydownEvent.mModifiers = keypressEvent.mModifiers;
+  }
+  // We've stopped dispatching "keypress" events of non-printable keys on
+  // the web.  Therefore, we need to dispatch eKeyDown event here for web
+  // apps.  This is non-standard behavior if we've already dispatched a
+  // "keydown" event.  However, Chrome also dispatches such fake "keydown"
+  // (and "keypress") event for making same behavior as Safari.
   nsEventStatus status = nsEventStatus_eIgnore;
+  if (mDispatcher->DispatchKeyboardEvent(eKeyDown, keydownEvent, status, nullptr)) {
+    bool keydownHandled = status == nsEventStatus_eConsumeNoDefault;
+    if (currentKeyEvent) {
+      currentKeyEvent->mKeyDownDispatched = true;
+      currentKeyEvent->mKeyDownHandled |= keydownHandled;
+    }
+    if (keydownHandled) {
+      // Don't dispatch eKeyPress event if preceding eKeyDown event is
+      // consumed for conforming to UI Events.
+      // XXX Perhaps, we should ignore previous eKeyDown event result
+      //     even if we've already dispatched because it may notify web apps
+      //     of different key information, e.g., it's handled by IME, but
+      //     web apps want to handle only this key.
+      return true;
+    }
+  }
   bool keyPressDispatched =
       mDispatcher->MaybeDispatchKeypressEvents(keypressEvent, status, currentKeyEvent);
   bool keyPressHandled = (status == nsEventStatus_eConsumeNoDefault);
   // NOTE: mWidget might have become null here.
   if (keyPressDispatched) {
     // Record the keypress event state only when it dispatched actual Enter