Bug 1705842 - Move command registration and NSMenuItem target/action initialization into nsMenuItemX. r=harry

This means that it'll happen even if the menu item is not visible. This will make
it possible to make a menu item visible after initial menu construction.

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

widget/cocoa/nsMenuItemX.mm

@@ -90,6 +90,13 @@ nsMenuItemX::nsMenuItemX(nsMenuX* aParent, const nsString& aLabel, EMenuItemType
 
   mIsVisible = !nsMenuUtilsX::NodeIsHiddenOrCollapsed(mContent);
 
+  // All menu items share the same target and action, and are differentiated
+  // be a unique (representedObject, tag) pair.
+  mNativeMenuItem.target = nsMenuBarX::sNativeEventTarget;
+  mNativeMenuItem.action = @selector(menuItemHit:);
+  mNativeMenuItem.representedObject = mMenuGroupOwner->GetRepresentedObject();
+  mNativeMenuItem.tag = mMenuGroupOwner->RegisterForCommand(this);
+
   NS_OBJC_END_TRY_ABORT_BLOCK;
 }
 
@@ -109,6 +116,8 @@ nsMenuItemX::~nsMenuItemX() {
 
 void nsMenuItemX::DetachFromGroupOwner() {
   if (mMenuGroupOwner) {
+    mMenuGroupOwner->UnregisterCommand(mNativeMenuItem.tag);
+
     if (mContent) {
       mMenuGroupOwner->UnregisterForContentChanges(mContent);
     }

widget/cocoa/nsMenuX.h

@@ -173,7 +173,6 @@ class nsMenuX final : public nsMenuParentX,
   void LoadMenuItem(nsIContent* aMenuItemContent);
   void LoadSubMenu(nsIContent* aMenuContent);
   GeckoNSMenu* CreateMenuWithGeckoString(nsString& aMenuTitle);
-  void UnregisterCommands();
   void DidFirePopupShowing();
 
   // Calculates the index at which aChild's NSMenuItem should be inserted into our NSMenu.

widget/cocoa/nsMenuX.mm

@@ -166,7 +166,6 @@ void nsMenuX::DetachFromGroupOwnerRecursive() {
   if (mMenuGroupOwner && mContent) {
     mMenuGroupOwner->UnregisterForContentChanges(mContent);
   }
-  UnregisterCommands();
   mMenuGroupOwner = nullptr;
 
   // Also detach all our children.
@@ -197,20 +196,6 @@ void nsMenuX::OnMenuClosed(dom::Element* aPopupElement) {
   }
 }
 
-void nsMenuX::UnregisterCommands() {
-  NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
-
-  if (!mMenuGroupOwner || !mNativeMenu) {
-    return;
-  }
-
-  for (NSMenuItem* item in mNativeMenu.itemArray) {
-    mMenuGroupOwner->UnregisterCommand(static_cast<uint32_t>(item.tag));
-  }
-
-  NS_OBJC_END_TRY_ABORT_BLOCK;
-}
-
 void nsMenuX::AddMenuItem(RefPtr<nsMenuItemX>&& aMenuItem) {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
 
@@ -220,21 +205,9 @@ void nsMenuX::AddMenuItem(RefPtr<nsMenuItemX>&& aMenuItem) {
     return;
   }
 
+  [mNativeMenu addItem:aMenuItem->NativeNSMenuItem()];
   ++mVisibleItemsCount;
 
-  NSMenuItem* newNativeMenuItem = aMenuItem->NativeNSMenuItem();
-
-  // add the menu item to this menu
-  [mNativeMenu addItem:newNativeMenuItem];
-
-  // set up target/action
-  newNativeMenuItem.target = nsMenuBarX::sNativeEventTarget;
-  newNativeMenuItem.action = @selector(menuItemHit:);
-
-  // set its command. we get the unique command id from the menubar
-  newNativeMenuItem.tag = mMenuGroupOwner->RegisterForCommand(aMenuItem);
-  newNativeMenuItem.representedObject = mMenuGroupOwner->GetRepresentedObject();
-
   aMenuItem->SetupIcon();
 
   NS_OBJC_END_TRY_ABORT_BLOCK;
@@ -329,7 +302,6 @@ Maybe<nsMenuX::MenuChild> nsMenuX::GetItemForElement(Element* aMenuChildElement)
 nsresult nsMenuX::RemoveAll() {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
 
-  UnregisterCommands();
   [mNativeMenu removeAllItems];
 
   for (auto& child : mMenuChildren) {