fix: menu state in macOS dock menus (#49627)

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

shell/browser/mac/electron_application_delegate.mm

@@ -56,11 +56,6 @@ static NSDictionary* UNNotificationResponseToNSDictionary(
 }
 
 - (void)applicationWillFinishLaunching:(NSNotification*)notify {
-  // Don't add the "Enter Full Screen" menu item automatically.
-  [[NSUserDefaults standardUserDefaults]
-      setBool:NO
-       forKey:@"NSFullScreenMenuItemEverywhere"];
-
   [[[NSWorkspace sharedWorkspace] notificationCenter]
       addObserver:self
          selector:@selector(willPowerOff:)
@@ -114,7 +109,14 @@ static NSDictionary* UNNotificationResponseToNSDictionary(
 }
 
 - (NSMenu*)applicationDockMenu:(NSApplication*)sender {
-  return menu_controller_ ? menu_controller_.menu : nil;
+  if (!menu_controller_)
+    return nil;
+
+  // Manually refresh menu state since menuWillOpen: is not called
+  // by macOS for dock menus for some reason before they are displayed.
+  NSMenu* menu = menu_controller_.menu;
+  [menu_controller_ refreshMenuTree:menu];
+  return menu;
 }
 
 - (BOOL)application:(NSApplication*)sender openFile:(NSString*)filename {

shell/browser/ui/cocoa/electron_menu_controller.h

@@ -57,6 +57,10 @@ class ElectronMenuModel;
 // Whether the menu is currently open.
 - (BOOL)isMenuOpen;
 
+// Recursively refreshes the menu tree starting from |menu|, applying the
+// model state (enabled, checked, hidden etc) to each menu item.
+- (void)refreshMenuTree:(NSMenu*)menu;
+
 // NSMenuDelegate methods this class implements. Subclasses should call super
 // if extending the behavior.
 - (void)menuWillOpen:(NSMenu*)menu;

shell/browser/ui/cocoa/electron_menu_controller.mm

@@ -490,8 +490,6 @@ NSArray* ConvertSharingItemToNS(const SharingItem& item) {
                                              : 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];
@@ -557,6 +555,14 @@ NSArray* ConvertSharingItemToNS(const SharingItem& item) {
 
 - (void)menuWillOpen:(NSMenu*)menu {
   isMenuOpen_ = YES;
+
+  // macOS automatically injects a duplicate "Toggle Full Screen" menu item
+  // when we set menu.delegate on submenus. Remove hidden duplicates.
+  for (NSMenuItem* item in menu.itemArray) {
+    if (item.isHidden && item.action == @selector(toggleFullScreenMode:))
+      [menu removeItem:item];
+  }
+
   [self refreshMenuTree:menu];
   if (model_)
     model_->MenuWillShow();

spec/api-app-spec.ts

@@ -2,7 +2,6 @@ import { app, BrowserWindow, Menu, session, net as electronNet, WebContents, uti
 
 import { assert, expect } from 'chai';
 import * as semver from 'semver';
-import split = require('split')
 
 import * as cp from 'node:child_process';
 import { once } from 'node:events';
@@ -11,6 +10,7 @@ import * as http from 'node:http';
 import * as https from 'node:https';
 import * as net from 'node:net';
 import * as path from 'node:path';
+import * as readline from 'node:readline';
 import { setTimeout } from 'node:timers/promises';
 import { promisify } from 'node:util';
 
@@ -260,11 +260,11 @@ describe('app module', () => {
       const firstExited = once(first, 'exit');
 
       // Wait for the first app to boot.
-      const firstStdoutLines = first.stdout.pipe(split());
-      while ((await once(firstStdoutLines, 'data')).toString() !== 'started') {
+      const firstStdoutLines = readline.createInterface({ input: first.stdout });
+      while ((await once(firstStdoutLines, 'line')).toString() !== 'started') {
         // wait.
       }
-      const additionalDataPromise = once(firstStdoutLines, 'data');
+      const additionalDataPromise = once(firstStdoutLines, 'line');
 
       const secondInstanceArgs = [process.execPath, appPath, ...testArgs.args, '--some-switch', 'some-arg'];
       const second = cp.spawn(secondInstanceArgs[0], secondInstanceArgs.slice(1));

spec/package.json

@@ -23,7 +23,6 @@
     "@types/mocha": "^7.0.2",
     "@types/send": "^0.14.5",
     "@types/sinon": "^9.0.4",
-    "@types/split": "^1.0.5",
     "@types/uuid": "^3.4.6",
     "@types/w3c-web-serial": "^1.0.7",
     "@types/ws": "^7.2.0",
@@ -46,7 +45,6 @@
     "q": "^1.5.1",
     "send": "^0.19.0",
     "sinon": "^9.0.1",
-    "split": "^1.0.1",
     "uuid": "^3.3.3",
     "winreg": "1.2.4",
     "ws": "^7.5.10",

yarn.lock

@@ -1990,16 +1990,6 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@types/split@npm:^1.0.5":
-  version: 1.0.5
-  resolution: "@types/split@npm:1.0.5"
-  dependencies:
-    "@types/node": "npm:*"
-    "@types/through": "npm:*"
-  checksum: 10c0/eb187a3b07e5064928e49bffd5c45ad1f1109135fee52344bb7623cdb55e2ebb16bd6ca009a30a0a6e2b262f7ebb7bf18030ff873819e80fafd4cbb51dba1a74
-  languageName: node
-  linkType: hard
-
 "@types/stream-chain@npm:*":
   version: 2.0.0
   resolution: "@types/stream-chain@npm:2.0.0"
@@ -2042,15 +2032,6 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@types/through@npm:*":
-  version: 0.0.33
-  resolution: "@types/through@npm:0.0.33"
-  dependencies:
-    "@types/node": "npm:*"
-  checksum: 10c0/6a8edd7f40cd7e197318e86310a40e568cddd380609dde59b30d5cc6c5f8276ddc698905eac4b3b429eb39f2e8ee326bc20dc6e95a2cdc41c4d3fc9a1ebd4929
-  languageName: node
-  linkType: hard
-
 "@types/unist@npm:*, @types/unist@npm:^2.0.0":
   version: 2.0.6
   resolution: "@types/unist@npm:2.0.6"
@@ -4543,7 +4524,6 @@ __metadata:
     "@types/mocha": "npm:^7.0.2"
     "@types/send": "npm:^0.14.5"
     "@types/sinon": "npm:^9.0.4"
-    "@types/split": "npm:^1.0.5"
     "@types/uuid": "npm:^3.4.6"
     "@types/w3c-web-serial": "npm:^1.0.7"
     "@types/ws": "npm:^7.2.0"
@@ -4566,7 +4546,6 @@ __metadata:
     q: "npm:^1.5.1"
     send: "npm:^0.19.0"
     sinon: "npm:^9.0.1"
-    split: "npm:^1.0.1"
     uuid: "npm:^3.3.3"
     winreg: "npm:1.2.4"
     ws: "npm:^7.5.10"