From 5356ceb23cca764874d675fc01f3c09839cb26f0 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Sun, 10 May 2020 09:50:13 +0200 Subject: [PATCH] windows - ensure new-window request has right window context (fix #97172) --- src/vs/code/electron-main/app.ts | 4 +- .../electron-main/electronMainService.ts | 5 ++- .../launch/electron-main/launchMainService.ts | 2 +- .../platform/menubar/electron-main/menubar.ts | 8 ++-- .../platform/windows/electron-main/windows.ts | 9 ++++- .../electron-main/windowsMainService.ts | 38 +++++++++++-------- 6 files changed, 40 insertions(+), 26 deletions(-) diff --git a/src/vs/code/electron-main/app.ts b/src/vs/code/electron-main/app.ts index 3a20695a397..fd287c25045 100644 --- a/src/vs/code/electron-main/app.ts +++ b/src/vs/code/electron-main/app.ts @@ -125,7 +125,7 @@ export class CodeApplication extends Disposable { // Mac only event: open new window when we get activated if (!hasVisibleWindows && this.windowsMainService) { - this.windowsMainService.openEmptyWindow(OpenContext.DOCK); + this.windowsMainService.openEmptyWindow({ context: OpenContext.DOCK }); } }); @@ -258,7 +258,7 @@ export class CodeApplication extends Disposable { app.on('new-window-for-tab', () => { if (this.windowsMainService) { - this.windowsMainService.openEmptyWindow(OpenContext.DESKTOP); //macOS native tab "+" button + this.windowsMainService.openEmptyWindow({ context: OpenContext.DESKTOP }); //macOS native tab "+" button } }); diff --git a/src/vs/platform/electron/electron-main/electronMainService.ts b/src/vs/platform/electron/electron-main/electronMainService.ts index a8cc247f89d..c21fb966278 100644 --- a/src/vs/platform/electron/electron-main/electronMainService.ts +++ b/src/vs/platform/electron/electron-main/electronMainService.ts @@ -114,7 +114,10 @@ export class ElectronMainService implements IElectronMainService { } private async doOpenEmptyWindow(windowId: number | undefined, options?: IOpenEmptyWindowOptions): Promise { - this.windowsMainService.openEmptyWindow(OpenContext.API, options); + this.windowsMainService.openEmptyWindow({ + context: OpenContext.API, + contextWindowId: windowId + }, options); } async toggleFullScreen(windowId: number | undefined): Promise { diff --git a/src/vs/platform/launch/electron-main/launchMainService.ts b/src/vs/platform/launch/electron-main/launchMainService.ts index 17698833e34..c2ede9faa64 100644 --- a/src/vs/platform/launch/electron-main/launchMainService.ts +++ b/src/vs/platform/launch/electron-main/launchMainService.ts @@ -84,7 +84,7 @@ export class LaunchMainService implements ILaunchMainService { // Create a window if there is none if (this.windowsMainService.getWindowCount() === 0) { - const window = this.windowsMainService.openEmptyWindow(OpenContext.DESKTOP)[0]; + const window = this.windowsMainService.openEmptyWindow({ context: OpenContext.DESKTOP })[0]; whenWindowReady = window.ready(); } diff --git a/src/vs/platform/menubar/electron-main/menubar.ts b/src/vs/platform/menubar/electron-main/menubar.ts index dd0251290f2..ac66913edbf 100644 --- a/src/vs/platform/menubar/electron-main/menubar.ts +++ b/src/vs/platform/menubar/electron-main/menubar.ts @@ -61,7 +61,7 @@ export class Menubar { private keybindings: { [commandId: string]: IMenubarKeybinding }; - private fallbackMenuHandlers: { [id: string]: (menuItem: MenuItem, browserWindow: BrowserWindow, event: Event) => void } = {}; + private readonly fallbackMenuHandlers: { [id: string]: (menuItem: MenuItem, browserWindow: BrowserWindow, event: Event) => void } = Object.create(null); constructor( @IUpdateService private readonly updateService: IUpdateService, @@ -113,8 +113,8 @@ export class Menubar { private addFallbackHandlers(): void { // File Menu Items - this.fallbackMenuHandlers['workbench.action.files.newUntitledFile'] = () => this.windowsMainService.openEmptyWindow(OpenContext.MENU); - this.fallbackMenuHandlers['workbench.action.newWindow'] = () => this.windowsMainService.openEmptyWindow(OpenContext.MENU); + this.fallbackMenuHandlers['workbench.action.files.newUntitledFile'] = (menuItem, win, event) => this.windowsMainService.openEmptyWindow({ context: OpenContext.MENU, contextWindowId: win.id }); + this.fallbackMenuHandlers['workbench.action.newWindow'] = (menuItem, win, event) => this.windowsMainService.openEmptyWindow({ context: OpenContext.MENU, contextWindowId: win.id }); this.fallbackMenuHandlers['workbench.action.files.openFileFolder'] = (menuItem, win, event) => this.electronMainService.pickFileFolderAndOpen(undefined, { forceNewWindow: this.isOptionClick(event), telemetryExtraData: { from: telemetryFrom } }); this.fallbackMenuHandlers['workbench.action.openWorkspace'] = (menuItem, win, event) => this.electronMainService.pickWorkspaceAndOpen(undefined, { forceNewWindow: this.isOptionClick(event), telemetryExtraData: { from: telemetryFrom } }); @@ -266,7 +266,7 @@ export class Menubar { this.appMenuInstalled = true; const dockMenu = new Menu(); - dockMenu.append(new MenuItem({ label: this.mnemonicLabel(nls.localize({ key: 'miNewWindow', comment: ['&& denotes a mnemonic'] }, "New &&Window")), click: () => this.windowsMainService.openEmptyWindow(OpenContext.DOCK) })); + dockMenu.append(new MenuItem({ label: this.mnemonicLabel(nls.localize({ key: 'miNewWindow', comment: ['&& denotes a mnemonic'] }, "New &&Window")), click: () => this.windowsMainService.openEmptyWindow({ context: OpenContext.DOCK }) })); app.dock.setMenu(dockMenu); } diff --git a/src/vs/platform/windows/electron-main/windows.ts b/src/vs/platform/windows/electron-main/windows.ts index 0db6846abd9..5c6facab0a9 100644 --- a/src/vs/platform/windows/electron-main/windows.ts +++ b/src/vs/platform/windows/electron-main/windows.ts @@ -105,7 +105,7 @@ export interface IWindowsMainService { readonly onWindowsCountChanged: Event; open(openConfig: IOpenConfiguration): ICodeWindow[]; - openEmptyWindow(context: OpenContext, options?: IOpenEmptyWindowOptions): ICodeWindow[]; + openEmptyWindow(openConfig: IOpenEmptyConfiguration, options?: IOpenEmptyWindowOptions): ICodeWindow[]; openExtensionDevelopmentHostWindow(extensionDevelopmentPath: string[], openConfig: IOpenConfiguration): ICodeWindow[]; sendToFocused(channel: string, ...args: any[]): void; @@ -118,9 +118,12 @@ export interface IWindowsMainService { getWindowCount(): number; } -export interface IOpenConfiguration { +export interface IBaseOpenConfiguration { readonly context: OpenContext; readonly contextWindowId?: number; +} + +export interface IOpenConfiguration extends IBaseOpenConfiguration { readonly cli: ParsedArgs; readonly userEnv?: IProcessEnvironment; readonly urisToOpen?: IWindowOpenable[]; @@ -136,3 +139,5 @@ export interface IOpenConfiguration { readonly initialStartup?: boolean; readonly noRecentEntry?: boolean; } + +export interface IOpenEmptyConfiguration extends IBaseOpenConfiguration { } diff --git a/src/vs/platform/windows/electron-main/windowsMainService.ts b/src/vs/platform/windows/electron-main/windowsMainService.ts index 266c235a0fe..cdd6731748d 100644 --- a/src/vs/platform/windows/electron-main/windowsMainService.ts +++ b/src/vs/platform/windows/electron-main/windowsMainService.ts @@ -24,7 +24,7 @@ import { IWindowSettings, IPath, isFileToOpen, isWorkspaceToOpen, isFolderToOpen import { getLastActiveWindow, findBestWindowOrFolderForFile, findWindowOnWorkspace, findWindowOnExtensionDevelopmentPath, findWindowOnWorkspaceOrFolderUri, INativeWindowConfiguration, OpenContext, IAddFoldersRequest, IPathsToWaitFor } from 'vs/platform/windows/node/window'; import { Emitter } from 'vs/base/common/event'; import product from 'vs/platform/product/common/product'; -import { IWindowsMainService, IOpenConfiguration, IWindowsCountChangedEvent, ICodeWindow, IWindowState as ISingleWindowState, WindowMode } from 'vs/platform/windows/electron-main/windows'; +import { IWindowsMainService, IOpenConfiguration, IWindowsCountChangedEvent, ICodeWindow, IWindowState as ISingleWindowState, WindowMode, IOpenEmptyConfiguration } from 'vs/platform/windows/electron-main/windows'; import { IWorkspacesHistoryMainService } from 'vs/platform/workspaces/electron-main/workspacesHistoryMainService'; import { IProcessEnvironment, isMacintosh, isWindows } from 'vs/base/common/platform'; import { IWorkspaceIdentifier, isSingleFolderWorkspaceIdentifier, hasWorkspaceFileExtension, IRecent } from 'vs/platform/workspaces/common/workspaces'; @@ -393,7 +393,7 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic }; } - openEmptyWindow(context: OpenContext, options?: IOpenEmptyWindowOptions): ICodeWindow[] { + openEmptyWindow(openConfig: IOpenEmptyConfiguration, options?: IOpenEmptyWindowOptions): ICodeWindow[] { let cli = this.environmentService.args; const remote = options?.remoteAuthority; if (cli && (cli.remote !== remote)) { @@ -403,7 +403,7 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic const forceReuseWindow = options?.forceReuseWindow; const forceNewWindow = !forceReuseWindow; - return this.open({ context, cli, forceEmpty: true, forceNewWindow, forceReuseWindow }); + return this.open({ ...openConfig, cli, forceEmpty: true, forceNewWindow, forceReuseWindow }); } open(openConfig: IOpenConfiguration): ICodeWindow[] { @@ -474,7 +474,6 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic // Make sure to pass focus to the most relevant of the windows if we open multiple if (usedWindows.length > 1) { - const focusLastActive = this.windowsState.lastActiveWindow && !openConfig.forceEmpty && openConfig.cli._.length && !openConfig.cli['file-uri'] && !openConfig.cli['folder-uri'] && !(openConfig.urisToOpen && openConfig.urisToOpen.length); let focusLastOpened = true; let focusLastWindow = true; @@ -753,15 +752,7 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic const remoteAuthority = fileInputs ? fileInputs.remoteAuthority : (openConfig.cli && openConfig.cli.remote || undefined); for (let i = 0; i < emptyToOpen; i++) { - usedWindows.push(this.openInBrowserWindow({ - userEnv: openConfig.userEnv, - cli: openConfig.cli, - initialStartup: openConfig.initialStartup, - remoteAuthority, - forceNewWindow: openFolderInNewWindow, - forceNewTabbedWindow: openConfig.forceNewTabbedWindow, - fileInputs - })); + usedWindows.push(this.doOpenEmpty(openConfig, openFolderInNewWindow, remoteAuthority, fileInputs)); // Reset these because we handled them fileInputs = undefined; @@ -801,12 +792,29 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic return window; } + private doOpenEmpty(openConfig: IOpenConfiguration, forceNewWindow: boolean, remoteAuthority: string | undefined, fileInputs: IFileInputs | undefined, windowToUse?: ICodeWindow): ICodeWindow { + if (!forceNewWindow && !windowToUse && typeof openConfig.contextWindowId === 'number') { + windowToUse = this.getWindowById(openConfig.contextWindowId); // fix for https://github.com/microsoft/vscode/issues/97172 + } + + return this.openInBrowserWindow({ + userEnv: openConfig.userEnv, + cli: openConfig.cli, + initialStartup: openConfig.initialStartup, + remoteAuthority, + forceNewWindow, + forceNewTabbedWindow: openConfig.forceNewTabbedWindow, + fileInputs, + windowToUse + }); + } + private doOpenFolderOrWorkspace(openConfig: IOpenConfiguration, folderOrWorkspace: IPathToOpen, forceNewWindow: boolean, fileInputs: IFileInputs | undefined, windowToUse?: ICodeWindow): ICodeWindow { if (!forceNewWindow && !windowToUse && typeof openConfig.contextWindowId === 'number') { windowToUse = this.getWindowById(openConfig.contextWindowId); // fix for https://github.com/Microsoft/vscode/issues/49587 } - const browserWindow = this.openInBrowserWindow({ + return this.openInBrowserWindow({ userEnv: openConfig.userEnv, cli: openConfig.cli, initialStartup: openConfig.initialStartup, @@ -818,8 +826,6 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic forceNewTabbedWindow: openConfig.forceNewTabbedWindow, windowToUse }); - - return browserWindow; } private getPathsToOpen(openConfig: IOpenConfiguration): IPathToOpen[] { -- GitLab