Keep compact/overflow menubar submenus in sync on updateMenu (fixes stale Auto Save checkmark on web) (#344678)

Keep compact/overflow menubar submenus in sync on updateMenu

MenuBar.updateMenu only replaced the top-level menu's actions, but the
compact (hamburger) and "More" overflow menus open their own
SubmenuActions that captured the previous actions array at layout time.
Context-key driven changes (e.g. the File > Auto Save checkmark) were
therefore stale until an unrelated relayout.

Fixes #160894

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

src/vs/base/browser/ui/menu/menubar.ts

@@ -426,6 +426,18 @@ export class MenuBar extends Disposable {
 		if (menuToUpdate && menuToUpdate.length) {
 			menuToUpdate[0].actions = menu.actions;
 		}
+
+		// The overflow menu holds its own submenu actions for menus that do not fit
+		// (or all menus in compact mode), so keep the matching entry in sync
+		const overflowSubmenuId = MenuBar.getOverflowSubmenuId(menu.label);
+		const overflowIndex = this.overflowMenu.actions.findIndex(action => action.id === overflowSubmenuId);
+		if (overflowIndex !== -1) {
+			this.overflowMenu.actions[overflowIndex] = new SubmenuAction(overflowSubmenuId, menu.label, menu.actions);
+		}
+	}
+
+	private static getOverflowSubmenuId(label: string): string {
+		return `menubar.submenu.${label}`;
 	}
 
 	override dispose(): void {
@@ -523,7 +535,7 @@ export class MenuBar extends Disposable {
 		if (this.isCompact) {
 			this.overflowMenu.actions = [];
 			for (let idx = this.numMenusShown; idx < this.menus.length; idx++) {
-				this.overflowMenu.actions.push(new SubmenuAction(`menubar.submenu.${this.menus[idx].label}`, this.menus[idx].label, this.menus[idx].actions || []));
+				this.overflowMenu.actions.push(new SubmenuAction(MenuBar.getOverflowSubmenuId(this.menus[idx].label), this.menus[idx].label, this.menus[idx].actions || []));
 			}
 
 			const compactMenuActions = this.options.getCompactMenuActions?.();
@@ -544,7 +556,7 @@ export class MenuBar extends Disposable {
 
 			this.overflowMenu.actions = [];
 			for (let idx = this.numMenusShown; idx < showableMenus.length; idx++) {
-				this.overflowMenu.actions.push(new SubmenuAction(`menubar.submenu.${showableMenus[idx].label}`, showableMenus[idx].label, showableMenus[idx].actions || []));
+				this.overflowMenu.actions.push(new SubmenuAction(MenuBar.getOverflowSubmenuId(showableMenus[idx].label), showableMenus[idx].label, showableMenus[idx].actions || []));
 			}
 
 			if (this.overflowMenu.buttonElement.nextElementSibling !== showableMenus[this.numMenusShown].buttonElement) {

src/vs/base/test/browser/ui/menu/menubar.test.ts

@@ -4,9 +4,11 @@
  *--------------------------------------------------------------------------------------------*/
 
 import assert from 'assert';
-import { $, ModifierKeyEmitter } from '../../../../browser/dom.js';
-import { unthemedMenuStyles } from '../../../../browser/ui/menu/menu.js';
+import { $, getWindow, ModifierKeyEmitter } from '../../../../browser/dom.js';
+import { HorizontalDirection, unthemedMenuStyles, VerticalDirection } from '../../../../browser/ui/menu/menu.js';
 import { MenuBar } from '../../../../browser/ui/menu/menubar.js';
+import { mainWindow } from '../../../../browser/window.js';
+import { toAction } from '../../../../common/actions.js';
 import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../common/utils.js';
 
 function getButtonElementByAriaLabel(menubarElement: HTMLElement, ariaLabel: string): HTMLElement | null {
@@ -94,4 +96,99 @@ suite('Menubar', () => {
 			validateMenuBarItem(menubar, container, '文件(&F)', '文件', 'F');
 		});
 	});
+
+	suite('compact mode', () => {
+		let compactContainer: HTMLElement;
+
+		setup(() => {
+			compactContainer = mainWindow.document.body.appendChild($('.compact-container'));
+		});
+
+		teardown(() => {
+			compactContainer.remove();
+			ModifierKeyEmitter.disposeInstance();
+		});
+
+		function createCompactMenubar(): MenuBar {
+			return new MenuBar(compactContainer, {
+				enableMnemonics: true,
+				visibility: 'classic',
+				compactMode: { horizontal: HorizontalDirection.Right, vertical: VerticalDirection.Below },
+				getCompactMenuActions: () => [toAction({ id: 'web.nav', label: 'Go Home', run: () => { } })]
+			}, unthemedMenuStyles);
+		}
+
+		function nextAnimationFrame(): Promise<void> {
+			return new Promise(resolve => getWindow(compactContainer).requestAnimationFrame(() => resolve()));
+		}
+
+		function dispatchKeyUp(target: HTMLElement): void {
+			target.dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter', code: 'Enter', keyCode: 13, bubbles: true }));
+		}
+
+		function getItemLabels(menuElement: Element): string[] {
+			const actionsContainer = menuElement.querySelector('.actions-container');
+			return [...actionsContainer?.querySelectorAll(':scope > .action-item') ?? []].map(item => item.querySelector('.action-label')?.textContent?.trim() ?? '');
+		}
+
+		/**
+		 * Opens the compact (overflow) menu, then the submenu with the given label,
+		 * and returns the rendered top-level labels and submenu items.
+		 */
+		function readOverflowMenu(menubar: MenuBar, submenuLabel: string): { topLevel: string[]; submenu: { label: string; checked: string | null }[] } {
+			const overflowButton = compactContainer.querySelector<HTMLElement>('.menubar-menu-button[aria-label="Application Menu"]');
+			assert.ok(overflowButton);
+			dispatchKeyUp(overflowButton);
+
+			const overflowMenu = overflowButton.querySelector('.monaco-menu');
+			assert.ok(overflowMenu);
+			const submenuItem = [...overflowMenu.querySelectorAll<HTMLElement>('.action-item')].find(item => item.querySelector('.action-label')?.textContent?.trim() === submenuLabel);
+			assert.ok(submenuItem);
+			dispatchKeyUp(submenuItem);
+
+			const submenu = submenuItem.querySelector('.monaco-submenu .monaco-menu');
+			assert.ok(submenu);
+			const result = {
+				topLevel: getItemLabels(overflowMenu),
+				submenu: [...submenu.querySelectorAll('.action-menu-item')].map(item => ({
+					label: item.querySelector('.action-label')?.textContent?.trim() ?? '',
+					checked: item.getAttribute('aria-checked')
+				}))
+			};
+
+			menubar.blur();
+			return result;
+		}
+
+		test('updateMenu refreshes menus already placed in the overflow menu (#160894)', async () => {
+			const menubar = createCompactMenubar();
+			try {
+				menubar.push([
+					{ label: '&File', actions: [toAction({ id: 'autoSave', label: 'Auto Save', checked: true, run: () => { } })] },
+					{ label: '&Edit', actions: [toAction({ id: 'undo', label: 'Undo', run: () => { } })] }
+				]);
+
+				// Lay out first so the overflow menu captures the initial actions
+				menubar.update();
+				await nextAnimationFrame();
+
+				const states: ReturnType<typeof readOverflowMenu>[] = [];
+				for (const checked of [false, true]) {
+					menubar.updateMenu({ label: '&File', actions: [toAction({ id: 'autoSave', label: 'Auto Save', checked, run: () => { } })] });
+					states.push(readOverflowMenu(menubar, 'File'));
+					states.push(readOverflowMenu(menubar, 'Edit'));
+				}
+
+				const topLevel = ['File', 'Edit', '', 'Go Home'];
+				assert.deepStrictEqual(states, [
+					{ topLevel, submenu: [{ label: 'Auto Save', checked: 'false' }] },
+					{ topLevel, submenu: [{ label: 'Undo', checked: '' }] },
+					{ topLevel, submenu: [{ label: 'Auto Save', checked: 'true' }] },
+					{ topLevel, submenu: [{ label: 'Undo', checked: '' }] },
+				]);
+			} finally {
+				menubar.dispose();
+			}
+		});
+	});
 });