From 91e59a296e4f3477715e3525357f7ae3458745d3 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Thu, 5 Oct 2023 09:51:10 +0200 Subject: [PATCH] aux window - better focus and reveal handling for editors (#194828) * first cut focus handling * :lipstick: * implement moveToTop * cleanup --- build/gulpfile.vscode.js | 1 + src/vs/base/browser/dom.ts | 14 ++- .../sandbox/electron-sandbox/electronTypes.ts | 107 ++++++++++-------- .../sandbox/electron-sandbox/preload-slim.js | 54 +++++++++ src/vs/code/electron-main/app.ts | 28 ++--- src/vs/platform/native/common/native.ts | 1 + .../electron-main/nativeHostMainService.ts | 7 ++ .../windows/electron-main/auxiliaryWindow.ts | 74 ++++++++++++ src/vs/workbench/browser/composite.ts | 10 +- src/vs/workbench/browser/panecomposite.ts | 2 + .../browser/parts/editor/editorPanes.ts | 22 +++- .../browser/parts/editor/editorPlaceholder.ts | 6 +- .../browser/parts/editor/sideBySideEditor.ts | 2 + .../browser/parts/editor/textCodeEditor.ts | 2 + .../browser/parts/editor/textDiffEditor.ts | 2 + .../contrib/chat/browser/chatEditor.ts | 2 + .../extensions/browser/extensionEditor.ts | 2 + .../interactive/browser/interactiveEditor.ts | 2 + .../mergeEditor/browser/view/mergeEditor.ts | 2 + .../browser/diff/notebookDiffEditor.ts | 4 - .../preferences/browser/keybindingsEditor.ts | 2 + .../preferences/browser/settingsEditor2.ts | 2 + .../searchEditor/browser/searchEditor.ts | 2 + .../terminal/browser/terminalEditor.ts | 2 + .../browser/gettingStarted.ts | 2 + .../browser/walkThroughPart.ts | 2 + .../workspace/browser/workspaceTrustEditor.ts | 2 + .../browser/auxiliaryWindowService.ts | 24 ++-- .../auxiliaryWindowService.ts | 59 ++++++++++ .../host/browser/browserHostService.ts | 4 + .../workbench/services/host/browser/host.ts | 5 + .../electron-sandbox/nativeHostService.ts | 11 ++ .../test/browser/workbenchTestServices.ts | 1 + .../electron-sandbox/workbenchTestServices.ts | 1 + src/vs/workbench/workbench.common.main.ts | 1 - src/vs/workbench/workbench.desktop.main.ts | 1 + src/vs/workbench/workbench.web.main.ts | 1 + 37 files changed, 375 insertions(+), 91 deletions(-) create mode 100644 src/vs/base/parts/sandbox/electron-sandbox/preload-slim.js create mode 100644 src/vs/platform/windows/electron-main/auxiliaryWindow.ts create mode 100644 src/vs/workbench/services/auxiliaryWindow/electron-sandbox/auxiliaryWindowService.ts diff --git a/build/gulpfile.vscode.js b/build/gulpfile.vscode.js index 5f7f5ce4470..4095f0f7419 100644 --- a/build/gulpfile.vscode.js +++ b/build/gulpfile.vscode.js @@ -64,6 +64,7 @@ const vscodeResources = [ 'out-build/vs/base/node/{stdForkStart.js,terminateProcess.sh,cpuUsage.sh,ps.sh}', 'out-build/vs/base/browser/ui/codicons/codicon/**', 'out-build/vs/base/parts/sandbox/electron-sandbox/preload.js', + 'out-build/vs/base/parts/sandbox/electron-sandbox/preload-slim.js', 'out-build/vs/workbench/browser/media/*-theme.css', 'out-build/vs/workbench/contrib/debug/**/*.json', 'out-build/vs/workbench/contrib/externalTerminal/**/*.scpt', diff --git a/src/vs/base/browser/dom.ts b/src/vs/base/browser/dom.ts index 0866bd9ba22..57aace68687 100644 --- a/src/vs/base/browser/dom.ts +++ b/src/vs/base/browser/dom.ts @@ -722,7 +722,7 @@ export function getActiveElement(): Element | null { * Use this instead of `document` when reacting to dom events to handle multiple windows. */ export function getActiveDocument(): Document { - const documents = Array.from(getWindows()).map(w => w.document); + const documents = Array.from(getWindows()).map(window => window.document); return documents.find(doc => doc.hasFocus()) ?? document; } @@ -731,7 +731,10 @@ export function getActiveWindow(): Window & typeof globalThis { return document.defaultView?.window ?? window; } -function getWindow(e: unknown): Window & typeof globalThis { +export function getWindow(element: Node): Window & typeof globalThis; +export function getWindow(event: UIEvent): Window & typeof globalThis; +export function getWindow(obj: unknown): Window & typeof globalThis; +export function getWindow(e: unknown): Window & typeof globalThis { const candidateNode = e as Node | undefined; if (candidateNode?.ownerDocument?.defaultView) { return candidateNode.ownerDocument.defaultView.window; @@ -745,6 +748,13 @@ function getWindow(e: unknown): Window & typeof globalThis { return window; } +export function focusWindow(element: Node): void { + const window = getWindow(element); + if (window !== getActiveWindow()) { + window.focus(); + } +} + export function createStyleSheet(container: HTMLElement = document.getElementsByTagName('head')[0], beforeAppend?: (style: HTMLStyleElement) => void): HTMLStyleElement { const style = document.createElement('style'); style.type = 'text/css'; diff --git a/src/vs/base/parts/sandbox/electron-sandbox/electronTypes.ts b/src/vs/base/parts/sandbox/electron-sandbox/electronTypes.ts index 58589dbec1d..d32188d1e97 100644 --- a/src/vs/base/parts/sandbox/electron-sandbox/electronTypes.ts +++ b/src/vs/base/parts/sandbox/electron-sandbox/electronTypes.ts @@ -7,10 +7,15 @@ // ####################################################################### // ### ### // ### electron.d.ts types we expose from electron-sandbox ### -// ### (copied from Electron 16.x) ### +// ### (copied from Electron 25.x) ### // ### ### // ####################################################################### +type Event = { + preventDefault: () => void; + readonly defaultPrevented: boolean; +} & Params; + export interface IpcRendererEvent extends Event { // Docs: https://electronjs.org/docs/api/structures/ipc-renderer-event @@ -33,12 +38,49 @@ export interface IpcRendererEvent extends Event { * `event.senderId` to `0`. */ senderId: number; + /** + * Whether the message sent via ipcRenderer.sendTo was sent by the main frame. This + * is relevant when `nodeIntegrationInSubFrames` is enabled in the originating + * `webContents`. + */ + senderIsMainFrame?: boolean; } export interface IpcRenderer { // Docs: https://electronjs.org/docs/api/ipc-renderer + /** + * Resolves with the response from the main process. + * + * Send a message to the main process via `channel` and expect a result + * asynchronously. Arguments will be serialized with the Structured Clone + * Algorithm, just like `window.postMessage`, so prototype chains will not be + * included. Sending Functions, Promises, Symbols, WeakMaps, or WeakSets will throw + * an exception. + * + * The main process should listen for `channel` with `ipcMain.handle()`. + * + * For example: + * + * If you need to transfer a `MessagePort` to the main process, use + * `ipcRenderer.postMessage`. + * + * If you do not need a response to the message, consider using `ipcRenderer.send`. + * + * > **Note** Sending non-standard JavaScript types such as DOM objects or special + * Electron objects will throw an exception. + * + * Since the main process does not have support for DOM objects such as + * `ImageBitmap`, `File`, `DOMMatrix` and so on, such objects cannot be sent over + * Electron's IPC to the main process, as the main process would have no way to + * decode them. Attempting to send such objects over IPC will result in an error. + * + * > **Note** If the handler in the main process throws an error, the promise + * returned by `invoke` will reject. However, the `Error` object in the renderer + * process will not be the same as the one thrown in the main process. + */ + invoke(channel: string, ...args: any[]): Promise; /** * Listens to `channel`, when a new message arrives `listener` would be called with * `listener(event, args...)`. @@ -53,6 +95,22 @@ export interface IpcRenderer { * Removes the specified `listener` from the listener array for the specified * `channel`. */ + // Note: API with `Transferable` intentionally commented out because you + // cannot transfer these when `contextIsolation: true`. + // /** + // * Send a message to the main process, optionally transferring ownership of zero or + // * more `MessagePort` objects. + // * + // * The transferred `MessagePort` objects will be available in the main process as + // * `MessagePortMain` objects by accessing the `ports` property of the emitted + // * event. + // * + // * For example: + // * + // * For more information on using `MessagePort` and `MessageChannel`, see the MDN + // * documentation. + // */ + // postMessage(channel: string, message: any, transfer?: MessagePort[]): void; removeListener(channel: string, listener: (...args: any[]) => void): this; /** * Send an asynchronous message to the main process via `channel`, along with @@ -79,57 +137,14 @@ export interface IpcRenderer { * of a method call, consider using `ipcRenderer.invoke`. */ send(channel: string, ...args: any[]): void; - /** - * Resolves with the response from the main process. - * - * Send a message to the main process via `channel` and expect a result - * asynchronously. Arguments will be serialized with the Structured Clone - * Algorithm, just like `window.postMessage`, so prototype chains will not be - * included. Sending Functions, Promises, Symbols, WeakMaps, or WeakSets will throw - * an exception. - * - * > **NOTE:** Sending non-standard JavaScript types such as DOM objects or special - * Electron objects will throw an exception. - * - * Since the main process does not have support for DOM objects such as - * `ImageBitmap`, `File`, `DOMMatrix` and so on, such objects cannot be sent over - * Electron's IPC to the main process, as the main process would have no way to - * decode them. Attempting to send such objects over IPC will result in an error. - * - * The main process should listen for `channel` with `ipcMain.handle()`. - * - * For example: - * - * If you need to transfer a `MessagePort` to the main process, use - * `ipcRenderer.postMessage`. - * - * If you do not need a response to the message, consider using `ipcRenderer.send`. - */ - invoke(channel: string, ...args: any[]): Promise; - - // Note: API with `Transferable` intentionally commented out because you - // cannot transfer these when `contextIsolation: true`. - // /** - // * Send a message to the main process, optionally transferring ownership of zero or - // * more `MessagePort` objects. - // * - // * The transferred `MessagePort` objects will be available in the main process as - // * `MessagePortMain` objects by accessing the `ports` property of the emitted - // * event. - // * - // * For example: - // * - // * For more information on using `MessagePort` and `MessageChannel`, see the MDN - // * documentation. - // */ - // postMessage(channel: string, message: any, transfer?: MessagePort[]): void; } export interface WebFrame { /** * Changes the zoom level to the specified level. The original size is 0 and each * increment above or below represents zooming 20% larger or smaller to default - * limits of 300% and 50% of original size, respectively. + * limits of 300% and 50% of original size, respectively. The formula for this is + * `scale := 1.2 ^ level`. * * > **NOTE**: The zoom policy at the Chromium level is same-origin, meaning that * the zoom level for a specific domain propagates across all instances of windows diff --git a/src/vs/base/parts/sandbox/electron-sandbox/preload-slim.js b/src/vs/base/parts/sandbox/electron-sandbox/preload-slim.js new file mode 100644 index 00000000000..05d390e37b7 --- /dev/null +++ b/src/vs/base/parts/sandbox/electron-sandbox/preload-slim.js @@ -0,0 +1,54 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +// @ts-check +(function () { + 'use strict'; + + const { ipcRenderer, contextBridge } = require('electron'); + + /** + * @param {string} channel + * @returns {true | never} + */ + function validateIPC(channel) { + if (!channel || !channel.startsWith('vscode:')) { + throw new Error(`Unsupported event IPC channel '${channel}'`); + } + + return true; + } + + const globals = { + + /** + * A minimal set of methods exposed from Electron's `ipcRenderer` + * to support communication to main process. + * + * @typedef {Pick} IpcRenderer + * + * @type {IpcRenderer} + */ + + ipcRenderer: { + + /** + * @param {string} channel + * @param {any[]} args + */ + send(channel, ...args) { + if (validateIPC(channel)) { + ipcRenderer.send(channel, ...args); + } + } + } + }; + + try { + contextBridge.exposeInMainWorld('vscode', globals); + } catch (error) { + console.error(error); + } +}()); diff --git a/src/vs/code/electron-main/app.ts b/src/vs/code/electron-main/app.ts index b35bb6d7135..642abb829c2 100644 --- a/src/vs/code/electron-main/app.ts +++ b/src/vs/code/electron-main/app.ts @@ -84,8 +84,8 @@ import { NativeURLService } from 'vs/platform/url/common/urlService'; import { ElectronURLListener } from 'vs/platform/url/electron-main/electronUrlListener'; import { IWebviewManagerService } from 'vs/platform/webview/common/webviewManagerService'; import { WebviewMainService } from 'vs/platform/webview/electron-main/webviewMainService'; -import { IWindowOpenable, IWindowSettings, zoomLevelToZoomFactor } from 'vs/platform/window/common/window'; -import { defaultBrowserWindowOptions, IWindowsMainService, OpenContext } from 'vs/platform/windows/electron-main/windows'; +import { IWindowOpenable } from 'vs/platform/window/common/window'; +import { IWindowsMainService, OpenContext } from 'vs/platform/windows/electron-main/windows'; import { ICodeWindow } from 'vs/platform/window/electron-main/window'; import { WindowsMainService } from 'vs/platform/windows/electron-main/windowsMainService'; import { ActiveWindowManager } from 'vs/platform/windows/node/windowTracker'; @@ -118,6 +118,7 @@ import { ElectronPtyHostStarter } from 'vs/platform/terminal/electron-main/elect import { PtyHostService } from 'vs/platform/terminal/node/ptyHostService'; import { NODE_REMOTE_RESOURCE_CHANNEL_NAME, NODE_REMOTE_RESOURCE_IPC_METHOD_NAME, NodeRemoteResourceResponse, NodeRemoteResourceRouter } from 'vs/platform/remote/common/electronRemoteResources'; import { Lazy } from 'vs/base/common/lazy'; +import { AuxiliaryWindow } from 'vs/platform/windows/electron-main/auxiliaryWindow'; /** * The main VS Code application. There will only ever be one instance, @@ -382,6 +383,12 @@ export class CodeApplication extends Disposable { // app.on('web-contents-created', (event, contents) => { + // Child Window: delegate to `AuxiliaryWindow` class + const isChildWindow = contents?.opener?.url.startsWith(`${Schemas.vscodeFileResource}://${VSCODE_AUTHORITY}/`); + if (isChildWindow) { + this.mainInstantiationService.createInstance(AuxiliaryWindow, contents); + } + // Block any in-page navigation contents.on('will-navigate', event => { this.logService.error('webContents#will-navigate: Prevented webcontent navigation'); @@ -389,21 +396,6 @@ export class CodeApplication extends Disposable { event.preventDefault(); }); - // Child Window: apply zoom after loading finished and handle --open-devtools - const isChildWindow = contents?.opener?.url.startsWith(`${Schemas.vscodeFileResource}://${VSCODE_AUTHORITY}/`); - if (isChildWindow) { - contents.on('dom-ready', () => { - const windowZoomLevel = this.configurationService.getValue('window')?.zoomLevel ?? 0; - - contents.setZoomLevel(windowZoomLevel); - contents.setZoomFactor(zoomLevelToZoomFactor(windowZoomLevel)); - }); - - if (this.environmentMainService.args['open-devtools'] === true) { - contents.openDevTools({ mode: 'bottom' }); - } - } - // All Windows: only allow about:blank child windows to open // For all other URLs, delegate to the OS. contents.setWindowOpenHandler(handler => { @@ -414,7 +406,7 @@ export class CodeApplication extends Disposable { return { action: 'allow', - overrideBrowserWindowOptions: this.mainInstantiationService.invokeFunction(defaultBrowserWindowOptions) + overrideBrowserWindowOptions: AuxiliaryWindow.open(this.mainInstantiationService) }; } diff --git a/src/vs/platform/native/common/native.ts b/src/vs/platform/native/common/native.ts index 5f063c31719..539e8daf1d3 100644 --- a/src/vs/platform/native/common/native.ts +++ b/src/vs/platform/native/common/native.ts @@ -75,6 +75,7 @@ export interface ICommonNativeHostService { maximizeWindow(): Promise; unmaximizeWindow(): Promise; minimizeWindow(): Promise; + moveWindowTop(): Promise; /** * Only supported on Windows and macOS. Updates the window controls to match the title bar size. diff --git a/src/vs/platform/native/electron-main/nativeHostMainService.ts b/src/vs/platform/native/electron-main/nativeHostMainService.ts index eac0530b40c..ea1a1daf0ba 100644 --- a/src/vs/platform/native/electron-main/nativeHostMainService.ts +++ b/src/vs/platform/native/electron-main/nativeHostMainService.ts @@ -213,6 +213,13 @@ export class NativeHostMainService extends Disposable implements INativeHostMain } } + async moveWindowTop(windowId: number | undefined): Promise { + const window = this.windowById(windowId); + if (window?.win) { + window.win.moveTop(); + } + } + async updateWindowControls(windowId: number | undefined, options: { height?: number; backgroundColor?: string; foregroundColor?: string }): Promise { const window = this.windowById(windowId); if (window) { diff --git a/src/vs/platform/windows/electron-main/auxiliaryWindow.ts b/src/vs/platform/windows/electron-main/auxiliaryWindow.ts new file mode 100644 index 00000000000..c6412ceeed0 --- /dev/null +++ b/src/vs/platform/windows/electron-main/auxiliaryWindow.ts @@ -0,0 +1,74 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { BrowserWindow, BrowserWindowConstructorOptions, WebContents } from 'electron'; +import { FileAccess } from 'vs/base/common/network'; +import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; +import { IEnvironmentMainService } from 'vs/platform/environment/electron-main/environmentMainService'; +import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; +import { IWindowSettings, zoomLevelToZoomFactor } from 'vs/platform/window/common/window'; +import { defaultBrowserWindowOptions } from 'vs/platform/windows/electron-main/windows'; + +export class AuxiliaryWindow { + + static open(instantiationService: IInstantiationService): BrowserWindowConstructorOptions { + return instantiationService.invokeFunction(defaultBrowserWindowOptions, undefined, { + webPreferences: { + preload: FileAccess.asFileUri('vs/base/parts/sandbox/electron-sandbox/preload-slim.js').fsPath + } + }); + } + + constructor( + private readonly contents: WebContents, + @IConfigurationService private readonly configurationService: IConfigurationService, + @IEnvironmentMainService private readonly environmentMainService: IEnvironmentMainService + ) { + + this.create(); + this.registerListeners(); + } + + private create(): void { + + // Apply zoom level when DOM is ready + this.contents.on('dom-ready', () => { + const windowZoomLevel = this.configurationService.getValue('window')?.zoomLevel ?? 0; + + this.contents.setZoomLevel(windowZoomLevel); + this.contents.setZoomFactor(zoomLevelToZoomFactor(windowZoomLevel)); + }); + + // Handle devtools argument + if (this.environmentMainService.args['open-devtools'] === true) { + this.contents.openDevTools({ mode: 'bottom' }); + } + } + + private registerListeners(): void { + + // Support a small set of IPC calls + this.contents.ipc.on('vscode:focusAuxiliaryWindow', () => { + this.withWindow(window => window.focus(), true /* restore */); + }); + + this.contents.ipc.on('vscode:moveAuxiliaryWindowTop', () => { + this.withWindow(window => window.moveTop(), true /* restore */); + }); + } + + private withWindow(callback: (window: BrowserWindow) => void, restore?: boolean): void { + const window = BrowserWindow.fromWebContents(this.contents); + if (window) { + if (restore) { + if (window.isMinimized()) { + window.restore(); + } + } + + callback(window); + } + } +} diff --git a/src/vs/workbench/browser/composite.ts b/src/vs/workbench/browser/composite.ts index 678d5f19c64..6072689b476 100644 --- a/src/vs/workbench/browser/composite.ts +++ b/src/vs/workbench/browser/composite.ts @@ -10,7 +10,7 @@ import { IComposite, ICompositeControl } from 'vs/workbench/common/composite'; import { Event, Emitter } from 'vs/base/common/event'; import { IThemeService } from 'vs/platform/theme/common/themeService'; import { IConstructorSignature, IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; -import { trackFocus, Dimension, IDomPosition } from 'vs/base/browser/dom'; +import { trackFocus, Dimension, IDomPosition, focusWindow } from 'vs/base/browser/dom'; import { IStorageService } from 'vs/platform/storage/common/storage'; import { Disposable } from 'vs/base/common/lifecycle'; import { assertIsDefined } from 'vs/base/common/types'; @@ -148,7 +148,13 @@ export abstract class Composite extends Component implements IComposite { * Called when this composite should receive keyboard focus. */ focus(): void { - // Subclasses can implement + const container = this.getContainer(); + if (container) { + // Make sure to focus the window of the container + // because it is possible that the composite is + // opened in a auxiliary window that is not focussed. + focusWindow(container); + } } /** diff --git a/src/vs/workbench/browser/panecomposite.ts b/src/vs/workbench/browser/panecomposite.ts index 541cd9f742b..fa228cdc284 100644 --- a/src/vs/workbench/browser/panecomposite.ts +++ b/src/vs/workbench/browser/panecomposite.ts @@ -149,6 +149,8 @@ export abstract class PaneComposite extends Composite implements IPaneComposite override focus(): void { this.viewPaneContainer?.focus(); + + super.focus(); } protected abstract createViewPaneContainer(parent: HTMLElement): ViewPaneContainer; diff --git a/src/vs/workbench/browser/parts/editor/editorPanes.ts b/src/vs/workbench/browser/parts/editor/editorPanes.ts index 8860623bde1..69911cb3d1b 100644 --- a/src/vs/workbench/browser/parts/editor/editorPanes.ts +++ b/src/vs/workbench/browser/parts/editor/editorPanes.ts @@ -10,7 +10,7 @@ import Severity from 'vs/base/common/severity'; import { Disposable, DisposableStore } from 'vs/base/common/lifecycle'; import { EditorExtensions, EditorInputCapabilities, IEditorOpenContext, IVisibleEditorPane, isEditorOpenError } from 'vs/workbench/common/editor'; import { EditorInput } from 'vs/workbench/common/editor/editorInput'; -import { Dimension, show, hide, IDomNodePagePosition, isAncestor } from 'vs/base/browser/dom'; +import { Dimension, show, hide, IDomNodePagePosition, isAncestor, getWindow, getActiveWindow } from 'vs/base/browser/dom'; import { Registry } from 'vs/platform/registry/common/platform'; import { IEditorPaneRegistry, IEditorPaneDescriptor } from 'vs/workbench/browser/editor'; import { IWorkbenchLayoutService } from 'vs/workbench/services/layout/browser/layoutService'; @@ -27,6 +27,7 @@ import { toErrorMessage } from 'vs/base/common/errorMessage'; import { ILogService } from 'vs/platform/log/common/log'; import { IDialogService, IPromptButton, IPromptCancelButton } from 'vs/platform/dialogs/common/dialogs'; import { IBoundarySashes } from 'vs/base/browser/ui/sash/sash'; +import { IHostService } from 'vs/workbench/services/host/browser/host'; export interface IOpenEditorResult { @@ -99,7 +100,8 @@ export class EditorPanes extends Disposable { @IEditorProgressService private readonly editorProgressService: IEditorProgressService, @IWorkspaceTrustManagementService private readonly workspaceTrustService: IWorkspaceTrustManagementService, @ILogService private readonly logService: ILogService, - @IDialogService private readonly dialogService: IDialogService + @IDialogService private readonly dialogService: IDialogService, + @IHostService private readonly hostService: IHostService ) { super(); @@ -251,10 +253,18 @@ export class EditorPanes extends Disposable { // Apply input to pane const { changed, cancelled } = await this.doSetInput(pane, editor, options, context); - // Focus only if not cancelled and not prevented - const focus = !options || !options.preserveFocus; - if (!cancelled && focus && this.shouldRestoreFocus(activeElement)) { - pane.focus(); + // Make sure to pass focus to the pane or otherwise + // make sure that the pane window is visible. + if (!cancelled) { + const focus = !options || !options.preserveFocus; + if (focus && this.shouldRestoreFocus(activeElement)) { + pane.focus(); + } else { + const paneWindow = getWindow(pane.getContainer()); + if (paneWindow !== getActiveWindow()) { + this.hostService.moveTop(paneWindow); + } + } } return { pane, changed, cancelled }; diff --git a/src/vs/workbench/browser/parts/editor/editorPlaceholder.ts b/src/vs/workbench/browser/parts/editor/editorPlaceholder.ts index b59fbf67805..746070f464a 100644 --- a/src/vs/workbench/browser/parts/editor/editorPlaceholder.ts +++ b/src/vs/workbench/browser/parts/editor/editorPlaceholder.ts @@ -17,7 +17,7 @@ import { Dimension, size, clearNode, $, EventHelper } from 'vs/base/browser/dom' import { CancellationToken } from 'vs/base/common/cancellation'; import { DisposableStore, IDisposable, MutableDisposable } from 'vs/base/common/lifecycle'; import { IStorageService } from 'vs/platform/storage/common/storage'; -import { assertIsDefined, assertAllDefined } from 'vs/base/common/types'; +import { assertAllDefined } from 'vs/base/common/types'; import { ICommandService } from 'vs/platform/commands/common/commands'; import { IWorkspaceContextService, isSingleFolderWorkspaceIdentifier, toWorkspaceIdentifier } from 'vs/platform/workspace/common/workspace'; import { EditorOpenSource, IEditorOptions } from 'vs/platform/editor/common/editor'; @@ -166,9 +166,9 @@ export abstract class EditorPlaceholder extends EditorPane { } override focus(): void { - const container = assertIsDefined(this.container); + this.container?.focus(); - container.focus(); + super.focus(); } override dispose(): void { diff --git a/src/vs/workbench/browser/parts/editor/sideBySideEditor.ts b/src/vs/workbench/browser/parts/editor/sideBySideEditor.ts index 4c5b3425a57..d9666d18dd2 100644 --- a/src/vs/workbench/browser/parts/editor/sideBySideEditor.ts +++ b/src/vs/workbench/browser/parts/editor/sideBySideEditor.ts @@ -419,6 +419,8 @@ export class SideBySideEditor extends AbstractEditorWithViewState extends override focus(): void { this.editorControl?.focus(); + + super.focus(); } override hasFocus(): boolean { diff --git a/src/vs/workbench/browser/parts/editor/textDiffEditor.ts b/src/vs/workbench/browser/parts/editor/textDiffEditor.ts index 5cbf571c2d2..7af75ed8903 100644 --- a/src/vs/workbench/browser/parts/editor/textDiffEditor.ts +++ b/src/vs/workbench/browser/parts/editor/textDiffEditor.ts @@ -339,6 +339,8 @@ export class TextDiffEditor extends AbstractTextEditor imp override focus(): void { this.diffEditorControl?.focus(); + + super.focus(); } override hasFocus(): boolean { diff --git a/src/vs/workbench/contrib/chat/browser/chatEditor.ts b/src/vs/workbench/contrib/chat/browser/chatEditor.ts index b0d54e30850..4082f00dc6b 100644 --- a/src/vs/workbench/contrib/chat/browser/chatEditor.ts +++ b/src/vs/workbench/contrib/chat/browser/chatEditor.ts @@ -75,6 +75,8 @@ export class ChatEditor extends EditorPane { if (this.widget) { this.widget.focusInput(); } + + super.focus(); } override clearInput(): void { diff --git a/src/vs/workbench/contrib/extensions/browser/extensionEditor.ts b/src/vs/workbench/contrib/extensions/browser/extensionEditor.ts index dfbc2063d48..0abbb01f1a7 100644 --- a/src/vs/workbench/contrib/extensions/browser/extensionEditor.ts +++ b/src/vs/workbench/contrib/extensions/browser/extensionEditor.ts @@ -624,6 +624,8 @@ export class ExtensionEditor extends EditorPane { override focus(): void { this.activeElement?.focus(); + + super.focus(); } showFind(): void { diff --git a/src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts b/src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts index 11644704a78..28aa5559b52 100644 --- a/src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts +++ b/src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts @@ -675,6 +675,8 @@ export class InteractiveEditor extends EditorPane { override focus() { this._notebookWidget.value?.onShow(); this._codeEditorWidget.focus(); + + super.focus(); } focusHistory() { diff --git a/src/vs/workbench/contrib/mergeEditor/browser/view/mergeEditor.ts b/src/vs/workbench/contrib/mergeEditor/browser/view/mergeEditor.ts index 6521589e940..a711a39431e 100644 --- a/src/vs/workbench/contrib/mergeEditor/browser/view/mergeEditor.ts +++ b/src/vs/workbench/contrib/mergeEditor/browser/view/mergeEditor.ts @@ -454,6 +454,8 @@ export class MergeEditor extends AbstractTextEditor { override focus(): void { (this.getControl() ?? this.inputResultView.editor).focus(); + + super.focus(); } override hasFocus(): boolean { diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts index 955cf94c74e..2d6eb9ea124 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts @@ -967,10 +967,6 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD super.setEditorVisible(visible, group); } - override focus() { - super.focus(); - } - override clearInput(): void { super.clearInput(); diff --git a/src/vs/workbench/contrib/preferences/browser/keybindingsEditor.ts b/src/vs/workbench/contrib/preferences/browser/keybindingsEditor.ts index 167296e4c70..7c3a32305fa 100644 --- a/src/vs/workbench/contrib/preferences/browser/keybindingsEditor.ts +++ b/src/vs/workbench/contrib/preferences/browser/keybindingsEditor.ts @@ -192,6 +192,8 @@ export class KeybindingsEditor extends EditorPane implements IKeybindingsEditorP } else if (!isIOS) { this.searchWidget.focus(); } + + super.focus(); } get activeKeybindingEntry(): IKeybindingItemEntry | null { diff --git a/src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts b/src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts index 4f656a0c467..7f30da60612 100644 --- a/src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts +++ b/src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts @@ -493,6 +493,8 @@ export class SettingsEditor2 extends EditorPane { } else if (this._currentFocusContext === SettingsFocusContext.TableOfContents) { this.tocTree.domFocus(); } + + super.focus(); } protected override setEditorVisible(visible: boolean, group: IEditorGroup | undefined): void { diff --git a/src/vs/workbench/contrib/searchEditor/browser/searchEditor.ts b/src/vs/workbench/contrib/searchEditor/browser/searchEditor.ts index 81f2a3ea90e..f73d0ddbcbe 100644 --- a/src/vs/workbench/contrib/searchEditor/browser/searchEditor.ts +++ b/src/vs/workbench/contrib/searchEditor/browser/searchEditor.ts @@ -278,6 +278,8 @@ export class SearchEditor extends AbstractTextCodeEditor } else { this.queryEditorWidget.focus(); } + + super.focus(); } focusSearchInput() { diff --git a/src/vs/workbench/contrib/terminal/browser/terminalEditor.ts b/src/vs/workbench/contrib/terminal/browser/terminalEditor.ts index 24e872ca88e..23719c3bb76 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalEditor.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalEditor.ts @@ -98,6 +98,8 @@ export class TerminalEditor extends EditorPane { override focus() { this._editorInput?.terminalInstance?.focus(); + + super.focus(); } // eslint-disable-next-line @typescript-eslint/naming-convention diff --git a/src/vs/workbench/contrib/welcomeGettingStarted/browser/gettingStarted.ts b/src/vs/workbench/contrib/welcomeGettingStarted/browser/gettingStarted.ts index d42c329a868..17bdabb3297 100644 --- a/src/vs/workbench/contrib/welcomeGettingStarted/browser/gettingStarted.ts +++ b/src/vs/workbench/contrib/welcomeGettingStarted/browser/gettingStarted.ts @@ -1586,6 +1586,8 @@ export class GettingStartedPage extends EditorPane { // This prevents us from stealing back focus from other focused elements such as quick pick due to delayed load. this.container.focus(); } + + super.focus(); } } diff --git a/src/vs/workbench/contrib/welcomeWalkthrough/browser/walkThroughPart.ts b/src/vs/workbench/contrib/welcomeWalkthrough/browser/walkThroughPart.ts index ba5dd1f6ea9..3a411232561 100644 --- a/src/vs/workbench/contrib/welcomeWalkthrough/browser/walkThroughPart.ts +++ b/src/vs/workbench/contrib/welcomeWalkthrough/browser/walkThroughPart.ts @@ -232,6 +232,8 @@ export class WalkThroughPart extends EditorPane { (this.lastFocus || this.content).focus(); } this.editorFocus.set(true); + + super.focus(); } arrowUp() { diff --git a/src/vs/workbench/contrib/workspace/browser/workspaceTrustEditor.ts b/src/vs/workbench/contrib/workspace/browser/workspaceTrustEditor.ts index 317b40cdfce..59d1c84e515 100644 --- a/src/vs/workbench/contrib/workspace/browser/workspaceTrustEditor.ts +++ b/src/vs/workbench/contrib/workspace/browser/workspaceTrustEditor.ts @@ -747,6 +747,8 @@ export class WorkspaceTrustEditor extends EditorPane { override focus() { this.rootElement.focus(); + + super.focus(); } override async setInput(input: WorkspaceTrustEditorInput, options: IEditorOptions | undefined, context: IEditorOpenContext, token: CancellationToken): Promise { diff --git a/src/vs/workbench/services/auxiliaryWindow/browser/auxiliaryWindowService.ts b/src/vs/workbench/services/auxiliaryWindow/browser/auxiliaryWindowService.ts index c96bb817d80..7a2f043a00e 100644 --- a/src/vs/workbench/services/auxiliaryWindow/browser/auxiliaryWindowService.ts +++ b/src/vs/workbench/services/auxiliaryWindow/browser/auxiliaryWindowService.ts @@ -32,7 +32,9 @@ export interface IAuxiliaryWindow extends IDisposable { readonly container: HTMLElement; } -export class AuxiliaryWindowService implements IAuxiliaryWindowService { +type AuxiliaryWindow = Window & typeof globalThis; + +export class BrowserAuxiliaryWindowService implements IAuxiliaryWindowService { declare readonly _serviceBrand: undefined; @@ -45,11 +47,11 @@ export class AuxiliaryWindowService implements IAuxiliaryWindowService { open(): IAuxiliaryWindow { const disposables = new DisposableStore(); - const auxiliaryWindow = assertIsDefined(window.open('about:blank')?.window); + const auxiliaryWindow = assertIsDefined(window.open('about:blank')?.window) as AuxiliaryWindow; disposables.add(registerWindow(auxiliaryWindow)); disposables.add(toDisposable(() => auxiliaryWindow.close())); - this.blockMethods(auxiliaryWindow); + this.patchMethods(auxiliaryWindow); this.applyMeta(auxiliaryWindow); this.applyCSS(auxiliaryWindow, disposables); @@ -69,7 +71,7 @@ export class AuxiliaryWindowService implements IAuxiliaryWindowService { }; } - private applyMeta(auxiliaryWindow: Window): void { + private applyMeta(auxiliaryWindow: AuxiliaryWindow): void { const metaCharset = auxiliaryWindow.document.head.appendChild(document.createElement('meta')); metaCharset.setAttribute('charset', 'utf-8'); @@ -80,7 +82,7 @@ export class AuxiliaryWindowService implements IAuxiliaryWindowService { } } - private applyCSS(auxiliaryWindow: Window, disposables: DisposableStore): void { + private applyCSS(auxiliaryWindow: AuxiliaryWindow, disposables: DisposableStore): void { // Clone all style elements and stylesheet links from the window to the child window for (const element of document.head.querySelectorAll('link[rel="stylesheet"], style')) { @@ -107,7 +109,7 @@ export class AuxiliaryWindowService implements IAuxiliaryWindowService { } } - private applyHTML(auxiliaryWindow: Window, disposables: DisposableStore): HTMLElement { + private applyHTML(auxiliaryWindow: AuxiliaryWindow, disposables: DisposableStore): HTMLElement { // Create workbench container and apply classes const container = document.createElement('div'); @@ -121,7 +123,7 @@ export class AuxiliaryWindowService implements IAuxiliaryWindowService { return container; } - private registerListeners(auxiliaryWindow: Window & typeof globalThis, container: HTMLElement, disposables: DisposableStore) { + private registerListeners(auxiliaryWindow: AuxiliaryWindow, container: HTMLElement, disposables: DisposableStore) { const onDidClose = disposables.add(new Emitter()); disposables.add(addDisposableListener(auxiliaryWindow, 'unload', () => { onDidClose.fire(); @@ -153,11 +155,15 @@ export class AuxiliaryWindowService implements IAuxiliaryWindowService { return { onDidResize, onDidClose }; } - private blockMethods(auxiliaryWindow: Window): void { + protected patchMethods(auxiliaryWindow: AuxiliaryWindow): void { + + // Disallow `createElement` because it would create + // HTML Elements in the "wrong" context and break + // code that does "instanceof HTMLElement" etc. auxiliaryWindow.document.createElement = function () { throw new Error('Not allowed to create elements in child window JavaScript context. Always use the main window so that "xyz instanceof HTMLElement" continues to work.'); }; } } -registerSingleton(IAuxiliaryWindowService, AuxiliaryWindowService, InstantiationType.Delayed); +registerSingleton(IAuxiliaryWindowService, BrowserAuxiliaryWindowService, InstantiationType.Delayed); diff --git a/src/vs/workbench/services/auxiliaryWindow/electron-sandbox/auxiliaryWindowService.ts b/src/vs/workbench/services/auxiliaryWindow/electron-sandbox/auxiliaryWindowService.ts new file mode 100644 index 00000000000..ed06aa9e265 --- /dev/null +++ b/src/vs/workbench/services/auxiliaryWindow/electron-sandbox/auxiliaryWindowService.ts @@ -0,0 +1,59 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { InstantiationType, registerSingleton } from 'vs/platform/instantiation/common/extensions'; +import { IWorkbenchLayoutService } from 'vs/workbench/services/layout/browser/layoutService'; +import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService'; +import { ILifecycleService } from 'vs/workbench/services/lifecycle/common/lifecycle'; +import { BrowserAuxiliaryWindowService, IAuxiliaryWindowService } from 'vs/workbench/services/auxiliaryWindow/browser/auxiliaryWindowService'; + +type AuxiliaryWindow = Window & typeof globalThis & { + vscode: { + ipcRenderer: Pick; + }; + moveTop: () => void; +}; + +export function isAuxiliaryWindow(obj: unknown): obj is AuxiliaryWindow { + const candidate = obj as AuxiliaryWindow | undefined; + + return typeof candidate?.vscode?.ipcRenderer?.send === 'function'; +} + +export class NativeAuxiliaryWindowService extends BrowserAuxiliaryWindowService { + + constructor( + @IWorkbenchLayoutService layoutService: IWorkbenchLayoutService, + @IWorkbenchEnvironmentService environmentService: IWorkbenchEnvironmentService, + @ILifecycleService lifecycleService: ILifecycleService + ) { + super(layoutService, environmentService, lifecycleService); + } + + protected override patchMethods(auxiliaryWindow: AuxiliaryWindow): void { + super.patchMethods(auxiliaryWindow); + + // Enable `window.focus()` to work in Electron by + // asking the main process to focus the window. + const originalWindowFocus = auxiliaryWindow.focus.bind(auxiliaryWindow); + auxiliaryWindow.focus = function () { + originalWindowFocus(); + + auxiliaryWindow.vscode.ipcRenderer.send('vscode:focusAuxiliaryWindow'); + }; + + // Add a method to move window to the top (TODO@bpasero better to go entirely through native host service) + Object.defineProperty(auxiliaryWindow, 'moveTop', { + value: () => { + auxiliaryWindow.vscode.ipcRenderer.send('vscode:moveAuxiliaryWindowTop'); + }, + writable: false, + enumerable: false, + configurable: false + }); + } +} + +registerSingleton(IAuxiliaryWindowService, NativeAuxiliaryWindowService, InstantiationType.Delayed); diff --git a/src/vs/workbench/services/host/browser/browserHostService.ts b/src/vs/workbench/services/host/browser/browserHostService.ts index 911ad41e1ed..008e5065ab3 100644 --- a/src/vs/workbench/services/host/browser/browserHostService.ts +++ b/src/vs/workbench/services/host/browser/browserHostService.ts @@ -501,6 +501,10 @@ export class BrowserHostService extends Disposable implements IHostService { } } + async moveTop(window: Window & typeof globalThis): Promise { + // There seems to be no API to bring a window to front in browsers + } + //#endregion //#region Lifecycle diff --git a/src/vs/workbench/services/host/browser/host.ts b/src/vs/workbench/services/host/browser/host.ts index 6072cff2843..569f40e6c60 100644 --- a/src/vs/workbench/services/host/browser/host.ts +++ b/src/vs/workbench/services/host/browser/host.ts @@ -69,6 +69,11 @@ export interface IHostService { */ toggleFullScreen(): Promise; + /** + * Bring a window to the front and restore it if needed. + */ + moveTop(window: Window & typeof globalThis): Promise; + //#endregion //#region Lifecycle diff --git a/src/vs/workbench/services/host/electron-sandbox/nativeHostService.ts b/src/vs/workbench/services/host/electron-sandbox/nativeHostService.ts index 131972fa538..6007a21ff11 100644 --- a/src/vs/workbench/services/host/electron-sandbox/nativeHostService.ts +++ b/src/vs/workbench/services/host/electron-sandbox/nativeHostService.ts @@ -14,6 +14,7 @@ import { Disposable } from 'vs/base/common/lifecycle'; import { NativeHostService } from 'vs/platform/native/electron-sandbox/nativeHostService'; import { INativeWorkbenchEnvironmentService } from 'vs/workbench/services/environment/electron-sandbox/environmentService'; import { IMainProcessService } from 'vs/platform/ipc/common/mainProcessService'; +import { isAuxiliaryWindow } from 'vs/workbench/services/auxiliaryWindow/electron-sandbox/auxiliaryWindowService'; class WorkbenchNativeHostService extends NativeHostService { @@ -114,6 +115,16 @@ class WorkbenchHostService extends Disposable implements IHostService { return this.nativeHostService.toggleFullScreen(); } + async moveTop(win: Window & typeof globalThis): Promise { + if (win === window) { + return this.nativeHostService.moveWindowTop(); + } + + if (isAuxiliaryWindow(win)) { + return win.moveTop(); + } + } + //#endregion diff --git a/src/vs/workbench/test/browser/workbenchTestServices.ts b/src/vs/workbench/test/browser/workbenchTestServices.ts index a6e3f76309e..33d485df930 100644 --- a/src/vs/workbench/test/browser/workbenchTestServices.ts +++ b/src/vs/workbench/test/browser/workbenchTestServices.ts @@ -1463,6 +1463,7 @@ export class TestHostService implements IHostService { } async focus(options?: { force: boolean }): Promise { } + async moveTop(): Promise { } async openWindow(arg1?: IOpenEmptyWindowOptions | IWindowOpenable[], arg2?: IOpenWindowOptions): Promise { } diff --git a/src/vs/workbench/test/electron-sandbox/workbenchTestServices.ts b/src/vs/workbench/test/electron-sandbox/workbenchTestServices.ts index b5ad4d0e96f..18e85e8af00 100644 --- a/src/vs/workbench/test/electron-sandbox/workbenchTestServices.ts +++ b/src/vs/workbench/test/electron-sandbox/workbenchTestServices.ts @@ -93,6 +93,7 @@ export class TestNativeHostService implements INativeHostService { async maximizeWindow(): Promise { } async unmaximizeWindow(): Promise { } async minimizeWindow(): Promise { } + async moveWindowTop(): Promise { } async updateWindowControls(options: { height?: number; backgroundColor?: string; foregroundColor?: string }): Promise { } async setMinimumSize(width: number | undefined, height: number | undefined): Promise { } async saveWindowSplash(value: IPartsSplash): Promise { } diff --git a/src/vs/workbench/workbench.common.main.ts b/src/vs/workbench/workbench.common.main.ts index f01a05e602c..81e55f19de6 100644 --- a/src/vs/workbench/workbench.common.main.ts +++ b/src/vs/workbench/workbench.common.main.ts @@ -114,7 +114,6 @@ import 'vs/workbench/services/textMate/browser/textMateTokenizationFeature.contr import 'vs/workbench/services/userActivity/common/userActivityService'; import 'vs/workbench/services/userActivity/browser/userActivityBrowser'; import 'vs/workbench/services/issue/browser/issueTroubleshoot'; -import 'vs/workbench/services/auxiliaryWindow/browser/auxiliaryWindowService'; import { InstantiationType, registerSingleton } from 'vs/platform/instantiation/common/extensions'; import { ExtensionGalleryService } from 'vs/platform/extensionManagement/common/extensionGalleryService'; diff --git a/src/vs/workbench/workbench.desktop.main.ts b/src/vs/workbench/workbench.desktop.main.ts index c7e3960445e..4f5ab26bb35 100644 --- a/src/vs/workbench/workbench.desktop.main.ts +++ b/src/vs/workbench/workbench.desktop.main.ts @@ -88,6 +88,7 @@ import 'vs/workbench/services/workingCopy/electron-sandbox/workingCopyHistorySer import 'vs/workbench/services/userDataSync/browser/userDataSyncEnablementService'; import 'vs/workbench/services/extensions/electron-sandbox/nativeExtensionService'; import 'vs/platform/userDataProfile/electron-sandbox/userDataProfileStorageService'; +import 'vs/workbench/services/auxiliaryWindow/electron-sandbox/auxiliaryWindowService'; import { InstantiationType, registerSingleton } from 'vs/platform/instantiation/common/extensions'; import { IUserDataInitializationService, UserDataInitializationService } from 'vs/workbench/services/userData/browser/userDataInit'; diff --git a/src/vs/workbench/workbench.web.main.ts b/src/vs/workbench/workbench.web.main.ts index 3b8c28f5937..c61a78d6fc1 100644 --- a/src/vs/workbench/workbench.web.main.ts +++ b/src/vs/workbench/workbench.web.main.ts @@ -63,6 +63,7 @@ import 'vs/workbench/services/userDataSync/browser/webUserDataSyncEnablementServ import 'vs/workbench/services/userDataProfile/browser/userDataProfileStorageService'; import 'vs/workbench/services/configurationResolver/browser/configurationResolverService'; import 'vs/platform/extensionResourceLoader/browser/extensionResourceLoaderService'; +import 'vs/workbench/services/auxiliaryWindow/browser/auxiliaryWindowService'; import { InstantiationType, registerSingleton } from 'vs/platform/instantiation/common/extensions'; import { IAccessibilityService } from 'vs/platform/accessibility/common/accessibility';