Bug 1430628 - TextInputHandler::HandleCommand() should use native key event when it dispatches a keypress event either initializing with native event or creating fake event. r=m_kato, a=lizzard
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 24 Jan 2018 19:13:01 +0900
changeset 454658 95291d42399e45ad8beaeb59a79addf578c66eda
parent 454657 2169cf07d96fb2f0b0afc842c1dc0c41dbf811df
child 454659 beb7c47f05a233f1b7afa76bce838c46f42c69a5
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato, lizzard
bugs1430628
milestone59.0
Bug 1430628 - TextInputHandler::HandleCommand() should use native key event when it dispatches a keypress event either initializing with native event or creating fake event. r=m_kato, a=lizzard TextInputHandler::HandleCommand() has two bugs. One is, checking whether the key event has caused composition events. Even if it caused composition events, we decided to dispatch keypress event for emulating native behavior. Therefore, this patch removes the check of |currentKeyEvent->CanDispatchKeyPress()|. The other is, for making content handle dispatching keypress event as given command, it needs to dispatch a keypress event whose key combination will cause the command. However, HandleCommand() needs to set native key event since content may not refer key combination for some edit actions, they just refer command which is computed with native key event with NativeKeyBindings. Therefore, even if current native key event has already caused dispatching some events, HandleCommand() needs to set WidgetKeyboardEvent::mNativeKeyEvent to current native key event for NativeKeyBindings. Although it must be rare case, given key could be not related to the command or not key could cause the command. In this case, and perhaps in all cases, we should set all commands of dispatching keypress event before dispatching it. Howevever, this needs more work, so, we shouldn't do it in this bug to making it possible to uplift. Therefore, this patch makes always set mNativeKeyEvent to current native key event. So, just warning it when command is caused without native key event. MozReview-Commit-ID: 2MvDTw4ruAu
widget/cocoa/TextInputHandler.mm
--- a/widget/cocoa/TextInputHandler.mm
+++ b/widget/cocoa/TextInputHandler.mm
@@ -2467,74 +2467,76 @@ TextInputHandler::HandleCommand(Command 
   //       command to the keypress event and it should be handled as
   //       the key press in editor.
 
   // If it's handling actual key event and hasn't cause any composition
   // events nor other key events, we should expose actual modifier state.
   // Otherwise, we should adjust Control, Option and Command state since
   // editor may behave differently if some of them are active.
   bool dispatchFakeKeyPress =
-    !(currentKeyEvent && currentKeyEvent->IsProperKeyEvent(aCommand) &&
-      currentKeyEvent->CanDispatchKeyPressEvent());
+    !(currentKeyEvent && currentKeyEvent->IsProperKeyEvent(aCommand));
 
   WidgetKeyboardEvent keypressEvent(true, eKeyPress, widget);
   if (!dispatchFakeKeyPress) {
     // If we're acutally handling a key press, we should dispatch
     // the keypress event as-is.
     currentKeyEvent->InitKeyEvent(this, keypressEvent);
   } else {
     // Otherwise, we should dispatch "fake" keypress event.
+    // However, for making it possible to compute edit commands, we need to
+    // set current native key event to the fake keyboard event even if it's
+    // not same as what we expect since the native keyboard event caused
+    // this command.
+    NSEvent* keyEvent =
+      currentKeyEvent ? currentKeyEvent->mKeyEvent : nullptr;
+    keypressEvent.mNativeKeyEvent = keyEvent;
+    NS_WARNING_ASSERTION(keypressEvent.mNativeKeyEvent,
+      "Without native key event, NativeKeyBindings cannot compute aCommand");
     switch (aCommand) {
       case CommandInsertLineBreak:
       case CommandInsertParagraph: {
         // Although, Shift+Enter and Enter are work differently in HTML
         // editor, we should expose actual Shift state if it's caused by
         // Enter key for compatibility with Chromium.  Chromium breaks
         // line in HTML editor with default pargraph separator when Enter
         // is pressed, with <br> element when Shift+Enter.  Safari breaks
         // line in HTML editor with default paragraph separator when
         // Enter, Shift+Enter or Option+Enter.  So, we should not change
         // Shift+Enter meaning when there was composition string or not.
-        NSEvent* keyEvent =
-          currentKeyEvent ? currentKeyEvent->mKeyEvent : nullptr;
         nsCocoaUtils::InitInputEvent(keypressEvent, keyEvent);
         keypressEvent.mKeyCode = NS_VK_RETURN;
         keypressEvent.mKeyNameIndex = KEY_NAME_INDEX_Enter;
         keypressEvent.mModifiers &= ~(MODIFIER_CONTROL |
                                       MODIFIER_ALT |
                                       MODIFIER_META);
         if (aCommand == CommandInsertLineBreak) {
           // In default settings, Ctrl + Enter causes insertLineBreak command.
           // So, let's make Ctrl state active of the keypress event.
           keypressEvent.mModifiers |= MODIFIER_CONTROL;
         }
         break;
       }
       case CommandDeleteCharBackward:
       case CommandDeleteToBeginningOfLine:
       case CommandDeleteWordBackward: {
-        NSEvent* keyEvent =
-          currentKeyEvent ? currentKeyEvent->mKeyEvent : nullptr;
         nsCocoaUtils::InitInputEvent(keypressEvent, keyEvent);
         keypressEvent.mKeyCode = NS_VK_BACK;
         keypressEvent.mKeyNameIndex = KEY_NAME_INDEX_Backspace;
         keypressEvent.mModifiers &= ~(MODIFIER_CONTROL |
                                       MODIFIER_ALT |
                                       MODIFIER_META);
         if (aCommand == CommandDeleteToBeginningOfLine) {
           keypressEvent.mModifiers |= MODIFIER_META;
         } else if (aCommand == CommandDeleteWordBackward) {
           keypressEvent.mModifiers |= MODIFIER_ALT;
         }
         break;
       }
       case CommandDeleteCharForward:
       case CommandDeleteWordForward: {
-        NSEvent* keyEvent =
-          currentKeyEvent ? currentKeyEvent->mKeyEvent : nullptr;
         nsCocoaUtils::InitInputEvent(keypressEvent, keyEvent);
         keypressEvent.mKeyCode = NS_VK_DELETE;
         keypressEvent.mKeyNameIndex = KEY_NAME_INDEX_Delete;
         keypressEvent.mModifiers &= ~(MODIFIER_CONTROL |
                                       MODIFIER_ALT |
                                       MODIFIER_META);
         if (aCommand == CommandDeleteWordForward) {
           keypressEvent.mModifiers |= MODIFIER_ALT;
@@ -2542,18 +2544,16 @@ TextInputHandler::HandleCommand(Command 
         break;
       }
       case CommandCharNext:
       case CommandSelectCharNext:
       case CommandWordNext:
       case CommandSelectWordNext:
       case CommandEndLine:
       case CommandSelectEndLine: {
-        NSEvent* keyEvent =
-          currentKeyEvent ? currentKeyEvent->mKeyEvent : nullptr;
         nsCocoaUtils::InitInputEvent(keypressEvent, keyEvent);
         keypressEvent.mKeyCode = NS_VK_RIGHT;
         keypressEvent.mKeyNameIndex = KEY_NAME_INDEX_ArrowRight;
         keypressEvent.mModifiers &= ~(MODIFIER_CONTROL |
                                       MODIFIER_ALT |
                                       MODIFIER_META);
         if (aCommand == CommandSelectCharNext ||
             aCommand == CommandSelectWordNext ||
@@ -2571,18 +2571,16 @@ TextInputHandler::HandleCommand(Command 
         break;
       }
       case CommandCharPrevious:
       case CommandSelectCharPrevious:
       case CommandWordPrevious:
       case CommandSelectWordPrevious:
       case CommandBeginLine:
       case CommandSelectBeginLine: {
-        NSEvent* keyEvent =
-          currentKeyEvent ? currentKeyEvent->mKeyEvent : nullptr;
         nsCocoaUtils::InitInputEvent(keypressEvent, keyEvent);
         keypressEvent.mKeyCode = NS_VK_LEFT;
         keypressEvent.mKeyNameIndex = KEY_NAME_INDEX_ArrowLeft;
         keypressEvent.mModifiers &= ~(MODIFIER_CONTROL |
                                       MODIFIER_ALT |
                                       MODIFIER_META);
         if (aCommand == CommandSelectCharPrevious ||
             aCommand == CommandSelectWordPrevious ||
@@ -2598,18 +2596,16 @@ TextInputHandler::HandleCommand(Command 
           keypressEvent.mModifiers |= MODIFIER_META;
         }
         break;
       }
       case CommandLinePrevious:
       case CommandSelectLinePrevious:
       case CommandMoveTop:
       case CommandSelectTop: {
-        NSEvent* keyEvent =
-          currentKeyEvent ? currentKeyEvent->mKeyEvent : nullptr;
         nsCocoaUtils::InitInputEvent(keypressEvent, keyEvent);
         keypressEvent.mKeyCode = NS_VK_UP;
         keypressEvent.mKeyNameIndex = KEY_NAME_INDEX_ArrowUp;
         keypressEvent.mModifiers &= ~(MODIFIER_CONTROL |
                                       MODIFIER_ALT |
                                       MODIFIER_META);
         if (aCommand == CommandSelectLinePrevious ||
             aCommand == CommandSelectTop) {
@@ -2620,18 +2616,16 @@ TextInputHandler::HandleCommand(Command 
           keypressEvent.mModifiers |= MODIFIER_META;
         }
         break;
       }
       case CommandLineNext:
       case CommandSelectLineNext:
       case CommandMoveBottom:
       case CommandSelectBottom: {
-        NSEvent* keyEvent =
-          currentKeyEvent ? currentKeyEvent->mKeyEvent : nullptr;
         nsCocoaUtils::InitInputEvent(keypressEvent, keyEvent);
         keypressEvent.mKeyCode = NS_VK_DOWN;
         keypressEvent.mKeyNameIndex = KEY_NAME_INDEX_ArrowDown;
         keypressEvent.mModifiers &= ~(MODIFIER_CONTROL |
                                       MODIFIER_ALT |
                                       MODIFIER_META);
         if (aCommand == CommandSelectLineNext ||
             aCommand == CommandSelectBottom) {
@@ -2640,65 +2634,57 @@ TextInputHandler::HandleCommand(Command 
         if (aCommand == CommandMoveBottom ||
             aCommand == CommandSelectBottom) {
           keypressEvent.mModifiers |= MODIFIER_META;
         }
         break;
       }
       case CommandScrollPageUp:
       case CommandSelectPageUp: {
-        NSEvent* keyEvent =
-          currentKeyEvent ? currentKeyEvent->mKeyEvent : nullptr;
         nsCocoaUtils::InitInputEvent(keypressEvent, keyEvent);
         keypressEvent.mKeyCode = NS_VK_PAGE_UP;
         keypressEvent.mKeyNameIndex = KEY_NAME_INDEX_PageUp;
         keypressEvent.mModifiers &= ~(MODIFIER_CONTROL |
                                       MODIFIER_ALT |
                                       MODIFIER_META);
         if (aCommand == CommandSelectPageUp) {
           keypressEvent.mModifiers |= MODIFIER_SHIFT;
         }
         break;
       }
       case CommandScrollPageDown:
       case CommandSelectPageDown: {
-        NSEvent* keyEvent =
-          currentKeyEvent ? currentKeyEvent->mKeyEvent : nullptr;
         nsCocoaUtils::InitInputEvent(keypressEvent, keyEvent);
         keypressEvent.mKeyCode = NS_VK_PAGE_DOWN;
         keypressEvent.mKeyNameIndex = KEY_NAME_INDEX_PageDown;
         keypressEvent.mModifiers &= ~(MODIFIER_CONTROL |
                                       MODIFIER_ALT |
                                       MODIFIER_META);
         if (aCommand == CommandSelectPageDown) {
           keypressEvent.mModifiers |= MODIFIER_SHIFT;
         }
         break;
       }
       case CommandScrollBottom:
       case CommandScrollTop: {
-        NSEvent* keyEvent =
-          currentKeyEvent ? currentKeyEvent->mKeyEvent : nullptr;
         nsCocoaUtils::InitInputEvent(keypressEvent, keyEvent);
         if (aCommand == CommandScrollBottom) {
           keypressEvent.mKeyCode = NS_VK_END;
           keypressEvent.mKeyNameIndex = KEY_NAME_INDEX_End;
         } else {
           keypressEvent.mKeyCode = NS_VK_HOME;
           keypressEvent.mKeyNameIndex = KEY_NAME_INDEX_Home;
         }
         keypressEvent.mModifiers &= ~(MODIFIER_CONTROL |
                                       MODIFIER_ALT |
                                       MODIFIER_META);
         break;
       }
       case CommandCancelOperation:
       case CommandComplete: {
-        NSEvent* keyEvent =
-          currentKeyEvent ? currentKeyEvent->mKeyEvent : nullptr;
         nsCocoaUtils::InitInputEvent(keypressEvent, keyEvent);
         keypressEvent.mKeyCode = NS_VK_ESCAPE;
         keypressEvent.mKeyNameIndex = KEY_NAME_INDEX_Escape;
         keypressEvent.mModifiers &= ~(MODIFIER_CONTROL |
                                       MODIFIER_ALT |
                                       MODIFIER_META);
         if (aCommand == CommandComplete) {
           keypressEvent.mModifiers |= MODIFIER_ALT;