Bug 1280053 - TextInputHandler should initialize WidgetKeyboardEvent without already handled characters. r=m_kato, a=sylvestre
authorMasayuki Nakano <masayuki@d-toybox.com>
Sun, 19 Jun 2016 01:13:16 +0900
changeset 341729 fe554517e03254038f631fa624f9f0badf83ca15
parent 341728 74763893c0f373a9fa4a1c61cbf9d1b49cc9bb5f
child 341730 15cc9baf2b6f43859d4a34d7de14a7b31093cf76
push id1183
push userraliiev@mozilla.com
push dateMon, 05 Sep 2016 20:01:49 +0000
treeherdermozilla-release@3148731bed45 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato, sylvestre
bugs1280053
milestone49.0a2
Bug 1280053 - TextInputHandler should initialize WidgetKeyboardEvent without already handled characters. r=m_kato, a=sylvestre TextInputHandler may dispatch keypress events after InsertText() is called if there was composition and it's committed by "current" keydown event. In that case, [NSEvent characters] may have the committing string. For example, when Opt+e of US keyboard layout started composition, Cmd+v causes committing the "`" character and pasting the clipboard text. Then, the "v" key's keydown event's |characters| is "`v". So, after InsertText() is called with "`", TextInputHandler shouldn't dispatch keypress event for "`" again. I.e., the KeyboardEvent.key value should be "v" rather than "`v". For solving this issue, TextInputHandlerBase::AutoInsertStringClearer which is created at every InsertText() call should store the inserted string to TextInputHandlerBase::KeyEventState. However, for making the implemntation simpler, it should recode only when the inserting string is actually a part of [mKeyEvent characters]. Then, TextInputHandlerBase::KeyEventState can compute unhandled insert string at initializing WidgetKeyboardEvent. So, finally, TextInputHandlerBase::InitKeyEvent() should be called via TextInputHandlerBase::KeyEventState::InitKeyEvent(). This ensures that all key events which may cause InsertText() calls are always initialized with unhandled string. MozReview-Commit-ID: A9o8o9pV2XV
widget/cocoa/TextInputHandler.h
widget/cocoa/TextInputHandler.mm
--- a/widget/cocoa/TextInputHandler.h
+++ b/widget/cocoa/TextInputHandler.h
@@ -496,16 +496,19 @@ protected:
    */
   struct KeyEventState
   {
     // Handling native key event
     NSEvent* mKeyEvent;
     // String specified by InsertText().  This is not null only during a
     // call of InsertText().
     nsAString* mInsertString;
+    // String which are included in [mKeyEvent characters] and already handled
+    // by InsertText() call(s).
+    nsString mInsertedString;
     // Whether keydown event was consumed by web contents or chrome contents.
     bool mKeyDownHandled;
     // Whether keypress event was dispatched for mKeyEvent.
     bool mKeyPressDispatched;
     // Whether keypress event was consumed by web contents or chrome contents.
     bool mKeyPressHandled;
     // Whether the key event causes other key events via IME or something.
     bool mCausedOtherKeyEvents;
@@ -516,27 +519,17 @@ protected:
     }    
 
     explicit KeyEventState(NSEvent* aNativeKeyEvent) : mKeyEvent(nullptr)
     {
       Clear();
       Set(aNativeKeyEvent);
     }
 
