From 99336e02ef2400756a8c6c219674efcb67a96e1e Mon Sep 17 00:00:00 2001 From: Stephen A Pohl Date: Mon, 15 May 2017 22:59:35 -0400 Subject: [PATCH] Bug 429824: Properly forward native OSX events to the native menu bar if they haven't been handled by the child process in e10s. r=mstange,masayuki --- dom/events/EventStateManager.cpp | 12 +++++ dom/ipc/TabChild.cpp | 1 + dom/ipc/TabParent.cpp | 10 ++++ widget/TextEvents.h | 4 +- widget/cocoa/TextInputHandler.h | 32 ++++++++---- widget/cocoa/TextInputHandler.mm | 12 ++++- widget/cocoa/nsChildView.h | 6 +++ widget/cocoa/nsChildView.mm | 87 ++++++++++++++++++++++++-------- widget/nsBaseWidget.cpp | 5 ++ widget/nsIWidget.h | 8 +++ 10 files changed, 140 insertions(+), 37 deletions(-) diff --git a/dom/events/EventStateManager.cpp b/dom/events/EventStateManager.cpp index 6d00f15b4c7a..1f9b4e470403 100644 --- a/dom/events/EventStateManager.cpp +++ b/dom/events/EventStateManager.cpp @@ -2840,6 +2840,18 @@ EventStateManager::PostHandleKeyboardEvent(WidgetKeyboardEvent* aKeyboardEvent, return; } + if (!dispatchedToContentProcess) { + // The widget expects a reply for every keyboard event. If the event wasn't + // dispatched to a content process (non-e10s or no content process + // running), we need to short-circuit here. Otherwise, we need to wait for + // the content process to handle the event. + aKeyboardEvent->mWidget->PostHandleKeyEvent(aKeyboardEvent); + if (aKeyboardEvent->DefaultPrevented()) { + aStatus = nsEventStatus_eConsumeNoDefault; + return; + } + } + // XXX Currently, our automated tests don't support mKeyNameIndex. // Therefore, we still need to handle this with keyCode. switch(aKeyboardEvent->mKeyCode) { diff --git a/dom/ipc/TabChild.cpp b/dom/ipc/TabChild.cpp index da07e4c3c986..bd8702a9665d 100644 --- a/dom/ipc/TabChild.cpp +++ b/dom/ipc/TabChild.cpp @@ -1973,6 +1973,7 @@ TabChild::RecvRealKeyEvent(const WidgetKeyboardEvent& aEvent, WidgetKeyboardEvent localEvent(aEvent); localEvent.mWidget = mPuppetWidget; + localEvent.mUniqueId = aEvent.mUniqueId; nsEventStatus status = APZCCallbackHelper::DispatchWidgetEvent(localEvent); // Update the end time of the possible repeated event so that we can skip diff --git a/dom/ipc/TabParent.cpp b/dom/ipc/TabParent.cpp index 670e7839ed04..6cbe15dbc2f8 100644 --- a/dom/ipc/TabParent.cpp +++ b/dom/ipc/TabParent.cpp @@ -2002,6 +2002,16 @@ TabParent::RecvReplyKeyEvent(const WidgetKeyboardEvent& aEvent) &localEvent, doc); EventDispatcher::Dispatch(mFrameElement, presContext, &localEvent); + + if (!localEvent.DefaultPrevented() && + !localEvent.mFlags.mIsSynthesizedForTests) { + nsCOMPtr widget = GetWidget(); + if (widget) { + widget->PostHandleKeyEvent(&localEvent); + localEvent.StopPropagation(); + } + } + return IPC_OK(); } diff --git a/widget/TextEvents.h b/widget/TextEvents.h index 4e85cca3d1c0..4b4c07310bfe 100644 --- a/widget/TextEvents.h +++ b/widget/TextEvents.h @@ -240,9 +240,7 @@ public: // handling should be delayed until it is determined that there exists no // overriding access key in the content process. bool mAccessKeyForwardedToChild; - // Unique id associated with a keydown / keypress event. Used in identifing - // keypress events for removal from async event dispatch queue in metrofx - // after preventDefault is called on keydown events. It's ok if this wraps + // Unique id associated with a keydown / keypress event. It's ok if this wraps // over long periods. uint32_t mUniqueId; diff --git a/widget/cocoa/TextInputHandler.h b/widget/cocoa/TextInputHandler.h index c4e71ad71302..573e1345bae4 100644 --- a/widget/cocoa/TextInputHandler.h +++ b/widget/cocoa/TextInputHandler.h @@ -529,6 +529,9 @@ protected: // String which are included in [mKeyEvent characters] and already handled // by InsertText() call(s). nsString mInsertedString; + // Unique id associated with a keydown / keypress event. It's ok if this + // wraps over long periods. + uint32_t mUniqueId; // Whether keydown event was consumed by web contents or chrome contents. bool mKeyDownHandled; // Whether keypress event was dispatched for mKeyEvent. @@ -542,15 +545,19 @@ protected: // if it dispatches keypress event. bool mCompositionDispatched; - KeyEventState() : mKeyEvent(nullptr) + KeyEventState() + : mKeyEvent(nullptr) + , mUniqueId(0) { Clear(); - } + } - explicit KeyEventState(NSEvent* aNativeKeyEvent) : mKeyEvent(nullptr) + explicit KeyEventState(NSEvent* aNativeKeyEvent, uint32_t aUniqueId = 0) + : mKeyEvent(nullptr) + , mUniqueId(0) { Clear(); - Set(aNativeKeyEvent); + Set(aNativeKeyEvent, aUniqueId); } KeyEventState(const KeyEventState &aOther) = delete; @@ -560,11 +567,12 @@ protected: Clear(); } - void Set(NSEvent* aNativeKeyEvent) + void Set(NSEvent* aNativeKeyEvent, uint32_t aUniqueId = 0) { NS_PRECONDITION(aNativeKeyEvent, "aNativeKeyEvent must not be NULL"); Clear(); mKeyEvent = [aNativeKeyEvent retain]; + mUniqueId = aUniqueId; } void Clear() @@ -572,6 +580,7 @@ protected: if (mKeyEvent) { [mKeyEvent release]; mKeyEvent = nullptr; + mUniqueId = 0; } mInsertString = nullptr; mInsertedString.Truncate(); @@ -670,7 +679,7 @@ protected: /** * PushKeyEvent() adds the current key event to mCurrentKeyEvents. */ - KeyEventState* PushKeyEvent(NSEvent* aNativeKeyEvent) + KeyEventState* PushKeyEvent(NSEvent* aNativeKeyEvent, uint32_t aUniqueId = 0) { uint32_t nestCount = mCurrentKeyEvents.Length(); for (uint32_t i = 0; i < nestCount; i++) { @@ -681,10 +690,10 @@ protected: KeyEventState* keyEvent = nullptr; if (nestCount == 0) { - mFirstKeyEvent.Set(aNativeKeyEvent); + mFirstKeyEvent.Set(aNativeKeyEvent, aUniqueId); keyEvent = &mFirstKeyEvent; } else { - keyEvent = new KeyEventState(aNativeKeyEvent); + keyEvent = new KeyEventState(aNativeKeyEvent, aUniqueId); } return *mCurrentKeyEvents.AppendElement(keyEvent); } @@ -1111,10 +1120,11 @@ public: * KeyDown event handler. * * @param aNativeEvent A native NSKeyDown event. - * @return TRUE if the event is consumed by web contents - * or chrome contents. Otherwise, FALSE. + * @param aUniqueId A unique ID for the event. + * @return TRUE if the event is dispatched to web + * contents or chrome contents. Otherwise, FALSE. */ - bool HandleKeyDownEvent(NSEvent* aNativeEvent); + bool HandleKeyDownEvent(NSEvent* aNativeEvent, uint32_t aUniqueId); /** * KeyUp event handler. diff --git a/widget/cocoa/TextInputHandler.mm b/widget/cocoa/TextInputHandler.mm index 353303fb833b..f98158d972be 100644 --- a/widget/cocoa/TextInputHandler.mm +++ b/widget/cocoa/TextInputHandler.mm @@ -1566,7 +1566,8 @@ TextInputHandler::~TextInputHandler() } bool -TextInputHandler::HandleKeyDownEvent(NSEvent* aNativeEvent) +TextInputHandler::HandleKeyDownEvent(NSEvent* aNativeEvent, + uint32_t aUniqueId) { NS_OBJC_BEGIN_TRY_ABORT_BLOCK_RETURN; @@ -1601,7 +1602,7 @@ TextInputHandler::HandleKeyDownEvent(NSEvent* aNativeEvent) RefPtr widget(mWidget); - KeyEventState* currentKeyEvent = PushKeyEvent(aNativeEvent); + KeyEventState* currentKeyEvent = PushKeyEvent(aNativeEvent, aUniqueId); AutoKeyEventStateCleaner remover(this); ComplexTextInputPanel* ctiPanel = ComplexTextInputPanel::GetSharedComplexTextInputPanel(); @@ -2807,6 +2808,12 @@ IMEInputHandler::WillDispatchKeyboardEvent( KeyEventState* currentKeyEvent = static_cast(aData); NSEvent* nativeEvent = currentKeyEvent->mKeyEvent; nsAString* insertString = currentKeyEvent->mInsertString; + if (aKeyboardEvent.mMessage == eKeyPress && aIndexOfKeypress == 0 && + (!insertString || insertString->IsEmpty())) { + // Inform the child process that this is an event that we want a reply + // from. + aKeyboardEvent.mFlags.mWantReplyFromContentProcess = true; + } if (KeyboardLayoutOverrideRef().mOverrideEnabled) { TISInputSourceWrapper tis; tis.InitByLayoutID(KeyboardLayoutOverrideRef().mKeyboardLayout, true); @@ -4706,6 +4713,7 @@ TextInputHandlerBase::KeyEventState::InitKeyEvent( keyCode:[mKeyEvent keyCode]]; } + aKeyEvent.mUniqueId = mUniqueId; aHandler->InitKeyEvent(nativeEvent, aKeyEvent, mInsertString); NS_OBJC_END_TRY_ABORT_BLOCK; diff --git a/widget/cocoa/nsChildView.h b/widget/cocoa/nsChildView.h index f65bca535095..20220b0fbb06 100644 --- a/widget/cocoa/nsChildView.h +++ b/widget/cocoa/nsChildView.h @@ -262,6 +262,10 @@ class WidgetRenderingContext; - (void)setUsingOMTCompositor:(BOOL)aUseOMTC; - (NSEvent*)lastKeyDownEvent; + ++ (uint32_t)sUniqueKeyEventId; + ++ (NSMutableDictionary*)sNativeKeyEventsMap; @end class ChildViewMouseTracker { @@ -375,6 +379,8 @@ public: virtual bool HasPendingInputEvent() override; + bool SendEventToNativeMenuSystem(NSEvent* aEvent); + virtual void PostHandleKeyEvent(mozilla::WidgetKeyboardEvent* aEvent) override; virtual nsresult ActivateNativeMenuItemAt(const nsAString& indexString) override; virtual nsresult ForceUpdateNativeMenuAt(const nsAString& indexString) override; virtual MOZ_MUST_USE nsresult diff --git a/widget/cocoa/nsChildView.mm b/widget/cocoa/nsChildView.mm index 0d457d26b629..a78aa1ebc7ca 100644 --- a/widget/cocoa/nsChildView.mm +++ b/widget/cocoa/nsChildView.mm @@ -154,6 +154,11 @@ static uint32_t gNumberOfWidgetsNeedingEventThread = 0; static bool sIsTabletPointerActivated = false; +static uint32_t sUniqueKeyEventId = 0; + +static NSMutableDictionary* sNativeKeyEventsMap = + [NSMutableDictionary dictionary]; + @interface ChildView(Private) // sets up our view, attaching it to its owning gecko view @@ -1249,6 +1254,50 @@ static NSMenuItem* NativeMenuItemWithLocation(NSMenu* menubar, NSString* locatio return nil; } +bool +nsChildView::SendEventToNativeMenuSystem(NSEvent* aEvent) +{ + bool handled = false; + nsCocoaWindow* widget = GetXULWindowWidget(); + if (widget) { + nsMenuBarX* mb = widget->GetMenuBar(); + if (mb) { + // Check if main menu wants to handle the event. + handled = mb->PerformKeyEquivalent(aEvent); + } + } + + if (!handled && sApplicationMenu) { + // Check if application menu wants to handle the event. + handled = [sApplicationMenu performKeyEquivalent:aEvent]; + } + + return handled; +} + +void +nsChildView::PostHandleKeyEvent(mozilla::WidgetKeyboardEvent* aEvent) +{ + NS_OBJC_BEGIN_TRY_ABORT_BLOCK; + + // We always allow keyboard events to propagate to keyDown: but if they are + // not handled we give menu items a chance to act. This allows for handling of + // custom shortcuts. Note that existing shortcuts cannot be reassigned yet and + // will have been handled by keyDown: before we get here. + NSEvent* cocoaEvent = + [sNativeKeyEventsMap objectForKey:@(aEvent->mUniqueId)]; + [sNativeKeyEventsMap removeObjectForKey:@(aEvent->mUniqueId)]; + if (!cocoaEvent) { + return; + } + + if (SendEventToNativeMenuSystem(cocoaEvent)) { + aEvent->PreventDefault(); + } + + NS_OBJC_END_TRY_ABORT_BLOCK; +} + // Used for testing native menu system structure and event handling. nsresult nsChildView::ActivateNativeMenuItemAt(const nsAString& indexString) @@ -5549,29 +5598,21 @@ GetIntegerDeltaForEvent(NSEvent* aEvent) #endif // #if !defined(RELEASE_OR_BETA) || defined(DEBUG) nsAutoRetainCocoaObject kungFuDeathGrip(self); - bool handled = false; - if (mGeckoChild && mTextInputHandler) { - handled = mTextInputHandler->HandleKeyDownEvent(theEvent); - } - - // We always allow keyboard events to propagate to keyDown: but if they are - // not handled we give menu items a chance to act. This allows for handling of - // custom shortcuts. Note that existing shortcuts cannot be reassigned yet and - // will have been handled by keyDown: before we get here. - if (!handled && mGeckoChild) { - nsCocoaWindow* widget = mGeckoChild->GetXULWindowWidget(); - if (widget) { - nsMenuBarX* mb = widget->GetMenuBar(); - if (mb) { - // Check if main menu wants to handle the event. - handled = mb->PerformKeyEquivalent(theEvent); - } + if (mGeckoChild) { + if (mTextInputHandler) { + sUniqueKeyEventId++; + [sNativeKeyEventsMap setObject:theEvent forKey:@(sUniqueKeyEventId)]; + // Purge old native events, in case we're still holding on to them. We + // keep at most 10 references to 10 different native events. + [sNativeKeyEventsMap removeObjectForKey:@(sUniqueKeyEventId - 10)]; + mTextInputHandler->HandleKeyDownEvent(theEvent, sUniqueKeyEventId); + } else { + // There was no text input handler. Offer the event to the native menu + // system to check if there are any registered custom shortcuts for this + // event. + mGeckoChild->SendEventToNativeMenuSystem(theEvent); } } - if (!handled && sApplicationMenu) { - // Check if application menu wants to handle the event. - handled = [sApplicationMenu performKeyEquivalent:theEvent]; - } NS_OBJC_END_TRY_ABORT_BLOCK; } @@ -6499,6 +6540,10 @@ nsChildView::GetSelectionAsPlaintext(nsAString& aResult) #endif /* ACCESSIBILITY */ ++ (uint32_t)sUniqueKeyEventId { return sUniqueKeyEventId; } + ++ (NSMutableDictionary*)sNativeKeyEventsMap { return sNativeKeyEventsMap; } + @end #pragma mark - diff --git a/widget/nsBaseWidget.cpp b/widget/nsBaseWidget.cpp index 3a85f8bdf973..1ff8d2dc1a6e 100644 --- a/widget/nsBaseWidget.cpp +++ b/widget/nsBaseWidget.cpp @@ -2283,6 +2283,11 @@ nsIWidget::OnWindowedPluginKeyEvent(const NativeEventData& aKeyEventData, return NS_ERROR_NOT_IMPLEMENTED; } +void +nsIWidget::PostHandleKeyEvent(mozilla::WidgetKeyboardEvent* aEvent) +{ +} + namespace mozilla { namespace widget { diff --git a/widget/nsIWidget.h b/widget/nsIWidget.h index 0c7ec3adfd31..94258bf0b203 100644 --- a/widget/nsIWidget.h +++ b/widget/nsIWidget.h @@ -1687,6 +1687,14 @@ private: static int32_t sPointerIdCounter; public: + /** + * If key events have not been handled by content or XBL handlers, they can + * be offered to the system (for custom application shortcuts set in system + * preferences, for example). + */ + virtual void + PostHandleKeyEvent(mozilla::WidgetKeyboardEvent* aEvent); + /** * Activates a native menu item at the position specified by the index * string. The index string is a string of positive integers separated