fix(settings): suspend the renderer unload guard for updater installs

The main-process allowNextClose() bypass was not enough: with a dirty
form the renderer's beforeunload handler still cancelled the updater's
window close at the DOM layer and treated the attempt as a reload,
stranding the install. Installing now suspends the whole unload guard
(beforeunload handler and main-process mirror) before quitAndInstall is
invoked; a reply proving no quit happened restores the protection.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Fable 5 committed 2026-08-09 11:32:11 +02:00
1 parent 769d519a61
commit 18093b0850
4 files changed
+135 -6

No files matched your search

@@ -12,6 +12,7 @@ import { ElectronServiceStub } from '../services/electron.service.stub';
import { SettingsService } from '../services/settings.service';
import { AppUpdateReleaseNotesDialogComponent } from './app-update-release-notes-dialog.component';
import { SettingsAppUpdateFacade } from './settings-app-update.facade';
import { SettingsUnloadGuardService } from './settings-unload-guard.service';
import {
createElectronStub,
DEFAULT_APP_UPDATE_STATUS,
@@ -46,6 +47,10 @@ describe('SettingsAppUpdateFacade', () => {
{ provide: DataService, useClass: ElectronServiceStub },
MockProvider(MatDialog, { open: jest.fn() }),
{ provide: SettingsService, useClass: MockSettingsService },
MockProvider(SettingsUnloadGuardService, {
resumeAfterAbortedAppQuit: jest.fn(),
suspendForAppQuit: jest.fn(),
}),
],
imports: [TranslateModule.forRoot()],
});
@@ -149,6 +154,33 @@ describe('SettingsAppUpdateFacade', () => {
expect(window.electron.installAppUpdate).toHaveBeenCalledTimes(1);
});
it('keeps the unload guard down while a real install quits the app', async () => {
// The updater closes the window while the settings form may still be
// dirty; an armed beforeunload would cancel that close at the DOM
// layer and strand the install.
const unloadGuard = TestBed.inject(SettingsUnloadGuardService);
(window.electron.installAppUpdate as jest.Mock).mockResolvedValue({
...DEFAULT_APP_UPDATE_STATUS,
status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded,
});
await facade.installAppUpdate();
expect(unloadGuard.suspendForAppQuit).toHaveBeenCalledTimes(1);
expect(unloadGuard.resumeAfterAbortedAppQuit).not.toHaveBeenCalled();
});
it('restores the unload guard when the install did not quit', async () => {
const unloadGuard = TestBed.inject(SettingsUnloadGuardService);
// The default stub reply stays Idle: nothing installable, no quit.
await facade.installAppUpdate();
expect(unloadGuard.suspendForAppQuit).toHaveBeenCalledTimes(1);
expect(unloadGuard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes(
1
);
});
it('opens the manual release URL from unsupported update status', () => {
const openSpy = jest.spyOn(window, 'open').mockReturnValue(null);
facade.status.set({
@@ -9,6 +9,7 @@ import { TranslateService } from '@ngx-translate/core';
import { take } from 'rxjs';
import { SettingsService } from '../services/settings.service';
import { AppUpdateReleaseNotesDialogComponent } from './app-update-release-notes-dialog.component';
import { SettingsUnloadGuardService } from './settings-unload-guard.service';
const APP_UPDATE_STATUS_LOAD_ATTEMPTS = 60;
const APP_UPDATE_STATUS_LOAD_RETRY_DELAY_MS = 250;
@@ -24,6 +25,7 @@ export class SettingsAppUpdateFacade {
private readonly runtime = inject(RuntimeCapabilitiesService);
private readonly settingsService = inject(SettingsService);
private readonly translate = inject(TranslateService);
private readonly unloadGuard = inject(SettingsUnloadGuardService);
/** Latest updater status, polled once and then pushed by the backend */
readonly status = signal<ElectronBridgeAppUpdateStatus | null>(null);
@@ -68,7 +70,22 @@ export class SettingsAppUpdateFacade {
return;
}
this.status.set(await window.electron.installAppUpdate());
// Installing quits the app; the unsaved-settings unload guard must
// not fight a quit the user just asked for — its `beforeunload`
// would cancel the updater's window close at the DOM layer and
// strand the install. A 'downloaded' reply means quitAndInstall ran
// and the app is going down; anything else means no quit happened,
// so the protection comes back.
this.unloadGuard.suspendForAppQuit();
const status = await window.electron.installAppUpdate();
this.status.set(status);
if (
status?.status !== ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded
) {
this.unloadGuard.resumeAfterAbortedAppQuit();
}
}
openManualAppUpdate(): void {
@@ -155,6 +155,51 @@ describe('SettingsUnloadGuardService', () => {
});
});
describe('updater-driven app quit (Electron)', () => {
it('suspends both protection layers for the quit', () => {
activateInElectron();
form.markAsDirty();
service.suspendForAppQuit();
// The main-process mirror is disarmed...
expect(electronStub.setWindowCloseGuard).toHaveBeenLastCalledWith(
false
);
// ...and the DOM layer no longer cancels the unload, so the
// updater's window close passes without turning into a reload.
const event = dispatchBeforeUnload();
expect(event.defaultPrevented).toBe(false);
});
it('keeps the mirror down when the form changes while suspended', () => {
activateInElectron();
form.markAsDirty();
service.suspendForAppQuit();
form.markAsPristine();
form.markAsDirty();
expect(electronStub.setWindowCloseGuard).toHaveBeenLastCalledWith(
false
);
});
it('restores the protection when the quit did not happen', () => {
activateInElectron();
form.markAsDirty();
service.suspendForAppQuit();
service.resumeAfterAbortedAppQuit();
expect(electronStub.setWindowCloseGuard).toHaveBeenLastCalledWith(
true
);
const event = dispatchBeforeUnload();
expect(event.defaultPrevented).toBe(true);
});
});
describe('intercepted window close (Electron)', () => {
it('confirms the close once the user saves or discards', async () => {
activateInElectron();
@@ -43,6 +43,8 @@ export class SettingsUnloadGuardService implements OnDestroy {
private confirmationPending = false;
/** Last guard state mirrored to the main process. */
private guardArmed = false;
/** True while an updater-driven app quit must pass unchallenged. */
private suspended = false;
/** Indirection because `window.location.reload` cannot be stubbed. */
reloadPage: () => void = () => window.location.reload();
@@ -74,12 +76,41 @@ export class SettingsUnloadGuardService implements OnDestroy {
this.dispose();
}
/**
* Stands every protection layer down for an app quit the user explicitly
* requested from inside settings — installing a downloaded update. The
* updater closes the window while the form may still be dirty; a
* `beforeunload` cancellation at the DOM layer would strand the install
* (and even morph it into a reload), so the quit must pass unchallenged.
*/
suspendForAppQuit(): void {
if (!this.host || this.suspended) {
return;
}
this.suspended = true;
window.removeEventListener('beforeunload', this.beforeUnloadHandler);
this.syncCloseGuard(this.host.form.dirty);
}
/** Restores the protection when the requested quit did not happen. */
resumeAfterAbortedAppQuit(): void {
if (!this.host || !this.suspended) {
return;
}
this.suspended = false;
window.addEventListener('beforeunload', this.beforeUnloadHandler);
this.syncCloseGuard(this.host.form.dirty);
}
private dispose(): void {
window.removeEventListener('beforeunload', this.beforeUnloadHandler);
this.dirtySubscription?.unsubscribe();
this.dirtySubscription = null;
this.unsubscribeCloseRequests?.();
this.unsubscribeCloseRequests = null;
this.suspended = false;
this.syncCloseGuard(false);
this.host = null;
}
@@ -98,8 +129,10 @@ export class SettingsUnloadGuardService implements OnDestroy {
if (window.electron) {
// Electron cancelled the unload without any prompt. Window close
// never lands here (the main process intercepts it first), so
// this can only be a reload — ask, then re-trigger it.
// never lands here while the guard is armed (the main process
// intercepts it first), and an updater-driven quit suspends this
// handler entirely — so this can only be a reload: ask, then
// re-trigger it.
setTimeout(() => {
this.zone.run(() => void this.handleCloseRequest('reload'));
});
@@ -139,11 +172,13 @@ export class SettingsUnloadGuardService implements OnDestroy {
}
private syncCloseGuard(active: boolean): void {
if (this.guardArmed === active) {
const effective = active && !this.suspended;
if (this.guardArmed === effective) {
return;
}
this.guardArmed = active;
void window.electron?.setWindowCloseGuard?.(active);
this.guardArmed = effective;
void window.electron?.setWindowCloseGuard?.(effective);
}
}