From 518c343dc9aadf3dc8cdcfa5d4e0e3ac1efa955b Mon Sep 17 00:00:00 2001 From: skjnldsv Date: Thu, 17 Sep 2026 22:12:36 +0200 Subject: [PATCH] fix(history): clear openfile before unwinding, not after Closing the viewer jumps back past the entries it pushed, and history.go() lands on a later task. Until it does, the URL still says openfile=true, and the Files list runs the default action again for anything that makes it re-read the route in that window: a second viewer opens over the one that is closing. Seen on a shared CI runner, where the window is wide: three media tests failed on their close assertion with two modal containers in the DOM, one per viewer. The flag now comes off the entry being left before the jump is asked for. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: skjnldsv --- __tests__/history.spec.ts | 28 ++++++++++++++++++++++++++++ lib/utils/history.ts | 19 ++++++++++++++----- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/__tests__/history.spec.ts b/__tests__/history.spec.ts index 55e41be..a4ccd8d 100644 --- a/__tests__/history.spec.ts +++ b/__tests__/history.spec.ts @@ -152,6 +152,34 @@ describe('openWithHistory', () => { expect(removeSpy).toHaveBeenCalledWith('popstate', expect.any(Function)) }) + it('drops the openfile flag before the jump, not after it', () => { + const router = setRouter() + const file = makeFile({ id: 1 }) + openWithHistory([file], file, view, folder) + router.query.openfile = 'true' + + const order: string[] = [] + vi.mocked(router.goToRoute).mockImplementation(() => { + order.push('route') + }) + goSpy.mockImplementation(() => { + order.push('go') + }) + + openOptions().onClose() + + // history.go() lands on a later task. Until it does the URL still says + // openfile=true, and the Files list opens the file again if anything + // makes it re-read the route in that window. + expect(order).toEqual(['route', 'go']) + expect(router.goToRoute).toHaveBeenCalledWith( + 'filelist', + router.params, + expect.not.objectContaining({ openfile: 'true' }), + true, + ) + }) + it('drops the openfile flag in place when closing a refresh-opened viewer', () => { const router = setRouter({ openfile: 'true', dir: '/photos' }) const file = makeFile({ id: 1 }) diff --git a/lib/utils/history.ts b/lib/utils/history.ts index f1c688a..7700ee2 100644 --- a/lib/utils/history.ts +++ b/lib/utils/history.ts @@ -163,19 +163,28 @@ function closeHistory(): void { return } + const query = { ...router.query } + delete query.openfile + delete query.editing + const offset = currentOffset() if (offset > 0) { + // Drop the flag on the entry being left before jumping. history.go() is + // asynchronous, and until it lands the URL still says openfile=true: + // anything that makes the Files list re-read the route in that window + // runs the default action again and opens a second viewer over the one + // that is closing. + router.goToRoute(routeName(router), router.params, query, true) + // Jump back past every entry the viewer added, in one step, so the back // button returns to the opening page instead of a previously shown file. window.history.go(-offset) return } - // Opened from an openfile URL with no pre-viewer entry to return to (refresh): - // just drop the openfile flag on the current entry. - const query = { ...router.query } - delete query.openfile - delete query.editing + // Opened from an openfile URL with no pre-viewer entry to return to + // (refresh): the flag comes off the current entry and there is nothing to + // unwind. router.goToRoute(routeName(router), router.params, query, true) }