Bug 1343451 - part 3-1: Make KeymapWrapper::InitKeyEvent() mark given key event as "processed by IME" if it has been handled by IME r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 22 Feb 2018 19:52:53 +0900
changeset 464105 0c24a392e924c90e0b40764ceec147fc5af26288
parent 464104 73a1a183e75f3ae349647977689e800b9a173547
child 464106 5ff56d8f616c0a219d2d8381735911f3482742f9
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1343451
milestone61.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 1343451 - part 3-1: Make KeymapWrapper::InitKeyEvent() mark given key event as "processed by IME" if it has been handled by IME r=m_kato For conforming UI Events spec, KeymapWrapper::InitKeyEvent() should initialize mKeyCode and mKeyNameIndex with NS_VK_PROCESSKEY and KEY_NAME_INDEX_Process if given keyboard event has already been handled by IME. For making it know if given keyboard event has been handled by IME, this patch adds additional bool argument to it and its callers. Note that this patch changes keyCode value and key value of "keydown" event if it's fired before "compositionstart" since Chromium does so on Linux. MozReview-Commit-ID: FC3tfyeeopU
widget/gtk/IMContextWrapper.cpp
widget/gtk/nsGtkKeyUtils.cpp
widget/gtk/nsGtkKeyUtils.h
widget/gtk/nsWindow.cpp
widget/gtk/nsWindow.h
--- a/widget/gtk/IMContextWrapper.cpp
+++ b/widget/gtk/IMContextWrapper.cpp
@@ -1407,19 +1407,21 @@ IMContextWrapper::DispatchCompositionSta
         // overwritten with different context if calling this method
         // recursively.
         // Note that we don't need to grab the context here because |context|
         // will be used only for checking if it's same as mComposingContext.
         GtkIMContext* context = mComposingContext;
 
         // If this composition is started by a native keydown event, we need to
         // dispatch our keydown event here (before composition start).
+        // Note that make the preceding eKeyDown event marked as "processed
+        // by IME" for compatibility with Chromium.
         bool isCancelled;
