Skip to content

Commit

Permalink
GH-8188: Workaround for application.confirmExit
Browse files Browse the repository at this point in the history
 - Moved the `beforeunload` handling logic to the frontend.
 - Removed the `attachWillPreventUnload` from the main electron app.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
  • Loading branch information
Akos Kitta authored and kittaakos committed Nov 18, 2020
1 parent 56c2274 commit e209832
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 32 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<a name="breaking_changes_1.8.0">[Breaking Changes:](#breaking_changes_1.8.0)</a>

- [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

Expand Down
7 changes: 5 additions & 2 deletions packages/core/src/browser/frontend-application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand Down
28 changes: 23 additions & 5 deletions packages/core/src/browser/window/default-window-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -25,6 +26,11 @@ export class DefaultWindowService implements WindowService, FrontendApplicationC

protected frontendApplication: FrontendApplication;

protected onUnloadEmitter = new Emitter<void>();
get onUnload(): Event<void> {
return this.onUnloadEmitter.event;
}

@inject(CorePreferences)
protected readonly corePreferences: CorePreferences;

Expand All @@ -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 {
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/browser/window/test/mock-window-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> { return Event.None; }
}
8 changes: 8 additions & 0 deletions packages/core/src/browser/window/window-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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<void>;

}
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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.
}
}
23 changes: 1 addition & 22 deletions packages/core/src/electron-main/electron-main-application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
*/
Expand Down

0 comments on commit e209832

Please sign in to comment.