From ea8db4711269ca67802958cb0c1f272ee36b186d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=A4der?= Date: Wed, 12 Oct 2022 16:02:42 +0200 Subject: [PATCH 1/2] Add secondary window support for Electron. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #11642 The main change is to prevent the secondary window from closing until the extracted widget is removed from the window. This includes waiting until any close handling (including dialogs) are finished. Since we cannot reliably prevent closing windows in the browser case, we either save or discard unsaved changes according to the autosave settings Contributed on behalf of ST Microelectronics. Signed-off-by: Thomas Mäder --- .../src/generator/frontend-generator.ts | 5 +- examples/electron/package.json | 1 + examples/electron/tsconfig.json | 3 + .../src/browser/secondary-window-handler.ts | 41 ++++++++---- .../src/browser/shell/application-shell.ts | 23 ++++--- .../src/browser/widgets/extractable-widget.ts | 3 + packages/core/src/browser/widgets/widget.ts | 7 ++- .../default-secondary-window-service.ts | 25 +++++--- .../window/secondary-window-service.ts | 6 +- .../electron-secondary-window-service.ts | 62 ++++++++++++++----- .../messaging/electron-messages.ts | 10 +++ .../electron-main/theia-electron-window.ts | 58 ++++++++++++++++- .../custom-editors/custom-editors-main.ts | 31 ++++------ .../src/main/browser/webview/webview.ts | 9 +++ .../src/browser/terminal-widget-impl.ts | 1 + 15 files changed, 214 insertions(+), 71 deletions(-) diff --git a/dev-packages/application-manager/src/generator/frontend-generator.ts b/dev-packages/application-manager/src/generator/frontend-generator.ts index a81d391ef3e3c..aa21793fdb43b 100644 --- a/dev-packages/application-manager/src/generator/frontend-generator.ts +++ b/dev-packages/application-manager/src/generator/frontend-generator.ts @@ -230,7 +230,10 @@ module.exports = Promise.resolve()${this.compileElectronMainModuleImports(electr // Only process messages from Theia main window if (e.source === window.opener) { // Delegate message to iframe - document.getElementsByTagName('iframe').item(0).contentWindow.postMessage({ ...e.data }, '*'); + const f = document.getElementsByTagName('iframe'); + if (f) { + f.item(0).contentWindow.postMessage({ ...e.data }, '*'); + } } }); diff --git a/examples/electron/package.json b/examples/electron/package.json index dd2939afd36d0..fb1c0cbda939f 100644 --- a/examples/electron/package.json +++ b/examples/electron/package.json @@ -48,6 +48,7 @@ "@theia/scm": "1.30.0", "@theia/scm-extra": "1.30.0", "@theia/search-in-workspace": "1.30.0", + "@theia/secondary-window": "1.30.0", "@theia/task": "1.30.0", "@theia/terminal": "1.30.0", "@theia/timeline": "1.30.0", diff --git a/examples/electron/tsconfig.json b/examples/electron/tsconfig.json index a9ea549221e47..ccd4b9fbd4d34 100644 --- a/examples/electron/tsconfig.json +++ b/examples/electron/tsconfig.json @@ -107,6 +107,9 @@ { "path": "../../packages/search-in-workspace" }, + { + "path": "../../packages/secondary-window" + }, { "path": "../../packages/task" }, diff --git a/packages/core/src/browser/secondary-window-handler.ts b/packages/core/src/browser/secondary-window-handler.ts index 377bce895cbd8..1ef14e3c21b6d 100644 --- a/packages/core/src/browser/secondary-window-handler.ts +++ b/packages/core/src/browser/secondary-window-handler.ts @@ -14,7 +14,6 @@ // SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 // ***************************************************************************** -import debounce = require('lodash.debounce'); import { inject, injectable } from 'inversify'; import { BoxLayout, BoxPanel, ExtractableWidget, Widget } from './widgets'; import { MessageService } from '../common/message-service'; @@ -23,6 +22,8 @@ import { Emitter } from '../common/event'; import { SecondaryWindowService } from './window/secondary-window-service'; import { KeybindingRegistry } from './keybinding'; import { ColorApplicationContribution } from './color-application-contribution'; +import debounce = require('lodash.debounce'); +import { Saveable } from './saveable'; /** Widget to be contained directly in a secondary window. */ class SecondaryWindowRootWidget extends Widget { @@ -40,7 +41,6 @@ class SecondaryWindowRootWidget extends Widget { } /** - * Offers functionality to move a widget out of the main window to a newly created window. * Widgets must explicitly implement the `ExtractableWidget` interface to support this. * * This handler manages the opened secondary windows and sets up messaging between them and the Theia main window. @@ -139,13 +139,32 @@ export class SecondaryWindowHandler { return; } - const newWindow = this.secondaryWindowService.createSecondaryWindow(closed => { - this.applicationShell.closeWidget(widget.id); - const extIndex = this.secondaryWindows.indexOf(closed); - if (extIndex > -1) { - this.secondaryWindows.splice(extIndex, 1); - } - }); + const newWindow = this.secondaryWindowService.createSecondaryWindow( + () => { + const saveable = Saveable.get(widget); + return !!saveable && saveable.dirty && saveable.autoSave === 'off'; + }, + async trySaving => { + widget.isClosing = true; + // if trySaving is true, we should let the widget decide what to do + const saveable = Saveable.get(widget); + let closeOptions = undefined; + if (!trySaving) { + closeOptions = { + save: !!saveable && saveable.dirty && saveable.autoSave !== 'off' + }; + } + + await this.applicationShell.closeWidget(widget.id, closeOptions); + widget.isClosing = false; + return widget.isDisposed; + }, + closedWindow => { + const extIndex = this.secondaryWindows.indexOf(closedWindow); + if (extIndex > -1) { + this.secondaryWindows.splice(extIndex, 1); + } + }); if (!newWindow) { this.messageService.error('The widget could not be moved to a secondary window because the window creation failed. Please make sure to allow popups.'); @@ -172,6 +191,7 @@ export class SecondaryWindowHandler { widget.secondaryWindow = newWindow; const rootWidget = new SecondaryWindowRootWidget(); rootWidget.addClass('secondary-widget-root'); + rootWidget.id = 'root-' + widget.id; Widget.attach(rootWidget, element); rootWidget.addWidget(widget); widget.show(); @@ -183,9 +203,6 @@ export class SecondaryWindowHandler { widget.disposed.connect(() => { unregisterWithColorContribution.dispose(); this.removeWidget(widget); - if (!newWindow.closed) { - newWindow.close(); - } }); // debounce to avoid rapid updates while resizing the secondary window diff --git a/packages/core/src/browser/shell/application-shell.ts b/packages/core/src/browser/shell/application-shell.ts index 272a06bfde5f8..82c14d036a778 100644 --- a/packages/core/src/browser/shell/application-shell.ts +++ b/packages/core/src/browser/shell/application-shell.ts @@ -34,7 +34,7 @@ import { FrontendApplicationStateService } from '../frontend-application-state'; import { TabBarToolbarRegistry, TabBarToolbarFactory } from './tab-bar-toolbar'; import { ContextKeyService } from '../context-key-service'; import { Emitter } from '../../common/event'; -import { waitForRevealed, waitForClosed, PINNED_CLASS } from '../widgets'; +import { waitForRevealed, waitForClosed, PINNED_CLASS, ExtractableWidget } from '../widgets'; import { CorePreferences } from '../core-preferences'; import { BreadcrumbsRendererFactory } from '../breadcrumbs/breadcrumbs-renderer'; import { Deferred } from '../../common/promise-util'; @@ -1561,14 +1561,19 @@ export class ApplicationShell extends Widget { if (!current) { return undefined; } - const saveableOptions = options && { shouldSave: () => options.save }; - const pendingClose = SaveableWidget.is(current) - ? current.closeWithSaving(saveableOptions) - : (current.close(), waitForClosed(current)); - await Promise.all([ - pendingClose, - this.pendingUpdates - ]); + + if (ExtractableWidget.is(current) && current.secondaryWindow && !current.isClosing) { + current.secondaryWindow!.close(); + } else { + const saveableOptions = options && { shouldSave: () => options.save }; + const pendingClose = SaveableWidget.is(current) + ? current.closeWithSaving(saveableOptions) + : (current.close(), waitForClosed(current)); + await Promise.all([ + pendingClose, + this.pendingUpdates + ]); + } return stack[0] || current; } diff --git a/packages/core/src/browser/widgets/extractable-widget.ts b/packages/core/src/browser/widgets/extractable-widget.ts index 8a0ada5ce8f96..5d2dc697974a2 100644 --- a/packages/core/src/browser/widgets/extractable-widget.ts +++ b/packages/core/src/browser/widgets/extractable-widget.ts @@ -22,6 +22,9 @@ import { Widget } from './widget'; export interface ExtractableWidget extends Widget { /** Set to `true` to mark the widget to be extractable. */ isExtractable: boolean; + + /** State variable to keep track of recursive attempty to close the secondary window */ + isClosing: boolean; /** The secondary window that the window was extracted to or `undefined` if it is not yet extracted. */ secondaryWindow: Window | undefined; } diff --git a/packages/core/src/browser/widgets/widget.ts b/packages/core/src/browser/widgets/widget.ts index 5274d0f2386a6..86a7b11bb3449 100644 --- a/packages/core/src/browser/widgets/widget.ts +++ b/packages/core/src/browser/widgets/widget.ts @@ -336,16 +336,17 @@ export function waitForHidden(widget: Widget): Promise { } function waitForVisible(widget: Widget, visible: boolean, attached?: boolean): Promise { + const win = widget.node.ownerDocument.defaultView || window; if ((typeof attached !== 'boolean' || widget.isAttached === attached) && (widget.isVisible === visible || (widget.node.style.visibility !== 'hidden') === visible) ) { - return new Promise(resolve => window.requestAnimationFrame(() => resolve())); + return new Promise(resolve => win.requestAnimationFrame(() => resolve())); } return new Promise(resolve => { - const waitFor = () => window.requestAnimationFrame(() => { + const waitFor = () => win.requestAnimationFrame(() => { if ((typeof attached !== 'boolean' || widget.isAttached === attached) && (widget.isVisible === visible || (widget.node.style.visibility !== 'hidden') === visible)) { - window.requestAnimationFrame(() => resolve()); + win.requestAnimationFrame(() => resolve()); } else { waitFor(); } diff --git a/packages/core/src/browser/window/default-secondary-window-service.ts b/packages/core/src/browser/window/default-secondary-window-service.ts index 643afe79d9741..3d200b2d63587 100644 --- a/packages/core/src/browser/window/default-secondary-window-service.ts +++ b/packages/core/src/browser/window/default-secondary-window-service.ts @@ -46,33 +46,44 @@ export class DefaultSecondaryWindowService implements SecondaryWindowService { }); } - createSecondaryWindow(onClose?: (closedWin: Window) => void): Window | undefined { - const win = this.doCreateSecondaryWindow(onClose); + createSecondaryWindow(wouldLoseStateOnClosing: () => boolean, tryCloseWidget: (trySaving: boolean) => Promise, closed: (win: Window) => void): Window | undefined { + const id = this.nextWindowId(); + const win = this.doCreateSecondaryWindow(id, wouldLoseStateOnClosing, tryCloseWidget, closed); if (win) { this.secondaryWindows.push(win); } return win; } - protected doCreateSecondaryWindow(onClose?: (closedWin: Window) => void): Window | undefined { - const win = window.open(DefaultSecondaryWindowService.SECONDARY_WINDOW_URL, this.nextWindowId(), 'popup'); + protected doCreateSecondaryWindow(id: string, wouldLoseStateOnClosing: () => boolean, tryCloseWidget: (trySaving: boolean) => Promise, + closed: (win: Window) => void): Window | undefined { + const win = window.open(DefaultSecondaryWindowService.SECONDARY_WINDOW_URL, id, 'popup'); + if (win) { // Add the unload listener after the dom content was loaded because otherwise the unload listener is called already on open in some browsers (e.g. Chrome). win.addEventListener('DOMContentLoaded', () => { + win.addEventListener('beforeunload', evt => { + if (wouldLoseStateOnClosing()) { + evt.returnValue = ''; + evt.preventDefault(); + return ''; + } + }, { capture: true }); win.addEventListener('unload', () => { - this.handleWindowClosed(win, onClose); + tryCloseWidget(false); + this.handleWindowClosed(win); + closed(win); }); }); } return win ?? undefined; } - protected handleWindowClosed(win: Window, onClose?: (closedWin: Window) => void): void { + protected handleWindowClosed(win: Window): void { const extIndex = this.secondaryWindows.indexOf(win); if (extIndex > -1) { this.secondaryWindows.splice(extIndex, 1); }; - onClose?.(win); } focus(win: Window): void { diff --git a/packages/core/src/browser/window/secondary-window-service.ts b/packages/core/src/browser/window/secondary-window-service.ts index 784e32bf974e2..2d1f3dd4a4d46 100644 --- a/packages/core/src/browser/window/secondary-window-service.ts +++ b/packages/core/src/browser/window/secondary-window-service.ts @@ -26,10 +26,12 @@ export interface SecondaryWindowService { * Creates a new secondary window for a widget to be extracted from the application shell. * The created window is closed automatically when the current theia instance is closed. * - * @param onClose optional callback that is invoked when the secondary window is closed + * @param wouldLoseStateOnClosing callback to determine whether save is necessary + * @param tryCloseWidget callback to try and close the widget. If the promise returns true, the window will be closed. + * @param onClosed optional callback that is invoked when the secondary window is closed * @returns the created window or `undefined` if it could not be created */ - createSecondaryWindow(onClose?: (win: Window) => void): Window | undefined; + createSecondaryWindow(wouldLoseStateOnClosing: () => boolean, tryCloseWidget: (trySaving: boolean) => Promise, closed: (win: Window) => void): Window | undefined; /** Handles focussing the given secondary window in the browser and on Electron. */ focus(win: Window): void; diff --git a/packages/core/src/electron-browser/window/electron-secondary-window-service.ts b/packages/core/src/electron-browser/window/electron-secondary-window-service.ts index 987420e056fd3..ab4ced189c1ae 100644 --- a/packages/core/src/electron-browser/window/electron-secondary-window-service.ts +++ b/packages/core/src/electron-browser/window/electron-secondary-window-service.ts @@ -14,32 +14,53 @@ // SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 // ***************************************************************************** -import { BrowserWindow } from '../../../electron-shared/electron'; +import { ipcRenderer, BrowserWindow } from '../../../electron-shared/electron'; import * as electronRemote from '../../../electron-shared/@electron/remote'; -import { injectable } from 'inversify'; +import { injectable, postConstruct } from 'inversify'; import { DefaultSecondaryWindowService } from '../../browser/window/default-secondary-window-service'; +import { CloseSecondaryRequestArguments, CLOSE_SECONDARY_REQUESTED_SIGNAL } from '../../electron-common/messaging/electron-messages'; @injectable() export class ElectronSecondaryWindowService extends DefaultSecondaryWindowService { - protected electronWindows: Map = new Map(); - protected override doCreateSecondaryWindow(onClose?: (closedWin: Window) => void): Window | undefined { - const id = this.nextWindowId(); + private electronWindows: Map = new Map(); + private electronWindowsById: Map Promise> = new Map(); + + @postConstruct() + override init(): void { + super.init(); + ipcRenderer.addListener(CLOSE_SECONDARY_REQUESTED_SIGNAL, (_sender, args: CloseSecondaryRequestArguments) => this.handleCloseRequestedEvent(args)); + } + + protected async handleCloseRequestedEvent(event: CloseSecondaryRequestArguments): Promise { + const safeToClose = await this.safeToClose(event.windowId); + if (safeToClose) { + ipcRenderer.send(event.confirmChannel); + } else { + ipcRenderer.send(event.cancelChannel); + } + } + + protected override doCreateSecondaryWindow(id: string, wouldLoseStateOnClosing: () => boolean, tryCloseWidget: (trySaving: boolean) => Promise, + closed: (win: Window) => void): Window | undefined { + let win: Window | undefined = undefined; electronRemote.getCurrentWindow().webContents.once('did-create-window', newElectronWindow => { - newElectronWindow.setMenuBarVisibility(false); + // newElectronWindow.setMenuBarVisibility(false); this.electronWindows.set(id, newElectronWindow); - newElectronWindow.on('closed', () => { + const electronId = newElectronWindow.id.toString(); + this.electronWindowsById.set(electronId, () => tryCloseWidget(true)); + const closedHandler = () => { + if (closed) { + closed(win!); + } + this.electronWindows.delete(id); - const browserWin = this.secondaryWindows.find(w => w.name === id); - if (browserWin) { - this.handleWindowClosed(browserWin, onClose); - } else { - console.warn(`Could not execute proper close handling for secondary window '${id}' because its frontend window could not be found.`); - }; - }); + this.electronWindowsById.delete(electronId); + }; + newElectronWindow.once('closed', closedHandler); }); - const win = window.open(DefaultSecondaryWindowService.SECONDARY_WINDOW_URL, id); - return win ?? undefined; + win = window.open(DefaultSecondaryWindowService.SECONDARY_WINDOW_URL, id, 'popup') || undefined; + return win; } override focus(win: Window): void { @@ -54,4 +75,13 @@ export class ElectronSecondaryWindowService extends DefaultSecondaryWindowServic console.warn(`There is no known secondary window '${win.name}'. Thus, the window could not be focussed.`); } } + + safeToClose(windowId: string): Promise { + const closingHandler = this.electronWindowsById.get(windowId); + if (closingHandler) { + return closingHandler!(); + } else { + return Promise.resolve(true); + } + } } diff --git a/packages/core/src/electron-common/messaging/electron-messages.ts b/packages/core/src/electron-common/messaging/electron-messages.ts index 53ec84e7edb6d..7f7aa2859824e 100644 --- a/packages/core/src/electron-common/messaging/electron-messages.ts +++ b/packages/core/src/electron-common/messaging/electron-messages.ts @@ -26,6 +26,10 @@ export const Restart = 'restart'; * Emitted by main when close requested. */ export const CLOSE_REQUESTED_SIGNAL = 'close-requested'; +/** + * Emitted by main when close requested. + */ +export const CLOSE_SECONDARY_REQUESTED_SIGNAL = 'close-secondary-requested'; /** * Emitted by window when a reload is requested. */ @@ -40,3 +44,9 @@ export interface CloseRequestArguments { cancelChannel: string; reason: StopReason; } + +export interface CloseSecondaryRequestArguments { + windowId: string; + confirmChannel: string; + cancelChannel: string; +} diff --git a/packages/core/src/electron-main/theia-electron-window.ts b/packages/core/src/electron-main/theia-electron-window.ts index 9ac8e7c9a2acd..3369109ac8328 100644 --- a/packages/core/src/electron-main/theia-electron-window.ts +++ b/packages/core/src/electron-main/theia-electron-window.ts @@ -16,7 +16,8 @@ import { FrontendApplicationConfig } from '@theia/application-package'; import { FrontendApplicationState } from '../common/frontend-application-state'; -import { APPLICATION_STATE_CHANGE_SIGNAL, CLOSE_REQUESTED_SIGNAL, RELOAD_REQUESTED_SIGNAL, StopReason } from '../electron-common/messaging/electron-messages'; +import { APPLICATION_STATE_CHANGE_SIGNAL, CloseSecondaryRequestArguments, CLOSE_REQUESTED_SIGNAL, CLOSE_SECONDARY_REQUESTED_SIGNAL, RELOAD_REQUESTED_SIGNAL, StopReason } + from '../electron-common/messaging/electron-messages'; import { BrowserWindow, BrowserWindowConstructorOptions, ipcMain, IpcMainEvent } from '../../electron-shared/electron'; import { inject, injectable, postConstruct } from '../../shared/inversify'; import { ElectronMainApplicationGlobals } from './electron-main-constants'; @@ -44,6 +45,12 @@ export const TheiaBrowserWindowOptions = Symbol('TheiaBrowserWindowOptions'); export const WindowApplicationConfig = Symbol('WindowApplicationConfig'); export type WindowApplicationConfig = FrontendApplicationConfig; +enum ClosingState { + initial, + inProgress, + readyToClose +} + @injectable() export class TheiaElectronWindow { @inject(TheiaBrowserWindowOptions) protected readonly options: TheiaBrowserWindowOptions; @@ -75,6 +82,35 @@ export class TheiaElectronWindow { this.attachCloseListeners(); this.trackApplicationState(); this.attachReloadListener(); + this.attachSecondaryWindowListener(); + } + + protected attachSecondaryWindowListener(): void { + createDisposableListener(this._window.webContents, 'did-create-window', (newWindow: BrowserWindow) => { + let closingState = ClosingState.initial; + newWindow.on('close', event => { + if (closingState === ClosingState.initial) { + closingState = ClosingState.inProgress; + event.preventDefault(); + this.checkSafeToCloseSecondaryWindow(newWindow).then(shouldClose => { + if (shouldClose) { + // removing the focus from the secondary window before close is necessary + // to prevent "illegal access" errors. Unfortunately, it is unclear what + // exactly causes the errors. + this._window.focus(); + closingState = ClosingState.readyToClose; + newWindow.close(); + } else { + closingState = ClosingState.initial; + } + }); + } else if (closingState === ClosingState.inProgress) { + // When the extracted widget is disposed programmatically, a dispose listener on it will try to close the window. + // if we dispose the widget because of closing the window, we'll get a recursive call to window.close() + event.preventDefault(); + } + }); + }); } /** @@ -137,6 +173,26 @@ export class TheiaElectronWindow { return false; } + protected checkSafeToCloseSecondaryWindow(win: BrowserWindow): Promise { + const confirmChannel = `safe-to-close-${this._window.id}`; + const cancelChannel = `notSafeToClose-${this._window.id}`; + const temporaryDisposables = new DisposableCollection(); + return new Promise(resolve => { + const params: CloseSecondaryRequestArguments = { windowId: win.id.toString(), confirmChannel, cancelChannel }; + this._window.webContents.send(CLOSE_SECONDARY_REQUESTED_SIGNAL, params); + createDisposableListener(ipcMain, confirmChannel, (e: IpcMainEvent) => { + if (this.isSender(e)) { + resolve(true); + } + }, temporaryDisposables); + createDisposableListener(ipcMain, cancelChannel, (e: IpcMainEvent) => { + if (this.isSender(e)) { + resolve(false); + } + }, temporaryDisposables); + }).finally(() => temporaryDisposables.dispose()); + } + protected checkSafeToStop(reason: StopReason): Promise { const confirmChannel = `safe-to-close-${this._window.id}`; const cancelChannel = `notSafeToClose-${this._window.id}`; diff --git a/packages/plugin-ext/src/main/browser/custom-editors/custom-editors-main.ts b/packages/plugin-ext/src/main/browser/custom-editors/custom-editors-main.ts index 73511bf102426..f064b0c92a6cb 100644 --- a/packages/plugin-ext/src/main/browser/custom-editors/custom-editors-main.ts +++ b/packages/plugin-ext/src/main/browser/custom-editors/custom-editors-main.ts @@ -185,7 +185,7 @@ export class CustomEditorsMainImpl implements CustomEditorsMain, Disposable { switch (modelType) { case CustomEditorModelType.Text: { - const model = CustomTextEditorModel.create(viewType, resource, this.textModelService, this.fileService, this.editorPreferences); + const model = CustomTextEditorModel.create(viewType, resource, this.textModelService, this.fileService); return this.customEditorService.models.add(resource, viewType, model); } case CustomEditorModelType.Custom: { @@ -520,19 +520,16 @@ export class CustomTextEditorModel implements CustomEditorModel { private readonly toDispose = new DisposableCollection(); private readonly onDirtyChangedEmitter = new Emitter(); readonly onDirtyChanged = this.onDirtyChangedEmitter.event; - autoSave: 'off' | 'afterDelay' | 'onFocusChange' | 'onWindowChange'; - autoSaveDelay: number; static async create( viewType: string, resource: TheiaURI, editorModelService: EditorModelService, - fileService: FileService, - editorPreferences: EditorPreferences, + fileService: FileService ): Promise { const model = await editorModelService.createModelReference(resource); model.object.suppressOpenEditorWhenDirty = true; - return new CustomTextEditorModel(viewType, resource, model, fileService, editorPreferences); + return new CustomTextEditorModel(viewType, resource, model, fileService); } constructor( @@ -540,7 +537,6 @@ export class CustomTextEditorModel implements CustomEditorModel { readonly editorResource: TheiaURI, private readonly model: Reference, private readonly fileService: FileService, - private readonly editorPreferences: EditorPreferences ) { this.toDispose.push( this.editorTextModel.onDirtyChanged(e => { @@ -549,19 +545,6 @@ export class CustomTextEditorModel implements CustomEditorModel { ); this.toDispose.push(this.onDirtyChangedEmitter); - this.autoSave = this.editorPreferences.get('files.autoSave', undefined, editorResource.toString()); - this.autoSaveDelay = this.editorPreferences.get('files.autoSaveDelay', undefined, editorResource.toString()); - - this.toDispose.push( - this.editorPreferences.onPreferenceChanged(event => { - if (event.preferenceName === 'files.autoSave') { - this.autoSave = this.editorPreferences.get('files.autoSave', undefined, editorResource.toString()); - } - if (event.preferenceName === 'files.autoSaveDelay') { - this.autoSaveDelay = this.editorPreferences.get('files.autoSaveDelay', undefined, editorResource.toString()); - } - }) - ); this.toDispose.push(this.onDirtyChangedEmitter); } @@ -586,6 +569,14 @@ export class CustomTextEditorModel implements CustomEditorModel { return this.model.object; } + get autoSave(): 'off' | 'afterDelay' | 'onFocusChange' | 'onWindowChange' { + return this.editorTextModel.autoSave; + } + + get autoSaveDelay(): number { + return this.editorTextModel.autoSaveDelay; + } + revert(options?: Saveable.RevertOptions): Promise { return this.editorTextModel.revert(options); } diff --git a/packages/plugin-ext/src/main/browser/webview/webview.ts b/packages/plugin-ext/src/main/browser/webview/webview.ts index 1461e2ebfcb43..18ef00279f59c 100644 --- a/packages/plugin-ext/src/main/browser/webview/webview.ts +++ b/packages/plugin-ext/src/main/browser/webview/webview.ts @@ -175,6 +175,7 @@ export class WebviewWidget extends BaseWidget implements StatefulWidget, Extract protected hideTimeout: any | number | undefined; isExtractable: boolean = true; + isClosing: boolean = false; secondaryWindow: Window | undefined = undefined; @postConstruct() @@ -239,11 +240,17 @@ export class WebviewWidget extends BaseWidget implements StatefulWidget, Extract } protected forceHide(): void { + console.log('forcing hide'); clearTimeout(this.hideTimeout); this.hideTimeout = undefined; this.toHide.dispose(); } + override dispose(): void { + console.log('disposing webview'); + super.dispose(); + } + protected doShow(): void { clearTimeout(this.hideTimeout); this.hideTimeout = undefined; @@ -265,7 +272,9 @@ export class WebviewWidget extends BaseWidget implements StatefulWidget, Extract this.element = element; this.node.appendChild(this.element); this.toHide.push(Disposable.create(() => { + console.log('try remove element'); if (this.element) { + console.log('removing'); this.element.remove(); this.element = undefined; } diff --git a/packages/terminal/src/browser/terminal-widget-impl.ts b/packages/terminal/src/browser/terminal-widget-impl.ts index 92ca4047d5215..e8da4d580a3f5 100644 --- a/packages/terminal/src/browser/terminal-widget-impl.ts +++ b/packages/terminal/src/browser/terminal-widget-impl.ts @@ -49,6 +49,7 @@ export interface TerminalWidgetFactoryOptions extends Partial Date: Tue, 8 Nov 2022 17:07:20 +0100 Subject: [PATCH 2/2] Some changes to prevent "Illegal access" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Thomas Mäder --- .../browser/color-application-contribution.ts | 16 +++++++++++----- .../core/src/browser/secondary-window-handler.ts | 4 +++- .../src/electron-main/theia-electron-window.ts | 6 ++++-- .../src/main/browser/webview/webview.ts | 8 -------- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/packages/core/src/browser/color-application-contribution.ts b/packages/core/src/browser/color-application-contribution.ts index 2091116c5efb9..e81656801ad8a 100644 --- a/packages/core/src/browser/color-application-contribution.ts +++ b/packages/core/src/browser/color-application-contribution.ts @@ -34,6 +34,7 @@ export class ColorApplicationContribution implements FrontendApplicationContribu protected readonly onDidChangeEmitter = new Emitter(); readonly onDidChange = this.onDidChangeEmitter.event; private readonly windows: Set = new Set(); + protected readonly toUpdate = new Map(); @inject(ColorRegistry) protected readonly colors: ColorRegistry; @@ -61,21 +62,26 @@ export class ColorApplicationContribution implements FrontendApplicationContribu this.windows.add(win); this.updateWindow(win); this.onDidChangeEmitter.fire(); - return Disposable.create(() => this.windows.delete(win)); + return Disposable.create(() => { + this.toUpdate.delete(win); + this.windows.delete(win); + }); } - protected readonly toUpdate = new DisposableCollection(); protected update(): void { - this.toUpdate.dispose(); this.windows.forEach(win => this.updateWindow(win)); this.onDidChangeEmitter.fire(); } protected updateWindow(win: Window): void { + this.toUpdate.get(win)?.dispose(); const theme = 'theia-' + this.themeService.getCurrentTheme().type; + const localDisposables = new DisposableCollection(); + this.toUpdate.set(win, localDisposables); + win.document.body.classList.add(theme); - this.toUpdate.push(Disposable.create(() => win.document.body.classList.remove(theme))); + localDisposables.push(Disposable.create(() => win.document.body.classList.remove(theme))); const documentElement = win.document.documentElement; if (documentElement) { @@ -84,7 +90,7 @@ export class ColorApplicationContribution implements FrontendApplicationContribu if (variable) { const { name, value } = variable; documentElement.style.setProperty(name, value); - this.toUpdate.push(Disposable.create(() => documentElement.style.removeProperty(name))); + localDisposables.push(Disposable.create(() => documentElement.style.removeProperty(name))); } } } diff --git a/packages/core/src/browser/secondary-window-handler.ts b/packages/core/src/browser/secondary-window-handler.ts index 1ef14e3c21b6d..5256042f4502e 100644 --- a/packages/core/src/browser/secondary-window-handler.ts +++ b/packages/core/src/browser/secondary-window-handler.ts @@ -175,7 +175,6 @@ export class SecondaryWindowHandler { const mainWindowTitle = document.title; newWindow.onload = () => { - this.keybindings.registerEventListeners(newWindow); // Use the widget's title as the window title // Even if the widget's label were malicious, this should be safe against XSS because the HTML standard defines this is inserted via a text node. // See https://html.spec.whatwg.org/multipage/dom.html#document.title @@ -186,6 +185,8 @@ export class SecondaryWindowHandler { console.error('Could not find dom element to attach to in secondary window'); return; } + + const disposKeybindingsListener = this.keybindings.registerEventListeners(newWindow); const unregisterWithColorContribution = this.colorAppContribution.registerWindow(newWindow); widget.secondaryWindow = newWindow; @@ -201,6 +202,7 @@ export class SecondaryWindowHandler { // Close the window if the widget is disposed, e.g. by a command closing all widgets. widget.disposed.connect(() => { + disposKeybindingsListener.dispose(); unregisterWithColorContribution.dispose(); this.removeWidget(widget); }); diff --git a/packages/core/src/electron-main/theia-electron-window.ts b/packages/core/src/electron-main/theia-electron-window.ts index 3369109ac8328..633b2946d3106 100644 --- a/packages/core/src/electron-main/theia-electron-window.ts +++ b/packages/core/src/electron-main/theia-electron-window.ts @@ -98,8 +98,10 @@ export class TheiaElectronWindow { // to prevent "illegal access" errors. Unfortunately, it is unclear what // exactly causes the errors. this._window.focus(); - closingState = ClosingState.readyToClose; - newWindow.close(); + setTimeout(() => { + closingState = ClosingState.readyToClose; + newWindow.close(); + }, 100); } else { closingState = ClosingState.initial; } diff --git a/packages/plugin-ext/src/main/browser/webview/webview.ts b/packages/plugin-ext/src/main/browser/webview/webview.ts index 18ef00279f59c..b89710dd4342a 100644 --- a/packages/plugin-ext/src/main/browser/webview/webview.ts +++ b/packages/plugin-ext/src/main/browser/webview/webview.ts @@ -240,17 +240,11 @@ export class WebviewWidget extends BaseWidget implements StatefulWidget, Extract } protected forceHide(): void { - console.log('forcing hide'); clearTimeout(this.hideTimeout); this.hideTimeout = undefined; this.toHide.dispose(); } - override dispose(): void { - console.log('disposing webview'); - super.dispose(); - } - protected doShow(): void { clearTimeout(this.hideTimeout); this.hideTimeout = undefined; @@ -272,9 +266,7 @@ export class WebviewWidget extends BaseWidget implements StatefulWidget, Extract this.element = element; this.node.appendChild(this.element); this.toHide.push(Disposable.create(() => { - console.log('try remove element'); if (this.element) { - console.log('removing'); this.element.remove(); this.element = undefined; }