Bug 1312649 part.1 TextInputHandler::InsertText() should dispatch composition events instead of keypress events when it replaces a range which is different from current selection r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 07 Nov 2016 10:30:05 +0900
changeset 348230 6bfd79a3ab98c93f7a6bff0ff27b24a9164ea59d
parent 348229 00f8bca94661bc5be13c1635b63e086ba7f03ce6
child 348231 05f8aed1d9c5d35cfac1dda1410542fa337617a1
push id10298
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:33:03 +0000
treeherdermozilla-aurora@7e29173b1641 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1312649
milestone52.0a1
Bug 1312649 part.1 TextInputHandler::InsertText() should dispatch composition events instead of keypress events when it replaces a range which is different from current selection r=m_kato Vietnamese Telex IME handles Backspace key immediately after inputting a word even when there is no marked text. At this time, it tries to replace the word with specific string. In such case, our editor shouldn't remove anything at handling the Backspace keypress event. For avoiding this issue, InserText() should dispatch a set of composition for inserting the specified text into the range. Then, editor won't perform any action of the key. Additionally, when a Backspace keydown tries to remove the last character of the word, Telex removes it with a composition. At this time, it creates dummy marked text "a" and make it empty later. So, in this case, InsertText() won't be called, therefore, we need to consume the Backspace keypress when SetMarkedText() is called for preventing removing the previous character of the word. MozReview-Commit-ID: LfeEHDWn0cZ
widget/cocoa/TextInputHandler.mm
--- a/widget/cocoa/TextInputHandler.mm
+++ b/widget/cocoa/TextInputHandler.mm
@@ -1513,16 +1513,18 @@ TextInputHandler::HandleKeyDownEvent(NSE
 
   if (Destroyed()) {
     MOZ_LOG(gLog, LogLevel::Info,
       ("%p TextInputHandler::HandleKeyDownEvent, "
        "widget has been already destroyed", this));
     return false;
   }
 
+  // Insert empty line to the log for easier to read.
+  MOZ_LOG(gLog, LogLevel::Info, (""));
   MOZ_LOG(gLog, LogLevel::Info,
     ("%p TextInputHandler::HandleKeyDownEvent, aNativeEvent=%p, "
      "type=%s, keyCode=%lld (0x%X), modifierFlags=0x%X, characters=\"%s\", "
      "charactersIgnoringModifiers=\"%s\"",
      this, aNativeEvent, GetNativeKeyEventType(aNativeEvent),
      [aNativeEvent keyCode], [aNativeEvent keyCode],
      [aNativeEvent modifierFlags], GetCharacters([aNativeEvent characters]),
      GetCharacters([aNativeEvent charactersIgnoringModifiers])));
@@ -1681,16 +1683,18 @@ TextInputHandler::HandleKeyDownEvent(NSE
   // Note: mWidget might have become null here. Don't count on it from here on.
 
   MOZ_LOG(gLog, LogLevel::Info,
     ("%p TextInputHandler::HandleKeyDownEvent, "
      "keydown handled=%s, keypress handled=%s, causedOtherKeyEvents=%s",
      this, TrueOrFalse(currentKeyEvent->mKeyDownHandled),
      TrueOrFalse(currentKeyEvent->mKeyPressHandled),
      TrueOrFalse(currentKeyEvent->mCausedOtherKeyEvents)));
+  // Insert empty line to the log for easier to read.
+  MOZ_LOG(gLog, LogLevel::Info, (""));
   return currentKeyEvent->IsDefaultPrevented();
 
   NS_OBJC_END_TRY_ABORT_BLOCK_RETURN(false);
 }
 
 void
 TextInputHandler::HandleKeyUpEvent(NSEvent* aNativeEvent)
 {
@@ -2183,39 +2187,41 @@ TextInputHandler::InsertText(NSAttribute
     WidgetContentCommandEvent deleteCommandEvent(true, eContentCommandDelete,
                                                  mWidget);
     DispatchEvent(deleteCommandEvent);
     NS_ENSURE_TRUE_VOID(deleteCommandEvent.mSucceeded);
     // Be aware! The widget might be destroyed here.
     return;
   }
 
-  // If this is not caused by pressing a key or there is a composition, let's
+  bool isReplacingSpecifiedRange =
+    isEditable && aReplacementRange &&
+    aReplacementRange->location != NSNotFound &&
+    !NSEqualRanges(selectedRange, *aReplacementRange);
+
+  // If this is not caused by pressing a key, there is a composition or
+  // replacing a range which is different from current selection, let's
   // insert the text as committing a composition.
-  if (!currentKeyEvent || IsIMEComposing()) {
+  if (!currentKeyEvent || IsIMEComposing() || isReplacingSpecifiedRange) {
     InsertTextAsCommittingComposition(aAttrString, aReplacementRange);
+    if (currentKeyEvent) {
+      currentKeyEvent->mKeyPressHandled = true;
+      currentKeyEvent->mKeyPressDispatched = true;
+    }
     return;
   }
 
   // Don't let the same event be fired twice when hitting
   // enter/return! (Bug 420502)
   if (currentKeyEvent && !currentKeyEvent->CanDispatchKeyPressEvent()) {
     return;
   }
 
+  // XXX Shouldn't we hold mDispatcher instead of mWidget?
   RefPtr<nsChildView> widget(mWidget);
-
-  // If the replacement range is specified, select the range.  Then, the
-  // selection will be replaced by the later keypress event.
-  if (isEditable &&
-      aReplacementRange && aReplacementRange->location != NSNotFound &&
-      !NSEqualRanges(selectedRange, *aReplacementRange)) {
-    NS_ENSURE_TRUE_VOID(SetSelection(*aReplacementRange));
-  }
-
   nsresult rv = mDispatcher->BeginNativeInputTransaction();
   if (NS_WARN_IF(NS_FAILED(rv))) {
       MOZ_LOG(gLog, LogLevel::Error,
         ("%p IMEInputHandler::HandleKeyUpEvent, "
          "FAILED, due to BeginNativeInputTransaction() failure", this));
     return;
   }
 
@@ -3138,29 +3144,47 @@ IMEInputHandler::InsertTextAsCommittingC
 
 void
 IMEInputHandler::SetMarkedText(NSAttributedString* aAttrString,
                                NSRange& aSelectedRange,
                                NSRange* aReplacementRange)
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
 
+  KeyEventState* currentKeyEvent = GetCurrentKeyEvent();
+
   MOZ_LOG(gLog, LogLevel::Info,
     ("%p IMEInputHandler::SetMarkedText, "
      "aAttrString=\"%s\", aSelectedRange={ location=%llu, length=%llu }, "
      "aReplacementRange=%p { location=%llu, length=%llu }, "
      "Destroyed()=%s, IgnoreIMEComposition()=%s, IsIMEComposing()=%s, "
-     "mMarkedRange={ location=%llu, length=%llu }",
+     "mMarkedRange={ location=%llu, length=%llu }, keyevent=%p, "
+     "keydownHandled=%s, keypressDispatched=%s, causedOtherKeyEvents=%s",
      this, GetCharacters([aAttrString string]),
      aSelectedRange.location, aSelectedRange.length, aReplacementRange,
      aReplacementRange ? aReplacementRange->location : 0,
      aReplacementRange ? aReplacementRange->length : 0,
      TrueOrFalse(Destroyed()), TrueOrFalse(IgnoreIMEComposition()),
      TrueOrFalse(IsIMEComposing()),
-     mMarkedRange.location, mMarkedRange.length));
+     mMarkedRange.location, mMarkedRange.length,
+     currentKeyEvent ? currentKeyEvent->mKeyEvent : nullptr,
+     currentKeyEvent ?
+       TrueOrFalse(currentKeyEvent->mKeyDownHandled) : "N/A",
+     currentKeyEvent ?
+       TrueOrFalse(currentKeyEvent->mKeyPressDispatched) : "N/A",
+     currentKeyEvent ?
+       TrueOrFalse(currentKeyEvent->mCausedOtherKeyEvents) : "N/A"));
+
+  // If SetMarkedText() is called during composition, that means that
+  // the key event caused this composition.  So, keypress event shouldn't
+  // be dispatched later, let's consume it here.
+  if (currentKeyEvent) {
+    currentKeyEvent->mKeyPressHandled = true;
+    currentKeyEvent->mKeyPressDispatched = true;
+  }
 
   if (Destroyed() || IgnoreIMEComposition()) {
     return;
   }
 
   RefPtr<IMEInputHandler> kungFuDeathGrip(this);
 
   // First, commit current composition with the latest composition string if the