fix(settings): arm the close guard for the whole settings mount

Arming the main-process guard on the first dirty transition raced the
very close it protects against: the fire-and-forget IPC could lose to
an immediate window close, which then fell through to beforeunload and
was mis-handled as a reload. The guard is now armed when settings
mounts — before the user can possibly stage an edit — and a close with
a pristine form auto-confirms through the dialog-less path, so the only
visible effect of mount-long arming is race-free protection. The
per-transition dirty mirror and its form subscription are gone.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Fable 5 committed 2026-08-09 12:04:05 +02:00
1 parent ea12b06480
commit ecdf3b9b9e
3 files changed
+25 -65

No files matched your search

+1 -1
View File
@@ -514,7 +514,7 @@ See `docs/architecture/m3u-playlist-module.md` for complete documentation.
`/workspace/xtreams/:id/downloads/:downloadId` and
`/workspace/stalker/:id/downloads/:downloadId`. Focused download details hide
the workspace context panel.
- Settings: `/workspace/settings/:section` — one page per section (`general`, `playback`, `epg`, `dashboard`, `remote-control`, `tmdb`, `backup`, `reset`, `about`); `/workspace/settings` redirects to `general`, unknown or capability-gated sections redirect there too, and `/settings` redirects into the workspace. The shared form lives on the parent `SettingsComponent`, so edits survive section switches; a floating unsaved-changes bar (Save/Discard) replaces the old always-visible footer Save button. Leaving the settings AREA with a dirty form triggers `settingsUnsavedChangesGuard` (canDeactivate) and a save/discard/stay dialog — section switches deliberately bypass it, and a failed save cancels the navigation. Non-router exits are covered too: `SettingsUnloadGuardService` (provided by `SettingsComponent`) arms a `beforeunload` handler while the form is dirty (native leave prompt in the PWA) and mirrors the dirty state to the Electron main process (`window-close-guard.service.ts`), which intercepts window close/app quit before `beforeunload` fires and completes the original intent only after the renderer confirms through the same dialog; Electron reloads are cancelled and re-triggered the same way, and a failed save always keeps the window open
- Settings: `/workspace/settings/:section` — one page per section (`general`, `playback`, `epg`, `dashboard`, `remote-control`, `tmdb`, `backup`, `reset`, `about`); `/workspace/settings` redirects to `general`, unknown or capability-gated sections redirect there too, and `/settings` redirects into the workspace. The shared form lives on the parent `SettingsComponent`, so edits survive section switches; a floating unsaved-changes bar (Save/Discard) replaces the old always-visible footer Save button. Leaving the settings AREA with a dirty form triggers `settingsUnsavedChangesGuard` (canDeactivate) and a save/discard/stay dialog — section switches deliberately bypass it, and a failed save cancels the navigation. Non-router exits are covered too: `SettingsUnloadGuardService` (provided by `SettingsComponent`) arms a `beforeunload` handler while the form is dirty (native leave prompt in the PWA) and arms an Electron main-process close guard (`window-close-guard.service.ts`) for the whole settings mount — mount-long on purpose, since arming on the first edit would race the close it protects against. The guard intercepts window close/app quit before `beforeunload` fires and completes the original intent only after the renderer confirms through the same dialog (a pristine form auto-confirms); Electron reloads are cancelled and re-triggered the same way, a failed save always keeps the window open, and installing an app update suspends the whole guard so the updater's quit passes unchallenged
**Service Architecture** (Factory Pattern):
@@ -112,31 +112,19 @@ describe('SettingsUnloadGuardService', () => {
});
describe('close guard mirroring (Electron)', () => {
it('arms the main-process guard when the form becomes dirty', () => {
it('arms the main-process guard for the whole settings mount', () => {
// Mount-long, not per dirty transition: arming on the first
// edit would race the very close it protects against, since
// the IPC is asynchronous. A pristine close auto-confirms.
activateInElectron();
expect(electronStub.setWindowCloseGuard).not.toHaveBeenCalled();
form.markAsDirty();
expect(electronStub.setWindowCloseGuard).toHaveBeenCalledWith(
true
);
});
it('disarms the guard when the form returns to pristine', () => {
it('disarms the guard on destroy and unsubscribes the push', () => {
activateInElectron();
form.markAsDirty();
form.markAsPristine();
expect(electronStub.setWindowCloseGuard).toHaveBeenLastCalledWith(
false
);
});
it('disarms an armed guard on destroy and unsubscribes the push', () => {
activateInElectron();
form.markAsDirty();
service.ngOnDestroy();
@@ -145,14 +133,6 @@ describe('SettingsUnloadGuardService', () => {
);
expect(unsubscribeCloseRequests).toHaveBeenCalled();
});
it('leaves the bridge untouched while the form stays pristine', () => {
activateInElectron();
service.ngOnDestroy();
expect(electronStub.setWindowCloseGuard).not.toHaveBeenCalled();
});
});
describe('updater-driven app quit (Electron)', () => {
@@ -172,19 +152,6 @@ describe('SettingsUnloadGuardService', () => {
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();
@@ -1,6 +1,4 @@
import { inject, Injectable, NgZone, OnDestroy } from '@angular/core';
import { Subscription } from 'rxjs';
import { distinctUntilChanged, map, startWith } from 'rxjs/operators';
import { SettingsForm } from './settings-form.utils';
export interface SettingsUnloadGuardHost {
@@ -18,15 +16,19 @@ export interface SettingsUnloadGuardHost {
* Protects unsaved settings edits on the exits the router never sees:
* closing the window, quitting the app, and reloading the page.
*
* Three cooperating layers, all armed only while the form is dirty:
* Three cooperating layers:
*
* - A `beforeunload` handler. In a browser (PWA) it triggers the native
* leave-page prompt — custom UI is not possible there. In Electron it
* silently cancels the unload (reloads only — see below) and follows up
* with the app's own save/discard/stay dialog.
* - In Electron, the dirty state is mirrored to the main process
* (`setWindowCloseGuard`), which intercepts window close / app quit
* *before* `beforeunload` fires and pushes the decision back here.
* - A `beforeunload` handler that engages while the form is dirty. In a
* browser (PWA) it triggers the native leave-page prompt — custom UI is
* not possible there. In Electron it silently cancels the unload (reloads
* only — see below) and follows up with the app's own save/discard/stay
* dialog.
* - In Electron, a main-process close guard (`setWindowCloseGuard`) armed
* for the WHOLE settings mount, not per dirty transition — arming on the
* first edit would race the very close it protects against, since the
* IPC is asynchronous. It intercepts window close / app quit *before*
* `beforeunload` fires and pushes the decision back here; a pristine
* form auto-confirms without showing anything.
* - `confirmWindowClose` completes an intercepted close once the user
* saved or discarded; staying — or a save that failed — never confirms,
* so the window stays open with the edits intact.
@@ -38,7 +40,6 @@ export interface SettingsUnloadGuardHost {
export class SettingsUnloadGuardService implements OnDestroy {
private readonly zone = inject(NgZone);
private host: SettingsUnloadGuardHost | null = null;
private dirtySubscription: Subscription | null = null;
private unsubscribeCloseRequests: (() => void) | null = null;
private confirmationPending = false;
/**
@@ -60,23 +61,17 @@ export class SettingsUnloadGuardService implements OnDestroy {
this.dispose();
this.host = host;
// `events` fires on every pristine transition (and more); mapping to
// the current dirty flag with distinctUntilChanged keeps the mirror
// exact without depending on which control emitted.
this.dirtySubscription = host.form.events
.pipe(
map(() => host.form.dirty),
startWith(host.form.dirty),
distinctUntilChanged()
)
.subscribe((dirty) => this.syncCloseGuard(dirty));
window.addEventListener('beforeunload', this.beforeUnloadHandler);
this.unsubscribeCloseRequests =
window.electron?.onWindowCloseRequested?.(() => {
this.zone.run(() => void this.handleCloseRequest('close'));
}) ?? null;
// Armed before the user can possibly stage an edit; a close with a
// pristine form auto-confirms through the dialog-less path, so the
// only visible effect of mount-long arming is race-free protection.
this.syncCloseGuard(true);
}
ngOnDestroy(): void {
@@ -97,7 +92,7 @@ export class SettingsUnloadGuardService implements OnDestroy {
this.suspended = true;
window.removeEventListener('beforeunload', this.beforeUnloadHandler);
this.syncCloseGuard(this.host.form.dirty);
this.syncCloseGuard(true);
}
/** Restores the protection when the requested quit did not happen. */
@@ -108,13 +103,11 @@ export class SettingsUnloadGuardService implements OnDestroy {
this.suspended = false;
window.addEventListener('beforeunload', this.beforeUnloadHandler);
this.syncCloseGuard(this.host.form.dirty);
this.syncCloseGuard(true);
}
private dispose(): void {
window.removeEventListener('beforeunload', this.beforeUnloadHandler);
this.dirtySubscription?.unsubscribe();
this.dirtySubscription = null;
this.unsubscribeCloseRequests?.();
this.unsubscribeCloseRequests = null;
this.suspended = false;