Bug 1707598 - Rename ActivateItemAndClose and call cancelTrackingWithoutAnimation in NativeMenuMac::ActivateItem. r=harry

This also makes sure we call ActivateItemAfterMenuClosing on the submenu that
immediately contains the activated menu item, and not on the root menu.
This usually doesn't make a difference, except in the case where something
calls FlushMenuClosedRunnable afterwards; in that case, before this patch, we
might accidentally fired the popuphidden event for the submenu before the
command event for the clicked item.

Depends on D113733

Differential Revision: https://phabricator.services.mozilla.com/D113987

widget/cocoa/NativeMenuMac.mm

@@ -315,8 +315,15 @@ void NativeMenuMac::ActivateItem(dom::Element* aItemElement, Modifiers aModifier
     return;
   }
 
-  mMenu->ActivateItemAndClose(std::move(item->as<RefPtr<nsMenuItemX>>()),
-                              ConvertModifierFlags(aModifiers), aButton);
+  menu->ActivateItemAfterClosing(std::move(item->as<RefPtr<nsMenuItemX>>()),
+                                 ConvertModifierFlags(aModifiers), aButton);
+
+  // Close the menu.
+  // cancelTracking(WithoutAnimation) is asynchronous; the menu only hides once the stack unwinds
+  // from NSMenu's nested "tracking" event loop.
+  // However, cancelTrackingWithoutAnimation synchronously calls the menu delegate's menuDidClose
+  // handler, at least on macOS 11.
+  [mMenu->NativeNSMenu() cancelTrackingWithoutAnimation];
 }
 
 void NativeMenuMac::OpenSubmenu(dom::Element* aMenuElement) {

widget/cocoa/nsMenuX.h

@@ -98,9 +98,9 @@ class nsMenuX final : public nsMenuParentX,
 
   mozilla::Maybe<MenuChild> GetItemForElement(mozilla::dom::Element* aMenuChildElement);
 
-  // Asynchronously runs the command event on aItem, and closes the menu.
-  void ActivateItemAndClose(RefPtr<nsMenuItemX>&& aItem, NSEventModifierFlags aModifiers,
-                            int16_t aButton);
+  // Asynchronously runs the command event on aItem, after the root menu has closed.
+  void ActivateItemAfterClosing(RefPtr<nsMenuItemX>&& aItem, NSEventModifierFlags aModifiers,
+                                int16_t aButton);
 
   bool IsOpenForGecko() const { return mIsOpenForGecko; }
 
@@ -234,7 +234,7 @@ class nsMenuX final : public nsMenuParentX,
   RefPtr<mozilla::CancelableRunnable> mPendingAsyncMenuCloseRunnable;
 
   // Any runnables for running asynchronous command events.
-  // These are only used during automated tests, via ActivateItemAndClose.
+  // These are only used during automated tests, via ActivateItemAfterClosing.
   // We keep track of them here so that we can ensure they're run before popuphiding/popuphidden.
   nsTArray<RefPtr<mozilla::Runnable>> mPendingCommandRunnables;
 

widget/cocoa/nsMenuX.mm

@@ -567,8 +567,8 @@ void nsMenuX::MenuClosedAsync() {
   }
 }
 
-void nsMenuX::ActivateItemAndClose(RefPtr<nsMenuItemX>&& aItem, NSEventModifierFlags aModifiers,
-                                   int16_t aButton) {
+void nsMenuX::ActivateItemAfterClosing(RefPtr<nsMenuItemX>&& aItem, NSEventModifierFlags aModifiers,
+                                       int16_t aButton) {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
 
   // Run the command asynchronously so that the menu can hide before the command runs.
@@ -598,15 +598,6 @@ void nsMenuX::ActivateItemAndClose(RefPtr<nsMenuItemX>&& aItem, NSEventModifierF
   mPendingCommandRunnables.AppendElement(doCommandAsync);
   NS_DispatchToCurrentThread(doCommandAsync);
 
-  if (mIsOpen) {
-    // cancelTracking(WithoutAnimation) is asynchronous; the menu only hides once the stack unwinds
-    // from NSMenu's nested "tracking" event loop.
-    [mNativeMenu cancelTrackingWithoutAnimation];
-  }
-
-  // We don't call FlushMenuClosedRunnable() here, so that the command event runs before
-  // MenuClosedAsync().
-
   NS_OBJC_END_TRY_ABORT_BLOCK;
 }