Bug 1280053 TextInputHandler should initialize WidgetKeyboardEvent without already handled characters r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Sun, 19 Jun 2016 01:13:16 +0900
changeset 302302 08e186081db2275a88437b51e925b92f13f572dc
parent 302301 da005aa1d83c4a32c2ff1d211d359581013352ed
child 302303 0b088b6f83128a841076379faceb11416b4006ce
push id30356
push usercbook@mozilla.com
push dateWed, 22 Jun 2016 11:45:58 +0000
treeherdermozilla-central@4e17dca08962 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1280053
milestone50.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1280053 TextInputHandler should initialize WidgetKeyboardEvent without already handled characters r=m_kato 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
@@ -502,16 +502,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;
@@ -522,27 +525,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)
     {
@@ -553,31 +546,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:
@@ -596,22 +603,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
@@ -1549,17 +1549,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;
   }
 
@@ -1570,17 +1570,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()) {
@@ -1643,17 +1643,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.
@@ -2221,18 +2221,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.
   }
 
@@ -2286,17 +2285,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,
@@ -4309,8 +4308,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;
+  }
+}