diff --git a/.changes/window-controls-fullscreen-exit.md b/.changes/window-controls-fullscreen-exit.md new file mode 100644 index 000000000..65607da9b --- /dev/null +++ b/.changes/window-controls-fullscreen-exit.md @@ -0,0 +1,9 @@ +--- +type: fix +area: window-controls +--- + +On Windows, the minimize, maximize and close buttons disappeared for good after +leaving fullscreen video playback — the only way to get them back was to restart +the app. They now reappear as soon as you exit fullscreen, and the maximize +button no longer gets stuck on the wrong icon afterwards. diff --git a/apps/electron-backend-e2e/src/window-controls.e2e.ts b/apps/electron-backend-e2e/src/window-controls.e2e.ts index 5d4f36c99..d65b463bd 100644 --- a/apps/electron-backend-e2e/src/window-controls.e2e.ts +++ b/apps/electron-backend-e2e/src/window-controls.e2e.ts @@ -103,6 +103,67 @@ test.describe('Custom window controls', () => { } }); + test('@electron controls reappear after leaving HTML element fullscreen', async ({ + dataDir, + }) => { + test.skip( + isWindowManagerlessCi, + 'xvfb on Linux CI has no window manager' + ); + const app = await launchElectronApp(dataDir); + + try { + const controls = app.mainWindow.locator('app-window-controls'); + await expect(controls).not.toHaveClass(/is-hidden/); + + // The video players fullscreen their player root through the + // HTML element-fullscreen API. That API needs transient user + // activation, so trigger it from a real click on a temporary + // overlay instead of calling it directly from evaluate(). + await app.mainWindow.evaluate(() => { + const overlay = document.createElement('div'); + overlay.id = 'e2e-fullscreen-trigger'; + overlay.style.cssText = + 'position: fixed; inset: 0; z-index: 2147483647;'; + overlay.addEventListener('click', () => { + overlay.remove(); + void document.documentElement.requestFullscreen(); + }); + document.body.append(overlay); + }); + await app.mainWindow.locator('#e2e-fullscreen-trigger').click(); + + const isHtmlFullScreen = () => + app.mainWindow.evaluate( + () => document.fullscreenElement !== null + ); + + await expect.poll(isHtmlFullScreen, { timeout: 10_000 }).toBe( + true + ); + await expect(controls).toHaveClass(/is-hidden/, { + timeout: 10_000, + }); + + await app.mainWindow.evaluate(() => document.exitFullscreen()); + await expect.poll(isHtmlFullScreen, { timeout: 10_000 }).toBe( + false + ); + + // Regression: the exit push must not carry a stale + // isFullScreen=true read mid-transition — the controls have to + // come back once fullscreen is left. + await expect(controls).not.toHaveClass(/is-hidden/, { + timeout: 10_000, + }); + await expect( + app.mainWindow.getByTestId('window-minimize') + ).toBeVisible(); + } finally { + await closeElectronApp(app); + } + }); + test('@electron minimize button minimizes the window', async ({ dataDir, }) => { diff --git a/apps/electron-backend/src/app/app-window-state.spec.ts b/apps/electron-backend/src/app/app-window-state.spec.ts new file mode 100644 index 000000000..f550142df --- /dev/null +++ b/apps/electron-backend/src/app/app-window-state.spec.ts @@ -0,0 +1,242 @@ +/** + * Regression coverage for the WINDOW:STATE_CHANGED pushes that drive the + * renderer-drawn window controls on Windows/Linux. + * + * The handlers must never re-read window state at event time: on Windows, + * isFullScreen() can still report true while 'leave-full-screen' fires for + * HTML-element fullscreen (video player) exits, and isMaximized() reports + * false while the window is fullscreen. Polling pushed a stale + * `isFullScreen: true` on exit — leaving the custom window controls hidden + * forever — and cleared the companion `isMaximized`, sticking the + * maximize/restore glyph on the wrong icon. Instead the state is seeded once + * at attach time and each event patches only the flag it names. + */ + +jest.mock('electron', () => ({ + app: { + getPath: jest.fn(() => '/tmp'), + isPackaged: false, + isReady: jest.fn(() => false), + on: jest.fn(), + }, + BrowserWindow: jest.fn(), + Menu: { + buildFromTemplate: jest.fn(), + }, + screen: { + getPrimaryDisplay: jest.fn(), + }, + session: { + defaultSession: { + clearStorageData: jest.fn(), + }, + }, + shell: { + openExternal: jest.fn(), + }, +})); + +jest.mock('./services/store.service', () => ({ + store: { + get: jest.fn(), + set: jest.fn(), + }, + WINDOW_BOUNDS: 'windowBounds', +})); + +import { WINDOW_STATE_CHANGED } from '@iptvnator/shared/interfaces'; +import App from './app'; + +type MockStateWindow = { + isDestroyed: jest.Mock; + isFullScreen: jest.Mock; + isMaximized: jest.Mock; + on: jest.Mock void]>; + webContents: { + send: jest.Mock; + }; +}; + +function createMockStateWindow(): MockStateWindow { + return { + isDestroyed: jest.fn().mockReturnValue(false), + isFullScreen: jest.fn().mockReturnValue(false), + isMaximized: jest.fn().mockReturnValue(false), + on: jest.fn void]>(), + webContents: { + send: jest.fn(), + }, + }; +} + +function attachWindowStateEvents(win: MockStateWindow): void { + ( + App as unknown as { + attachWindowStateEvents: (win: MockStateWindow) => void; + } + ).attachWindowStateEvents(win); +} + +function fireWindowEvent(win: MockStateWindow, eventName: string): void { + const handlers = win.on.mock.calls + .filter(([name]) => name === eventName) + .map(([, handler]) => handler); + + expect(handlers).toHaveLength(1); + handlers[0](); +} + +describe('window state change pushes', () => { + const originalPlatform = Object.getOwnPropertyDescriptor( + process, + 'platform' + ); + + function setProcessPlatform(platform: NodeJS.Platform): void { + Object.defineProperty(process, 'platform', { + configurable: true, + value: platform, + }); + } + + beforeEach(() => { + setProcessPlatform('win32'); + }); + + afterEach(() => { + if (originalPlatform) { + Object.defineProperty(process, 'platform', originalPlatform); + } + }); + + it('registers no window-state listeners on macOS', () => { + setProcessPlatform('darwin'); + const win = createMockStateWindow(); + + attachWindowStateEvents(win); + + expect(win.on).not.toHaveBeenCalled(); + }); + + it('pushes isFullScreen=false on leave-full-screen even while the window still reports fullscreen', () => { + const win = createMockStateWindow(); + attachWindowStateEvents(win); + // Windows still reports the old state while the transition runs. + win.isFullScreen.mockReturnValue(true); + + fireWindowEvent(win, 'leave-full-screen'); + + expect(win.webContents.send).toHaveBeenCalledWith( + WINDOW_STATE_CHANGED, + { isMaximized: false, isFullScreen: false } + ); + }); + + it('pushes isFullScreen=false on leave-html-full-screen even while the window still reports fullscreen', () => { + const win = createMockStateWindow(); + attachWindowStateEvents(win); + win.isFullScreen.mockReturnValue(true); + + fireWindowEvent(win, 'leave-html-full-screen'); + + expect(win.webContents.send).toHaveBeenCalledWith( + WINDOW_STATE_CHANGED, + { isMaximized: false, isFullScreen: false } + ); + }); + + it('pushes isFullScreen=true on enter events before the window reports fullscreen', () => { + const win = createMockStateWindow(); + attachWindowStateEvents(win); + win.isFullScreen.mockReturnValue(false); + + fireWindowEvent(win, 'enter-full-screen'); + fireWindowEvent(win, 'enter-html-full-screen'); + + expect(win.webContents.send).toHaveBeenCalledTimes(2); + expect(win.webContents.send).toHaveBeenLastCalledWith( + WINDOW_STATE_CHANGED, + { isMaximized: false, isFullScreen: true } + ); + }); + + it('derives isMaximized from the maximize/unmaximize events instead of polling', () => { + const win = createMockStateWindow(); + attachWindowStateEvents(win); + + win.isMaximized.mockReturnValue(false); + fireWindowEvent(win, 'maximize'); + expect(win.webContents.send).toHaveBeenLastCalledWith( + WINDOW_STATE_CHANGED, + { isMaximized: true, isFullScreen: false } + ); + + win.isMaximized.mockReturnValue(true); + fireWindowEvent(win, 'unmaximize'); + expect(win.webContents.send).toHaveBeenLastCalledWith( + WINDOW_STATE_CHANGED, + { isMaximized: false, isFullScreen: false } + ); + }); + + it('keeps the maximized flag across a fullscreen round-trip while the window misreports it', () => { + const win = createMockStateWindow(); + win.isMaximized.mockReturnValue(true); + attachWindowStateEvents(win); + // Windows reports a fullscreen window as not maximized, so polling + // the companion flag here would clear it and leave the glyph on + // "maximize" after the controls come back. + win.isMaximized.mockReturnValue(false); + + fireWindowEvent(win, 'enter-html-full-screen'); + expect(win.webContents.send).toHaveBeenLastCalledWith( + WINDOW_STATE_CHANGED, + { isMaximized: true, isFullScreen: true } + ); + + fireWindowEvent(win, 'leave-html-full-screen'); + expect(win.webContents.send).toHaveBeenLastCalledWith( + WINDOW_STATE_CHANGED, + { isMaximized: true, isFullScreen: false } + ); + }); + + it('keeps the fullscreen flag when a maximize event arrives mid-transition', () => { + const win = createMockStateWindow(); + win.isFullScreen.mockReturnValue(true); + attachWindowStateEvents(win); + win.isFullScreen.mockReturnValue(false); + + fireWindowEvent(win, 'maximize'); + + expect(win.webContents.send).toHaveBeenLastCalledWith( + WINDOW_STATE_CHANGED, + { isMaximized: true, isFullScreen: true } + ); + }); + + it('sends an independent payload per push', () => { + const win = createMockStateWindow(); + attachWindowStateEvents(win); + + fireWindowEvent(win, 'enter-full-screen'); + const firstPayload = win.webContents.send.mock.calls[0][1]; + fireWindowEvent(win, 'leave-full-screen'); + + // The renderer must not see the earlier push mutate under it. + expect(firstPayload).toEqual({ + isMaximized: false, + isFullScreen: true, + }); + }); + + it('skips pushes for destroyed windows', () => { + const win = createMockStateWindow(); + attachWindowStateEvents(win); + win.isDestroyed.mockReturnValue(true); + + fireWindowEvent(win, 'leave-full-screen'); + + expect(win.webContents.send).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/electron-backend/src/app/app.spec.ts b/apps/electron-backend/src/app/app.spec.ts index 639d86d48..bfd5a24a2 100644 --- a/apps/electron-backend/src/app/app.spec.ts +++ b/apps/electron-backend/src/app/app.spec.ts @@ -55,6 +55,10 @@ import { store } from './services/store.service'; type MockMainWindow = { center: jest.Mock; getNormalBounds: jest.Mock; + // Read once by attachWindowStateEvents to seed the tracked window + // state; only reached off macOS, where the custom controls exist. + isFullScreen: jest.Mock; + isMaximized: jest.Mock; loadFile: jest.Mock, [string]>; loadURL: jest.Mock, [string]>; on: jest.Mock void]>; @@ -72,6 +76,8 @@ function createMockMainWindow(): MockMainWindow { return { center: jest.fn(), getNormalBounds: jest.fn().mockReturnValue({}), + isFullScreen: jest.fn().mockReturnValue(false), + isMaximized: jest.fn().mockReturnValue(false), loadFile: jest.fn, [string]>().mockResolvedValue(), loadURL: jest.fn, [string]>().mockResolvedValue(), on: jest.fn void]>(), diff --git a/apps/electron-backend/src/app/app.ts b/apps/electron-backend/src/app/app.ts index 00d7d940b..7aef4a9ae 100644 --- a/apps/electron-backend/src/app/app.ts +++ b/apps/electron-backend/src/app/app.ts @@ -1,5 +1,8 @@ import { app, BrowserWindow, Menu, screen, session, shell } from 'electron'; -import { WINDOW_STATE_CHANGED } from '@iptvnator/shared/interfaces'; +import { + ElectronBridgeWindowState, + WINDOW_STATE_CHANGED, +} from '@iptvnator/shared/interfaces'; import { join, resolve } from 'path'; import { fileURLToPath } from 'url'; import { rendererAppName, rendererAppPort } from './constants'; @@ -318,21 +321,44 @@ export default class App { return; } - const sendWindowState = () => { + // Window state is never re-read at event time: on Windows both + // isFullScreen() and isMaximized() can still report the + // pre-transition value while the matching event fires (notably for + // HTML-element fullscreen, i.e. the video player). Since the + // renderer replaces both flags on every push and no later event + // corrects a stale one, polling left the controls hidden forever + // after leaving fullscreen — and, for the companion flag, the + // maximize/restore glyph stuck on the wrong icon. + // + // Instead the state is seeded once here (window creation, so no + // transition is in flight) and each event patches only the flag it + // names. + const state: ElectronBridgeWindowState = { + isMaximized: win.isMaximized(), + isFullScreen: win.isFullScreen(), + }; + + const push = (patch: Partial) => { + Object.assign(state, patch); + if (win.isDestroyed()) { return; } - win.webContents.send(WINDOW_STATE_CHANGED, { - isMaximized: win.isMaximized(), - isFullScreen: win.isFullScreen(), - }); + // A copy per push: the renderer must not observe later + // mutations of the tracked state. + win.webContents.send(WINDOW_STATE_CHANGED, { ...state }); }; - win.on('maximize', sendWindowState); - win.on('unmaximize', sendWindowState); - win.on('enter-full-screen', sendWindowState); - win.on('leave-full-screen', sendWindowState); + win.on('maximize', () => push({ isMaximized: true })); + win.on('unmaximize', () => push({ isMaximized: false })); + // The html variants cover HTML-element fullscreen; not every + // platform/trigger emits both pairs, and duplicate pushes with the + // same payload are harmless. + win.on('enter-full-screen', () => push({ isFullScreen: true })); + win.on('enter-html-full-screen', () => push({ isFullScreen: true })); + win.on('leave-full-screen', () => push({ isFullScreen: false })); + win.on('leave-html-full-screen', () => push({ isFullScreen: false })); } private static initMainWindow() { diff --git a/docs/architecture/workspace-shell.md b/docs/architecture/workspace-shell.md index 5a0b6c9c0..c0f4ffbb3 100644 --- a/docs/architecture/workspace-shell.md +++ b/docs/architecture/workspace-shell.md @@ -255,10 +255,25 @@ handlers in `apps/electron-backend/src/app/events/window.events.ts`): WebContents. Close goes through `win.close()` so the existing window-bounds persistence in `app.ts` still runs. 2. `WINDOW:STATE_CHANGED` is pushed main → renderer on - maximize/unmaximize/enter-full-screen/leave-full-screen so the - maximize/restore glyph stays correct for externally triggered changes - (double-click on a drag region, OS snap, F11). The controls hide - themselves while the window is fullscreen. + maximize/unmaximize and on the fullscreen events — + `enter/leave-full-screen` plus the `enter/leave-html-full-screen` + variants emitted for HTML-element fullscreen (video player + fullscreen) — so the maximize/restore glyph stays correct for + externally triggered changes (double-click on a drag region, OS + snap, F11). The controls hide themselves while the window is + fullscreen. + + Window state is **never re-read at event time**. `attachWindowStateEvents` + seeds `{ isMaximized, isFullScreen }` once at window creation and each + event patches only the flag it names; every push carries a copy of that + tracked state. On Windows both getters can still report the + pre-transition value while the matching event fires — `isFullScreen()` + stays `true` during an HTML fullscreen exit, and `isMaximized()` reads + `false` while the window is fullscreen. Because the renderer replaces + both flags on every push and no later event corrects a stale one, + polling left the controls hidden forever after leaving fullscreen and + stuck the maximize/restore glyph on the wrong icon. Regression coverage: + `app-window-state.spec.ts` and `window-controls.e2e.ts`. Layout integration: