Bug 1811487 - Clean-up popup hide / rollup APIs. r=cmartin

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
dothq · Jan 23, 2023 · e8d9ad8 · e8d9ad8
1 parent 0807bf1
commit e8d9ad8
Show file tree

Hide file tree

Showing 19 changed files with 221 additions and 195 deletions.
diff --git a/browser/components/customizableui/test/browser_PanelMultiView_keyboard.js b/browser/components/customizableui/test/browser_PanelMultiView_keyboard.js
@@ -290,14 +290,12 @@ 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") {

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -62,6 +62,7 @@
 #include <algorithm>
 
 #include "X11UndefineNone.h"
+#include "nsXULPopupManager.h"
 
 using namespace mozilla;
 using mozilla::dom::Document;
@@ -2463,7 +2464,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;