Fix menubar dropdown infinite loop and visibility on Windows

- Remove focus() calls from cleanupCustomMenu to prevent infinite recursion
- Add re-entrancy guard to prevent recursive cleanup calls
- Fix menu visibility and height calculation on Windows
- Ensure proper z-index and positioning when menu is appended to document.body
- Let Menu widget handle theming via IMenuStyles (no color overrides)
- Add validation for empty actions before creating menus
- Fix bounding rect fallback for invalid measurements

Author: Tajudeen

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

@@ -118,6 +118,28 @@ body > .menubar-menu-items-holder.monaco-menu-container {
 	isolation: auto;
 	/* Ensure menu appears above all workbench content including titlebar with backdrop-filter */
 	will-change: auto;
+	/* Ensure menu container doesn't interfere with theme - Menu widget handles theming */
+	background: transparent !important;
+}
+
+/* Ensure proper height calculation for menu content on Windows */
+/* Note: Menu widget applies colors via IMenuStyles, so we only fix sizing here */
+body > .menubar-menu-items-holder.monaco-menu-container .monaco-scrollable-element {
+	/* Ensure proper height calculation - let content determine height */
+	height: auto !important;
+	min-height: fit-content !important;
+}
+
+body > .menubar-menu-items-holder.monaco-menu-container .monaco-menu {
+	/* Ensure proper height calculation */
+	height: auto !important;
+	min-height: fit-content !important;
+}
+
+body > .menubar-menu-items-holder.monaco-menu-container .actions-container {
+	/* Ensure proper height calculation */
+	height: auto !important;
+	min-height: fit-content !important;
 }
 
 .menubar .toolbar-toggle-more {

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

@@ -87,6 +87,7 @@ export class MenuBar extends Disposable {
 
 	private numMenusShown: number = 0;
 	private overflowLayoutScheduled: IDisposable | undefined = undefined;
+	private isCleaningUp: boolean = false;
 
 	private readonly menuDisposables = this._register(new DisposableStore());
 
@@ -998,64 +999,89 @@ export class MenuBar extends Disposable {
 	}
 
 	private cleanupCustomMenu(): void {
-		if (this.focusedMenu) {
-			// Remove focus from the menus first
-			if (this.focusedMenu.index === MenuBar.OVERFLOW_INDEX) {
-				this.overflowMenu.buttonElement.focus();
-			} else {
-				this.menus[this.focusedMenu.index].buttonElement?.focus();
-			}
-
-			if (this.focusedMenu.holder) {
-				// Remove 'open' class from button element (not from document.body on Windows)
-				const actualMenuIndex = this.focusedMenu.index >= this.numMenusShown ? MenuBar.OVERFLOW_INDEX : this.focusedMenu.index;
-				const customMenu = actualMenuIndex === MenuBar.OVERFLOW_INDEX ? this.overflowMenu : this.menus[actualMenuIndex];
-				customMenu.buttonElement?.classList.remove('open');
+		// Prevent re-entrancy - if we're already cleaning up, don't do it again
+		if (this.isCleaningUp) {
+			return;
+		}
 
-				this.focusedMenu.holder.remove();
-			}
+		this.isCleaningUp = true;
+		try {
+			const focusedMenu = this.focusedMenu;
+			if (focusedMenu) {
+				// Defensive check: ensure holder exists before accessing it
+				// This prevents errors when cleanupCustomMenu is called in edge cases
+				if (focusedMenu.holder) {
+					// Remove 'open' class from button element (not from document.body on Windows)
+					const actualMenuIndex = focusedMenu.index >= this.numMenusShown ? MenuBar.OVERFLOW_INDEX : focusedMenu.index;
+					const customMenu = actualMenuIndex === MenuBar.OVERFLOW_INDEX ? this.overflowMenu : this.menus[actualMenuIndex];
+					customMenu.buttonElement?.classList.remove('open');
+
+					focusedMenu.holder.remove();
+				}
 
-			this.focusedMenu.widget?.dispose();
+				focusedMenu.widget?.dispose();
 
-			this.focusedMenu = { index: this.focusedMenu.index };
+				// Only update focusedMenu if it hasn't been cleared by another operation
+				if (this.focusedMenu === focusedMenu) {
+					this.focusedMenu = { index: focusedMenu.index };
+				}
+			}
+			this.menuDisposables.clear();
+		} finally {
+			this.isCleaningUp = false;
 		}
-		this.menuDisposables.clear();
 	}
 
 	private showCustomMenu(menuIndex: number, selectFirst = true): void {
 		const actualMenuIndex = menuIndex >= this.numMenusShown ? MenuBar.OVERFLOW_INDEX : menuIndex;
 		const customMenu = actualMenuIndex === MenuBar.OVERFLOW_INDEX ? this.overflowMenu : this.menus[actualMenuIndex];
 
-		if (!customMenu.actions || !customMenu.buttonElement || !customMenu.titleElement) {
+		if (!customMenu.actions || customMenu.actions.length === 0 || !customMenu.buttonElement || !customMenu.titleElement) {
 			return;
 		}
 
 		const menuHolder = $('div.menubar-menu-items-holder', { 'title': '' });
 
 		customMenu.buttonElement.classList.add('open');
 
+		// Ensure the title element is visible and laid out before getting bounding rect
+		// This is critical on Windows where elements might not be laid out immediately
 		const titleBoundingRect = customMenu.titleElement.getBoundingClientRect();
 		const titleBoundingRectZoom = DOM.getDomNodeZoomLevel(customMenu.titleElement);
 
+		// Validate bounding rect - if invalid, use button element as fallback
+		let validBoundingRect = titleBoundingRect;
+		if (titleBoundingRect.width === 0 && titleBoundingRect.height === 0) {
+			validBoundingRect = customMenu.buttonElement.getBoundingClientRect();
+		}
+
 		if (this.options.compactMode?.horizontal === HorizontalDirection.Right) {
-			menuHolder.style.left = `${titleBoundingRect.left + this.container.clientWidth}px`;
+			menuHolder.style.left = `${validBoundingRect.left + this.container.clientWidth}px`;
 		} else if (this.options.compactMode?.horizontal === HorizontalDirection.Left) {
 			const windowWidth = DOM.getWindow(this.container).innerWidth;
-			menuHolder.style.right = `${windowWidth - titleBoundingRect.left}px`;
+			menuHolder.style.right = `${windowWidth - validBoundingRect.left}px`;
 			menuHolder.style.left = 'auto';
 		} else {
-			menuHolder.style.left = `${titleBoundingRect.left * titleBoundingRectZoom}px`;
+			menuHolder.style.left = `${validBoundingRect.left * titleBoundingRectZoom}px`;
 		}
 
 		if (this.options.compactMode?.vertical === VerticalDirection.Above) {
 			// TODO@benibenj Do not hardcode the height of the menu holder
-			menuHolder.style.top = `${titleBoundingRect.top - this.menus.length * 30 + this.container.clientHeight}px`;
+			menuHolder.style.top = `${validBoundingRect.top - this.menus.length * 30 + this.container.clientHeight}px`;
 		} else if (this.options.compactMode?.vertical === VerticalDirection.Below) {
-			menuHolder.style.top = `${titleBoundingRect.top}px`;
+			menuHolder.style.top = `${validBoundingRect.top}px`;
 		} else {
-			menuHolder.style.top = `${titleBoundingRect.bottom * titleBoundingRectZoom}px`;
+			menuHolder.style.top = `${validBoundingRect.bottom * titleBoundingRectZoom}px`;
 		}
 
+		// Ensure menu holder is visible with explicit styles
+		menuHolder.style.position = 'fixed';
+		menuHolder.style.zIndex = '10000';
+		menuHolder.style.visibility = 'visible';
+		menuHolder.style.display = 'block';
+		menuHolder.style.opacity = '1';
+		menuHolder.style.pointerEvents = 'auto';
+
 		// On Windows, append dropdown to document.body to avoid stacking context issues
 		// where the dropdown renders behind the workbench content
 		if (isWindows) {