-    KeyEventState(const KeyEventState &aOther) : mKeyEvent(nullptr)
-    {
-      Clear();
-      if (aOther.mKeyEvent) {
-        mKeyEvent = [aOther.mKeyEvent retain];
-      }
-      mKeyDownHandled = aOther.mKeyDownHandled;
-      mKeyPressDispatched = aOther.mKeyPressDispatched;
-      mKeyPressHandled = aOther.mKeyPressHandled;
-      mCausedOtherKeyEvents = aOther.mCausedOtherKeyEvents;
-    }
+    KeyEventState(const KeyEventState &aOther) = delete;
 
     ~KeyEventState()
     {
       Clear();
     }
 
     void Set(NSEvent* aNativeKeyEvent)
     {
@@ -547,31 +540,45 @@ protected:
 
     void Clear()
     {
       if (mKeyEvent) {
         [mKeyEvent release];
         mKeyEvent = nullptr;
       }
       mInsertString = nullptr;
+      mInsertedString.Truncate();
       mKeyDownHandled = false;
       mKeyPressDispatched = false;
       mKeyPressHandled = false;
       mCausedOtherKeyEvents = false;
     }
 
     bool IsDefaultPrevented() const
     {
       return mKeyDownHandled || mKeyPressHandled || mCausedOtherKeyEvents;
     }
 
     bool CanDispatchKeyPressEvent() const
     {
       return !mKeyPressDispatched && !IsDefaultPrevented();
     }
+
+    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|
+     * is |`v|.  Then, after |`| is committed with a call of InsertString(),
+     * this returns only 'v'.
+     */
+    void GetUnhandledString(nsAString& aUnhandledString) const;
   };
 
   /**
    * Helper classes for guaranteeing cleaning mCurrentKeyEvent
    */
   class AutoKeyEventStateCleaner
   {
   public:
@@ -590,22 +597,18 @@ protected:
 
   class MOZ_STACK_CLASS AutoInsertStringClearer
   {
   public:
     explicit AutoInsertStringClearer(KeyEventState* aState)
       : mState(aState)
     {
     }
-    ~AutoInsertStringClearer()
-    {
-      if (mState) {
-        mState->mInsertString = nullptr;
-      }
-    }
+    ~AutoInsertStringClearer();
+
   private:
     KeyEventState* mState;
   };
 
   /**
    * mCurrentKeyEvents stores all key events which are being processed.
    * When we call interpretKeyEvents, IME may generate other key events.
    * mCurrentKeyEvents[0] is the latest key event.
--- a/widget/cocoa/TextInputHandler.mm
+++ b/widget/cocoa/TextInputHandler.mm
@@ -1547,17 +1547,17 @@ TextInputHandler::HandleKeyDownEvent(NSE
         MOZ_LOG(gLog, LogLevel::Error,
           ("%p IMEInputHandler::HandleKeyDownEvent, "
            "FAILED, due to BeginNativeInputTransaction() failure "
            "at dispatching keydown for ComplexTextInputPanel", this));
         return false;
       }
 
       WidgetKeyboardEvent imeEvent(true, eKeyDown, mWidget);
-      InitKeyEvent(aNativeEvent, imeEvent);
+      currentKeyEvent->InitKeyEvent(this, imeEvent);
       imeEvent.mPluginTextEventString.Assign(committed);
       nsEventStatus status = nsEventStatus_eIgnore;
       mDispatcher->DispatchKeyboardEvent(eKeyDown, imeEvent, status,
                                          currentKeyEvent);
     }
     return true;
   }
 
@@ -1568,17 +1568,17 @@ TextInputHandler::HandleKeyDownEvent(NSE
       MOZ_LOG(gLog, LogLevel::Error,
         ("%p IMEInputHandler::HandleKeyDownEvent, "
          "FAILED, due to BeginNativeInputTransaction() failure "
          "at dispatching keydown for ordinal cases", this));
     return false;
   }
 
   WidgetKeyboardEvent keydownEvent(true, eKeyDown, mWidget);
-  InitKeyEvent(aNativeEvent, keydownEvent);
+  currentKeyEvent->InitKeyEvent(this, keydownEvent);
 
   nsEventStatus status = nsEventStatus_eIgnore;
   mDispatcher->DispatchKeyboardEvent(eKeyDown, keydownEvent, status,
                                      currentKeyEvent);
   currentKeyEvent->mKeyDownHandled =
     (status == nsEventStatus_eConsumeNoDefault);
 
   if (Destroyed()) {
@@ -1641,17 +1641,17 @@ TextInputHandler::HandleKeyDownEvent(NSE
         MOZ_LOG(gLog, LogLevel::Error,
           ("%p IMEInputHandler::HandleKeyDownEvent, "
            "FAILED, due to BeginNativeInputTransaction() failure "
            "at dispatching keypress", this));
       return false;
     }
 
     WidgetKeyboardEvent keypressEvent(true, eKeyPress, mWidget);
-    InitKeyEvent(aNativeEvent, keypressEvent);
+    currentKeyEvent->InitKeyEvent(this, keypressEvent);
 
     // If we called interpretKeyEvents and this isn't normal character input
     // then IME probably ate the event for some reason. We do not want to
     // send a key press event in that case.
     // TODO:
     // There are some other cases which IME eats the current event.
     // 1. If key events were nested during calling interpretKeyEvents, it means
     //    that IME did something.  Then, we should do nothing.
@@ -2219,18 +2219,17 @@ TextInputHandler::InsertText(NSAttribute
   //     must not be caused by a key operation, a part of IME's processing.
   keypressEvent.mIsChar = IsPrintableChar(str.CharAt(0));
 
   // Don't set other modifiers from the current event, because here in
   // -insertText: they've already been taken into account in creating
   // the input string.
 
   if (currentKeyEvent) {
-    NSEvent* keyEvent = currentKeyEvent->mKeyEvent;
-    InitKeyEvent(keyEvent, keypressEvent, &str);
+    currentKeyEvent->InitKeyEvent(this, keypressEvent);
   } else {
     nsCocoaUtils::InitInputEvent(keypressEvent, static_cast<NSEvent*>(nullptr));
     keypressEvent.mKeyNameIndex = KEY_NAME_INDEX_USE_STRING;
     keypressEvent.mKeyValue = str;
     // FYI: TextEventDispatcher will set mKeyCode to 0 for printable key's
     //      keypress events even if they don't cause inputting non-empty string.
   }
 
@@ -2284,17 +2283,17 @@ TextInputHandler::DoCommandBySelector(co
         MOZ_LOG(gLog, LogLevel::Error,
           ("%p IMEInputHandler::DoCommandBySelector, "
            "FAILED, due to BeginNativeInputTransaction() failure "
            "at dispatching keypress", this));
       return false;
     }
 
     WidgetKeyboardEvent keypressEvent(true, eKeyPress, mWidget);
-    InitKeyEvent(currentKeyEvent->mKeyEvent, keypressEvent);
+    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,
@@ -4307,8 +4306,107 @@ TextInputHandlerBase::IsSecureEventInput
 
 /* static */ void
 TextInputHandlerBase::EnsureSecureEventInputDisabled()
 {
   while (sSecureEventInputCount) {
     TextInputHandlerBase::DisableSecureEventInput();
   }
 }
+
+#pragma mark -
+
+
+/******************************************************************************
+ *
+ *  TextInputHandlerBase::KeyEventState implementation
+ *
+ ******************************************************************************/
+
+void
+TextInputHandlerBase::KeyEventState::InitKeyEvent(
+                                       TextInputHandlerBase* aHandler,
+                                       WidgetKeyboardEvent& aKeyEvent)
+{
+  NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
+
+  MOZ_ASSERT(aHandler);
+  MOZ_RELEASE_ASSERT(mKeyEvent);
+
+  NSEvent* nativeEvent = mKeyEvent;
+  if (!mInsertedString.IsEmpty()) {
+    nsAutoString unhandledString;
+    GetUnhandledString(unhandledString);
+    NSString* unhandledNSString =
+      nsCocoaUtils::ToNSString(unhandledString);
+    // If the key event's some characters were already handled by
+    // InsertString() calls, we need to create a dummy event which doesn't
+    // include the handled characters.
+    nativeEvent =
+      [NSEvent keyEventWithType:[mKeyEvent type]
+                       location:[mKeyEvent locationInWindow]
+                  modifierFlags:[mKeyEvent modifierFlags]
+                      timestamp:[mKeyEvent timestamp]
+                   windowNumber:[mKeyEvent windowNumber]
+                        context:[mKeyEvent context]
+                     characters:unhandledNSString
+    charactersIgnoringModifiers:[mKeyEvent charactersIgnoringModifiers]
+                      isARepeat:[mKeyEvent isARepeat]
+                        keyCode:[mKeyEvent keyCode]];
+  }
+
+  aHandler->InitKeyEvent(nativeEvent, aKeyEvent, mInsertString);
+
+  NS_OBJC_END_TRY_ABORT_BLOCK;
+}
+
+void
+TextInputHandlerBase::KeyEventState::GetUnhandledString(
+                                       nsAString& aUnhandledString) const
+{
+  aUnhandledString.Truncate();
+  if (NS_WARN_IF(!mKeyEvent)) {
+    return;
+  }
+  nsAutoString characters;
+  nsCocoaUtils::GetStringForNSString([mKeyEvent characters],
+                                     characters);
+  if (characters.IsEmpty()) {
+    return;
+  }
+  if (mInsertedString.IsEmpty()) {
+    aUnhandledString = characters;
+    return;
+  }
+
+  // The insertes string must match with the start of characters.
+  MOZ_ASSERT(StringBeginsWith(characters, mInsertedString));
+
+  aUnhandledString = nsDependentSubstring(characters, mInsertedString.Length());
+}
+
+#pragma mark -
+
+
+/******************************************************************************
+ *
+ *  TextInputHandlerBase::AutoInsertStringClearer implementation
+ *
+ ******************************************************************************/
+
+TextInputHandlerBase::AutoInsertStringClearer::~AutoInsertStringClearer()
+{
+  if (mState && mState->mInsertString) {
+    // If inserting string is a part of characters of the event,
+    // we should record it as inserted string.
+    nsAutoString characters;
+    nsCocoaUtils::GetStringForNSString([mState->mKeyEvent characters],
+                                       characters);
+    nsAutoString insertedString(mState->mInsertedString);
+    insertedString += *mState->mInsertString;
+    if (StringBeginsWith(characters, insertedString)) {
+      mState->mInsertedString = insertedString;
+    }
+  }
+  if (mState) {
+    mState->mInsertString = nullptr;
+  }
+}