make menubarControl prefer mnemonicTitle, make MenuItemAction only implement IAction

src/vs/platform/actions/common/actions.ts

@@ -58,7 +58,7 @@ export interface IMenuItem {
 }
 
 export interface ISubmenuItem {
-	title: string | ILocalizedString;
+	title: string | ICommandActionTitle;
 	submenu: MenuId;
 	icon?: Icon;
 	when?: ContextKeyExpression;
@@ -350,44 +350,54 @@ export class SubmenuItemAction extends SubmenuAction {
 	}
 }
 
-export class MenuItemAction extends ExecuteCommandAction {
+// implements IAction, does NOT extend Action, so that no one
+// subscribes to events of Action or modified properties
+export class MenuItemAction implements IAction {
 
 	readonly item: ICommandAction;
 	readonly alt: MenuItemAction | undefined;
 
 	private readonly _options: IMenuActionOptions | undefined;
 
+	readonly id: string;
+	readonly label: string;
+	readonly tooltip: string;
+	readonly class: string | undefined;
+	readonly enabled: boolean;
+	readonly checked: boolean;
+
 	constructor(
 		item: ICommandAction,
 		alt: ICommandAction | undefined,
-		options: IMenuActionOptions,
+		options: IMenuActionOptions | undefined,
 		@IContextKeyService contextKeyService: IContextKeyService,
-		@ICommandService commandService: ICommandService
+		@ICommandService private _commandService: ICommandService
 	) {
-		typeof item.title === 'string' ? super(item.id, item.title, commandService) : super(item.id, item.title.value, commandService);
-
-		this._cssClass = undefined;
-		this._enabled = !item.precondition || contextKeyService.contextMatchesRules(item.precondition);
-		this._tooltip = item.tooltip;
+		this.id = item.id;
+		this.label = typeof item.title === 'string' ? item.title : item.title.value;
+		this.tooltip = item.tooltip ?? '';
+		this.enabled = !item.precondition || contextKeyService.contextMatchesRules(item.precondition);
+		this.checked = false;
 
 		if (item.toggled) {
 			const toggled = ((item.toggled as { condition: ContextKeyExpression }).condition ? item.toggled : { condition: item.toggled }) as {
 				condition: ContextKeyExpression, icon?: Icon, tooltip?: string | ILocalizedString
 			};
-			this._checked = contextKeyService.contextMatchesRules(toggled.condition);
-			if (this._checked && toggled.tooltip) {
-				this._tooltip = typeof toggled.tooltip === 'string' ? toggled.tooltip : toggled.tooltip.value;
+			this.checked = contextKeyService.contextMatchesRules(toggled.condition);
+			if (this.checked && toggled.tooltip) {
+				this.tooltip = typeof toggled.tooltip === 'string' ? toggled.tooltip : toggled.tooltip.value;
 			}
 		}
 
 		this.item = item;
-		this.alt = alt ? new MenuItemAction(alt, undefined, options, contextKeyService, commandService) : undefined;
+		this.alt = alt ? new MenuItemAction(alt, undefined, options, contextKeyService, _commandService) : undefined;
 		this._options = options;
 	}
 
 	dispose(): void {
-		this.alt?.dispose();
-		super.dispose();
+		// there is NOTHING to dispose and the MenuItemAction should
+		// never have anything to dispose as it is a convenience type
+		// to bridge into the rendering world.
 	}
 
 	run(...args: any[]): Promise<any> {
@@ -401,7 +411,7 @@ export class MenuItemAction extends ExecuteCommandAction {
 			runArgs = [...runArgs, ...args];
 		}
 
-		return super.run(...runArgs);
+		return this._commandService.executeCommand(this.id, ...runArgs);
 	}
 }
 

src/vs/workbench/browser/parts/titlebar/menubarControl.ts

@@ -37,6 +37,7 @@ import { BrowserFeatures } from 'vs/base/browser/canIUse';
 import { KeyCode } from 'vs/base/common/keyCodes';
 import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry';
 import { IsWebContext } from 'vs/platform/contextkey/common/contextkeys';
+import { ICommandService } from 'vs/platform/commands/common/commands';
 
 export abstract class MenubarControl extends Disposable {
 
@@ -91,7 +92,8 @@ export abstract class MenubarControl extends Disposable {
 		protected readonly preferencesService: IPreferencesService,
 		protected readonly environmentService: IWorkbenchEnvironmentService,
 		protected readonly accessibilityService: IAccessibilityService,
-		protected readonly hostService: IHostService
+		protected readonly hostService: IHostService,
+		protected readonly commandService: ICommandService
 	) {
 
 		super();
@@ -308,9 +310,10 @@ export class CustomMenubarControl extends MenubarControl {
 		@IAccessibilityService accessibilityService: IAccessibilityService,
 		@IThemeService private readonly themeService: IThemeService,
 		@IWorkbenchLayoutService private readonly layoutService: IWorkbenchLayoutService,
-		@IHostService protected readonly hostService: IHostService
+		@IHostService protected readonly hostService: IHostService,
+		@ICommandService commandService: ICommandService
 	) {
-		super(menuService, workspacesService, contextKeyService, keybindingService, configurationService, labelService, updateService, storageService, notificationService, preferencesService, environmentService, accessibilityService, hostService);
+		super(menuService, workspacesService, contextKeyService, keybindingService, configurationService, labelService, updateService, storageService, notificationService, preferencesService, environmentService, accessibilityService, hostService, commandService);
 
 		this._onVisibilityChange = this._register(new Emitter<boolean>());
 		this._onFocusStateChange = this._register(new Emitter<boolean>());
@@ -592,6 +595,12 @@ export class CustomMenubarControl extends MenubarControl {
 
 				for (let action of actions) {
 					this.insertActionsBefore(action, target);
+
+					// use mnemonicTitle whenever possible
+					const title = typeof action.item.title === 'string'
+						? action.item.title
+						: action.item.title.mnemonicTitle ?? action.item.title.value;
+
 					if (action instanceof SubmenuItemAction) {
 						let submenu = this.menus[action.item.submenu.id];
 						if (!submenu) {
@@ -609,10 +618,12 @@ export class CustomMenubarControl extends MenubarControl {
 
 						const submenuActions: SubmenuAction[] = [];
 						updateActions(submenu, submenuActions, topLevelTitle);
-						target.push(new SubmenuAction(action.id, mnemonicMenuLabel(action.label), submenuActions));
+						target.push(new SubmenuAction(action.id, mnemonicMenuLabel(title), submenuActions));
 					} else {
-						action.label = mnemonicMenuLabel(this.calculateActionLabel(action));
-						target.push(action);
+						const newAction = new Action(action.id, mnemonicMenuLabel(title), action.class, action.enabled, () => this.commandService.executeCommand(action.id));
+						newAction.tooltip = action.tooltip;
+						newAction.checked = action.checked;
+						target.push(newAction);
 					}
 				}
 

src/vs/workbench/electron-sandbox/parts/titlebar/menubarControl.ts

@@ -25,6 +25,7 @@ import { withNullAsUndefined } from 'vs/base/common/types';
 import { INativeHostService } from 'vs/platform/native/electron-sandbox/native';
 import { IHostService } from 'vs/workbench/services/host/browser/host';
 import { IPreferencesService } from 'vs/workbench/services/preferences/common/preferences';
+import { ICommandService } from 'vs/platform/commands/common/commands';
 
 export class NativeMenubarControl extends MenubarControl {
 
@@ -43,9 +44,10 @@ export class NativeMenubarControl extends MenubarControl {
 		@IAccessibilityService accessibilityService: IAccessibilityService,
 		@IMenubarService private readonly menubarService: IMenubarService,
 		@IHostService hostService: IHostService,
-		@INativeHostService private readonly nativeHostService: INativeHostService
+		@INativeHostService private readonly nativeHostService: INativeHostService,
+		@ICommandService commandService: ICommandService,
 	) {
-		super(menuService, workspacesService, contextKeyService, keybindingService, configurationService, labelService, updateService, storageService, notificationService, preferencesService, environmentService, accessibilityService, hostService);
+		super(menuService, workspacesService, contextKeyService, keybindingService, configurationService, labelService, updateService, storageService, notificationService, preferencesService, environmentService, accessibilityService, hostService, commandService);
 
 		if (isMacintosh) {
 			this.menus['Preferences'] = this._register(this.menuService.createMenu(MenuId.MenubarPreferencesMenu, this.contextKeyService));