Bug 1341960 TextInputHandler shouldn't ignore InsertText() calls even if TextInputHandler has already dispatched keypress events and/or composition events for the key down but InsertText() is called again for inserting printable text r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 23 Feb 2017 11:45:13 +0900
changeset 488494 e1e1531c39d1caf0d5e680a82819a0fffea39f53
parent 488302 32dcdde1fc64fc39a9065dc4218265dbc727673f
child 546738 e67e74e851cf1c31acc50587a050c4ac3fe6c1b9
push id46540
push usermasayuki@d-toybox.com
push dateThu, 23 Feb 2017 05:52:10 +0000
reviewersm_kato
bugs1341960
milestone54.0a1
Bug 1341960 TextInputHandler shouldn't ignore InsertText() calls even if TextInputHandler has already dispatched keypress events and/or composition events for the key down but InsertText() is called again for inserting printable text r?m_kato Korean IME and some other simple IME may not use marked text at composing text. Such IME modifies composing character with InsertText() with aReplacementRange. Then, when user types new character, such IME may replace the previous character with a call of InsertText() with aReplacementRange and insert the next character with additional call of InsertText() without aReplacementRange. In this case, InsertText() ignores the call when the native keydown event is already caused composition events. However, we need to allow to dispatch keypress event for supporting to insert text even in such case. This patch checks if inserting text is printable. If it's not a printable character such as U+000A at pressing Enter, keeping current behavior. Otherwise, dispatching keypress event instead. MozReview-Commit-ID: 5NcCgXfvniO
widget/cocoa/TextInputHandler.mm
--- a/widget/cocoa/TextInputHandler.mm
+++ b/widget/cocoa/TextInputHandler.mm
@@ -2221,18 +2221,30 @@ TextInputHandler::InsertText(NSAttribute
     InsertTextAsCommittingComposition(aAttrString, aReplacementRange);
     if (currentKeyEvent) {
       currentKeyEvent->mCompositionDispatched = true;
     }
     return;
   }
 
   // Don't let the same event be fired twice when hitting
-  // enter/return! (Bug 420502)
-  if (currentKeyEvent && !currentKeyEvent->CanDispatchKeyPressEvent()) {
+  // enter/return for Bug 420502.  However, Korean IME (or some other
+  // simple IME) may work without marked text.  For example, composing
+  // character may be inserted as committed text and it's modified with
+  // aReplacementRange.  When a keydown starts new composition with
+  // committing previous character, InsertText() may be called twice,
+  // one is for committing previous character and then, inserting new
+  // composing character as committed character.  In the latter case,
+  // |CanDispatchKeyPressEvent()| returns true but we need to dispatch
+  // keypress event for the new character.  So, when IME tries to insert
+  // printable characters, we should ignore current key event state even
+  // after the keydown has already caused dispatching composition event.
+  // XXX Anyway, we should sort out around this at fixing bug 1338460.
+  if (currentKeyEvent && !currentKeyEvent->CanDispatchKeyPressEvent() &&
+      (str.IsEmpty() || (str.Length() == 1 && !IsPrintableChar(str[0])))) {
     return;
   }
 
   // XXX Shouldn't we hold mDispatcher instead of mWidget?
   RefPtr<nsChildView> widget(mWidget);
   nsresult rv = mDispatcher->BeginNativeInputTransaction();
   if (NS_WARN_IF(NS_FAILED(rv))) {
       MOZ_LOG(gLog, LogLevel::Error,