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 draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 22 Feb 2018 19:52:53 +0900
changeset 766628 9db6aee9190dfe66a5c245bd2cd5a9ba42b178ac
parent 766627 5014c861df0e174a0174ce17ba44ae3babe42680
child 766629 46a2dcc9ccc49743d9308ef1403f7882cdb9bead
push id102374
push usermasayuki@d-toybox.com
push dateTue, 13 Mar 2018 06:34:13 +0000
reviewersm_kato
bugs1343451
milestone61.0a1
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*