diff --git a/dom/events/EventStateManager.cpp b/dom/events/EventStateManager.cpp index 12400c77ab9b..05891c7996bb 100644 --- a/dom/events/EventStateManager.cpp +++ b/dom/events/EventStateManager.cpp @@ -763,11 +763,34 @@ EventStateManager::PreHandleEvent(nsPresContext* aPresContext, WidgetKeyboardEvent* keyEvent = aEvent->AsKeyboardEvent(); if (keyEvent->ModifiersMatchWithAccessKey(AccessKeyType::eChrome) || keyEvent->ModifiersMatchWithAccessKey(AccessKeyType::eContent)) { - AutoTArray accessCharCodes; - keyEvent->GetAccessKeyCandidates(accessCharCodes); + // If the eKeyPress event will be sent to a remote process, this + // process needs to wait reply from the remote process for checking if + // preceding eKeyDown event is consumed. If preceding eKeyDown event + // is consumed in the remote process, TabChild won't send the event + // back to this process. So, only when this process receives a reply + // eKeyPress event in TabParent, we should handle accesskey in this + // process. + if (IsRemoteTarget(GetFocusedContent())) { + // However, if there is no accesskey target for the key combination, + // we don't need to wait reply from the remote process. Otherwise, + // Mark the event as waiting reply from remote process and stop + // propagation in this process. + if (CheckIfEventMatchesAccessKey(keyEvent, aPresContext)) { + keyEvent->StopPropagation(); + keyEvent->MarkAsWaitingReplyFromRemoteProcess(); + } + } + // If the event target is in this process, we can handle accesskey now + // since if preceding eKeyDown event was consumed, eKeyPress event + // won't be dispatched by widget. So, coming eKeyPress event means + // that the preceding eKeyDown event wasn't consumed in this case. + else { + AutoTArray accessCharCodes; + keyEvent->GetAccessKeyCandidates(accessCharCodes); - if (HandleAccessKey(keyEvent, aPresContext, accessCharCodes)) { - *aStatus = nsEventStatus_eConsumeNoDefault; + if (HandleAccessKey(keyEvent, aPresContext, accessCharCodes)) { + *aStatus = nsEventStatus_eConsumeNoDefault; + } } } } @@ -1102,7 +1125,13 @@ HandleAccessKeyInRemoteChild(TabParent* aTabParent, void* aArg) bool active; aTabParent->GetDocShellIsActive(&active); if (active) { - accessKeyInfo->event->mAccessKeyForwardedToChild = true; + // Even if there is no target for the accesskey in this process, + // the event may match with a content accesskey. If so, the keyboard + // event should be handled with reply event for preventing double action. + // (e.g., Alt+Shift+F on Windows may focus a content in remote and open + // "File" menu.) + accessKeyInfo->event->StopPropagation(); + accessKeyInfo->event->MarkAsWaitingReplyFromRemoteProcess(); aTabParent->HandleAccessKey(*accessKeyInfo->event, accessKeyInfo->charCodes); return true; @@ -1202,16 +1231,22 @@ EventStateManager::WalkESMTreeToHandleAccessKey( if (aExecute && aEvent->ModifiersMatchWithAccessKey(AccessKeyType::eContent) && mDocument && mDocument->GetWindow()) { - // If the focus is currently on a node with a TabParent, the key event will - // get forwarded to the child process and HandleAccessKey called from there. + // If the focus is currently on a node with a TabParent, the key event + // should've gotten forwarded to the child process and HandleAccessKey + // called from there. + if (TabParent::GetFrom(GetFocusedContent())) { + // If access key may be only in remote contents, this method won't handle + // access key synchronously. In this case, only reply event should reach + // here. + MOZ_ASSERT(aEvent->IsHandledInRemoteProcess() || + !aEvent->IsWaitingReplyFromRemoteProcess()); + } // If focus is somewhere else, then we need to check the remote children. - nsFocusManager* fm = nsFocusManager::GetFocusManager(); - nsIContent* focusedContent = fm ? fm->GetFocusedContent() : nullptr; - if (TabParent::GetFrom(focusedContent)) { - // A remote child process is focused. The key event should get sent to - // the child process. - aEvent->mAccessKeyForwardedToChild = true; - } else { + // However, if the event has already been handled in a remote process, + // then, focus is moved from the remote process after posting the event. + // In such case, we shouldn't retry to handle access keys in remote + // processes. + else if (!aEvent->IsHandledInRemoteProcess()) { AccessKeyInfo accessKeyInfo(aEvent, aAccessCharCodes); nsContentUtils::CallOnAllRemoteChildren(mDocument->GetWindow(), HandleAccessKeyInRemoteChild, diff --git a/dom/ipc/TabChild.cpp b/dom/ipc/TabChild.cpp index efae1e69b68a..ce774c0650a4 100644 --- a/dom/ipc/TabChild.cpp +++ b/dom/ipc/TabChild.cpp @@ -1599,7 +1599,7 @@ TabChild::RecvRealMouseButtonEvent(const WidgetMouseEvent& aEvent, localEvent.mWidget = mPuppetWidget; APZCCallbackHelper::ApplyCallbackTransform(localEvent, aGuid, mPuppetWidget->GetDefaultScale()); - APZCCallbackHelper::DispatchWidgetEvent(localEvent); + DispatchWidgetEventViaAPZ(localEvent); if (aInputBlockId && aEvent.mFlags.mHandledByAPZ) { mAPZEventState->ProcessMouseEvent(aEvent, aGuid, aInputBlockId); @@ -1648,6 +1648,13 @@ TabChild::MaybeCoalesceWheelEvent(const WidgetWheelEvent& aEvent, return false; } +nsEventStatus +TabChild::DispatchWidgetEventViaAPZ(WidgetGUIEvent& aEvent) +{ + aEvent.ResetWaitingReplyFromRemoteProcessState(); + return APZCCallbackHelper::DispatchWidgetEvent(aEvent); +} + void TabChild::MaybeDispatchCoalescedWheelEvent() { @@ -1678,7 +1685,7 @@ TabChild::DispatchWheelEvent(const WidgetWheelEvent& aEvent, localEvent.mWidget = mPuppetWidget; APZCCallbackHelper::ApplyCallbackTransform(localEvent, aGuid, mPuppetWidget->GetDefaultScale()); - APZCCallbackHelper::DispatchWidgetEvent(localEvent); + DispatchWidgetEventViaAPZ(localEvent); if (localEvent.mCanTriggerSwipe) { SendRespondStartSwipeEvent(aInputBlockId, localEvent.TriggersSwipe()); @@ -1745,7 +1752,7 @@ TabChild::RecvRealTouchEvent(const WidgetTouchEvent& aEvent, } // Dispatch event to content (potentially a long-running operation) - nsEventStatus status = APZCCallbackHelper::DispatchWidgetEvent(localEvent); + nsEventStatus status = DispatchWidgetEventViaAPZ(localEvent); if (!AsyncPanZoomEnabled()) { // We shouldn't have any e10s platforms that have touch events enabled @@ -1805,7 +1812,7 @@ TabChild::RecvRealDragEvent(const WidgetDragEvent& aEvent, } } - APZCCallbackHelper::DispatchWidgetEvent(localEvent); + DispatchWidgetEventViaAPZ(localEvent); return IPC_OK(); } @@ -1814,7 +1821,7 @@ TabChild::RecvPluginEvent(const WidgetPluginEvent& aEvent) { WidgetPluginEvent localEvent(aEvent); localEvent.mWidget = mPuppetWidget; - nsEventStatus status = APZCCallbackHelper::DispatchWidgetEvent(localEvent); + nsEventStatus status = DispatchWidgetEventViaAPZ(localEvent); if (status != nsEventStatus_eConsumeNoDefault) { // If not consumed, we should call default action SendDefaultProcOfPluginEvent(aEvent); @@ -1916,7 +1923,7 @@ TabChild::RecvRealKeyEvent(const WidgetKeyboardEvent& aEvent) WidgetKeyboardEvent localEvent(aEvent); localEvent.mWidget = mPuppetWidget; localEvent.mUniqueId = aEvent.mUniqueId; - nsEventStatus status = APZCCallbackHelper::DispatchWidgetEvent(localEvent); + nsEventStatus status = DispatchWidgetEventViaAPZ(localEvent); // Update the end time of the possible repeated event so that we can skip // some incoming events in case event handling took long time. @@ -1931,16 +1938,21 @@ TabChild::RecvRealKeyEvent(const WidgetKeyboardEvent& aEvent) } // If a response is desired from the content process, resend the key event. - // If mAccessKeyForwardedToChild is set, then don't resend the key event yet - // as RecvHandleAccessKey will do this. - if (localEvent.WantReplyFromContentProcess()) { + if (aEvent.WantReplyFromContentProcess()) { + // If the event's default isn't prevented but the status is no default, + // That means that the event was consumed by EventStateManager or something + // which is not a usual event handler. In such case, prevent its default + // as a default handler. For example, when an eKeyPress event matches + // with a content accesskey, and it's executed, peventDefault() of the + // event won't be called but the status is set to "no default". Then, + // the event shouldn't be handled by nsMenuBarListener in the main process. + if (!localEvent.DefaultPrevented() && + status == nsEventStatus_eConsumeNoDefault) { + localEvent.PreventDefault(); + } SendReplyKeyEvent(localEvent); } - if (localEvent.mAccessKeyForwardedToChild) { - SendAccessKeyNotHandled(localEvent); - } - return IPC_OK(); } @@ -1962,7 +1974,7 @@ TabChild::RecvCompositionEvent(const WidgetCompositionEvent& aEvent) { WidgetCompositionEvent localEvent(aEvent); localEvent.mWidget = mPuppetWidget; - APZCCallbackHelper::DispatchWidgetEvent(localEvent); + DispatchWidgetEventViaAPZ(localEvent); Unused << SendOnEventNeedingAckHandled(aEvent.mMessage); return IPC_OK(); } @@ -1972,7 +1984,7 @@ TabChild::RecvSelectionEvent(const WidgetSelectionEvent& aEvent) { WidgetSelectionEvent localEvent(aEvent); localEvent.mWidget = mPuppetWidget; - APZCCallbackHelper::DispatchWidgetEvent(localEvent); + DispatchWidgetEventViaAPZ(localEvent); Unused << SendOnEventNeedingAckHandled(aEvent.mMessage); return IPC_OK(); } diff --git a/dom/ipc/TabChild.h b/dom/ipc/TabChild.h index 3bd9272881d1..438ca410b188 100644 --- a/dom/ipc/TabChild.h +++ b/dom/ipc/TabChild.h @@ -802,6 +802,11 @@ private: void MaybeDispatchCoalescedWheelEvent(); + /** + * Dispatch aEvent on aEvent.mWidget. + */ + nsEventStatus DispatchWidgetEventViaAPZ(WidgetGUIEvent& aEvent); + void DispatchWheelEvent(const WidgetWheelEvent& aEvent, const ScrollableLayerGuid& aGuid, const uint64_t& aInputBlockId); diff --git a/dom/ipc/TabParent.cpp b/dom/ipc/TabParent.cpp index cdf4f68581d6..18f546ff8a2f 100644 --- a/dom/ipc/TabParent.cpp +++ b/dom/ipc/TabParent.cpp @@ -2046,7 +2046,23 @@ TabParent::RecvReplyKeyEvent(const WidgetKeyboardEvent& aEvent) AutoHandlingUserInputStatePusher userInpStatePusher(localEvent.IsTrusted(), &localEvent, doc); - EventDispatcher::Dispatch(mFrameElement, presContext, &localEvent); + nsEventStatus status = nsEventStatus_eIgnore; + + // Handle access key in this process before dispatching reply event because + // ESM handles it before dispatching the event to the DOM tree. + if (localEvent.mMessage == eKeyPress && + (localEvent.ModifiersMatchWithAccessKey(AccessKeyType::eChrome) || + localEvent.ModifiersMatchWithAccessKey(AccessKeyType::eContent))) { + RefPtr esm = presContext->EventStateManager(); + AutoTArray accessCharCodes; + localEvent.GetAccessKeyCandidates(accessCharCodes); + if (esm->HandleAccessKey(&localEvent, presContext, accessCharCodes)) { + status = nsEventStatus_eConsumeNoDefault; + } + } + + EventDispatcher::Dispatch(mFrameElement, presContext, &localEvent, nullptr, + &status); if (!localEvent.DefaultPrevented() && !localEvent.mFlags.mIsSynthesizedForTests) { @@ -2065,10 +2081,16 @@ TabParent::RecvAccessKeyNotHandled(const WidgetKeyboardEvent& aEvent) { NS_ENSURE_TRUE(mFrameElement, IPC_OK()); + // This is called only when this process had focus and HandleAccessKey + // message was posted to all remote process and each remote process didn't + // execute any content access keys. + // XXX If there were two or more remote processes, this may be called + // twice or more for a keyboard event, that must be a bug. But how to + // detect if received event has already been handled? + WidgetKeyboardEvent localEvent(aEvent); localEvent.MarkAsHandledInRemoteProcess(); localEvent.mMessage = eAccessKeyNotFound; - localEvent.mAccessKeyForwardedToChild = false; // Here we convert the WidgetEvent that we received to an nsIDOMEvent // to be able to dispatch it to the element as the target element. diff --git a/layout/base/PresShell.cpp b/layout/base/PresShell.cpp index 468f18320dfe..4a4335b61e1f 100644 --- a/layout/base/PresShell.cpp +++ b/layout/base/PresShell.cpp @@ -8161,7 +8161,11 @@ PresShell::HandleEventInternal(WidgetEvent* aEvent, if (aEvent->mClass == eKeyboardEventClass) { nsContentUtils::SetIsHandlingKeyBoardEvent(true); } - if (aEvent->IsAllowedToDispatchDOMEvent()) { + // If EventStateManager or something wants reply from remote process, + // PresShell shouldn't dispatch the event into the DOM tree because they + // don't have a chance to stop propagation in the system event group. + if (aEvent->IsAllowedToDispatchDOMEvent() && + !aEvent->IsWaitingReplyFromRemoteProcess()) { MOZ_ASSERT(nsContentUtils::IsSafeToRunScript(), "Somebody changed aEvent to cause a DOM event!"); nsPresShellEventCB eventCB(this); diff --git a/layout/xul/nsMenuBarListener.cpp b/layout/xul/nsMenuBarListener.cpp index e9211c15b95b..7048d5c63fca 100644 --- a/layout/xul/nsMenuBarListener.cpp +++ b/layout/xul/nsMenuBarListener.cpp @@ -271,8 +271,7 @@ nsMenuBarListener::KeyPress(nsIDOMEvent* aKeyEvent) // the mozaccesskeynotfound event before handling accesskeys. WidgetKeyboardEvent* nativeKeyEvent = aKeyEvent->WidgetEventPtr()->AsKeyboardEvent(); - if (!nativeKeyEvent || - (nativeKeyEvent && nativeKeyEvent->mAccessKeyForwardedToChild)) { + if (!nativeKeyEvent) { return NS_OK; } @@ -301,6 +300,9 @@ nsMenuBarListener::KeyPress(nsIDOMEvent* aKeyEvent) // so, we'll know the menu got activated. nsMenuFrame* result = mMenuBarFrame->FindMenuWithShortcut(keyEvent); if (result) { + // TODO: If the event target is a remote process and not stopped + // sending the event to it, we need to mark the event as + // waiting reply from remote process and stop handling the event. mMenuBarFrame->SetActiveByKeyboard(); mMenuBarFrame->SetActive(true); result->OpenMenu(true); diff --git a/widget/BasicEvents.h b/widget/BasicEvents.h index 95c18ee1f574..758f154e440b 100644 --- a/widget/BasicEvents.h +++ b/widget/BasicEvents.h @@ -252,6 +252,20 @@ public: mNoRemoteProcessDispatch = false; mWantReplyFromContentProcess = true; } + /** + * Reset "waiting reply from remote process" state. This is useful when + * you dispatch a copy of an event coming from different process. + */ + inline void ResetWaitingReplyFromRemoteProcessState() + { + if (IsWaitingReplyFromRemoteProcess()) { + // FYI: mWantReplyFromContentProcess is also used for indicating + // "handled in remote process" state. Therefore, only when + // IsWaitingReplyFromRemoteProcess() returns true, this should + // reset the flag. + mWantReplyFromContentProcess = false; + } + } /** * Return true if the event handler should wait reply event. I.e., if this * returns true, any event handler should do nothing with the event. @@ -301,6 +315,12 @@ public: { MOZ_ASSERT(!IsCrossProcessForwardingStopped()); mPostedToRemoteProcess = false; + // Ignore propagation state in the different process if it's marked as + // "waiting reply from remote process" because the process needs to + // stop propagation in the process until receiving a reply event. + if (IsWaitingReplyFromRemoteProcess()) { + mPropagationStopped = mImmediatePropagationStopped = false; + } } /** * Return true if the event has been posted to a remote process. @@ -629,6 +649,14 @@ public: { mFlags.MarkAsWaitingReplyFromRemoteProcess(); } + /** + * Reset "waiting reply from remote process" state. This is useful when + * you dispatch a copy of an event coming from different process. + */ + inline void ResetWaitingReplyFromRemoteProcessState() + { + mFlags.ResetWaitingReplyFromRemoteProcessState(); + } /** * Return true if the event handler should wait reply event. I.e., if this * returns true, any event handler should do nothing with the event. diff --git a/widget/TextEvents.h b/widget/TextEvents.h index 3ceb6568b6fc..c1f5178fe025 100644 --- a/widget/TextEvents.h +++ b/widget/TextEvents.h @@ -154,7 +154,6 @@ protected: , mCharCode(0) , mPseudoCharCode(0) , mLocation(eKeyLocationStandard) - , mAccessKeyForwardedToChild(false) , mUniqueId(0) #ifdef XP_MACOSX , mNativeModifierFlags(0) @@ -183,7 +182,6 @@ public: , mCharCode(0) , mPseudoCharCode(0) , mLocation(eKeyLocationStandard) - , mAccessKeyForwardedToChild(false) , mUniqueId(0) #ifdef XP_MACOSX , mNativeModifierFlags(0) @@ -279,11 +277,6 @@ public: uint32_t mPseudoCharCode; // One of eKeyLocation* uint32_t mLocation; - // True if accesskey handling was forwarded to the child via - // TabParent::HandleAccessKey. In this case, parent process menu access key - // 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. It's ok if this wraps // over long periods. uint32_t mUniqueId; @@ -509,7 +502,6 @@ public: mAlternativeCharCodes = aEvent.mAlternativeCharCodes; mIsRepeat = aEvent.mIsRepeat; mIsComposing = aEvent.mIsComposing; - mAccessKeyForwardedToChild = aEvent.mAccessKeyForwardedToChild; mKeyNameIndex = aEvent.mKeyNameIndex; mCodeNameIndex = aEvent.mCodeNameIndex; mKeyValue = aEvent.mKeyValue; diff --git a/widget/nsGUIEventIPC.h b/widget/nsGUIEventIPC.h index df254be9681e..7ca55b7048d4 100644 --- a/widget/nsGUIEventIPC.h +++ b/widget/nsGUIEventIPC.h @@ -449,7 +449,6 @@ struct ParamTraits WriteParam(aMsg, aParam.mPseudoCharCode); WriteParam(aMsg, aParam.mAlternativeCharCodes); WriteParam(aMsg, aParam.mIsRepeat); - WriteParam(aMsg, aParam.mAccessKeyForwardedToChild); WriteParam(aMsg, aParam.mLocation); WriteParam(aMsg, aParam.mUniqueId); WriteParam(aMsg, aParam.mIsSynthesizedByTIP); @@ -487,7 +486,6 @@ struct ParamTraits ReadParam(aMsg, aIter, &aResult->mPseudoCharCode) && ReadParam(aMsg, aIter, &aResult->mAlternativeCharCodes) && ReadParam(aMsg, aIter, &aResult->mIsRepeat) && - ReadParam(aMsg, aIter, &aResult->mAccessKeyForwardedToChild) && ReadParam(aMsg, aIter, &aResult->mLocation) && ReadParam(aMsg, aIter, &aResult->mUniqueId) && ReadParam(aMsg, aIter, &aResult->mIsSynthesizedByTIP) &&