-        mLastFocusedWindow->DispatchKeyDownEvent(mProcessingKeyEvent,
-                                                 &isCancelled);
+        mLastFocusedWindow->DispatchKeyDownOrKeyUpEvent(mProcessingKeyEvent,
+                                                        true, &isCancelled);
         MOZ_LOG(gGtkIMLog, LogLevel::Debug,
             ("0x%p   DispatchCompositionStart(), preceding keydown event is "
              "dispatched",
              this));
         if (lastFocusedWindow->IsDestroyed() ||
             lastFocusedWindow != mLastFocusedWindow) {
             MOZ_LOG(gGtkIMLog, LogLevel::Warning,
                 ("0x%p   DispatchCompositionStart(), Warning, the focused "
--- a/widget/gtk/nsGtkKeyUtils.cpp
+++ b/widget/gtk/nsGtkKeyUtils.cpp
@@ -926,40 +926,46 @@ KeymapWrapper::ComputeDOMCodeNameIndex(c
             break;
     }
 
     return CODE_NAME_INDEX_UNKNOWN;
 }
 
 /* static */ void
 KeymapWrapper::InitKeyEvent(WidgetKeyboardEvent& aKeyEvent,
-                            GdkEventKey* aGdkKeyEvent)
+                            GdkEventKey* aGdkKeyEvent,
+                            bool aIsProcessedByIME)
 {
+    MOZ_ASSERT(!aIsProcessedByIME || aKeyEvent.mMessage != eKeyPress,
+      "If the key event is handled by IME, keypress event shouldn't be fired");
+
     KeymapWrapper* keymapWrapper = GetInstance();
 
     aKeyEvent.mCodeNameIndex = ComputeDOMCodeNameIndex(aGdkKeyEvent);
     MOZ_ASSERT(aKeyEvent.mCodeNameIndex != CODE_NAME_INDEX_USE_STRING);
     aKeyEvent.mKeyNameIndex =
-        keymapWrapper->ComputeDOMKeyNameIndex(aGdkKeyEvent);
+        aIsProcessedByIME ? KEY_NAME_INDEX_Process :
+                            keymapWrapper->ComputeDOMKeyNameIndex(aGdkKeyEvent);
     if (aKeyEvent.mKeyNameIndex == KEY_NAME_INDEX_Unidentified) {
         uint32_t charCode = GetCharCodeFor(aGdkKeyEvent);
         if (!charCode) {
             charCode = keymapWrapper->GetUnmodifiedCharCodeFor(aGdkKeyEvent);
         }
         if (charCode) {
             aKeyEvent.mKeyNameIndex = KEY_NAME_INDEX_USE_STRING;
             MOZ_ASSERT(aKeyEvent.mKeyValue.IsEmpty(),
                        "Uninitialized mKeyValue must be empty");
             AppendUCS4ToUTF16(charCode, aKeyEvent.mKeyValue);
         }
     }
-    aKeyEvent.mKeyCode = ComputeDOMKeyCode(aGdkKeyEvent);
 
-    if (aKeyEvent.mKeyNameIndex != KEY_NAME_INDEX_USE_STRING ||
-        aKeyEvent.mMessage != eKeyPress) {
+    if (aIsProcessedByIME) {
+        aKeyEvent.mKeyCode = NS_VK_PROCESSKEY;
+    } else if (aKeyEvent.mKeyNameIndex != KEY_NAME_INDEX_USE_STRING ||
+               aKeyEvent.mMessage != eKeyPress) {
         aKeyEvent.mKeyCode = ComputeDOMKeyCode(aGdkKeyEvent);
     } else {
         aKeyEvent.mKeyCode = 0;
     }
 
     // NOTE: The state of given key event indicates adjacent state of
     // modifier keys.  E.g., even if the event is Shift key press event,
     // the bit for Shift is still false.  By the same token, even if the
--- a/widget/gtk/nsGtkKeyUtils.h
+++ b/widget/gtk/nsGtkKeyUtils.h
@@ -126,19 +126,21 @@ public:
 
     /**
      * InitKeyEvent() intializes aKeyEvent's modifier key related members
      * and keycode related values.
      *
      * @param aKeyEvent         It's an WidgetKeyboardEvent which needs to be
      *                          initialized.
      * @param aGdkKeyEvent      A native GDK key event.
+     * @param aIsProcessedByIME true if aGdkKeyEvent is handled by IME.
      */
     static void InitKeyEvent(WidgetKeyboardEvent& aKeyEvent,
-                             GdkEventKey* aGdkKeyEvent);
+                             GdkEventKey* aGdkKeyEvent,
+                             bool aIsProcessedByIME);
 
     /**
      * WillDispatchKeyboardEvent() is called via
      * TextEventDispatcherListener::WillDispatchKeyboardEvent().
      *
      * @param aKeyEvent         An instance of KeyboardEvent which will be
      *                          dispatched.  This method should set charCode
      *                          and alternative char codes if it's necessary.
--- a/widget/gtk/nsWindow.cpp
+++ b/widget/gtk/nsWindow.cpp
@@ -2932,39 +2932,42 @@ static bool
 IsCtrlAltTab(GdkEventKey *aEvent)
 {
     return aEvent->keyval == GDK_Tab &&
         KeymapWrapper::AreModifiersActive(
             KeymapWrapper::CTRL | KeymapWrapper::ALT, aEvent->state);
 }
 
 bool
-nsWindow::DispatchKeyDownEvent(GdkEventKey *aEvent, bool *aCancelled)
-{
-    NS_PRECONDITION(aCancelled, "aCancelled must not be null");
-
-    *aCancelled = false;
-
-    if (IsCtrlAltTab(aEvent)) {
+nsWindow::DispatchKeyDownOrKeyUpEvent(GdkEventKey* aEvent,
+                                      bool aIsProcessedByIME,
+                                      bool* aIsCancelled)
+{
+    MOZ_ASSERT(aIsCancelled, "aCancelled must not be null");
+
+    *aIsCancelled = false;
+
+    if (aEvent->type == GDK_KEY_PRESS && IsCtrlAltTab(aEvent)) {
         return false;
     }
 
     RefPtr<TextEventDispatcher> dispatcher = GetTextEventDispatcher();
     nsresult rv = dispatcher->BeginNativeInputTransaction();
     if (NS_WARN_IF(NS_FAILED(rv))) {
         return FALSE;
     }
 
-    WidgetKeyboardEvent keydownEvent(true, eKeyDown, this);
-    KeymapWrapper::InitKeyEvent(keydownEvent, aEvent);
+    EventMessage message =
+        aEvent->type == GDK_KEY_PRESS ? eKeyDown : eKeyUp;
+    WidgetKeyboardEvent keyEvent(true, message, this);
+    KeymapWrapper::InitKeyEvent(keyEvent, aEvent, aIsProcessedByIME);
     nsEventStatus status = nsEventStatus_eIgnore;
     bool dispatched =
-        dispatcher->DispatchKeyboardEvent(eKeyDown, keydownEvent,
-                                          status, aEvent);
-    *aCancelled = (status == nsEventStatus_eConsumeNoDefault);
+        dispatcher->DispatchKeyboardEvent(message, keyEvent, status, aEvent);
+    *aIsCancelled = (status == nsEventStatus_eConsumeNoDefault);
     return dispatched ? TRUE : FALSE;
 }
 
 WidgetEventTime
 nsWindow::GetWidgetEventTime(guint32 aEventTime)
 {
   return WidgetEventTime(aEventTime, GetEventTimeStamp(aEventTime));
 }
@@ -3040,17 +3043,17 @@ nsWindow::OnKeyPressEvent(GdkEventKey *a
 
     // Dispatch keydown event always.  At auto repeating, we should send
     // KEYDOWN -> KEYPRESS -> KEYDOWN -> KEYPRESS ... -> KEYUP
     // However, old distributions (e.g., Ubuntu 9.10) sent native key
     // release event, so, on such platform, the DOM events will be:
     // KEYDOWN -> KEYPRESS -> KEYUP -> KEYDOWN -> KEYPRESS -> KEYUP...
 
     bool isKeyDownCancelled = false;
-    if (DispatchKeyDownEvent(aEvent, &isKeyDownCancelled) &&
+    if (DispatchKeyDownOrKeyUpEvent(aEvent, false, &isKeyDownCancelled) &&
         (MOZ_UNLIKELY(mIsDestroyed) || isKeyDownCancelled)) {
         return TRUE;
     }
 
     // If a keydown event handler causes to enable IME, i.e., it moves
     // focus from IME unusable content to IME usable editor, we should
     // send the native key event to IME for the first input on the editor.
     if (!IMEWasEnabled && mIMContext && mIMContext->IsEnabled()) {
@@ -3089,17 +3092,17 @@ nsWindow::OnKeyPressEvent(GdkEventKey *a
         case GDK_Redo:
             return DispatchContentCommandEvent(eContentCommandRedo);
         case GDK_Undo:
         case GDK_F14:
             return DispatchContentCommandEvent(eContentCommandUndo);
     }
 
     WidgetKeyboardEvent keypressEvent(true, eKeyPress, this);
-    KeymapWrapper::InitKeyEvent(keypressEvent, aEvent);
+    KeymapWrapper::InitKeyEvent(keypressEvent, aEvent, false);
 
     // before we dispatch a key, check if it's the context menu key.
     // If so, send a context menu key event instead.
     if (MaybeDispatchContextMenuEvent(aEvent)) {
         return TRUE;
     }
 
     RefPtr<TextEventDispatcher> dispatcher = GetTextEventDispatcher();
@@ -3173,34 +3176,28 @@ nsWindow::MaybeDispatchContextMenuEvent(
         contextMenuEvent.mModifiers &= ~MODIFIER_SHIFT;
     }
 
     DispatchInputEvent(&contextMenuEvent);
     return true;
 }
 
 gboolean
-nsWindow::OnKeyReleaseEvent(GdkEventKey *aEvent)
+nsWindow::OnKeyReleaseEvent(GdkEventKey* aEvent)
 {
     LOGFOCUS(("OnKeyReleaseEvent [%p]\n", (void *)this));
 
     if (mIMContext && mIMContext->OnKeyEvent(this, aEvent)) {
         return TRUE;
     }
 
-    RefPtr<TextEventDispatcher> dispatcher = GetTextEventDispatcher();
-    nsresult rv = dispatcher->BeginNativeInputTransaction();
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-        return false;
-    }
-
-    WidgetKeyboardEvent keyupEvent(true, eKeyUp, this);
-    KeymapWrapper::InitKeyEvent(keyupEvent, aEvent);
-    nsEventStatus status = nsEventStatus_eIgnore;
-    dispatcher->DispatchKeyboardEvent(eKeyUp, keyupEvent, status, aEvent);
+    bool isCancelled = false;
+    if (NS_WARN_IF(!DispatchKeyDownOrKeyUpEvent(aEvent, false, &isCancelled))) {
+        return FALSE;
+    }
 
     return TRUE;
 }
 
 void
 nsWindow::OnScrollEvent(GdkEventScroll *aEvent)
 {
     // check to see if we should rollup
--- a/widget/gtk/nsWindow.h
+++ b/widget/gtk/nsWindow.h
@@ -275,20 +275,30 @@ public:
     GdkWindow*         GetGdkWindow() { return mGdkWindow; }
     bool               IsDestroyed() { return mIsDestroyed; }
 
     void               DispatchDragEvent(mozilla::EventMessage aMsg,
                                          const LayoutDeviceIntPoint& aRefPoint,
                                          guint aTime);
     static void        UpdateDragStatus (GdkDragContext *aDragContext,
                                          nsIDragService *aDragService);
-    // If this dispatched the keydown event actually, this returns TRUE,
-    // otherwise, FALSE.
-    bool               DispatchKeyDownEvent(GdkEventKey *aEvent,
-                                            bool *aIsCancelled);
+    /**
+     * DispatchKeyDownOrKeyUpEvent() dispatches eKeyDown or eKeyUp event.
+     *
+     * @param aEvent            A native GDK_KEY_PRESS or GDK_KEY_RELEASE
+     *                          event.
+     * @param aProcessedByIME   true if the event is handled by IME.
+     * @param aIsCancelled      [Out] true if the default is prevented.
+     * @return                  true if eKeyDown event is actually dispatched.
+     *                          Otherwise, false.
+     */
+    bool DispatchKeyDownOrKeyUpEvent(GdkEventKey* aEvent,
+                                     bool aProcessedByIME,
+                                     bool* aIsCancelled);
+
     WidgetEventTime    GetWidgetEventTime(guint32 aEventTime);
     mozilla::TimeStamp GetEventTimeStamp(guint32 aEventTime);
     mozilla::CurrentX11TimeGetter* GetCurrentTimeGetter();
 
     virtual void SetInputContext(const InputContext& aContext,
                                  const InputContextAction& aAction) override;
     virtual InputContext GetInputContext() override;
     virtual TextEventDispatcherListener*