fix: revert allow disabling all NSMenuItems, fix menu crash (#48800)

Revert "fix: allow disabling all `NSMenuItems` (#48598)"

This reverts commit 0cb4fdd.

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Keeley Hammond <[email protected]>

Author: trop[bot]

lib/browser/api/menu.ts

@@ -25,30 +25,11 @@ Menu.prototype._isCommandIdChecked = function (id) {
 };
 
 Menu.prototype._isCommandIdEnabled = function (id) {
-  const item = this.commandsMap[id];
-  if (!item) return false;
-
-  const focusedWindow = BaseWindow.getFocusedWindow();
-
-  if (item.role === 'minimize' && focusedWindow) {
-    return focusedWindow.isMinimizable();
-  }
-
-  if (item.role === 'togglefullscreen' && focusedWindow) {
-    return focusedWindow.isFullScreenable();
-  }
-
-  if (item.role === 'close' && focusedWindow) {
-    return focusedWindow.isClosable();
-  }
-
-  return item.enabled;
+  return this.commandsMap[id] ? this.commandsMap[id].enabled : false;
 };
-
 Menu.prototype._shouldCommandIdWorkWhenHidden = function (id) {
   return this.commandsMap[id]?.acceleratorWorksWhenHidden ?? false;
 };
-
 Menu.prototype._isCommandIdVisible = function (id) {
   return this.commandsMap[id]?.visible ?? false;
 };

shell/browser/ui/cocoa/electron_menu_controller.mm

@@ -90,7 +90,6 @@ bool MenuHasVisibleItems(const electron::ElectronMenuModel* model) {
 // "(empty)" into the submenu. Matches Windows behavior.
 NSMenu* MakeEmptySubmenu() {
   NSMenu* submenu = [[NSMenu alloc] initWithTitle:@""];
-  submenu.autoenablesItems = NO;
   NSString* empty_menu_title =
       l10n_util::GetNSString(IDS_APP_MENU_EMPTY_SUBMENU);
 
@@ -232,9 +231,6 @@ - (void)cancel {
 // be invoked recursively.
 - (NSMenu*)menuFromModel:(electron::ElectronMenuModel*)model {
   NSMenu* menu = [[NSMenu alloc] initWithTitle:@""];
-  // We manually manage enabled/disabled/hidden state for every item,
-  // including Cocoa role-based selectors.
-  menu.autoenablesItems = NO;
 
   const int count = model->GetItemCount();
   for (int index = 0; index < count; index++) {
@@ -244,7 +240,6 @@ - (NSMenu*)menuFromModel:(electron::ElectronMenuModel*)model {
       [self addItemToMenu:menu atIndex:index fromModel:model];
   }
 
-  menu.delegate = self;
   return menu;
 }
 
@@ -299,11 +294,9 @@ - (NSMenu*)createShareMenuForItem:(const SharingItem&)item {
   if ([items count] == 0)
     return MakeEmptySubmenu();
   NSMenu* menu = [[NSMenu alloc] init];
-  menu.autoenablesItems = NO;
   NSArray* services = [NSSharingService sharingServicesForItems:items];
   for (NSSharingService* service in services)
     [menu addItem:[self menuItemForService:service withItems:items]];
-  [menu setDelegate:self];
   return menu;
 }
 
@@ -360,22 +353,27 @@ - (NSMenuItem*)makeMenuItemForIndex:(NSInteger)index
     std::u16string title = u"Services";
     NSString* sub_label = l10n_util::FixUpWindowsStyleLabel(title);
 
-    item.target = nil;
-    item.action = nil;
+    [item setTarget:nil];
+    [item setAction:nil];
     NSMenu* submenu = [[NSMenu alloc] initWithTitle:sub_label];
-    item.submenu = submenu;
+    [item setSubmenu:submenu];
     [NSApp setServicesMenu:submenu];
   } else if (role == u"sharemenu") {
     SharingItem sharing_item;
     model->GetSharingItemAt(index, &sharing_item);
-    item.target = nil;
-    item.action = nil;
+    [item setTarget:nil];
+    [item setAction:nil];
     [item setSubmenu:[self createShareMenuForItem:sharing_item]];
   } else if (type == electron::ElectronMenuModel::TYPE_SUBMENU &&
              model->IsVisibleAt(index)) {
+    // We need to specifically check that the submenu top-level item has been
+    // enabled as it's not validated by validateUserInterfaceItem
+    if (!model->IsEnabledAt(index))
+      [item setEnabled:NO];
+
     // Recursively build a submenu from the sub-model at this index.
-    item.target = nil;
-    item.action = nil;
+    [item setTarget:nil];
+    [item setAction:nil];
     electron::ElectronMenuModel* submenuModel =
         static_cast<electron::ElectronMenuModel*>(
             model->GetSubmenuModelAt(index));
@@ -390,12 +388,8 @@ - (NSMenuItem*)makeMenuItemForIndex:(NSInteger)index
       }
     }
 
-    submenu.title = item.title;
-    item.submenu = submenu;
-    item.tag = index;
-    item.representedObject =
-        [WeakPtrToElectronMenuModelAsNSObject weakPtrForModel:model];
-    submenu.delegate = self;
+    [submenu setTitle:[item title]];
+    [item setSubmenu:submenu];
 
     // Set submenu's role.
     if ((role == u"window" || role == u"windowmenu") && [submenu numberOfItems])
@@ -410,9 +404,9 @@ - (NSMenuItem*)makeMenuItemForIndex:(NSInteger)index
     // the model so hierarchical menus check the correct index in the correct
     // model. Setting the target to |self| allows this class to participate
     // in validation of the menu items.
-    item.tag = index;
-    item.representedObject =
-        [WeakPtrToElectronMenuModelAsNSObject weakPtrForModel:model];
+    [item setTag:index];
+    [item setRepresentedObject:[WeakPtrToElectronMenuModelAsNSObject
+                                   weakPtrForModel:model]];
     ui::Accelerator accelerator;
     if (model->GetAcceleratorAtWithParams(index, useDefaultAccelerator_,
                                           &accelerator)) {
@@ -440,20 +434,20 @@ - (NSMenuItem*)makeMenuItemForIndex:(NSInteger)index
         ui::MacKeyCodeForWindowsKeyCode(accelerator.key_code(), modifier_mask,
                                         nullptr, &character);
       }
-      item.keyEquivalent = [NSString stringWithFormat:@"%C", character];
-      item.keyEquivalentModifierMask = modifier_mask;
+      [item setKeyEquivalent:[NSString stringWithFormat:@"%C", character]];
+      [item setKeyEquivalentModifierMask:modifier_mask];
     }
 
     [(id)item
         setAllowsKeyEquivalentWhenHidden:(model->WorksWhenHiddenAt(index))];
 
     // Set menu item's role.
-    item.target = self;
+    [item setTarget:self];
     if (!role.empty()) {
       for (const Role& pair : kRolesMap) {
         if (role == base::ASCIIToUTF16(pair.role)) {
-          item.target = nil;
-          item.action = pair.selector;
+          [item setTarget:nil];
+          [item setAction:pair.selector];
           break;
         }
       }
@@ -463,37 +457,6 @@ - (NSMenuItem*)makeMenuItemForIndex:(NSInteger)index
   return item;
 }
 
-- (void)applyStateToMenuItem:(NSMenuItem*)item {
-  id represented = item.representedObject;
-  if (!represented)
-    return;
-
-  electron::ElectronMenuModel* model =
-      [WeakPtrToElectronMenuModelAsNSObject getFrom:represented];
-  if (!model)
-    return;
-
-  NSInteger index = item.tag;
-  int count = model->GetItemCount();
-  if (index < 0 || index >= count)
-    return;
-
-  item.hidden = !model->IsVisibleAt(index);
-  item.enabled = model->IsEnabledAt(index);
-  item.state = model->IsItemCheckedAt(index) ? NSControlStateValueOn
-                                             : NSControlStateValueOff;
-}
-
-// Recursively refreshes the menu tree starting from |menu|, applying the
-// model state to each menu item.
-- (void)refreshMenuTree:(NSMenu*)menu {
-  for (NSMenuItem* item in menu.itemArray) {
-    [self applyStateToMenuItem:item];
-    if (item.submenu)
-      [self refreshMenuTree:item.submenu];
-  }
-}
-
 // Adds an item or a hierarchical menu to the item at the |index|,
 // associated with the entry in the model identified by |modelIndex|.
 - (void)addItemToMenu:(NSMenu*)menu
@@ -503,6 +466,32 @@ - (void)addItemToMenu:(NSMenu*)menu
            atIndex:index];
 }
 
