From 94a3de309b266506368e6ed5143d8c4ac9e341a4 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Fri, 9 Oct 2026 08:14:25 -0400 Subject: [PATCH] Re-request the wake lock the browser lets go or a pending request misses Keep screen on now listens for each lock's release event: when the browser lets the lock go while the switch is on, the lock is asked for again at once if the page is visible (or on the next visibilitychange if hidden), so the switch never claims a lock that is not held. When a pending request is granted, the hook checks visibility and the lock again, so a hide-and-show during the request is no longer lost. Fixes review findings F-01 and F-02 on PR #151. Maestro task c7da10dc-7f27-4dd0-8548-28d6587bdabf. Co-Authored-By: Claude Opus 5.5 --- web/src/dinners/KitchenReading.test.tsx | 60 ++++++++++++++++++++++--- web/src/dinners/keepScreenOn.ts | 21 ++++++++- 2 files changed, 74 insertions(+), 7 deletions(-) diff --git a/web/src/dinners/KitchenReading.test.tsx b/web/src/dinners/KitchenReading.test.tsx index 314034e..d708fce 100644 --- a/web/src/dinners/KitchenReading.test.tsx +++ b/web/src/dinners/KitchenReading.test.tsx @@ -117,7 +117,14 @@ function currentStep() { } // FakeSentinel is a WakeLockSentinel as far as the screen uses one. -type FakeSentinel = { released: boolean; release: Mock<() => Promise> } +type FakeSentinel = EventTarget & { released: boolean; release: Mock<() => Promise> } + +// letGo releases a lock as the browser does, telling its listeners so. +function letGo(sentinel: FakeSentinel) { + if (sentinel.released) return + sentinel.released = true + sentinel.dispatchEvent(new Event('release')) +} // stubWakeLock gives jsdom, which has none, a Screen Wake Lock. Each request // is granted at once, or, given hold, only when the test calls grant. @@ -126,13 +133,13 @@ function stubWakeLock({ hold = false } = {}) { const pending: (() => void)[] = [] const request = vi.fn((type: string) => { expect(type).toBe('screen') - const sentinel: FakeSentinel = { + const sentinel: FakeSentinel = Object.assign(new EventTarget(), { released: false, release: vi.fn(() => { - sentinel.released = true + letGo(sentinel) return Promise.resolve() }), - } + }) sentinels.push(sentinel) if (!hold) return Promise.resolve(sentinel) return new Promise((resolve) => pending.push(() => resolve(sentinel))) @@ -471,8 +478,8 @@ describe('Kitchen reading mode', () => { await waitFor(() => expect(wakeLock.request).toHaveBeenCalledTimes(1)) // Hidden, the browser lets the lock go; nothing is asked while hidden. - wakeLock.sentinels[0]!.released = true act(() => setVisibility('hidden')) + act(() => letGo(wakeLock.sentinels[0]!)) expect(wakeLock.request).toHaveBeenCalledTimes(1) act(() => setVisibility('visible')) @@ -489,6 +496,49 @@ describe('Kitchen reading mode', () => { expect(wakeLock.request).toHaveBeenCalledTimes(2) }) + it('asks for the lock again when the browser lets it go while the page is visible', async () => { + const wakeLock = stubWakeLock() + serve() + await renderPage() + const keepOn = screen.getByRole('switch', { name: 'Keep screen on' }) + fireEvent.click(keepOn) + await waitFor(() => expect(wakeLock.request).toHaveBeenCalledTimes(1)) + + act(() => letGo(wakeLock.sentinels[0]!)) + await waitFor(() => expect(wakeLock.request).toHaveBeenCalledTimes(2)) + await act(async () => {}) + expect(keepOn).toHaveAttribute('aria-checked', 'true') + + // The fresh lock is watched too. + act(() => letGo(wakeLock.sentinels[1]!)) + await waitFor(() => expect(wakeLock.request).toHaveBeenCalledTimes(3)) + expect(wakeLock.sentinels[2]!.released).toBe(false) + expect(keepOn).toHaveAttribute('aria-checked', 'true') + }) + + it('asks for the lock again when the page was hidden and shown while a request was pending', async () => { + const wakeLock = stubWakeLock({ hold: true }) + serve() + await renderPage() + const keepOn = screen.getByRole('switch', { name: 'Keep screen on' }) + fireEvent.click(keepOn) + expect(wakeLock.request).toHaveBeenCalledTimes(1) + + // Hidden and shown again before the first lock is granted, which the + // browser has let go by the time it is. + act(() => setVisibility('hidden')) + act(() => setVisibility('visible')) + expect(wakeLock.request).toHaveBeenCalledTimes(1) + letGo(wakeLock.sentinels[0]!) + await act(async () => wakeLock.grant()) + expect(wakeLock.request).toHaveBeenCalledTimes(2) + + await act(async () => wakeLock.grant()) + expect(wakeLock.sentinels[1]!.released).toBe(false) + expect(wakeLock.request).toHaveBeenCalledTimes(2) + expect(keepOn).toHaveAttribute('aria-checked', 'true') + }) + it('reports a lock the browser refuses and turns the switch back off', async () => { const wakeLock = stubWakeLock() wakeLock.request.mockRejectedValueOnce(new DOMException('Blocked by permissions policy', 'NotAllowedError')) diff --git a/web/src/dinners/keepScreenOn.ts b/web/src/dinners/keepScreenOn.ts index a6907fe..63ed9a3 100644 --- a/web/src/dinners/keepScreenOn.ts +++ b/web/src/dinners/keepScreenOn.ts @@ -32,6 +32,15 @@ export function useKeepScreenOn(): KeepScreenOn { let requesting = false let left = false + // The browser may let a lock go while the switch is on: always when the + // page is hidden, and, the specification allows, while it is still + // visible. Either way the lock is asked for again at once if the page is + // visible, or when it is visible again if not, so the switch never claims + // a lock that is not held. + function onRelease() { + if (!left) acquire() + } + function acquire() { if (document.visibilityState !== 'visible' || requesting) return if (sentinel !== null && !sentinel.released) return @@ -39,8 +48,16 @@ export function useKeepScreenOn(): KeepScreenOn { wakeLock.request('screen').then( (granted) => { requesting = false - if (left) void granted.release() - else sentinel = granted + if (left) { + void granted.release() + return + } + sentinel = granted + granted.addEventListener('release', onRelease) + // While the request was pending the page may have been hidden and + // shown again (that visibilitychange was ignored) and the lock let + // go before it could be listened for, so check again now. + acquire() }, (err: unknown) => { requesting = false