fix: show macOS context menus reliably after a previous one is cancelled (#55235)

* fix: show macOS context menus reliably after a previous one is cancelled

-[ElectronMenuController cancel] dismissed the menu with -cancelTracking,
which plays a fade-out animation. While that animation runs, a following
-popUpMenuPositioningItem:atLocation:inView: returns without showing the
menu or calling -menuWillOpen:, so a Menu.popup() made right after
Menu.closePopup() (or after a cancelled popup on another window) showed
nothing, and neither 'menu-will-show', 'menu-will-close' nor the popup
callback ever fired.

Use -cancelTrackingWithoutAnimation instead, and clear isMenuOpen_ before
calling it so that a -menuDidClose: delivered during the cancel is ignored
and 'menu-will-close' and the close callback are still reported exactly
once, from -cancel.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LPMvJBQiyP6b7QjatACmnS

* test: cover reopening a macOS popup menu from a closed popup's callback

Pops a menu up, closes it with closePopup() once it shows, and pops it
up again from the first popup's callback. Both popups must emit exactly
one menu-will-show, one menu-will-close and one callback, in order.
With the animated -cancelTracking the second popup showed nothing, so
the test timed out. It runs with retry: 0 because every attempt repeats
the same sequence, so a retry could only hide a regression.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LPMvJBQiyP6b7QjatACmnS

---------

Co-authored-by: Claude <noreply@anthropic.com>

shell/browser/ui/cocoa/electron_menu_controller.mm

@@ -235,8 +235,12 @@ NSMenuItemBadge* CreateBadge(const electron::ElectronMenuModel::Badge& badge)
 
 - (void)cancel {
   if (isMenuOpen_) {
-    [menu_ cancelTracking];
+    // Clear the flag first so that a -menuDidClose: delivered during the
+    // cancel is ignored and the close is only reported once, from here.
     isMenuOpen_ = NO;
+    // Cancel without the fade-out animation: while it runs, a following
+    // -popUpMenuPositioningItem:atLocation:inView: silently shows nothing.
+    [menu_ cancelTrackingWithoutAnimation];
     if (model_)
       model_->MenuWillClose();
     if (!popupCloseCallback.is_null()) {

spec/api-menu.spec.ts

@@ -7,7 +7,7 @@ import { once } from 'node:events';
 import * as fs from 'node:fs';
 import * as os from 'node:os';
 import * as path from 'node:path';
-import { setTimeout } from 'node:timers/promises';
+import { setImmediate, setTimeout } from 'node:timers/promises';
 
 import { singleModifierCombinations } from './lib/accelerator-helpers.ts';
 import { defer, ifit, isWayland, spawnAndWait } from './lib/spec-helpers.ts';
@@ -842,6 +842,54 @@ describe('Menu module', () => {
       w = null as unknown as BrowserWindow;
     });
 
+    // Closing a popup with the animated -[NSMenu cancelTracking] made a popup
+    // opened during the fade-out show nothing, so it never emitted
+    // 'menu-will-show' or 'menu-will-close' or ran its callback. Every retry
+    // repeats the same sequence, so retries could only hide a regression.
+    ifit(process.platform === 'darwin')(
+      'reopens a menu from the callback of a popup closed with closePopup()',
+      { retry: 0 },
+      async () => {
+        const events: string[] = [];
+        let popup = 1;
+        menu.on('menu-will-show', () => {
+          events.push(`menu-will-show ${popup}`);
+          menu.closePopup(w);
+        });
+        menu.on('menu-will-close', () => {
+          events.push(`menu-will-close ${popup}`);
+        });
+
+        await new Promise<void>((resolve) => {
+          menu.popup({
+            window: w,
+            callback: () => {
+              events.push('callback 1');
+              popup = 2;
+              menu.popup({
+                window: w,
+                callback: () => {
+                  events.push('callback 2');
+                  resolve();
+                }
+              });
+            }
+          });
+        });
+        // Let any duplicate event that is already queued arrive before checking.
+        await setImmediate();
+
+        expect(events).toEqual([
+          'menu-will-show 1',
+          'menu-will-close 1',
+          'callback 1',
+          'menu-will-show 2',
+          'menu-will-close 2',
+          'callback 2'
+        ]);
+      }
+    );
+
     it('throws an error if options is not an object', () => {
       expect(() => {
         menu.popup('this is a string, not an object' as any);