From 3f9348584ac1204c32d2ef7bf3d9ac7b738fd889 Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 9 Aug 2026 10:51:44 +0200 Subject: [PATCH] fix(settings): protect unsaved edits on window close, quit, and reload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The settingsUnsavedChangesGuard only covers router navigation; closing the Electron window, quitting the app, or reloading the page silently dropped staged settings edits. Renderer: SettingsUnloadGuardService (provided by SettingsComponent) arms a beforeunload handler while the form is dirty — native leave prompt in the PWA, cancelled-and-redialogued reload in Electron — and mirrors the dirty state to the main process. Main process: WindowCloseGuard intercepts BrowserWindow close before beforeunload fires, pushes WINDOW:CLOSE_REQUESTED to the renderer for the same save/discard/stay dialog, and replays the original intent (close vs. quit, tracked via before-quit) only after the renderer confirms. A failed save never confirms, keeping the guard's semantics. The guard disarms on renderer navigation, crash, or destroyed webContents so a window can never become unclosable. Co-Authored-By: Claude Fable 5 --- .changes/settings-unsaved-close-protection.md | 10 + CLAUDE.md | 2 +- apps/electron-backend-e2e/src/settings.e2e.ts | 63 ++++ .../src/app/api/main.preload.ts | 11 + .../window-close-guard.service.spec.ts | 309 ++++++++++++++++++ .../services/window-close-guard.service.ts | 175 ++++++++++ apps/electron-backend/src/main.ts | 4 + .../settings-unload-guard.service.spec.ts | 233 +++++++++++++ .../settings/settings-unload-guard.service.ts | 143 ++++++++ .../src/app/settings/settings.component.ts | 10 + .../test-stubs/settings-test-harness.stub.ts | 3 + .../src/lib/electron-api.interface.ts | 12 + .../shared/interfaces/src/lib/ipc-commands.ts | 6 + 13 files changed, 980 insertions(+), 1 deletion(-) create mode 100644 .changes/settings-unsaved-close-protection.md create mode 100644 apps/electron-backend/src/app/services/window-close-guard.service.spec.ts create mode 100644 apps/electron-backend/src/app/services/window-close-guard.service.ts create mode 100644 apps/web/src/app/settings/settings-unload-guard.service.spec.ts create mode 100644 apps/web/src/app/settings/settings-unload-guard.service.ts diff --git a/.changes/settings-unsaved-close-protection.md b/.changes/settings-unsaved-close-protection.md new file mode 100644 index 000000000..37f62e770 --- /dev/null +++ b/.changes/settings-unsaved-close-protection.md @@ -0,0 +1,10 @@ +--- +type: fix +area: settings +--- + +Unsaved settings edits no longer vanish silently when you close the window, +quit the app, or reload the page. The desktop app now asks whether to save, +discard, or keep editing — the same dialog in-app navigation shows — and only +closes once your choice is applied; a failed save keeps the window open. In the +browser, the native leave-page warning appears instead. diff --git a/CLAUDE.md b/CLAUDE.md index 8eb867e36..eaee76c10 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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 +- 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 **Service Architecture** (Factory Pattern): diff --git a/apps/electron-backend-e2e/src/settings.e2e.ts b/apps/electron-backend-e2e/src/settings.e2e.ts index fe28297c1..3e1d623de 100644 --- a/apps/electron-backend-e2e/src/settings.e2e.ts +++ b/apps/electron-backend-e2e/src/settings.e2e.ts @@ -481,6 +481,69 @@ test.describe('Electron Settings', () => { } }); + test('@settings @electron intercepts window close while settings edits are unsaved', async ({ + dataDir, + }) => { + const app = await launchElectronApp(dataDir); + + try { + await openSettings(app.mainWindow); + await selectSettingsOption(app.mainWindow, 'select-language', 'de'); + await expect( + app.mainWindow.getByTestId('settings-unsaved-bar') + ).toBeVisible(); + // Round-trip on the same renderer->main IPC pipe: once this + // resolves, the earlier close-guard arming has been processed. + await app.mainWindow.evaluate(() => + window.electron.getWindowState() + ); + + const requestWindowClose = () => + app.electronApp.evaluate(({ BrowserWindow }) => { + BrowserWindow.getAllWindows()[0]?.close(); + }); + + await requestWindowClose(); + + // The close is intercepted: the window stays open and the same + // save/discard/stay dialog the router guard shows takes over. + await expect( + app.mainWindow.getByTestId('unsaved-dialog-stay') + ).toBeVisible(); + expect(app.electronApp.windows()).toHaveLength(1); + + await app.mainWindow.getByTestId('unsaved-dialog-stay').click(); + await expect( + app.mainWindow.getByTestId('unsaved-dialog-stay') + ).toHaveCount(0); + expect(app.electronApp.windows()).toHaveLength(1); + + // Second attempt, this time saving: the close then completes. + await requestWindowClose(); + await expect( + app.mainWindow.getByTestId('unsaved-dialog-save') + ).toBeVisible(); + await app.mainWindow + .getByTestId('unsaved-dialog-save') + .click({ noWaitAfter: true }); + await expect.poll(() => app.electronApp.windows().length).toBe(0); + } finally { + await closeElectronApp(app); + } + + // The save the dialog promised actually landed before the close. + const relaunch = await launchElectronApp(dataDir); + + try { + await openSettings(relaunch.mainWindow); + await expect( + relaunch.mainWindow.getByTestId('select-language') + ).toContainText('Deutsch'); + } finally { + await closeElectronApp(relaunch); + } + }); + test('@settings @persistence @electron persists the EPG view mode across app restart', async ({ dataDir, }) => { diff --git a/apps/electron-backend/src/app/api/main.preload.ts b/apps/electron-backend/src/app/api/main.preload.ts index 36379161e..ea2db5c0e 100644 --- a/apps/electron-backend/src/app/api/main.preload.ts +++ b/apps/electron-backend/src/app/api/main.preload.ts @@ -80,6 +80,9 @@ const WINDOW_TOGGLE_MAXIMIZE = 'WINDOW:TOGGLE_MAXIMIZE'; const WINDOW_CLOSE = 'WINDOW:CLOSE'; const WINDOW_GET_STATE = 'WINDOW:GET_STATE'; const WINDOW_STATE_CHANGED = 'WINDOW:STATE_CHANGED'; +const WINDOW_SET_CLOSE_GUARD = 'WINDOW:SET_CLOSE_GUARD'; +const WINDOW_CONFIRM_CLOSE = 'WINDOW:CONFIRM_CLOSE'; +const WINDOW_CLOSE_REQUESTED = 'WINDOW:CLOSE_REQUESTED'; const dbSaveContentProgressListeners = new Set< ( @@ -418,6 +421,14 @@ const electronApi: ElectronBridgeApi = { ipcRenderer.on(WINDOW_STATE_CHANGED, handler); return () => ipcRenderer.off(WINDOW_STATE_CHANGED, handler); }, + setWindowCloseGuard: (active: boolean) => + ipcRenderer.invoke(WINDOW_SET_CLOSE_GUARD, active), + confirmWindowClose: () => ipcRenderer.invoke(WINDOW_CONFIRM_CLOSE), + onWindowCloseRequested: (callback: () => void) => { + const handler = () => callback(); + ipcRenderer.on(WINDOW_CLOSE_REQUESTED, handler); + return () => ipcRenderer.off(WINDOW_CLOSE_REQUESTED, handler); + }, fetchPlaylistByUrl: ( url: string, title?: string, diff --git a/apps/electron-backend/src/app/services/window-close-guard.service.spec.ts b/apps/electron-backend/src/app/services/window-close-guard.service.spec.ts new file mode 100644 index 000000000..8565c0199 --- /dev/null +++ b/apps/electron-backend/src/app/services/window-close-guard.service.spec.ts @@ -0,0 +1,309 @@ +/** + * Regression coverage for the unsaved-settings close guard: while the + * renderer arms it, window close and app quit must be intercepted and + * replayed only after the renderer confirms — and the interception must + * never survive a renderer that can no longer answer (navigation, crash, + * destroyed webContents), or the window would become unclosable. + */ + +jest.mock('electron', () => ({ + app: { + on: jest.fn(), + quit: jest.fn(), + }, + ipcMain: { + handle: jest.fn(), + }, +})); + +import { ipcMain } from 'electron'; +import { + WINDOW_CLOSE_REQUESTED, + WINDOW_CONFIRM_CLOSE, + WINDOW_SET_CLOSE_GUARD, +} from '@iptvnator/shared/interfaces'; +import { + bootstrapWindowCloseGuard, + CloseGuardApp, + CloseGuardWindow, + WindowCloseGuard, +} from './window-close-guard.service'; + +type Listener = (...args: unknown[]) => void; + +interface FakeApp extends CloseGuardApp { + quit: jest.Mock; + fireBeforeQuit(): void; +} + +function createFakeApp(): FakeApp { + const beforeQuitListeners: Listener[] = []; + + return { + on: jest.fn((event: string, listener: Listener) => { + if (event === 'before-quit') { + beforeQuitListeners.push(listener); + } + }), + quit: jest.fn(), + fireBeforeQuit(): void { + beforeQuitListeners.forEach((listener) => listener()); + }, + }; +} + +interface FakeWindow extends CloseGuardWindow { + close: jest.Mock; + isDestroyed: jest.Mock; + webContents: CloseGuardWindow['webContents'] & { + isDestroyed: jest.Mock; + send: jest.Mock; + }; + /** Fires the window 'close' event; returns whether it was prevented. */ + fireClose(): boolean; + fireClosed(): void; + fireWebContentsEvent(event: string): void; +} + +function createFakeWindow(): FakeWindow { + const windowListeners = new Map(); + const webContentsListeners = new Map(); + + const addListener = + (target: Map) => + (event: string, listener: Listener) => { + target.set(event, [...(target.get(event) ?? []), listener]); + }; + + return { + close: jest.fn(), + isDestroyed: jest.fn(() => false), + on: jest.fn(addListener(windowListeners)), + webContents: { + isDestroyed: jest.fn(() => false), + send: jest.fn(), + on: jest.fn(addListener(webContentsListeners)), + }, + fireClose(): boolean { + let prevented = false; + const event = { + preventDefault: () => { + prevented = true; + }, + }; + + (windowListeners.get('close') ?? []).forEach((listener) => + listener(event) + ); + return prevented; + }, + fireClosed(): void { + (windowListeners.get('closed') ?? []).forEach((listener) => + listener() + ); + }, + fireWebContentsEvent(event: string): void { + (webContentsListeners.get(event) ?? []).forEach((listener) => + listener() + ); + }, + }; +} + +function createArmedGuard(): { + app: FakeApp; + guard: WindowCloseGuard; + win: FakeWindow; +} { + const app = createFakeApp(); + const guard = new WindowCloseGuard(app); + const win = createFakeWindow(); + + guard.trackQuitLifecycle(); + guard.attachToWindow(win); + guard.setGuardActive(true); + + return { app, guard, win }; +} + +describe('WindowCloseGuard', () => { + it('lets the window close while the guard is inactive', () => { + const app = createFakeApp(); + const guard = new WindowCloseGuard(app); + const win = createFakeWindow(); + + guard.trackQuitLifecycle(); + guard.attachToWindow(win); + + expect(win.fireClose()).toBe(false); + expect(win.webContents.send).not.toHaveBeenCalled(); + }); + + it('intercepts an armed close and asks the renderer instead', () => { + const { win } = createArmedGuard(); + + expect(win.fireClose()).toBe(true); + expect(win.webContents.send).toHaveBeenCalledWith( + WINDOW_CLOSE_REQUESTED + ); + expect(win.close).not.toHaveBeenCalled(); + }); + + it('replays an intercepted window close once the renderer confirms', () => { + const { app, guard, win } = createArmedGuard(); + + win.fireClose(); + guard.confirmClose(); + + expect(win.close).toHaveBeenCalledTimes(1); + expect(app.quit).not.toHaveBeenCalled(); + // The replayed close must pass through even though the guard is + // still armed — the renderer already decided. + expect(win.fireClose()).toBe(false); + }); + + it('resumes a quit as a quit, not as a plain window close', () => { + const { app, guard, win } = createArmedGuard(); + + app.fireBeforeQuit(); + expect(win.fireClose()).toBe(true); + + guard.confirmClose(); + + expect(app.quit).toHaveBeenCalledTimes(1); + expect(win.close).not.toHaveBeenCalled(); + }); + + it('forgets an aborted quit before the next close attempt', () => { + const { app, guard, win } = createArmedGuard(); + + // Quit intercepted, user stays: the quit is aborted. + app.fireBeforeQuit(); + win.fireClose(); + + // A later plain window close must not resurrect the quit. + win.fireClose(); + guard.confirmClose(); + + expect(win.close).toHaveBeenCalledTimes(1); + expect(app.quit).not.toHaveBeenCalled(); + }); + + it('confirms as a plain close when nothing was intercepted', () => { + const { app, guard, win } = createArmedGuard(); + + guard.confirmClose(); + + expect(win.close).toHaveBeenCalledTimes(1); + expect(app.quit).not.toHaveBeenCalled(); + }); + + it('disarms when the renderer navigates away', () => { + const { win } = createArmedGuard(); + + win.fireWebContentsEvent('did-navigate'); + + expect(win.fireClose()).toBe(false); + }); + + it('disarms when the render process is gone', () => { + const { win } = createArmedGuard(); + + win.fireWebContentsEvent('render-process-gone'); + + expect(win.fireClose()).toBe(false); + }); + + it('never blocks a close whose webContents is already destroyed', () => { + const { win } = createArmedGuard(); + + win.webContents.isDestroyed.mockReturnValue(true); + + expect(win.fireClose()).toBe(false); + }); + + it('drops all state once the window is closed', () => { + const { guard, win } = createArmedGuard(); + + win.fireClose(); + win.fireClosed(); + guard.confirmClose(); + + expect(win.close).not.toHaveBeenCalled(); + }); + + it('ignores confirmations for a destroyed window', () => { + const { guard, win } = createArmedGuard(); + + win.fireClose(); + win.isDestroyed.mockReturnValue(true); + guard.confirmClose(); + + expect(win.close).not.toHaveBeenCalled(); + }); + + it('starts a re-created window with a disarmed guard', () => { + const { guard } = createArmedGuard(); + const nextWin = createFakeWindow(); + + guard.attachToWindow(nextWin); + + expect(nextWin.fireClose()).toBe(false); + }); +}); + +describe('bootstrapWindowCloseGuard', () => { + beforeEach(() => { + (ipcMain.handle as jest.Mock).mockClear(); + }); + + function getIpcHandler(channel: string): Listener { + const call = (ipcMain.handle as jest.Mock).mock.calls.find( + ([registered]) => registered === channel + ); + + if (!call) { + throw new Error(`No IPC handler registered for ${channel}`); + } + + return call[1] as Listener; + } + + it('wires the IPC handlers to the guard', () => { + const attachListeners: Array<(win: Electron.BrowserWindow) => void> = + []; + + bootstrapWindowCloseGuard((listener) => + attachListeners.push(listener) + ); + + const win = createFakeWindow(); + attachListeners.forEach((listener) => + listener(win as unknown as Electron.BrowserWindow) + ); + + getIpcHandler(WINDOW_SET_CLOSE_GUARD)({}, true); + expect(win.fireClose()).toBe(true); + + getIpcHandler(WINDOW_CONFIRM_CLOSE)({}); + expect(win.close).toHaveBeenCalledTimes(1); + }); + + it('treats non-boolean guard payloads as disarm', () => { + const attachListeners: Array<(win: Electron.BrowserWindow) => void> = + []; + + bootstrapWindowCloseGuard((listener) => + attachListeners.push(listener) + ); + + const win = createFakeWindow(); + attachListeners.forEach((listener) => + listener(win as unknown as Electron.BrowserWindow) + ); + + getIpcHandler(WINDOW_SET_CLOSE_GUARD)({}, 'yes'); + + expect(win.fireClose()).toBe(false); + }); +}); diff --git a/apps/electron-backend/src/app/services/window-close-guard.service.ts b/apps/electron-backend/src/app/services/window-close-guard.service.ts new file mode 100644 index 000000000..ab012e89c --- /dev/null +++ b/apps/electron-backend/src/app/services/window-close-guard.service.ts @@ -0,0 +1,175 @@ +/** + * Main-process side of the unsaved-changes close protection. + * + * The settings page arms the guard while its form is dirty + * (`WINDOW:SET_CLOSE_GUARD`). While armed, closing the window — title-bar + * button, custom window controls, Cmd+W, or an app quit — is intercepted + * here and handed back to the renderer as a `WINDOW:CLOSE_REQUESTED` push, + * where the same save/discard/stay dialog the router guard uses decides the + * outcome. `WINDOW:CONFIRM_CLOSE` then replays the original intent (window + * close vs. app quit) with the guard bypassed; staying simply never confirms. + * + * The BrowserWindow `close` event fires before the DOM `beforeunload` + * (Electron contract), so an armed guard is always consulted first and the + * renderer's own `beforeunload` handler only ever sees reloads. + */ + +import { app, ipcMain } from 'electron'; +import { + WINDOW_CLOSE_REQUESTED, + WINDOW_CONFIRM_CLOSE, + WINDOW_SET_CLOSE_GUARD, +} from '@iptvnator/shared/interfaces'; + +type CloseIntent = 'close' | 'quit'; + +/** The slice of `Electron.App` the guard needs — injectable for tests. */ +export interface CloseGuardApp { + on(event: 'before-quit', listener: () => void): unknown; + quit(): void; +} + +/** The slice of `Electron.BrowserWindow` the guard needs. */ +export interface CloseGuardWindow { + isDestroyed(): boolean; + close(): void; + on(event: string, listener: (...args: unknown[]) => void): unknown; + webContents: { + isDestroyed(): boolean; + send(channel: string): void; + on(event: string, listener: (...args: unknown[]) => void): unknown; + }; +} + +export class WindowCloseGuard { + private guardActive = false; + private bypassClose = false; + private quitInProgress = false; + private pendingIntent: CloseIntent | null = null; + private window: CloseGuardWindow | null = null; + + constructor(private readonly electronApp: CloseGuardApp) {} + + /** + * Distinguishes "close this window" from "quit the app": `before-quit` + * fires before the quit closes the windows, so a close intercepted with + * this flag set must resume as a quit — on macOS a plain `win.close()` + * would leave the app running when the user asked it to exit. + */ + trackQuitLifecycle(): void { + this.electronApp.on('before-quit', () => { + this.quitInProgress = true; + }); + } + + /** + * Called for every main window (macOS can rebuild it while the process + * lives on); per-window guard state starts over with each new window. + */ + attachToWindow(win: CloseGuardWindow): void { + this.window = win; + this.guardActive = false; + this.bypassClose = false; + this.pendingIntent = null; + + win.on('close', (event) => + this.handleClose(event as { preventDefault(): void }, win) + ); + win.on('closed', () => { + if (this.window === win) { + this.window = null; + this.guardActive = false; + this.bypassClose = false; + this.pendingIntent = null; + } + }); + // A full navigation (reload included) discards the renderer state the + // guard was protecting; a gone renderer can no longer answer the + // close request. Either way an armed guard would make the window + // unclosable. + win.webContents.on('did-navigate', () => { + this.guardActive = false; + }); + win.webContents.on('render-process-gone', () => { + this.guardActive = false; + }); + } + + setGuardActive(active: boolean): void { + this.guardActive = active; + } + + /** + * Renderer verdict: safe to leave (settings saved or discarded). Replays + * the intercepted intent with the guard bypassed for exactly one close. + */ + confirmClose(): void { + const win = this.window; + + if (!win || win.isDestroyed()) { + return; + } + + const intent = this.pendingIntent ?? 'close'; + this.pendingIntent = null; + this.bypassClose = true; + + if (intent === 'quit') { + this.electronApp.quit(); + } else { + win.close(); + } + } + + private handleClose( + event: { preventDefault(): void }, + win: CloseGuardWindow + ): void { + // Consumed per close attempt: if this close is allowed through, the + // quit continues on its own; if it is intercepted, the quit is + // aborted and only `pendingIntent` remembers it. + const wasQuit = this.quitInProgress; + this.quitInProgress = false; + + if (!this.guardActive || this.bypassClose) { + return; + } + + if (win.webContents.isDestroyed()) { + return; + } + + event.preventDefault(); + this.pendingIntent = wasQuit ? 'quit' : 'close'; + win.webContents.send(WINDOW_CLOSE_REQUESTED); + } +} + +/** + * Creates the app-wide guard and registers its IPC handlers. The main-window + * hook is passed in (instead of importing `App`) to keep this module free of + * bootstrap-order side effects. + */ +export function bootstrapWindowCloseGuard( + onMainWindowCreated: ( + listener: (win: Electron.BrowserWindow) => void + ) => void +): WindowCloseGuard { + const guard = new WindowCloseGuard(app); + + guard.trackQuitLifecycle(); + // BrowserWindow satisfies CloseGuardWindow at runtime; the cast only + // bridges Electron's per-event `on` overloads to the structural type. + onMainWindowCreated((win) => + guard.attachToWindow(win as unknown as CloseGuardWindow) + ); + + ipcMain.handle(WINDOW_SET_CLOSE_GUARD, (_event, active: boolean) => { + guard.setGuardActive(active === true); + }); + ipcMain.handle(WINDOW_CONFIRM_CLOSE, () => { + guard.confirmClose(); + }); + + return guard; +} diff --git a/apps/electron-backend/src/main.ts b/apps/electron-backend/src/main.ts index 1dfc50e9e..8abd74480 100644 --- a/apps/electron-backend/src/main.ts +++ b/apps/electron-backend/src/main.ts @@ -30,6 +30,7 @@ import { registerStaticHeaderShims } from './app/services/request-header-overrid import { AppUpdateService } from './app/services/app-update.service'; import { databaseWorkerClient } from './app/services/database-worker-client'; import WindowEvents from './app/events/window.events'; +import { bootstrapWindowCloseGuard } from './app/services/window-close-guard.service'; import { registerStreamProbeHandlers } from './app/events/stream-probe'; import XtreamEvents from './app/events/xtream.events'; import { environment } from './environments/environment'; @@ -145,6 +146,9 @@ export default class Main { registerStaticHeaderShims(); ElectronEvents.bootstrapElectronEvents(); WindowEvents.bootstrapWindowEvents(); + bootstrapWindowCloseGuard((listener) => + App.onMainWindowCreated(listener) + ); EmbeddedMpvEvents.bootstrapEmbeddedMpvEvents(); PlaylistEvents.bootstrapPlaylistEvents(); PlaylistOpenEvents.bootstrapPlaylistOpenEvents(); diff --git a/apps/web/src/app/settings/settings-unload-guard.service.spec.ts b/apps/web/src/app/settings/settings-unload-guard.service.spec.ts new file mode 100644 index 000000000..ae6f8abfc --- /dev/null +++ b/apps/web/src/app/settings/settings-unload-guard.service.spec.ts @@ -0,0 +1,233 @@ +import { TestBed } from '@angular/core/testing'; +import { FormBuilder } from '@angular/forms'; +import { SettingsForm } from './settings-form.utils'; +import { + SettingsUnloadGuardHost, + SettingsUnloadGuardService, +} from './settings-unload-guard.service'; + +/** + * Regression coverage for the non-router exits from settings: window close / + * app quit (Electron main-process interception) and page reload + * (`beforeunload`). The router-navigation path is covered by + * `settings-unsaved-changes.guard.spec.ts`. + */ +describe('SettingsUnloadGuardService', () => { + let service: SettingsUnloadGuardService; + let form: SettingsForm; + let host: SettingsUnloadGuardHost & { confirmClose: jest.Mock }; + const originalElectron = window.electron; + + let closeRequestCallback: (() => void) | null; + let unsubscribeCloseRequests: jest.Mock; + let electronStub: { + setWindowCloseGuard: jest.Mock; + confirmWindowClose: jest.Mock; + onWindowCloseRequested: jest.Mock; + }; + + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [SettingsUnloadGuardService], + }); + + service = TestBed.inject(SettingsUnloadGuardService); + // The service only touches `events`, `dirty` and the pristine + // markers, so a minimal group stands in for the real settings form. + form = new FormBuilder().group({ + theme: [''], + }) as unknown as SettingsForm; + host = { + confirmClose: jest.fn().mockResolvedValue(true), + form, + }; + + closeRequestCallback = null; + unsubscribeCloseRequests = jest.fn(); + electronStub = { + setWindowCloseGuard: jest.fn().mockResolvedValue(undefined), + confirmWindowClose: jest.fn().mockResolvedValue(undefined), + onWindowCloseRequested: jest.fn((callback: () => void) => { + closeRequestCallback = callback; + return unsubscribeCloseRequests; + }), + }; + }); + + afterEach(() => { + service.ngOnDestroy(); + window.electron = originalElectron; + }); + + function activateInElectron(): void { + window.electron = electronStub as unknown as typeof window.electron; + service.activate(host); + } + + function activateInPwa(): void { + window.electron = undefined; + service.activate(host); + } + + function dispatchBeforeUnload(): Event { + const event = new Event('beforeunload', { cancelable: true }); + window.dispatchEvent(event); + return event; + } + + async function flushAsyncWork(): Promise { + await new Promise((resolve) => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); + } + + describe('beforeunload (PWA)', () => { + it('blocks the unload while the form is dirty', () => { + activateInPwa(); + form.markAsDirty(); + + const event = dispatchBeforeUnload(); + + expect(event.defaultPrevented).toBe(true); + }); + + it('lets a pristine form unload silently', () => { + activateInPwa(); + + const event = dispatchBeforeUnload(); + + expect(event.defaultPrevented).toBe(false); + }); + + it('stops blocking after destroy', () => { + activateInPwa(); + form.markAsDirty(); + service.ngOnDestroy(); + + const event = dispatchBeforeUnload(); + + expect(event.defaultPrevented).toBe(false); + }); + }); + + describe('close guard mirroring (Electron)', () => { + it('arms the main-process guard when the form becomes dirty', () => { + activateInElectron(); + expect(electronStub.setWindowCloseGuard).not.toHaveBeenCalled(); + + form.markAsDirty(); + + expect(electronStub.setWindowCloseGuard).toHaveBeenCalledWith( + true + ); + }); + + it('disarms the guard when the form returns to pristine', () => { + 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(); + + expect(electronStub.setWindowCloseGuard).toHaveBeenLastCalledWith( + false + ); + expect(unsubscribeCloseRequests).toHaveBeenCalled(); + }); + + it('leaves the bridge untouched while the form stays pristine', () => { + activateInElectron(); + + service.ngOnDestroy(); + + expect(electronStub.setWindowCloseGuard).not.toHaveBeenCalled(); + }); + }); + + describe('intercepted window close (Electron)', () => { + it('confirms the close once the user saves or discards', async () => { + activateInElectron(); + form.markAsDirty(); + + closeRequestCallback?.(); + await flushAsyncWork(); + + expect(host.confirmClose).toHaveBeenCalledTimes(1); + expect(electronStub.confirmWindowClose).toHaveBeenCalledTimes(1); + }); + + it('keeps the window open when the user stays or the save fails', async () => { + host.confirmClose.mockResolvedValue(false); + activateInElectron(); + form.markAsDirty(); + + closeRequestCallback?.(); + await flushAsyncWork(); + + expect(electronStub.confirmWindowClose).not.toHaveBeenCalled(); + }); + + it('ignores repeated close requests while a dialog is open', async () => { + let resolveConfirm: ((value: boolean) => void) | null = null; + host.confirmClose.mockImplementation( + () => + new Promise((resolve) => { + resolveConfirm = resolve; + }) + ); + activateInElectron(); + form.markAsDirty(); + + closeRequestCallback?.(); + closeRequestCallback?.(); + await flushAsyncWork(); + + expect(host.confirmClose).toHaveBeenCalledTimes(1); + + resolveConfirm?.(true); + await flushAsyncWork(); + + expect(electronStub.confirmWindowClose).toHaveBeenCalledTimes(1); + }); + }); + + describe('intercepted reload (Electron)', () => { + it('re-triggers the reload after the user saves or discards', async () => { + const reloadPage = jest.fn(); + activateInElectron(); + service.reloadPage = reloadPage; + form.markAsDirty(); + + const event = dispatchBeforeUnload(); + await flushAsyncWork(); + + expect(event.defaultPrevented).toBe(true); + expect(host.confirmClose).toHaveBeenCalledTimes(1); + expect(reloadPage).toHaveBeenCalledTimes(1); + // A reload decision must never complete a window close. + expect(electronStub.confirmWindowClose).not.toHaveBeenCalled(); + }); + + it('cancels the reload when the user stays', async () => { + const reloadPage = jest.fn(); + host.confirmClose.mockResolvedValue(false); + activateInElectron(); + service.reloadPage = reloadPage; + form.markAsDirty(); + + dispatchBeforeUnload(); + await flushAsyncWork(); + + expect(reloadPage).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/apps/web/src/app/settings/settings-unload-guard.service.ts b/apps/web/src/app/settings/settings-unload-guard.service.ts new file mode 100644 index 000000000..e175a7edd --- /dev/null +++ b/apps/web/src/app/settings/settings-unload-guard.service.ts @@ -0,0 +1,143 @@ +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 { + /** The shared settings form whose dirty state arms every protection. */ + form: SettingsForm; + /** + * The same save/discard/stay flow the router guard runs. Resolves true + * when leaving is safe — saved or discarded; a failed save resolves + * false and must cancel the close. + */ + confirmClose: () => Promise; +} + +/** + * 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: + * + * - 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. + * - `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. + * + * Provided by `SettingsComponent`, so its `ngOnDestroy` runs when the user + * leaves the settings area and every hook is released. + */ +@Injectable() +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; + /** Last guard state mirrored to the main process. */ + private guardArmed = false; + + /** Indirection because `window.location.reload` cannot be stubbed. */ + reloadPage: () => void = () => window.location.reload(); + + activate(host: SettingsUnloadGuardHost): void { + 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; + } + + ngOnDestroy(): void { + this.dispose(); + } + + private dispose(): void { + window.removeEventListener('beforeunload', this.beforeUnloadHandler); + this.dirtySubscription?.unsubscribe(); + this.dirtySubscription = null; + this.unsubscribeCloseRequests?.(); + this.unsubscribeCloseRequests = null; + this.syncCloseGuard(false); + this.host = null; + } + + private readonly beforeUnloadHandler = ( + event: BeforeUnloadEvent + ): void => { + if (!this.host?.form.dirty) { + return; + } + + event.preventDefault(); + // Chromium's legacy trigger for the native prompt; harmless in + // Electron, where cancellation comes from preventDefault(). + event.returnValue = ''; + + 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. + setTimeout(() => { + this.zone.run(() => void this.handleCloseRequest('reload')); + }); + } + }; + + private async handleCloseRequest( + intent: 'close' | 'reload' + ): Promise { + const host = this.host; + + if (!host || this.confirmationPending) { + return; + } + + this.confirmationPending = true; + + try { + if (!(await host.confirmClose())) { + return; + } + + if (intent === 'close') { + await window.electron?.confirmWindowClose?.(); + } else { + this.reloadPage(); + } + } finally { + this.confirmationPending = false; + } + } + + private syncCloseGuard(active: boolean): void { + if (this.guardArmed === active) { + return; + } + + this.guardArmed = active; + void window.electron?.setWindowCloseGuard?.(active); + } +} diff --git a/apps/web/src/app/settings/settings.component.ts b/apps/web/src/app/settings/settings.component.ts index e136cc871..56a07510c 100644 --- a/apps/web/src/app/settings/settings.component.ts +++ b/apps/web/src/app/settings/settings.component.ts @@ -49,6 +49,7 @@ import { SettingsUnsavedChangesDialogComponent, } from './settings-unsaved-changes-dialog.component'; import { SettingsLeaveConfirmation } from './settings-unsaved-changes.guard'; +import { SettingsUnloadGuardService } from './settings-unload-guard.service'; import { SettingsBackupFacade } from './settings-backup.facade'; import { SettingsPlaylistResetFacade } from './settings-playlist-reset.facade'; import { SettingsSnackbarService } from './settings-snackbar.service'; @@ -97,6 +98,7 @@ export const SETTINGS_DEFAULT_SECTION = 'general'; SettingsPlaylistResetFacade, SettingsRemoteControlFacade, SettingsSnackbarService, + SettingsUnloadGuardService, ], }) export class SettingsComponent @@ -112,6 +114,7 @@ export class SettingsComponent private readonly settingsCtx = inject(SettingsContextService); private readonly settingsSnackbar = inject(SettingsSnackbarService); + private readonly unloadGuard = inject(SettingsUnloadGuardService); private readonly runtime = inject(RuntimeCapabilitiesService); private readonly vodSourceDiscovery = inject(VodSourceDiscoveryService); private readonly matDialog = inject(MatDialog); @@ -214,6 +217,13 @@ export class SettingsComponent * storage (indexed db) */ async ngOnInit(): Promise { + // The router guard only covers in-app navigation; this protects the + // same edits against window close, app quit, and page reload. + this.unloadGuard.activate({ + form: this.settingsForm, + confirmClose: () => this.confirmLeaveWithUnsavedChanges(), + }); + // Wait for settings to load before setting the form await this.form.loadSettings(); this.form.hydrateFromStore(); diff --git a/apps/web/src/app/settings/test-stubs/settings-test-harness.stub.ts b/apps/web/src/app/settings/test-stubs/settings-test-harness.stub.ts index 2dd8cd1c5..8a50210ed 100644 --- a/apps/web/src/app/settings/test-stubs/settings-test-harness.stub.ts +++ b/apps/web/src/app/settings/test-stubs/settings-test-harness.stub.ts @@ -241,6 +241,9 @@ export function createElectronStub(): typeof window.electron { .fn() .mockResolvedValue(DEFAULT_APP_UPDATE_STATUS), onAppUpdateStatusChange: jest.fn(() => jest.fn()), + onWindowCloseRequested: jest.fn(() => jest.fn()), + setWindowCloseGuard: jest.fn().mockResolvedValue(undefined), + confirmWindowClose: jest.fn().mockResolvedValue(undefined), openInMpv: jest.fn(), openInVlc: jest.fn(), platform: 'linux', diff --git a/libs/shared/interfaces/src/lib/electron-api.interface.ts b/libs/shared/interfaces/src/lib/electron-api.interface.ts index 28cfa0707..e9e93c865 100644 --- a/libs/shared/interfaces/src/lib/electron-api.interface.ts +++ b/libs/shared/interfaces/src/lib/electron-api.interface.ts @@ -618,6 +618,18 @@ export interface ElectronBridgeApi { onWindowStateChange: ( callback: (state: ElectronBridgeWindowState) => void ) => () => void; + /** + * While active, the main process intercepts window close / app quit and + * pushes a close request to the renderer instead of closing. Used by the + * settings page while its form holds unsaved edits. + */ + setWindowCloseGuard: (active: boolean) => Promise; + /** + * Completes a close the guard intercepted: the main process re-runs the + * original intent (window close or app quit) with the guard bypassed. + */ + confirmWindowClose: () => Promise; + onWindowCloseRequested: (callback: () => void) => () => void; fetchPlaylistByUrl: ( url: string, title?: string, diff --git a/libs/shared/interfaces/src/lib/ipc-commands.ts b/libs/shared/interfaces/src/lib/ipc-commands.ts index fc95950dd..e2272ae44 100644 --- a/libs/shared/interfaces/src/lib/ipc-commands.ts +++ b/libs/shared/interfaces/src/lib/ipc-commands.ts @@ -113,3 +113,9 @@ export const WINDOW_TOGGLE_MAXIMIZE = 'WINDOW:TOGGLE_MAXIMIZE'; export const WINDOW_CLOSE = 'WINDOW:CLOSE'; export const WINDOW_GET_STATE = 'WINDOW:GET_STATE'; export const WINDOW_STATE_CHANGED = 'WINDOW:STATE_CHANGED'; + +// Close guard: while active, closing/quitting the app is intercepted in the +// main process and handed to the renderer for a save/discard/stay decision +export const WINDOW_SET_CLOSE_GUARD = 'WINDOW:SET_CLOSE_GUARD'; +export const WINDOW_CONFIRM_CLOSE = 'WINDOW:CONFIRM_CLOSE'; +export const WINDOW_CLOSE_REQUESTED = 'WINDOW:CLOSE_REQUESTED';