From 0c86eaff4ac1e5bf5c363191d66679debc385362 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Sat, 7 Nov 2020 18:29:30 +0100 Subject: [PATCH] GH-8188: Workaround for `application.confirmExit` - Moved the `beforeunload` handling logic to the frontend. - Removed the `attachWillPreventUnload` from the main electron app. Signed-off-by: Akos Kitta --- CHANGELOG.md | 1 + .../core/src/browser/frontend-application.ts | 7 +++-- .../browser/window/default-window-service.ts | 28 ++++++++++++++--- .../window/test/mock-window-service.ts | 2 ++ .../core/src/browser/window/window-service.ts | 8 +++++ .../window/electron-window-service.ts | 31 +++++++++++++++++-- .../electron-main-application.ts | 23 +------------- 7 files changed, 68 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c46b143eece8e..4716142a42989 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ [Breaking Changes:](#breaking_changes_1.8.0) - [file-search] Deprecate dependency on `@theia/process` and replaced its usage by node's `child_process` api. +- [electron] Removed `attachWillPreventUnload` method from the Electron main application. The `confirmExit` logic is handled on the frontend. [#8732](https://github.com/eclipse-theia/theia/pull/8732) ## v1.7.0 - 29/10/2020 diff --git a/packages/core/src/browser/frontend-application.ts b/packages/core/src/browser/frontend-application.ts index 25997c737a692..1d7c6a1ae870b 100644 --- a/packages/core/src/browser/frontend-application.ts +++ b/packages/core/src/browser/frontend-application.ts @@ -24,6 +24,7 @@ import { ShellLayoutRestorer, ApplicationShellLayoutMigrationError } from './she import { FrontendApplicationStateService } from './frontend-application-state'; import { preventNavigation, parseCssTime, animationFrame } from './browser'; import { CorePreferences } from './core-preferences'; +import { WindowService } from './window/window-service'; /** * Clients can implement to get a callback for contributing widgets to a shell on start. @@ -96,6 +97,9 @@ export class FrontendApplication { @inject(CorePreferences) protected readonly corePreferences: CorePreferences; + @inject(WindowService) + protected readonly windowsService: WindowService; + constructor( @inject(CommandRegistry) protected readonly commands: CommandRegistry, @inject(MenuModelRegistry) protected readonly menus: MenuModelRegistry, @@ -182,8 +186,7 @@ export class FrontendApplication { */ protected registerEventListeners(): void { this.registerCompositionEventListeners(); /* Hotfix. See above. */ - - window.addEventListener('beforeunload', () => { + this.windowsService.onUnload(() => { this.stateService.state = 'closing_window'; this.layoutRestorer.storeLayout(this); this.stopContributions(); diff --git a/packages/core/src/browser/window/default-window-service.ts b/packages/core/src/browser/window/default-window-service.ts index a2948fb7a351d..470c4c9bcc49d 100644 --- a/packages/core/src/browser/window/default-window-service.ts +++ b/packages/core/src/browser/window/default-window-service.ts @@ -15,6 +15,7 @@ ********************************************************************************/ import { inject, injectable, named } from 'inversify'; +import { Event, Emitter } from '../../common'; import { CorePreferences } from '../core-preferences'; import { ContributionProvider } from '../../common/contribution-provider'; import { FrontendApplicationContribution, FrontendApplication } from '../frontend-application'; @@ -25,6 +26,11 @@ export class DefaultWindowService implements WindowService, FrontendApplicationC protected frontendApplication: FrontendApplication; + protected onUnloadEmitter = new Emitter(); + get onUnload(): Event { + return this.onUnloadEmitter.event; + } + @inject(CorePreferences) protected readonly corePreferences: CorePreferences; @@ -34,11 +40,7 @@ export class DefaultWindowService implements WindowService, FrontendApplicationC onStart(app: FrontendApplication): void { this.frontendApplication = app; - window.addEventListener('beforeunload', event => { - if (!this.canUnload()) { - return this.preventUnload(event); - } - }); + this.registerUnloadListeners(); } openNewWindow(url: string): undefined { @@ -61,6 +63,22 @@ export class DefaultWindowService implements WindowService, FrontendApplicationC return confirmExit !== 'always'; } + /** + * Implement the mechanism to detect unloading of the page. + */ + protected registerUnloadListeners(): void { + window.addEventListener('beforeunload', event => { + if (!this.canUnload()) { + return this.preventUnload(event); + } + }); + // In a browser, `unload` is correctly fired when the page unloads, unlike Electron. + // If `beforeunload` is cancelled, the user will be prompted to leave or stay. + // If the user stays, the page won't be unloaded, so `unload` is not fired. + // If the user leaves, the page will be unloaded, so `unload` is fired. + window.addEventListener('unload', () => this.onUnloadEmitter.fire()); + } + /** * Ask the user to confirm if they want to unload the window. Prevent it if they do not. * @param event The beforeunload event diff --git a/packages/core/src/browser/window/test/mock-window-service.ts b/packages/core/src/browser/window/test/mock-window-service.ts index a0662fcfde2a1..a3cca33908fa3 100644 --- a/packages/core/src/browser/window/test/mock-window-service.ts +++ b/packages/core/src/browser/window/test/mock-window-service.ts @@ -14,10 +14,12 @@ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 ********************************************************************************/ import { injectable } from 'inversify'; +import { Event } from '../../../common/event'; import { WindowService } from '../window-service'; @injectable() export class MockWindowService implements WindowService { openNewWindow(): undefined { return undefined; } canUnload(): boolean { return true; } + get onUnload(): Event { return Event.None; } } diff --git a/packages/core/src/browser/window/window-service.ts b/packages/core/src/browser/window/window-service.ts index 1300b5af249c0..cec3b1e9a25f7 100644 --- a/packages/core/src/browser/window/window-service.ts +++ b/packages/core/src/browser/window/window-service.ts @@ -14,6 +14,8 @@ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 ********************************************************************************/ +import { Event } from '../../common/event'; + export interface NewWindowOptions { readonly external?: boolean; } @@ -38,4 +40,10 @@ export interface WindowService { */ canUnload(): boolean; + /** + * Fires when the `window` unloads. The unload event is inevitable. On this event, the frontend application can save its state and release resource. + * Saving the state and releasing any resources must be a synchronous call. Any asynchronous calls invoked after emitting this event might be ignored. + */ + readonly onUnload: Event; + } diff --git a/packages/core/src/electron-browser/window/electron-window-service.ts b/packages/core/src/electron-browser/window/electron-window-service.ts index 0bbda274987b9..3f550dae87189 100644 --- a/packages/core/src/electron-browser/window/electron-window-service.ts +++ b/packages/core/src/electron-browser/window/electron-window-service.ts @@ -15,6 +15,7 @@ ********************************************************************************/ import { injectable, inject } from 'inversify'; +import { remote } from 'electron'; import { NewWindowOptions } from '../../browser/window/window-service'; import { DefaultWindowService } from '../../browser/window/default-window-service'; import { ElectronMainWindowService } from '../../electron-common/electron-main-window-service'; @@ -30,9 +31,33 @@ export class ElectronWindowService extends DefaultWindowService { return undefined; } - protected preventUnload(event: BeforeUnloadEvent): string | void { - // The user will be shown a confirmation dialog by the will-prevent-unload handler in the Electron main script - event.returnValue = false; + registerUnloadListeners(): void { + window.addEventListener('beforeunload', event => { + // Either we can unload, or the user confirms that he wants to quit + if (this.canUnload() || this.shouldUnload()) { + // We are unloading + delete event.returnValue; + this.onUnloadEmitter.fire(); + } else { + // The user wants to stay, let's prevent unloading + return this.preventUnload(event); + } + }); } + /** + * When preventing `beforeunload` on Electron, no popup is shown. + * This method implements a modal to ask the user if he wants to quit the page. + */ + protected shouldUnload(): boolean { + const electronWindow = remote.getCurrentWindow(); + const response = remote.dialog.showMessageBoxSync(electronWindow, { + type: 'question', + buttons: ['Yes', 'No'], + title: 'Confirm', + message: 'Are you sure you want to quit?', + detail: 'Any unsaved changes will not be saved.' + }); + return response === 0; // 'Yes', close the window. + } } diff --git a/packages/core/src/electron-main/electron-main-application.ts b/packages/core/src/electron-main/electron-main-application.ts index e1a7194599c5c..dc7304c35c8e8 100644 --- a/packages/core/src/electron-main/electron-main-application.ts +++ b/packages/core/src/electron-main/electron-main-application.ts @@ -15,7 +15,7 @@ ********************************************************************************/ import { inject, injectable, named } from 'inversify'; -import { session, screen, globalShortcut, app, BrowserWindow, BrowserWindowConstructorOptions, Event as ElectronEvent, dialog } from 'electron'; +import { session, screen, globalShortcut, app, BrowserWindow, BrowserWindowConstructorOptions, Event as ElectronEvent } from 'electron'; import * as path from 'path'; import { Argv } from 'yargs'; import { AddressInfo } from 'net'; @@ -213,7 +213,6 @@ export class ElectronMainApplication { const electronWindow = new BrowserWindow(options); this.attachReadyToShow(electronWindow); this.attachSaveWindowState(electronWindow); - this.attachWillPreventUnload(electronWindow); this.attachGlobalShortcuts(electronWindow); this.restoreMaximizedState(electronWindow, options); return electronWindow; @@ -339,26 +338,6 @@ export class ElectronMainApplication { electronWindow.on('move', saveWindowStateDelayed); } - /** - * Catch window closing event and display a confirmation window. - */ - protected attachWillPreventUnload(electronWindow: BrowserWindow): void { - // Fired when a beforeunload handler tries to prevent the page unloading - electronWindow.webContents.on('will-prevent-unload', async event => { - const { response } = await dialog.showMessageBox(electronWindow, { - type: 'question', - buttons: ['Yes', 'No'], - title: 'Confirm', - message: 'Are you sure you want to quit?', - detail: 'Any unsaved changes will not be saved.' - }); - if (response === 0) { // 'Yes' - // This ignores the beforeunload callback, allowing the page to unload - event.preventDefault(); - } - }); - } - /** * Catch certain keybindings to prevent reloading the window using keyboard shortcuts. */