+// Called before the menu is to be displayed to update the state (enabled,
+// radio, etc) of each item in the menu.
+- (BOOL)validateUserInterfaceItem:(id<NSValidatedUserInterfaceItem>)item {
+  SEL action = [item action];
+  if (action == @selector(performShare:))
+    return YES;
+  if (action != @selector(itemSelected:))
+    return NO;
+
+  NSInteger modelIndex = [item tag];
+  electron::ElectronMenuModel* model = [WeakPtrToElectronMenuModelAsNSObject
+      getFrom:[(id)item representedObject]];
+  DCHECK(model);
+  if (model) {
+    BOOL checked = model->IsItemCheckedAt(modelIndex);
+    DCHECK([(id)item isKindOfClass:[NSMenuItem class]]);
+
+    [(id)item
+        setState:(checked ? NSControlStateValueOn : NSControlStateValueOff)];
+    [(id)item setHidden:(!model->IsVisibleAt(modelIndex))];
+
+    return model->IsEnabledAt(modelIndex);
+  }
+  return NO;
+}
+
 // Called when the user chooses a particular menu item. |sender| is the menu
 // item chosen.
 - (void)itemSelected:(id)sender {
@@ -537,11 +526,10 @@ - (NSMenu*)menu {
     menu_ = menu;
   } else {
     menu_ = [[NSMenu alloc] initWithTitle:@""];
-    menu_.autoenablesItems = NO;
     if (model_)
       [self populateWithModel:model_.get()];
   }
-  menu_.delegate = self;
+  [menu_ setDelegate:self];
   return menu_;
 }
 
@@ -551,7 +539,6 @@ - (BOOL)isMenuOpen {
 
 - (void)menuWillOpen:(NSMenu*)menu {
   isMenuOpen_ = YES;
-  [self refreshMenuTree:menu];
   if (model_)
     model_->MenuWillShow();
 }