dialogs - have a helper for better native dialog styles

src/vs/code/electron-main/app.ts

@@ -12,7 +12,7 @@ import { onUnexpectedError, setUnexpectedErrorHandler } from 'vs/base/common/err
 import { isEqualOrParent } from 'vs/base/common/extpath';
 import { once } from 'vs/base/common/functional';
 import { stripComments } from 'vs/base/common/json';
-import { getPathLabel, mnemonicButtonLabel } from 'vs/base/common/labels';
+import { getPathLabel } from 'vs/base/common/labels';
 import { Disposable } from 'vs/base/common/lifecycle';
 import { Schemas } from 'vs/base/common/network';
 import { isAbsolute, join, posix } from 'vs/base/common/path';
@@ -112,6 +112,7 @@ import { ExtensionsProfileScannerService } from 'vs/platform/extensionManagement
 import { LoggerChannel } from 'vs/platform/log/electron-main/logIpc';
 import { ILoggerMainService } from 'vs/platform/log/electron-main/loggerService';
 import { IInitialProtocolUrls, IProtocolUrl } from 'vs/platform/url/electron-main/url';
+import { massageMessageBoxOptions } from 'vs/platform/dialogs/common/dialogs';
 
 /**
  * The main VS Code application. There will only ever be one instance,
@@ -664,20 +665,17 @@ export class CodeApplication extends Disposable {
 
 	private shouldBlockURI(uri: URI): boolean {
 		if (uri.authority === Schemas.file && isWindows) {
-			const res = dialog.showMessageBoxSync({
-				title: this.productService.nameLong,
+			const { options, buttonIndeces } = massageMessageBoxOptions({
 				type: 'question',
 				buttons: [
-					mnemonicButtonLabel(localize({ key: 'open', comment: ['&& denotes a mnemonic'] }, "&&Yes")),
-					mnemonicButtonLabel(localize({ key: 'cancel', comment: ['&& denotes a mnemonic'] }, "&&No")),
+					localize({ key: 'open', comment: ['&& denotes a mnemonic'] }, "&&Yes"),
+					localize({ key: 'cancel', comment: ['&& denotes a mnemonic'] }, "&&No")
 				],
-				defaultId: 0,
-				cancelId: 1,
 				message: localize('confirmOpenMessage', "An external application wants to open '{0}' in {1}. Do you want to open this file or folder?", getPathLabel(uri, { os: OS, tildify: this.environmentMainService }), this.productService.nameShort),
 				detail: localize('confirmOpenDetail', "If you did not initiate this request, it may represent an attempted attack on your system. Unless you took an explicit action to initiate this request, you should press 'No'"),
-				noLink: true
-			});
+			}, this.productService);
 
+			const res = buttonIndeces[dialog.showMessageBoxSync(options)];
 			if (res === 1) {
 				return true;
 			}

src/vs/code/electron-main/main.ts

@@ -14,7 +14,7 @@ import { toErrorMessage } from 'vs/base/common/errorMessage';
 import { ExpectedError, setUnexpectedErrorHandler } from 'vs/base/common/errors';
 import { IPathWithLineAndColumn, isValidBasename, parseLineAndColumnAware, sanitizeFilePath } from 'vs/base/common/extpath';
 import { once } from 'vs/base/common/functional';
-import { getPathLabel, mnemonicButtonLabel } from 'vs/base/common/labels';
+import { getPathLabel } from 'vs/base/common/labels';
 import { Schemas } from 'vs/base/common/network';
 import { basename, join, resolve } from 'vs/base/common/path';
 import { mark } from 'vs/base/common/performance';
@@ -69,6 +69,7 @@ import { IUriIdentityService } from 'vs/platform/uriIdentity/common/uriIdentity'
 import { UriIdentityService } from 'vs/platform/uriIdentity/common/uriIdentityService';
 import { ILoggerMainService, LoggerMainService } from 'vs/platform/log/electron-main/loggerService';
 import { LogService } from 'vs/platform/log/common/logService';
+import { massageMessageBoxOptions } from 'vs/platform/dialogs/common/dialogs';
 
 /**
  * The main VS Code entry point.
@@ -106,7 +107,7 @@ class CodeMain {
 			} catch (error) {
 
 				// Show a dialog for errors that can be resolved by the user
-				this.handleStartupDataDirError(environmentMainService, productService.nameLong, error);
+				this.handleStartupDataDirError(environmentMainService, productService, error);
 
 				throw error;
 			}
@@ -279,7 +280,7 @@ class CodeMain {
 			if (error.code !== 'EADDRINUSE') {
 
 				// Show a dialog for errors that can be resolved by the user
-				this.handleStartupDataDirError(environmentMainService, productService.nameLong, error);
+				this.handleStartupDataDirError(environmentMainService, productService, error);
 
 				// Any other runtime error is just printed to the console
 				throw error;
@@ -297,7 +298,7 @@ class CodeMain {
 						this.showStartupWarningDialog(
 							localize('secondInstanceAdmin', "Another instance of {0} is already running as administrator.", productService.nameShort),
 							localize('secondInstanceAdminDetail', "Please close the other instance and try again."),
-							productService.nameLong
+							productService
 						);
 					}
 
@@ -336,7 +337,7 @@ class CodeMain {
 					this.showStartupWarningDialog(
 						localize('secondInstanceNoResponse', "Another instance of {0} is running but not responding", productService.nameShort),
 						localize('secondInstanceNoResponseDetail', "Please close all other instances and try again."),
-						productService.nameLong
+						productService
 					);
 				}, 10000);
 			}
@@ -391,31 +392,30 @@ class CodeMain {
 		return mainProcessNodeIpcServer;
 	}
 
-	private handleStartupDataDirError(environmentMainService: IEnvironmentMainService, title: string, error: NodeJS.ErrnoException): void {
+	private handleStartupDataDirError(environmentMainService: IEnvironmentMainService, productService: IProductService, error: NodeJS.ErrnoException): void {
 		if (error.code === 'EACCES' || error.code === 'EPERM') {
 			const directories = coalesce([environmentMainService.userDataPath, environmentMainService.extensionsPath, XDG_RUNTIME_DIR]).map(folder => getPathLabel(URI.file(folder), { os: OS, tildify: environmentMainService }));
 
 			this.showStartupWarningDialog(
 				localize('startupDataDirError', "Unable to write program user data."),
 				localize('startupUserDataAndExtensionsDirErrorDetail', "{0}\n\nPlease make sure the following directories are writeable:\n\n{1}", toErrorMessage(error), directories.join('\n')),
-				title
+				productService
 			);
 		}
 	}
 
-	private showStartupWarningDialog(message: string, detail: string, title: string): void {
+	private showStartupWarningDialog(message: string, detail: string, productService: IProductService): void {
+
 		// use sync variant here because we likely exit after this method
 		// due to startup issues and otherwise the dialog seems to disappear
 		// https://github.com/microsoft/vscode/issues/104493
-		dialog.showMessageBoxSync({
-			title,
+
+		dialog.showMessageBoxSync(massageMessageBoxOptions({
 			type: 'warning',
-			buttons: [mnemonicButtonLabel(localize({ key: 'close', comment: ['&& denotes a mnemonic'] }, "&&Close"))],
+			buttons: [localize({ key: 'close', comment: ['&& denotes a mnemonic'] }, "&&Close")],
 			message,
-			detail,
-			defaultId: 0,
-			noLink: true
-		});
+			detail
+		}, productService).options);
 	}
 
 	private async windowsAllowSetForegroundWindow(launchMainService: ILaunchMainService, logService: ILogService): Promise<void> {

src/vs/platform/dialogs/common/dialogs.ts

@@ -12,6 +12,11 @@ import { URI } from 'vs/base/common/uri';
 import { localize } from 'vs/nls';
 import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
 import { ITelemetryData } from 'vs/platform/telemetry/common/telemetry';
+import { MessageBoxOptions } from 'vs/base/parts/sandbox/common/electronTypes';
+import { mnemonicButtonLabel } from 'vs/base/common/labels';
+import { isLinux, isMacintosh, isWindows } from 'vs/base/common/platform';
+import { IProductService } from 'vs/platform/product/common/productService';
+import { deepClone } from 'vs/base/common/objects';
 
 export interface FileFilter {
 	readonly extensions: string[];
@@ -402,3 +407,114 @@ export interface INativeOpenDialogOptions {
 	readonly telemetryEventName?: string;
 	readonly telemetryExtraData?: ITelemetryData;
 }
+
+export interface IMassagedMessageBoxOptions {
+
+	/**
+	 * OS massaged message box options.
+	 */
+	options: MessageBoxOptions;
+
+	/**
+	 * Since the massaged result of the message box options potentially
+	 * changes the order of buttons, we have to keep a map of these
+	 * changes so that we can still return the correct index to the caller.
+	 */
+	buttonIndeces: number[];
+}
+
+/**
+ * A utility method to ensure the options for the message box dialog
+ * are using properties that are consistent across all platforms and
+ * specific to the platform where necessary.
+ */
+export function massageMessageBoxOptions(options: MessageBoxOptions, productService: IProductService): IMassagedMessageBoxOptions {
+	const massagedOptions = deepClone(options);
+
+	let buttons = (massagedOptions.buttons ?? []).map(button => mnemonicButtonLabel(button));
+	let buttonIndeces = (options.buttons || []).map((button, index) => index);
+
+	let defaultId = 0; // by default the first button is default button
+	let cancelId = massagedOptions.cancelId ?? buttons.length - 1; // by default the last button is cancel button
+
+	// Apply HIG per OS when more than one button is used
+	if (buttons.length > 1) {
+		if (isLinux) {
+
+			// Linux: the GNOME HIG (https://developer.gnome.org/hig/patterns/feedback/dialogs.html?highlight=dialog)
+			// recommend the following:
+			// "Always ensure that the cancel button appears first, before the affirmative button. In left-to-right
+			//  locales, this is on the left. This button order ensures that users become aware of, and are reminded
+			//  of, the ability to cancel prior to encountering the affirmative button."
+			//
+			// Electron APIs do not reorder buttons for us, so we ensure a reverse order of buttons and a position
+			// of the cancel button (if provided) that matches the HIG
+
+			const cancelButton = typeof cancelId === 'number' ? buttons[cancelId] : undefined;
+
+			buttons = buttons.reverse();
+			buttonIndeces = buttonIndeces.reverse();
+
+			defaultId = buttons.length - 1;
+
+			if (typeof cancelButton === 'string') {
+				cancelId = buttons.indexOf(cancelButton);
+				if (cancelId !== buttons.length - 2 /* left to primary action */) {
+					buttons.splice(cancelId, 1);
+					buttons.splice(buttons.length - 1, 0, cancelButton);
+
+					const buttonIndex = buttonIndeces[cancelId];
+					buttonIndeces.splice(cancelId, 1);
+					buttonIndeces.splice(buttonIndeces.length - 1, 0, buttonIndex);
+
+					cancelId = buttons.length - 2;
+				}
+			}
+		} else if (isWindows) {
+
+			// Windows: the HIG (https://learn.microsoft.com/en-us/windows/win32/uxguide/win-dialog-box)
+			// recommend the following:
+			// "One of the following sets of concise commands: Yes/No, Yes/No/Cancel, [Do it]/Cancel,
+			//  [Do it]/[Don't do it], [Do it]/[Don't do it]/Cancel."
+			//
+			// Electron APIs do not reorder buttons for us, so we ensure the position of the cancel button
+			// (if provided) that matches the HIG
+
+			const cancelButton = typeof cancelId === 'number' ? buttons[cancelId] : undefined;
+
+			if (typeof cancelButton === 'string') {
+				cancelId = buttons.indexOf(cancelButton);
+				if (cancelId !== buttons.length - 1 /* right to primary action */) {
+					buttons.splice(cancelId, 1);
+					buttons.push(cancelButton);
+
+					const buttonIndex = buttonIndeces[cancelId];
+					buttonIndeces.splice(cancelId, 1);
+					buttonIndeces.push(buttonIndex);
+
+					cancelId = buttons.length - 1;
+				}
+			}
+		} else if (isMacintosh) {
+
+			// macOS: the HIG (https://developer.apple.com/design/human-interface-guidelines/components/presentation/alerts)
+			// recommend the following:
+			// "Place buttons where people expect. In general, place the button people are most likely to choose on the trailing side in a
+			//  row of buttons or at the top in a stack of buttons. Always place the default button on the trailing side of a row or at the
+			//  top of a stack. Cancel buttons are typically on the leading side of a row or at the bottom of a stack."
+			//
+			// Electron APIs ensure above for us, so there is nothing we have to do.
+		}
+	}
+
+	massagedOptions.buttons = buttons;
+	massagedOptions.defaultId = defaultId;
+	massagedOptions.cancelId = cancelId;
+	massagedOptions.noLink = true;
+	massagedOptions.title = massagedOptions.title || productService.nameLong;
+
+	return {
+		options: massagedOptions,
+		buttonIndeces
+	};
+}