From 46dd8a0d405edf393700142a7ba4d244a2386b41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 24 Jan 2023 15:43:49 +0000 Subject: [PATCH] Bug 1811487 - Clean-up popup hide / rollup APIs. r=cmartin,stransky I'm about to extend them for bug 1811486, where I want to force in some cases the rolled up popups to hide synchronously. These APIs use a ton of boolean arguments that make them error prone, so refactor them a bit to use strongly typed enums and flags. Differential Revision: https://phabricator.services.mozilla.com/D167381 --- ..._PanelMultiView_toggle_with_other_popup.js | 4 +- .../test/browser_PanelMultiView_keyboard.js | 14 +- dom/xul/XULButtonElement.cpp | 12 +- dom/xul/XULPopupElement.cpp | 9 +- dom/xul/nsXULPopupListener.cpp | 4 +- layout/xul/nsMenuBarListener.cpp | 2 +- layout/xul/nsMenuPopupFrame.cpp | 4 +- layout/xul/nsXULPopupManager.cpp | 163 +++++++++--------- layout/xul/nsXULPopupManager.h | 53 +++--- layout/xul/nsXULTooltipListener.cpp | 2 +- view/nsView.cpp | 5 +- widget/cocoa/nsChildView.mm | 91 +++++----- widget/cocoa/nsCocoaWindow.mm | 2 +- widget/cocoa/nsMenuX.mm | 2 +- widget/cocoa/nsToolkit.mm | 2 +- widget/gtk/nsWindow.cpp | 87 +++++----- widget/gtk/nsWindow.h | 3 +- widget/nsBaseDragService.cpp | 2 +- widget/nsIRollupListener.h | 29 ++-- widget/windows/nsWindow.cpp | 16 +- 20 files changed, 264 insertions(+), 242 deletions(-) diff --git a/browser/components/customizableui/test/browser_1484275_PanelMultiView_toggle_with_other_popup.js b/browser/components/customizableui/test/browser_1484275_PanelMultiView_toggle_with_other_popup.js index d7b3b8760f4d..651571df71f9 100644 --- a/browser/components/customizableui/test/browser_1484275_PanelMultiView_toggle_with_other_popup.js +++ b/browser/components/customizableui/test/browser_1484275_PanelMultiView_toggle_with_other_popup.js @@ -43,8 +43,8 @@ add_task(async function test_PanelMultiView_toggle_with_other_popup() { eventTypeToWait: "mouseup", }); - if (AppConstants.platform == "win") { - // On Windows, the operation will close both popups. + // On Windows and macOS, the operation will close both popups. + if (AppConstants.platform == "win" || AppConstants.platform == "macosx") { await gCUITestUtils.hidePanelMultiView(PanelUI.panel, clickFn); await new Promise(resolve => executeSoon(resolve)); diff --git a/browser/components/customizableui/test/browser_PanelMultiView_keyboard.js b/browser/components/customizableui/test/browser_PanelMultiView_keyboard.js index ae9a25b7bf79..435a6dc27c63 100644 --- a/browser/components/customizableui/test/browser_PanelMultiView_keyboard.js +++ b/browser/components/customizableui/test/browser_PanelMultiView_keyboard.js @@ -290,14 +290,11 @@ add_task(async function testTabOpenMenulist() { await shown; ok(gMainMenulist.open, "menulist open"); let menuHidden = BrowserTestUtils.waitForEvent(popup, "popuphidden"); - let panelHidden = BrowserTestUtils.waitForEvent(gPanel, "popuphidden"); EventUtils.synthesizeKey("KEY_Tab"); await menuHidden; ok(!gMainMenulist.open, "menulist closed after Tab"); - // Tab in an open menulist closes the menulist, but also dismisses the panel - // above it (bug 1566673). So, we just wait for the panel to hide rather than - // using hidePopup(). - await panelHidden; + is(gPanel.state, "open", "Panel should be open"); + await hidePopup(); }); if (AppConstants.platform == "macosx") { @@ -327,14 +324,11 @@ if (AppConstants.platform == "macosx") { ); let menuHidden = BrowserTestUtils.waitForEvent(popup, "popuphidden"); - let panelHidden = BrowserTestUtils.waitForEvent(gPanel, "popuphidden"); EventUtils.synthesizeKey("KEY_Tab"); await menuHidden; ok(!gMainMenulist.open, "menulist closed after Tab"); - // Tab in an open menulist closes the menulist, but also dismisses the panel - // above it (bug 1566673). So, we just wait for the panel to hide rather than - // using hidePopup(). - await panelHidden; + is(gPanel.state, "open", "Panel should be open"); + await hidePopup(); }); } diff --git a/dom/xul/XULButtonElement.cpp b/dom/xul/XULButtonElement.cpp index 709540f9b403..481a5f74705e 100644 --- a/dom/xul/XULButtonElement.cpp +++ b/dom/xul/XULButtonElement.cpp @@ -95,9 +95,9 @@ void XULButtonElement::HandleEnterKeyPress(WidgetEvent& aEvent) { #ifdef XP_WIN if (XULPopupElement* popup = GetContainingPopupElement()) { if (nsXULPopupManager* pm = nsXULPopupManager::GetInstance()) { - pm->HidePopup(popup, /* aHideChain = */ true, - /* aDeselectMenu = */ true, /* aAsynchronous = */ true, - /* aIsCancel = */ false); + pm->HidePopup( + popup, {HidePopupOption::HideChain, HidePopupOption::DeselectMenu, + HidePopupOption::Async}); } } #endif @@ -211,7 +211,11 @@ void XULButtonElement::CloseMenuPopup(bool aDeselectMenu) { return; } if (auto* popup = GetMenuPopupContent()) { - pm->HidePopup(popup, false, aDeselectMenu, true, false); + HidePopupOptions options{HidePopupOption::Async}; + if (aDeselectMenu) { + options += HidePopupOption::DeselectMenu; + } + pm->HidePopup(popup, options); } } diff --git a/dom/xul/XULPopupElement.cpp b/dom/xul/XULPopupElement.cpp index 22d86ed3ffde..29fde929a81e 100644 --- a/dom/xul/XULPopupElement.cpp +++ b/dom/xul/XULPopupElement.cpp @@ -106,9 +106,14 @@ void XULPopupElement::OpenPopupAtScreenRect(const nsAString& aPosition, void XULPopupElement::HidePopup(bool aCancel) { nsXULPopupManager* pm = nsXULPopupManager::GetInstance(); - if (pm) { - pm->HidePopup(this, false, true, false, aCancel); + if (!pm) { + return; } + HidePopupOptions options{HidePopupOption::DeselectMenu}; + if (aCancel) { + options += HidePopupOption::IsRollup; + } + pm->HidePopup(this, options); } static Modifiers ConvertModifiers(const ActivateMenuItemOptions& aModifiers) { diff --git a/dom/xul/nsXULPopupListener.cpp b/dom/xul/nsXULPopupListener.cpp index 825f7d15abd4..6f6ac3158274 100644 --- a/dom/xul/nsXULPopupListener.cpp +++ b/dom/xul/nsXULPopupListener.cpp @@ -170,7 +170,9 @@ void nsXULPopupListener::ClosePopup() { // popup is hidden. Use asynchronous hiding just to be safe so we don't // fire events during destruction. nsXULPopupManager* pm = nsXULPopupManager::GetInstance(); - if (pm) pm->HidePopup(mPopupContent, false, true, true, false); + if (pm) + pm->HidePopup(mPopupContent, + {HidePopupOption::DeselectMenu, HidePopupOption::Async}); mPopupContent = nullptr; // release the popup } } // ClosePopup diff --git a/layout/xul/nsMenuBarListener.cpp b/layout/xul/nsMenuBarListener.cpp index ed20499fa11b..caa4bd11f832 100644 --- a/layout/xul/nsMenuBarListener.cpp +++ b/layout/xul/nsMenuBarListener.cpp @@ -207,7 +207,7 @@ nsresult nsMenuBarListener::KeyUp(Event* aKeyEvent) { // handle key events when menubar is active and IME should be // disabled. if (nsXULPopupManager* pm = nsXULPopupManager::GetInstance()) { - pm->Rollup(0, false, nullptr, nullptr); + pm->Rollup({}); } // If menubar active state is changed or the menubar is destroyed // during closing the popups, we should do nothing anymore. diff --git a/layout/xul/nsMenuPopupFrame.cpp b/layout/xul/nsMenuPopupFrame.cpp index 4535b2031841..88adb3b64175 100644 --- a/layout/xul/nsMenuPopupFrame.cpp +++ b/layout/xul/nsMenuPopupFrame.cpp @@ -62,6 +62,7 @@ #include #include "X11UndefineNone.h" +#include "nsXULPopupManager.h" using namespace mozilla; using mozilla::dom::Document; @@ -2460,7 +2461,8 @@ void nsMenuPopupFrame::CheckForAnchorChange(nsRect& aRect) { if (pm) { // As the caller will be iterating over the open popups, hide // asyncronously. - pm->HidePopup(mContent, false, true, true, false); + pm->HidePopup(mContent, + {HidePopupOption::DeselectMenu, HidePopupOption::Async}); } return; diff --git a/layout/xul/nsXULPopupManager.cpp b/layout/xul/nsXULPopupManager.cpp index 1a9d68e227f7..0f02b571a595 100644 --- a/layout/xul/nsXULPopupManager.cpp +++ b/layout/xul/nsXULPopupManager.cpp @@ -278,14 +278,12 @@ nsXULPopupManager* nsXULPopupManager::GetInstance() { } bool nsXULPopupManager::RollupTooltips() { - return RollupInternal(RollupKind::Tooltip, UINT32_MAX, false, nullptr, - nullptr); + return RollupInternal(RollupKind::Tooltip, {}, nullptr); } -bool nsXULPopupManager::Rollup(uint32_t aCount, bool aFlush, - const LayoutDeviceIntPoint* aPos, +bool nsXULPopupManager::Rollup(const RollupOptions& aOptions, nsIContent** aLastRolledUp) { - return RollupInternal(RollupKind::Menu, aCount, aFlush, aPos, aLastRolledUp); + return RollupInternal(RollupKind::Menu, aOptions, aLastRolledUp); } bool nsXULPopupManager::RollupNativeMenu() { @@ -296,9 +294,8 @@ bool nsXULPopupManager::RollupNativeMenu() { return false; } -bool nsXULPopupManager::RollupInternal(RollupKind aKind, uint32_t aCount, - bool aFlush, - const LayoutDeviceIntPoint* pos, +bool nsXULPopupManager::RollupInternal(RollupKind aKind, + const RollupOptions& aOptions, nsIContent** aLastRolledUp) { if (aLastRolledUp) { *aLastRolledUp = nullptr; @@ -349,7 +346,7 @@ bool nsXULPopupManager::RollupInternal(RollupKind aKind, uint32_t aCount, // This would be used to allow adjusting the caret position in an // autocomplete field without hiding the popup for example. bool noRollupOnAnchor = - (!consume && pos && + (!consume && aOptions.mPoint && item->Frame()->GetContent()->AsElement()->AttrValueIs( kNameSpaceID_None, nsGkAtoms::norolluponanchor, nsGkAtoms::_true, eCaseMatters)); @@ -358,7 +355,7 @@ bool nsXULPopupManager::RollupInternal(RollupKind aKind, uint32_t aCount, // when the click was over the anchor. This way, clicking on a menu doesn't // reopen the menu. if ((consumeResult == ConsumeOutsideClicks_ParentOnly || noRollupOnAnchor) && - pos) { + aOptions.mPoint) { nsMenuPopupFrame* popupFrame = item->Frame(); CSSIntRect anchorRect = [&] { if (popupFrame->IsAnchored()) { @@ -399,7 +396,8 @@ bool nsXULPopupManager::RollupInternal(RollupKind aKind, uint32_t aCount, // event will get consumed, so here only a quick coordinates check is // done rather than a slower complete check of what is at that location. nsPresContext* presContext = item->Frame()->PresContext(); - CSSIntPoint posCSSPixels = presContext->DevPixelsToIntCSSPixels(*pos); + CSSIntPoint posCSSPixels = + presContext->DevPixelsToIntCSSPixels(*aOptions.mPoint); if (anchorRect.Contains(posCSSPixels)) { if (consumeResult == ConsumeOutsideClicks_ParentOnly) { consume = true; @@ -415,12 +413,13 @@ bool nsXULPopupManager::RollupInternal(RollupKind aKind, uint32_t aCount, return false; } - // if a number of popups to close has been specified, determine the last - // popup to close + // If a number of popups to close has been specified, determine the last + // popup to close. nsIContent* lastPopup = nullptr; - if (aCount != UINT32_MAX) { + uint32_t count = aOptions.mCount; + if (count && count != UINT32_MAX) { nsMenuChainItem* last = item; - while (--aCount && last->GetParent()) { + while (--count && last->GetParent()) { last = last->GetParent(); } if (last) { @@ -432,9 +431,12 @@ bool nsXULPopupManager::RollupInternal(RollupKind aKind, uint32_t aCount, RefPtr viewManager = presContext->PresShell()->GetViewManager(); - HidePopup(item->Content(), true, true, false, true, lastPopup); + HidePopup(item->Content(), + {HidePopupOption::HideChain, HidePopupOption::DeselectMenu, + HidePopupOption::IsRollup}, + lastPopup); - if (aFlush) { + if (aOptions.mFlush == FlushViews::Yes) { // The popup's visibility doesn't update until the minimize animation // has finished, so call UpdateWidgetGeometry to update it right away. viewManager->UpdateWidgetGeometry(); @@ -950,7 +952,7 @@ void nsXULPopupManager::OnNativeMenuClosed() { // menus. // Close the non-native menus now. This matches the HidePopup call in // nsXULMenuCommandEvent::Run. - HidePopup(mPopups->Content(), true, false, false, false); + HidePopup(mPopups->Content(), {HidePopupOption::HideChain}); } } @@ -1136,9 +1138,8 @@ nsMenuChainItem* nsXULPopupManager::FindPopup(nsIContent* aPopup) const { return nullptr; } -void nsXULPopupManager::HidePopup(nsIContent* aPopup, bool aHideChain, - bool aDeselectMenu, bool aAsynchronous, - bool aIsCancel, nsIContent* aLastPopup) { +void nsXULPopupManager::HidePopup(nsIContent* aPopup, HidePopupOptions aOptions, + nsIContent* aLastPopup) { if (mNativeMenu && mNativeMenu->Element() == aPopup) { RefPtr menu = mNativeMenu; (void)menu->Close(); @@ -1152,7 +1153,6 @@ void nsXULPopupManager::HidePopup(nsIContent* aPopup, bool aHideChain, nsMenuChainItem* foundPopup = FindPopup(aPopup); - bool deselectMenu = false; nsCOMPtr popupToHide, nextPopup, lastPopup; if (foundPopup) { @@ -1160,6 +1160,8 @@ void nsXULPopupManager::HidePopup(nsIContent* aPopup, bool aHideChain, // If this is a noautohide panel, remove it but don't close any other // panels. popupToHide = aPopup; + // XXX This preserves behavior but why is it the right thing to do? + aOptions -= HidePopupOption::DeselectMenu; } else { // At this point, foundPopup will be set to the found item in the list. If // foundPopup is the topmost menu, the one to remove, then there are no @@ -1189,14 +1191,15 @@ void nsXULPopupManager::HidePopup(nsIContent* aPopup, bool aHideChain, } } - deselectMenu = aDeselectMenu; popupToHide = topMenu->Content(); popupFrame = topMenu->Frame(); + const bool hideChain = aOptions.contains(HidePopupOption::HideChain); + // Close up another popup if there is one, and we are either hiding the // entire chain or the item to hide isn't the topmost popup. nsMenuChainItem* parent = topMenu->GetParent(); - if (parent && (aHideChain || topMenu != foundPopup)) { + if (parent && (hideChain || topMenu != foundPopup)) { while (parent && parent->IsNoAutoHide()) { parent = parent->GetParent(); } @@ -1206,41 +1209,42 @@ void nsXULPopupManager::HidePopup(nsIContent* aPopup, bool aHideChain, } } - lastPopup = aLastPopup ? aLastPopup : (aHideChain ? nullptr : aPopup); + lastPopup = aLastPopup ? aLastPopup : (hideChain ? nullptr : aPopup); } } else if (popupFrame->PopupState() == ePopupPositioning) { // When the popup is in the popuppositioning state, it will not be in the // mPopups list. We need another way to find it and make sure it does not // continue the popup showing process. - deselectMenu = aDeselectMenu; popupToHide = aPopup; } - if (popupToHide) { - nsPopupState state = popupFrame->PopupState(); - // If the popup is already being hidden, don't attempt to hide it again - if (state == ePopupHiding) { - return; - } + if (!popupToHide) { + return; + } - // Change the popup state to hiding. Don't set the hiding state if the - // popup is invisible, otherwise nsMenuPopupFrame::HidePopup will - // run again. In the invisible state, we just want the events to fire. - if (state != ePopupInvisible) { - popupFrame->SetPopupState(ePopupHiding); - } + nsPopupState state = popupFrame->PopupState(); + // If the popup is already being hidden, don't attempt to hide it again + if (state == ePopupHiding) { + return; + } - // For menus, popupToHide is always the frontmost item in the list to hide. - if (aAsynchronous) { - nsCOMPtr event = new nsXULPopupHidingEvent( - popupToHide, nextPopup, lastPopup, popupFrame->GetPopupType(), - deselectMenu, aIsCancel); - aPopup->OwnerDoc()->Dispatch(TaskCategory::Other, event.forget()); - } else { - RefPtr presContext = popupFrame->PresContext(); - FirePopupHidingEvent(popupToHide, nextPopup, lastPopup, presContext, - popupFrame->GetPopupType(), deselectMenu, aIsCancel); - } + // Change the popup state to hiding. Don't set the hiding state if the + // popup is invisible, otherwise nsMenuPopupFrame::HidePopup will + // run again. In the invisible state, we just want the events to fire. + if (state != ePopupInvisible) { + popupFrame->SetPopupState(ePopupHiding); + } + + // For menus, popupToHide is always the frontmost item in the list to hide. + if (aOptions.contains(HidePopupOption::Async)) { + nsCOMPtr event = + new nsXULPopupHidingEvent(popupToHide, nextPopup, lastPopup, + popupFrame->GetPopupType(), aOptions); + aPopup->OwnerDoc()->Dispatch(TaskCategory::Other, event.forget()); + } else { + RefPtr presContext = popupFrame->PresContext(); + FirePopupHidingEvent(popupToHide, nextPopup, lastPopup, presContext, + popupFrame->GetPopupType(), aOptions); } } @@ -1259,7 +1263,7 @@ void nsXULPopupManager::HideMenu(nsIContent* aMenu) { if (!popup) { return; } - HidePopup(popup, false, true, false, false); + HidePopup(popup, {HidePopupOption::DeselectMenu}); } // This is used to hide the popup after a transition finishes. @@ -1272,13 +1276,13 @@ class TransitionEnder final : public nsIDOMEventListener { virtual ~TransitionEnder() = default; public: - bool mDeselectMenu; + HidePopupOptions mOptions; NS_DECL_CYCLE_COLLECTING_ISUPPORTS NS_DECL_CYCLE_COLLECTION_CLASS(TransitionEnder) - TransitionEnder(nsIContent* aContent, bool aDeselectMenu) - : mContent(aContent), mDeselectMenu(aDeselectMenu) {} + TransitionEnder(nsIContent* aContent, HidePopupOptions aOptions) + : mContent(aContent), mOptions(aOptions) {} MOZ_CAN_RUN_SCRIPT NS_IMETHOD HandleEvent(Event* aEvent) override { mContent->RemoveSystemEventListener(u"transitionend"_ns, this, false); @@ -1293,7 +1297,7 @@ class TransitionEnder final : public nsIDOMEventListener { // the first one ending. if (RefPtr pm = nsXULPopupManager::GetInstance()) { pm->HidePopupCallback(mContent, popupFrame, nullptr, nullptr, - popupFrame->GetPopupType(), mDeselectMenu); + popupFrame->GetPopupType(), mOptions); } return NS_OK; @@ -1310,7 +1314,7 @@ NS_INTERFACE_MAP_END NS_IMPL_CYCLE_COLLECTION(TransitionEnder, mContent); void nsXULPopupManager::HidePopupCallback( nsIContent* aPopup, nsMenuPopupFrame* aPopupFrame, nsIContent* aNextPopup, - nsIContent* aLastPopup, PopupType aPopupType, bool aDeselectMenu) { + nsIContent* aLastPopup, PopupType aPopupType, HidePopupOptions aOptions) { if (mCloseTimer && mTimerMenu == aPopupFrame) { mCloseTimer->Cancel(); mCloseTimer = nullptr; @@ -1331,7 +1335,8 @@ void nsXULPopupManager::HidePopupCallback( } AutoWeakFrame weakFrame(aPopupFrame); - aPopupFrame->HidePopup(aDeselectMenu, ePopupClosed); + aPopupFrame->HidePopup(aOptions.contains(HidePopupOption::DeselectMenu), + ePopupClosed); NS_ENSURE_TRUE_VOID(weakFrame.IsAlive()); // send the popuphidden event synchronously. This event has no default @@ -1369,7 +1374,7 @@ void nsXULPopupManager::HidePopupCallback( RefPtr presContext = popupFrame->PresContext(); FirePopupHidingEvent(popupToHide, nextPopup, aLastPopup, presContext, - foundMenu->GetPopupType(), aDeselectMenu, false); + foundMenu->GetPopupType(), aOptions); } } } @@ -1626,10 +1631,12 @@ void nsXULPopupManager::BeginShowingPopup(const PendingPopup& aPendingPopup, } } -void nsXULPopupManager::FirePopupHidingEvent( - nsIContent* aPopup, nsIContent* aNextPopup, nsIContent* aLastPopup, - nsPresContext* aPresContext, PopupType aPopupType, bool aDeselectMenu, - bool aIsCancel) { +void nsXULPopupManager::FirePopupHidingEvent(nsIContent* aPopup, + nsIContent* aNextPopup, + nsIContent* aLastPopup, + nsPresContext* aPresContext, + PopupType aPopupType, + HidePopupOptions aOptions) { nsCOMPtr popup = aPopup; RefPtr presShell = aPresContext->PresShell(); Unused << presShell; // This presShell may be keeping things alive @@ -1700,7 +1707,8 @@ void nsXULPopupManager::FirePopupHidingEvent( } // If animate="cancel", only show the transition if cancelling the popup // or rolling up. - if (animate.EqualsLiteral("cancel") && !aIsCancel) { + if (animate.EqualsLiteral("cancel") && + !aOptions.contains(HidePopupOption::IsRollup)) { return false; } return true; @@ -1711,13 +1719,13 @@ void nsXULPopupManager::FirePopupHidingEvent( // view will be hidden and you won't be able to see it. if (shouldAnimate && AnimationUtils::HasCurrentTransitions( aPopup->AsElement(), PseudoStyleType::NotPseudo)) { - RefPtr ender = new TransitionEnder(aPopup, aDeselectMenu); + RefPtr ender = new TransitionEnder(aPopup, aOptions); aPopup->AddSystemEventListener(u"transitionend"_ns, ender, false, false); return; } HidePopupCallback(aPopup, popupFrame, aNextPopup, aLastPopup, aPopupType, - aDeselectMenu); + aOptions); } bool nsXULPopupManager::IsPopupOpen(nsIContent* aPopup) { @@ -1939,7 +1947,7 @@ void nsXULPopupManager::PopupDestroyed(nsMenuPopupFrame* aPopup) { } else { // HidePopup will take care of hiding any of its children, so // break out afterwards - HidePopup(child->Content(), false, false, true, false); + HidePopup(child->Content(), {HidePopupOption::Async}); break; } } @@ -2141,7 +2149,7 @@ void nsXULPopupManager::KillMenuTimer() { mCloseTimer = nullptr; if (mTimerMenu->IsOpen()) { - HidePopup(mTimerMenu->GetContent(), false, false, true, false); + HidePopup(mTimerMenu->GetContent(), {HidePopupOption::Async}); } } @@ -2378,9 +2386,7 @@ bool nsXULPopupManager::HandleKeyboardNavigationInPopup( // close a submenu when Left is pressed if (nsMenuPopupFrame* popupFrame = currentItem->GetMenuPopup(FlushType::None)) { - HidePopup(popupFrame->GetContent(), /* aHideChain = */ false, - /* aDeselectMenu = */ false, /* aAsynchronous = */ false, - /* aIsCancel = */ false); + HidePopup(popupFrame->GetContent(), {}); } return true; } @@ -2396,7 +2402,7 @@ bool nsXULPopupManager::HandleKeyboardEventWithKeyCode( if (aTopVisibleMenuItem && aTopVisibleMenuItem->GetPopupType() != PopupType::Menu) { if (keyCode == KeyboardEvent_Binding::DOM_VK_ESCAPE) { - HidePopup(aTopVisibleMenuItem->Content(), false, false, false, true); + HidePopup(aTopVisibleMenuItem->Content(), {HidePopupOption::IsRollup}); aKeyEvent->StopPropagation(); aKeyEvent->StopCrossProcessForwarding(); aKeyEvent->PreventDefault(); @@ -2412,7 +2418,7 @@ bool nsXULPopupManager::HandleKeyboardEventWithKeyCode( // roll up the popup when alt+up/down are pressed within a menulist. if (aKeyEvent->AltKey() && aTopVisibleMenuItem && aTopVisibleMenuItem->Frame()->IsMenuList()) { - Rollup(0, false, nullptr, nullptr); + Rollup({}); break; } [[fallthrough]]; @@ -2439,7 +2445,7 @@ bool nsXULPopupManager::HandleKeyboardEventWithKeyCode( // though in this latter case, a menu didn't actually close, the effect // ends up being the same. Similar for the tab key below. if (aTopVisibleMenuItem) { - HidePopup(aTopVisibleMenuItem->Content(), false, false, false, true); + HidePopup(aTopVisibleMenuItem->Content(), {HidePopupOption::IsRollup}); } else if (mActiveMenuBar) { mActiveMenuBar->MenuClosed(); } @@ -2454,7 +2460,7 @@ bool nsXULPopupManager::HandleKeyboardEventWithKeyCode( kNameSpaceID_None, nsGkAtoms::activateontab, nsGkAtoms::_true, eCaseMatters)) { // Close popups or deactivate menubar when Tab or F10 are pressed - Rollup(0, false, nullptr, nullptr); + Rollup({}); break; } else if (mActiveMenuBar) { mActiveMenuBar->MenuClosed(); @@ -2600,7 +2606,7 @@ nsresult nsXULPopupManager::KeyDown(KeyboardEvent* aKeyEvent) { // modifiers are already down. nsMenuChainItem* item = GetTopVisibleMenu(); if (item && !item->Frame()->IsMenuList()) { - Rollup(0, false, nullptr, nullptr); + Rollup({}); } else if (mActiveMenuBar) { mActiveMenuBar->MenuClosed(); } @@ -2663,7 +2669,7 @@ nsXULPopupHidingEvent::Run() { nsCOMPtr nextPopup = mNextPopup; nsCOMPtr lastPopup = mLastPopup; pm->FirePopupHidingEvent(popup, nextPopup, lastPopup, presContext, - mPopupType, mDeselectMenu, mIsRollup); + mPopupType, mOptions); } } return NS_OK; @@ -2816,8 +2822,11 @@ nsXULMenuCommandEvent::Run() { if (mCloseMenuMode != CloseMenuMode_None) { if (RefPtr popup = menu->GetContainingPopupElement()) { - pm->HidePopup(popup, mCloseMenuMode == CloseMenuMode_Auto, true, false, - false); + HidePopupOptions options{HidePopupOption::DeselectMenu}; + if (mCloseMenuMode == CloseMenuMode_Auto) { + options += HidePopupOption::HideChain; + } + pm->HidePopup(popup, options); } } diff --git a/layout/xul/nsXULPopupManager.h b/layout/xul/nsXULPopupManager.h index 753d995532f4..5b9053837465 100644 --- a/layout/xul/nsXULPopupManager.h +++ b/layout/xul/nsXULPopupManager.h @@ -161,6 +161,21 @@ enum nsIgnoreKeys { eIgnoreKeys_Shortcuts, }; +enum class HidePopupOption : uint8_t { + // If the entire chain of menus should be closed. + HideChain, + // If the parent of the popup should not be deselected. This will not + // be set when the menu is closed by pressing the Escape key. + DeselectMenu, + // If the first popuphiding event should be sent asynchrously. This should + // be set if HidePopup is called from a frame. + Async, + // If this popup is hiding due to being cancelled. + IsRollup, +}; + +using HidePopupOptions = mozilla::EnumSet; + #define NS_DIRECTION_IS_INLINE(dir) \ (dir == eNavigationDirection_Start || dir == eNavigationDirection_End) #define NS_DIRECTION_IS_BLOCK(dir) \ @@ -280,14 +295,13 @@ class nsXULPopupHidingEvent : public mozilla::Runnable { public: nsXULPopupHidingEvent(nsIContent* aPopup, nsIContent* aNextPopup, nsIContent* aLastPopup, PopupType aPopupType, - bool aDeselectMenu, bool aIsCancel) + HidePopupOptions aOptions) : mozilla::Runnable("nsXULPopupHidingEvent"), mPopup(aPopup), mNextPopup(aNextPopup), mLastPopup(aLastPopup), mPopupType(aPopupType), - mDeselectMenu(aDeselectMenu), - mIsRollup(aIsCancel) { + mOptions(aOptions) { NS_ASSERTION(aPopup, "null popup supplied to nsXULPopupHidingEvent constructor"); // aNextPopup and aLastPopup may be null @@ -300,8 +314,7 @@ class nsXULPopupHidingEvent : public mozilla::Runnable { nsCOMPtr mNextPopup; nsCOMPtr mLastPopup; PopupType mPopupType; - bool mDeselectMenu; - bool mIsRollup; + HidePopupOptions mOptions; }; // this class is used for dispatching popuppositioned events asynchronously. @@ -375,9 +388,8 @@ class nsXULPopupManager final : public nsIDOMEventListener, // nsIRollupListener MOZ_CAN_RUN_SCRIPT_BOUNDARY - bool Rollup(uint32_t aCount, bool aFlush, - const mozilla::LayoutDeviceIntPoint* aPos, - nsIContent** aLastRolledUp) override; + bool Rollup(const RollupOptions&, + nsIContent** aLastRolledUp = nullptr) override; bool ShouldRollupOnMouseWheelEvent() override; bool ShouldConsumeOnMouseWheelEvent() override; bool ShouldRollupOnMouseActivate() override; @@ -389,8 +401,7 @@ class nsXULPopupManager final : public nsIDOMEventListener, enum class RollupKind { Tooltip, Menu }; MOZ_CAN_RUN_SCRIPT - bool RollupInternal(RollupKind, uint32_t aCount, bool aFlush, - const mozilla::LayoutDeviceIntPoint* pos, + bool RollupInternal(RollupKind, const RollupOptions&, nsIContent** aLastRolledUp); // NativeMenu::Observer @@ -507,21 +518,10 @@ class nsXULPopupManager final : public nsIDOMEventListener, /* * Hide a popup aPopup. If the popup is in a , then also inform the * menu that the popup is being hidden. - * - * aHideChain - true if the entire chain of menus should be closed. If false, - * only this popup is closed. - * aDeselectMenu - true if the parent of the popup should be - * deselected. This will be false when the menu is closed by - * pressing the Escape key. - * aAsynchronous - true if the first popuphiding event should be sent - * asynchrously. This should be true if HidePopup is called - * from a frame. - * aIsCancel - true if this popup is hiding due to being cancelled. * aLastPopup - optional popup to close last when hiding a chain of menus. * If null, then all popups will be closed. */ - void HidePopup(nsIContent* aPopup, bool aHideChain, bool aDeselectMenu, - bool aAsynchronous, bool aIsCancel, + void HidePopup(nsIContent* aPopup, HidePopupOptions, nsIContent* aLastPopup = nullptr); /* @@ -754,7 +754,7 @@ class nsXULPopupManager final : public nsIDOMEventListener, bool aSelectFirstItem); MOZ_CAN_RUN_SCRIPT void HidePopupCallback( nsIContent* aPopup, nsMenuPopupFrame* aPopupFrame, nsIContent* aNextPopup, - nsIContent* aLastPopup, PopupType aPopupType, bool aDeselectMenu); + nsIContent* aLastPopup, PopupType aPopupType, HidePopupOptions); /** * Trigger frame construction and reflow in the popup, fire a popupshowing @@ -786,14 +786,13 @@ class nsXULPopupManager final : public nsIDOMEventListener, * aLastPopup - the last popup in the chain to hide * aPresContext - nsPresContext for the popup's frame * aPopupType - the PopupType of the frame. - * aDeselectMenu - true to unhighlight the menu when hiding it - * aIsCancel - true if this popup is hiding due to being cancelled. + * aOptions - the relevant options to hide the popup. Only a subset is looked + * at. */ MOZ_CAN_RUN_SCRIPT_BOUNDARY void FirePopupHidingEvent(nsIContent* aPopup, nsIContent* aNextPopup, nsIContent* aLastPopup, nsPresContext* aPresContext, - PopupType aPopupType, bool aDeselectMenu, - bool aIsCancel); + PopupType aPopupType, HidePopupOptions aOptions); /** * Handle keyboard navigation within a menu popup specified by aItem. diff --git a/layout/xul/nsXULTooltipListener.cpp b/layout/xul/nsXULTooltipListener.cpp index 799b767aef10..1af94b197b57 100644 --- a/layout/xul/nsXULTooltipListener.cpp +++ b/layout/xul/nsXULTooltipListener.cpp @@ -474,7 +474,7 @@ void nsXULTooltipListener::LaunchTooltip() { nsresult nsXULTooltipListener::HideTooltip() { if (nsCOMPtr currentTooltip = do_QueryReferent(mCurrentTooltip)) { if (nsXULPopupManager* pm = nsXULPopupManager::GetInstance()) { - pm->HidePopup(currentTooltip, false, false, false, false); + pm->HidePopup(currentTooltip, {}); } } diff --git a/view/nsView.cpp b/view/nsView.cpp index 8ffc5659ac91..9dfef415b482 100644 --- a/view/nsView.cpp +++ b/view/nsView.cpp @@ -1058,9 +1058,8 @@ void nsView::DynamicToolbarOffsetChanged(ScreenIntCoord aOffset) { bool nsView::RequestWindowClose(nsIWidget* aWidget) { if (mFrame && IsPopupWidget(aWidget) && mFrame->IsMenuPopupFrame()) { - nsXULPopupManager* pm = nsXULPopupManager::GetInstance(); - if (pm) { - pm->HidePopup(mFrame->GetContent(), false, true, false, false); + if (nsXULPopupManager* pm = nsXULPopupManager::GetInstance()) { + pm->HidePopup(mFrame->GetContent(), {HidePopupOption::DeselectMenu}); return true; } } diff --git a/widget/cocoa/nsChildView.mm b/widget/cocoa/nsChildView.mm index 2c488fc997d7..0f64ca46f1a3 100644 --- a/widget/cocoa/nsChildView.mm +++ b/widget/cocoa/nsChildView.mm @@ -2397,56 +2397,57 @@ NSEvent* gLastDragMouseDownEvent = nil; // [strong] } nsCOMPtr rollupWidget = rollupListener->GetRollupWidget(); - if (rollupWidget) { - NSWindow* currentPopup = static_cast(rollupWidget->GetNativeData(NS_NATIVE_WINDOW)); - if (!nsCocoaUtils::IsEventOverWindow(theEvent, currentPopup)) { - // event is not over the rollup window, default is to roll up - bool shouldRollup = true; + if (!rollupWidget) { + return consumeEvent; + } - // check to see if scroll/zoom events should roll up the popup - if (isWheelTypeEvent) { - shouldRollup = rollupListener->ShouldRollupOnMouseWheelEvent(); - // consume scroll events that aren't over the popup - // unless the popup is an arrow panel - consumeEvent = rollupListener->ShouldConsumeOnMouseWheelEvent(); - } + NSWindow* currentPopup = static_cast(rollupWidget->GetNativeData(NS_NATIVE_WINDOW)); + if (nsCocoaUtils::IsEventOverWindow(theEvent, currentPopup)) { + return consumeEvent; + } - // if we're dealing with menus, we probably have submenus and - // we don't want to rollup if the click is in a parent menu of - // the current submenu - uint32_t popupsToRollup = UINT32_MAX; - AutoTArray widgetChain; - uint32_t sameTypeCount = rollupListener->GetSubmenuWidgetChain(&widgetChain); - for (uint32_t i = 0; i < widgetChain.Length(); i++) { - nsIWidget* widget = widgetChain[i]; - NSWindow* currWindow = (NSWindow*)widget->GetNativeData(NS_NATIVE_WINDOW); - if (nsCocoaUtils::IsEventOverWindow(theEvent, currWindow)) { - // don't roll up if the mouse event occurred within a menu of the - // same type. If the mouse event occurred in a menu higher than - // that, roll up, but pass the number of popups to Rollup so - // that only those of the same type close up. - if (i < sameTypeCount) { - shouldRollup = false; - } else { - popupsToRollup = sameTypeCount; - } - break; - } - } - - if (shouldRollup) { - if ([theEvent type] == NSEventTypeLeftMouseDown) { - NSPoint point = [NSEvent mouseLocation]; - FlipCocoaScreenCoordinate(point); - LayoutDeviceIntPoint devPoint = mGeckoChild->CocoaPointsToDevPixels(point); - consumeEvent = (BOOL)rollupListener->Rollup(popupsToRollup, true, &devPoint, nullptr); - } else { - consumeEvent = (BOOL)rollupListener->Rollup(popupsToRollup, true, nullptr, nullptr); - } - } + // Check to see if scroll/zoom events should roll up the popup + if (isWheelTypeEvent) { + // consume scroll events that aren't over the popup unless the popup is an + // arrow panel. + consumeEvent = rollupListener->ShouldConsumeOnMouseWheelEvent(); + if (!rollupListener->ShouldRollupOnMouseWheelEvent()) { + return consumeEvent; } } + // if we're dealing with menus, we probably have submenus and + // we don't want to rollup if the click is in a parent menu of + // the current submenu + uint32_t popupsToRollup = UINT32_MAX; + AutoTArray widgetChain; + uint32_t sameTypeCount = rollupListener->GetSubmenuWidgetChain(&widgetChain); + for (uint32_t i = 0; i < widgetChain.Length(); i++) { + nsIWidget* widget = widgetChain[i]; + NSWindow* currWindow = (NSWindow*)widget->GetNativeData(NS_NATIVE_WINDOW); + if (nsCocoaUtils::IsEventOverWindow(theEvent, currWindow)) { + // don't roll up if the mouse event occurred within a menu of the + // same type. If the mouse event occurred in a menu higher than + // that, roll up, but pass the number of popups to Rollup so + // that only those of the same type close up. + if (i < sameTypeCount) { + return consumeEvent; + } + popupsToRollup = sameTypeCount; + break; + } + } + + LayoutDeviceIntPoint devPoint; + nsIRollupListener::RollupOptions rollupOptions{popupsToRollup, + nsIRollupListener::FlushViews::Yes}; + if ([theEvent type] == NSEventTypeLeftMouseDown) { + NSPoint point = [NSEvent mouseLocation]; + FlipCocoaScreenCoordinate(point); + devPoint = mGeckoChild->CocoaPointsToDevPixels(point); + rollupOptions.mPoint = &devPoint; + } + consumeEvent = (BOOL)rollupListener->Rollup(rollupOptions); return consumeEvent; NS_OBJC_END_TRY_BLOCK_RETURN(NO); diff --git a/widget/cocoa/nsCocoaWindow.mm b/widget/cocoa/nsCocoaWindow.mm index 2676a06599d9..42ca50609e44 100644 --- a/widget/cocoa/nsCocoaWindow.mm +++ b/widget/cocoa/nsCocoaWindow.mm @@ -120,7 +120,7 @@ static void RollUpPopups() { nsCOMPtr rollupWidget = rollupListener->GetRollupWidget(); if (!rollupWidget) return; - rollupListener->Rollup(0, true, nullptr, nullptr); + rollupListener->Rollup({0, nsIRollupListener::FlushViews::Yes}); } nsCocoaWindow::nsCocoaWindow() diff --git a/widget/cocoa/nsMenuX.mm b/widget/cocoa/nsMenuX.mm index f744701051cd..c32ee7ff444d 100644 --- a/widget/cocoa/nsMenuX.mm +++ b/widget/cocoa/nsMenuX.mm @@ -1159,7 +1159,7 @@ void nsMenuX::Dump(uint32_t aIndent) const { if (rollupListener) { nsCOMPtr rollupWidget = rollupListener->GetRollupWidget(); if (rollupWidget) { - rollupListener->Rollup(0, true, nullptr, nullptr); + rollupListener->Rollup({0, nsIRollupListener::FlushViews::Yes}); [menu cancelTracking]; return; } diff --git a/widget/cocoa/nsToolkit.mm b/widget/cocoa/nsToolkit.mm index 3125d9da0f05..01802620679e 100644 --- a/widget/cocoa/nsToolkit.mm +++ b/widget/cocoa/nsToolkit.mm @@ -179,7 +179,7 @@ void nsToolkit::MonitorAllProcessMouseEvents() { return; } - rollupListener->Rollup(0, false, nullptr, nullptr); + rollupListener->Rollup({}); }]; } diff --git a/widget/gtk/nsWindow.cpp b/widget/gtk/nsWindow.cpp index f83c576593ce..ae7419d16569 100644 --- a/widget/gtk/nsWindow.cpp +++ b/widget/gtk/nsWindow.cpp @@ -625,7 +625,7 @@ void nsWindow::Destroy() { if (rollupListener) { nsCOMPtr rollupWidget = rollupListener->GetRollupWidget(); if (static_cast(this) == rollupWidget) { - rollupListener->Rollup(0, false, nullptr, nullptr); + rollupListener->Rollup({}); } } @@ -4017,7 +4017,7 @@ gboolean nsWindow::OnConfigureEvent(GtkWidget* aWidget, // This check avoids unwanted rollup on spurious configure events from // Cygwin/X (bug 672103). if (mBounds.x != screenBounds.x || mBounds.y != screenBounds.y) { - CheckForRollup(0, 0, false, true); + RollupAllMenus(); } } @@ -4782,7 +4782,7 @@ void nsWindow::OnContainerFocusOutEvent(GdkEventFocus* aEvent) { }(); if (shouldRollupMenus) { - CheckForRollup(0, 0, false, true); + RollupAllMenus(); } if (RefPtr pm = nsXULPopupManager::GetInstance()) { @@ -7407,53 +7407,52 @@ bool nsWindow::CheckForRollup(gdouble aMouseX, gdouble aMouseY, bool aIsWheel, return false; } - bool retVal = false; auto* currentPopup = (GdkWindow*)rollupWidget->GetNativeData(NS_NATIVE_WINDOW); - if (aAlwaysRollup || !is_mouse_in_window(currentPopup, aMouseX, aMouseY)) { - bool rollup = true; - if (aIsWheel) { - rollup = rollupListener->ShouldRollupOnMouseWheelEvent(); - retVal = rollupListener->ShouldConsumeOnMouseWheelEvent(); + if (!aAlwaysRollup && is_mouse_in_window(currentPopup, aMouseX, aMouseY)) { + return false; + } + bool retVal = false; + if (aIsWheel) { + retVal = rollupListener->ShouldConsumeOnMouseWheelEvent(); + if (!rollupListener->ShouldRollupOnMouseWheelEvent()) { + return retVal; } - // if we're dealing with menus, we probably have submenus and - // we don't want to rollup if the click is in a parent menu of - // the current submenu - uint32_t popupsToRollup = UINT32_MAX; - if (!aAlwaysRollup) { - AutoTArray widgetChain; - uint32_t sameTypeCount = - rollupListener->GetSubmenuWidgetChain(&widgetChain); - for (unsigned long i = 0; i < widgetChain.Length(); ++i) { - nsIWidget* widget = widgetChain[i]; - auto* currWindow = (GdkWindow*)widget->GetNativeData(NS_NATIVE_WINDOW); - if (is_mouse_in_window(currWindow, aMouseX, aMouseY)) { - // don't roll up if the mouse event occurred within a - // menu of the same type. If the mouse event occurred - // in a menu higher than that, roll up, but pass the - // number of popups to Rollup so that only those of the - // same type close up. - if (i < sameTypeCount) { - rollup = false; - } else { - popupsToRollup = sameTypeCount; - } - break; + } + LayoutDeviceIntPoint point; + nsIRollupListener::RollupOptions options{0, + nsIRollupListener::FlushViews::Yes}; + // if we're dealing with menus, we probably have submenus and + // we don't want to rollup if the click is in a parent menu of + // the current submenu + if (!aAlwaysRollup) { + AutoTArray widgetChain; + uint32_t sameTypeCount = + rollupListener->GetSubmenuWidgetChain(&widgetChain); + for (unsigned long i = 0; i < widgetChain.Length(); ++i) { + nsIWidget* widget = widgetChain[i]; + auto* currWindow = (GdkWindow*)widget->GetNativeData(NS_NATIVE_WINDOW); + if (is_mouse_in_window(currWindow, aMouseX, aMouseY)) { + // Don't roll up if the mouse event occurred within a menu of the same + // type. + // If the mouse event occurred in a menu higher than that, roll up, but + // pass the number of popups to Rollup so that only those of the same + // type close up. + if (i < sameTypeCount) { + return retVal; } - } // foreach parent menu widget - } // if rollup listener knows about menus - - // if we've determined that we should still rollup, do it. - bool usePoint = !aIsWheel && !aAlwaysRollup; - LayoutDeviceIntPoint point; - if (usePoint) { + options.mCount = sameTypeCount; + break; + } + } // foreach parent menu widget + if (!aIsWheel) { point = GdkEventCoordsToDevicePixels(aMouseX, aMouseY); + options.mPoint = &point; } - if (rollup && - rollupListener->Rollup(popupsToRollup, true, - usePoint ? &point : nullptr, nullptr)) { - retVal = true; - } + } + + if (rollupListener->Rollup(options)) { + retVal = true; } return retVal; } diff --git a/widget/gtk/nsWindow.h b/widget/gtk/nsWindow.h index 1f3cf0f927f9..a77cd4051bb4 100644 --- a/widget/gtk/nsWindow.h +++ b/widget/gtk/nsWindow.h @@ -504,7 +504,8 @@ class nsWindow final : public nsBaseWidget { const mozilla::LayoutDeviceIntPoint& aRefPoint); bool CheckForRollup(gdouble aMouseX, gdouble aMouseY, bool aIsWheel, bool aAlwaysRollup); - void CheckForRollupDuringGrab() { CheckForRollup(0, 0, false, true); } + void RollupAllMenus() { CheckForRollup(0, 0, false, true); } + void CheckForRollupDuringGrab() { RollupAllMenus(); } bool GetDragInfo(mozilla::WidgetMouseEvent* aMouseEvent, GdkWindow** aWindow, gint* aButton, gint* aRootX, gint* aRootY); diff --git a/widget/nsBaseDragService.cpp b/widget/nsBaseDragService.cpp index 2dbb28b3601d..bd103c5a32f6 100644 --- a/widget/nsBaseDragService.cpp +++ b/widget/nsBaseDragService.cpp @@ -567,7 +567,7 @@ nsBaseDragService::EndDragSession(bool aDoneDrag, uint32_t aKeyModifiers) { if (mDragPopup) { nsXULPopupManager* pm = nsXULPopupManager::GetInstance(); if (pm) { - pm->HidePopup(mDragPopup, false, true, false, false); + pm->HidePopup(mDragPopup, {HidePopupOption::DeselectMenu}); } } diff --git a/widget/nsIRollupListener.h b/widget/nsIRollupListener.h index 1b5b1d6efcf5..083a9b68e00b 100644 --- a/widget/nsIRollupListener.h +++ b/widget/nsIRollupListener.h @@ -16,26 +16,29 @@ class nsIWidget; class nsIRollupListener { public: + enum class FlushViews : bool { No, Yes }; + struct RollupOptions { + // aCount is the number of popups in a chain to close. If this is + // zero, then all popups are closed. + uint32_t mCount = 0; + // If this is true, then views should be flushed after the rollup. + FlushViews mFlush = FlushViews::No; + // This is the mouse pointer position where the event that triggered the + // rollup occurred, which may be nullptr. + const mozilla::LayoutDeviceIntPoint* mPoint = nullptr; + }; + /** * Notifies the object to rollup, optionally returning the node that - * was just rolled up. + * was just rolled up in aLastRolledUp, if non-null. * - * If aFlush is true, then views should be flushed after the rollup. - * - * aPoint is the mouse pointer position where the event that triggered the - * rollup occurred, which may be nullptr. - * - * aCount is the number of popups in a chain to close. If this is - * UINT32_MAX, then all popups are closed. - * If aLastRolledUp is non-null, it will be set to the last rolled up popup, - * if this is supported. aLastRolledUp is not addrefed. + * aLastRolledUp is not addrefed. * * Returns true if the event that the caller is processing should be consumed. */ MOZ_CAN_RUN_SCRIPT_BOUNDARY - virtual bool Rollup(uint32_t aCount, bool aFlush, - const mozilla::LayoutDeviceIntPoint* aPoint, - nsIContent** aLastRolledUp) = 0; + virtual bool Rollup(const RollupOptions&, + nsIContent** aLastRolledUp = nullptr) = 0; /** * Asks the RollupListener if it should rollup on mouse wheel events diff --git a/widget/windows/nsWindow.cpp b/widget/windows/nsWindow.cpp index a76ca2316d74..4d9b653d8d38 100644 --- a/widget/windows/nsWindow.cpp +++ b/widget/windows/nsWindow.cpp @@ -7415,7 +7415,7 @@ void nsWindow::OnDestroy() { rollupWidget = rollupListener->GetRollupWidget(); } if (this == rollupWidget) { - if (rollupListener) rollupListener->Rollup(0, false, nullptr, nullptr); + rollupListener->Rollup({}); CaptureRollupEvents(false); } @@ -8410,6 +8410,11 @@ bool nsWindow::DealWithPopups(HWND aWnd, UINT aMessage, WPARAM aWParam, // Only need to deal with the last rollup for left mouse down events. NS_ASSERTION(!nsAutoRollup::GetLastRollup(), "last rollup is null"); + nsIRollupListener::RollupOptions rollupOptions{ + popupsToRollup, + nsIRollupListener::FlushViews::Yes, + }; + if (nativeMessage == WM_TOUCH || nativeMessage == WM_LBUTTONDOWN || nativeMessage == WM_POINTERDOWN) { LayoutDeviceIntPoint pos; @@ -8427,13 +8432,12 @@ bool nsWindow::DealWithPopups(HWND aWnd, UINT aMessage, WPARAM aWParam, pos = LayoutDeviceIntPoint(pt.x, pt.y); } - nsIContent* lastRollup; - consumeRollupEvent = - rollupListener->Rollup(popupsToRollup, true, &pos, &lastRollup); + rollupOptions.mPoint = &pos; + nsIContent* lastRollup = nullptr; + consumeRollupEvent = rollupListener->Rollup(rollupOptions, &lastRollup); nsAutoRollup::SetLastRollup(lastRollup); } else { - consumeRollupEvent = - rollupListener->Rollup(popupsToRollup, true, nullptr, nullptr); + consumeRollupEvent = rollupListener->Rollup(rollupOptions); } // Tell hook to stop processing messages