fix: allow disabling all NSMenuItems (#48712)

fix: allow disabling all NSMenuItems

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

lib/browser/api/menu.ts

@@ -25,11 +25,30 @@ Menu.prototype._isCommandIdChecked = function (id) {
 };
 
 Menu.prototype._isCommandIdEnabled = function (id) {
-  return this.commandsMap[id] ? this.commandsMap[id].enabled : false;
+  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;
 };
+
 Menu.prototype._shouldCommandIdWorkWhenHidden = function (id) {
   return this.commandsMap[id] ? !!this.commandsMap[id].acceleratorWorksWhenHidden : false;
 };
+
 Menu.prototype._isCommandIdVisible = function (id) {
   return this.commandsMap[id] ? this.commandsMap[id].visible : false;
 };

shell/browser/ui/cocoa/electron_menu_controller.mm

@@ -90,6 +90,7 @@ 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);
 
@@ -231,6 +232,9 @@ NSArray* ConvertSharingItemToNS(const SharingItem& item) {
 // 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++) {
@@ -240,6 +244,7 @@ NSArray* ConvertSharingItemToNS(const SharingItem& item) {
       [self addItemToMenu:menu atIndex:index fromModel:model];
   }
 
+  menu.delegate = self;
   return menu;
 }
 
@@ -294,9 +299,11 @@ NSArray* ConvertSharingItemToNS(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;
 }
 
@@ -353,27 +360,22 @@ NSArray* ConvertSharingItemToNS(const SharingItem& item) {
     std::u16string title = u"Services";
     NSString* sub_label = l10n_util::FixUpWindowsStyleLabel(title);
 
-    [item setTarget:nil];
+    item.target = nil;
-    [item setAction:nil];
+    item.action = nil;
     NSMenu* submenu = [[NSMenu alloc] initWithTitle:sub_label];
-    [item setSubmenu:submenu];
+    item.submenu = submenu;
     [NSApp setServicesMenu:submenu];
   } else if (role == u"sharemenu") {
     SharingItem sharing_item;
     model->GetSharingItemAt(index, &sharing_item);
-    [item setTarget:nil];
+    item.target = nil;
-    [item setAction:nil];
+    item.action = 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 setTarget:nil];
+    item.target = nil;
-    [item setAction:nil];
+    item.action = nil;
     electron::ElectronMenuModel* submenuModel =
         static_cast<electron::ElectronMenuModel*>(
             model->GetSubmenuModelAt(index));
@@ -388,8 +390,12 @@ NSArray* ConvertSharingItemToNS(const SharingItem& item) {
       }
     }
 
-    [submenu setTitle:[item title]];
+    submenu.title = item.title;
-    [item setSubmenu:submenu];
+    item.submenu = submenu;
+    item.tag = index;
+    item.representedObject =
+        [WeakPtrToElectronMenuModelAsNSObject weakPtrForModel:model];
+    submenu.delegate = self;
 
     // Set submenu's role.
     if ((role == u"window" || role == u"windowmenu") && [submenu numberOfItems])
@@ -404,9 +410,9 @@ NSArray* ConvertSharingItemToNS(const SharingItem& item) {
     // 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 setTag:index];
+    item.tag = index;
-    [item setRepresentedObject:[WeakPtrToElectronMenuModelAsNSObject
+    item.representedObject =
-                                   weakPtrForModel:model]];
+        [WeakPtrToElectronMenuModelAsNSObject weakPtrForModel:model];
     ui::Accelerator accelerator;
     if (model->GetAcceleratorAtWithParams(index, useDefaultAccelerator_,
                                           &accelerator)) {
@@ -434,20 +440,20 @@ NSArray* ConvertSharingItemToNS(const SharingItem& item) {
         ui::MacKeyCodeForWindowsKeyCode(accelerator.key_code(), modifier_mask,
                                         nullptr, &character);
       }
-      [item setKeyEquivalent:[NSString stringWithFormat:@"%C", character]];
+      item.keyEquivalent = [NSString stringWithFormat:@"%C", character];
-      [item setKeyEquivalentModifierMask:modifier_mask];
+      item.keyEquivalentModifierMask = modifier_mask;
     }
 
     [(id)item
         setAllowsKeyEquivalentWhenHidden:(model->WorksWhenHiddenAt(index))];
 
     // Set menu item's role.
-    [item setTarget:self];
+    item.target = self;
     if (!role.empty()) {
       for (const Role& pair : kRolesMap) {
         if (role == base::ASCIIToUTF16(pair.role)) {
-          [item setTarget:nil];
+          item.target = nil;
-          [item setAction:pair.selector];
+          item.action = pair.selector;
           break;
         }
       }
@@ -457,6 +463,37 @@ NSArray* ConvertSharingItemToNS(const SharingItem& item) {
   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
@@ -466,32 +503,6 @@ NSArray* ConvertSharingItemToNS(const SharingItem& item) {
            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 {
@@ -526,10 +537,11 @@ NSArray* ConvertSharingItemToNS(const SharingItem& item) {
     menu_ = menu;
   } else {
     menu_ = [[NSMenu alloc] initWithTitle:@""];
+    menu_.autoenablesItems = NO;
     if (model_)
       [self populateWithModel:model_.get()];
   }
-  [menu_ setDelegate:self];
+  menu_.delegate = self;
   return menu_;
 }
 
@@ -539,6 +551,7 @@ NSArray* ConvertSharingItemToNS(const SharingItem& item) {
 
 - (void)menuWillOpen:(NSMenu*)menu {
   isMenuOpen_ = YES;
+  [self refreshMenuTree:menu];
   if (model_)
     model_->MenuWillShow();
 }