Bug 1358958 - Don't consume command when neither keydown nor keypress event was consumed. r=m_kato, a=gchang
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 26 Apr 2017 20:39:13 +0900
changeset 396185 bf1dbe42eca073481652dfe13e59b8e0782a2109
parent 396184 90aefe217dc2271ed7f32cbcfb9ff072386a44f7
child 396186 0f9b796efd49f85906a9bea4d439e2cd34c6afe2
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato, gchang
bugs1358958
milestone54.0
Bug 1358958 - Don't consume command when neither keydown nor keypress event was consumed. r=m_kato, a=gchang When typing Enter key when active keyboard layout is Korean IME and it has composition string, the composition string is committed and then, "insertNewline:" command is sent. However, TextInputHandler::DoCommandBySelector() consumes the command because the key event has already modified the composition string. This patch makes TextInputHandler::DoCommandBySelector() consume the command if it's not handling keydown or neither dispatched keydown event nor dispatched keypress event (if it does) is consumed. Therefore, insertNewline:sender of nsChildView will be called later, then, it causes inserting a line break with a set of composition events. MozReview-Commit-ID: Afr1FKZbUtL
widget/cocoa/TextInputHandler.h
widget/cocoa/TextInputHandler.mm
--- a/widget/cocoa/TextInputHandler.h
+++ b/widget/cocoa/TextInputHandler.h
@@ -588,16 +588,21 @@ protected:
              mCompositionDispatched;
     }
 
     bool CanDispatchKeyPressEvent() const
     {
       return !mKeyPressDispatched && !IsDefaultPrevented();
     }
 
+    bool CanHandleCommand() const
+    {
+      return !mKeyDownHandled && !mKeyPressHandled;
+    }
+
     void InitKeyEvent(TextInputHandlerBase* aHandler,
                       WidgetKeyboardEvent& aKeyEvent);
 
     /**
      * GetUnhandledString() returns characters of the event which have not been
      * handled with InsertText() yet. For example, if there is a composition
      * caused by a dead key press like '`' and it's committed by some key
      * combinations like |Cmd+v|, then, the |v|'s KeyDown event's |characters|
--- a/widget/cocoa/TextInputHandler.mm
+++ b/widget/cocoa/TextInputHandler.mm
@@ -2372,44 +2372,63 @@ TextInputHandler::DoCommandBySelector(co
      this, aSelector ? aSelector : "", TrueOrFalse(Destroyed()),
      currentKeyEvent ?
        TrueOrFalse(currentKeyEvent->mKeyDownHandled) : "N/A",
      currentKeyEvent ?
        TrueOrFalse(currentKeyEvent->mKeyPressHandled) : "N/A",
      currentKeyEvent ?
        TrueOrFalse(currentKeyEvent->mCausedOtherKeyEvents) : "N/A"));
 
-  if (currentKeyEvent && currentKeyEvent->CanDispatchKeyPressEvent()) {
+  // If the command isn't caused by key operation, the command should
+  // be handled in the super class of the caller.
+  if (!currentKeyEvent) {
+    return Destroyed();
+  }
+
+  // If the key operation causes this command, should dispatch a keypress
+  // event.
+  // XXX This must be worng.  Even if this command is caused by the key
+  //     operation, its our default action can be different from the
+  //     command.  So, in this case, we should dispatch a keypress event
+  //     which have the command and editor should handle it.
+  if (currentKeyEvent->CanDispatchKeyPressEvent()) {
     nsresult rv = mDispatcher->BeginNativeInputTransaction();
     if (NS_WARN_IF(NS_FAILED(rv))) {
         MOZ_LOG(gLog, LogLevel::Error,
           ("%p IMEInputHandler::DoCommandBySelector, "
            "FAILED, due to BeginNativeInputTransaction() failure "
            "at dispatching keypress", this));
-      return false;
+      return Destroyed();
     }
 
     WidgetKeyboardEvent keypressEvent(true, eKeyPress, widget);
     currentKeyEvent->InitKeyEvent(this, keypressEvent);
 
     nsEventStatus status = nsEventStatus_eIgnore;
     currentKeyEvent->mKeyPressDispatched =
       mDispatcher->MaybeDispatchKeypressEvents(keypressEvent, status,
                                                currentKeyEvent);
     currentKeyEvent->mKeyPressHandled =
       (status == nsEventStatus_eConsumeNoDefault);
     MOZ_LOG(gLog, LogLevel::Info,
       ("%p TextInputHandler::DoCommandBySelector, keypress event "
        "dispatched, Destroyed()=%s, keypressHandled=%s",
        this, TrueOrFalse(Destroyed()),
        TrueOrFalse(currentKeyEvent->mKeyPressHandled)));
+    // This command is now dispatched with keypress event.
+    // So, this shouldn't be handled by nobody anymore.
+    return true;
   }
 
-  return (!Destroyed() && currentKeyEvent &&
-          currentKeyEvent->IsDefaultPrevented());
+  // If the key operation didn't cause keypress event or caused keypress event
+  // but not prevented its default, we need to honor the command.  For example,
+  // Korean IME sends "insertNewline:" when committing existing composition
+  // with Enter key press.  In such case, the key operation has been consumed
+  // by the committing composition but we still need to handle the command.
+  return Destroyed() || !currentKeyEvent->CanHandleCommand();
 }
 
 
 #pragma mark -
 
 
 /******************************************************************************
  *