From 6fcda66b603531b128554eb41c168d1e1feb360c Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Tue, 28 Apr 2026 00:32:04 +0800 Subject: [PATCH] fix(reader): close stuck reader window on book load failure, closes #3932 (#3980) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a book's underlying file is missing, opening it in a dedicated reader window showed an error toast then navigated that window to /library, leaving a leftover library-in-reader-window the user had to close manually. Route the recovery through a new closeReaderWindowOrGoToLibrary() that closes the dedicated reader window (after ensuring the main library window is visible) and only falls back to /library navigation in the main window or on web. Also fix a related macOS issue: the reader's CloseRequested handler was running handleCloseBooks and calling currentWindow.destroy() on the main window, which tore down the active book and bypassed the Rust close-to-hide handler — making Cmd+W / traffic-light close quit the app from the reader page (vs. correctly hiding from the library page) and lose the active book even when the window did hide. Skip both the cleanup and destroy on macOS for the main window so the Rust handler hides it with the book intact, matching the macOS minimize-to-dock convention. Co-authored-by: Claude Opus 4.7 (1M context) --- .../src/__tests__/utils/nav.test.ts | 87 ++++++++++++- .../src/__tests__/utils/window.test.ts | 115 ++++++++++++++++++ .../app/reader/components/ReaderContent.tsx | 11 +- apps/readest-app/src/utils/nav.ts | 23 +++- apps/readest-app/src/utils/window.ts | 7 ++ 5 files changed, 238 insertions(+), 5 deletions(-) create mode 100644 apps/readest-app/src/__tests__/utils/window.test.ts diff --git a/apps/readest-app/src/__tests__/utils/nav.test.ts b/apps/readest-app/src/__tests__/utils/nav.test.ts index 06c5748d..06d36204 100644 --- a/apps/readest-app/src/__tests__/utils/nav.test.ts +++ b/apps/readest-app/src/__tests__/utils/nav.test.ts @@ -7,7 +7,7 @@ vi.mock('next/navigation', () => ({ })); vi.mock('@tauri-apps/api/window', () => ({ - getCurrentWindow: vi.fn().mockReturnValue({ label: 'main' }), + getCurrentWindow: vi.fn().mockReturnValue({ label: 'main', close: vi.fn() }), })); vi.mock('@tauri-apps/api/webviewWindow', () => { @@ -22,6 +22,7 @@ vi.mock('@tauri-apps/api/webviewWindow', () => { vi.mock('@/services/environment', () => ({ isPWA: vi.fn().mockReturnValue(false), isWebAppPlatform: vi.fn().mockReturnValue(false), + isTauriAppPlatform: vi.fn().mockReturnValue(false), })); vi.mock('@/services/constants', () => ({ @@ -29,8 +30,9 @@ vi.mock('@/services/constants', () => ({ })); import { redirect } from 'next/navigation'; +import { getCurrentWindow } from '@tauri-apps/api/window'; import { WebviewWindow } from '@tauri-apps/api/webviewWindow'; -import { isPWA, isWebAppPlatform } from '@/services/environment'; +import { isPWA, isTauriAppPlatform, isWebAppPlatform } from '@/services/environment'; import { navigateToReader, navigateToLogin, @@ -42,6 +44,7 @@ import { showReaderWindow, showLibraryWindow, ensureMainLibraryWindow, + closeReaderWindowOrGoToLibrary, } from '@/utils/nav'; const WebviewWindowCtor = WebviewWindow as unknown as { getByLabel: ReturnType }; @@ -68,6 +71,13 @@ beforeEach(() => { // Reset default environment mock returns vi.mocked(isWebAppPlatform).mockReturnValue(false); vi.mocked(isPWA).mockReturnValue(false); + vi.mocked(isTauriAppPlatform).mockReturnValue(false); + + // Reset getCurrentWindow default + vi.mocked(getCurrentWindow).mockReturnValue({ + label: 'main', + close: vi.fn(), + } as unknown as ReturnType); // Reset window.location Object.defineProperty(window, 'location', { @@ -381,3 +391,76 @@ describe('ensureMainLibraryWindow', () => { expect((options as { url: string }).url).toBe('/library'); }); }); + +describe('closeReaderWindowOrGoToLibrary', () => { + function makeAppServiceWithWindow(hasWindow = true) { + return { isMacOSApp: false, hasWindow } as Record; + } + + test('on web platform, navigates current view to /library', async () => { + vi.mocked(isTauriAppPlatform).mockReturnValue(false); + + const router = mockRouter(); + await closeReaderWindowOrGoToLibrary(makeAppServiceWithWindow() as never, router); + + expect(router.replace).toHaveBeenCalledWith('/library', undefined); + expect(WebviewWindowCtor.getByLabel).not.toHaveBeenCalled(); + }); + + test('in Tauri main window, navigates the same window to /library', async () => { + vi.mocked(isTauriAppPlatform).mockReturnValue(true); + const close = vi.fn(); + vi.mocked(getCurrentWindow).mockReturnValue({ + label: 'main', + close, + } as unknown as ReturnType); + + const router = mockRouter(); + await closeReaderWindowOrGoToLibrary(makeAppServiceWithWindow() as never, router); + + expect(close).not.toHaveBeenCalled(); + expect(router.replace).toHaveBeenCalledWith('/library', undefined); + }); + + test('in dedicated reader window, ensures main library window and closes self', async () => { + vi.mocked(isTauriAppPlatform).mockReturnValue(true); + const close = vi.fn().mockResolvedValue(undefined); + vi.mocked(getCurrentWindow).mockReturnValue({ + label: 'reader-0', + close, + } as unknown as ReturnType); + const main = { + show: vi.fn().mockResolvedValue(undefined), + unminimize: vi.fn().mockResolvedValue(undefined), + setFocus: vi.fn().mockResolvedValue(undefined), + }; + WebviewWindowCtor.getByLabel.mockResolvedValue(main); + + const router = mockRouter(); + await closeReaderWindowOrGoToLibrary(makeAppServiceWithWindow() as never, router); + + expect(WebviewWindowCtor.getByLabel).toHaveBeenCalledWith('main'); + expect(main.show).toHaveBeenCalled(); + expect(close).toHaveBeenCalled(); + expect(router.replace).not.toHaveBeenCalled(); + }); + + test('uses lastLibraryParams from sessionStorage when navigating', async () => { + vi.mocked(isTauriAppPlatform).mockReturnValue(false); + sessionStorage.setItem('lastLibraryParams', 'sort=author'); + + const router = mockRouter(); + await closeReaderWindowOrGoToLibrary(makeAppServiceWithWindow() as never, router); + + expect(router.replace).toHaveBeenCalledWith('/library?sort=author', undefined); + }); + + test('falls back to navigation when appService is null', async () => { + vi.mocked(isTauriAppPlatform).mockReturnValue(true); + + const router = mockRouter(); + await closeReaderWindowOrGoToLibrary(null, router); + + expect(router.replace).toHaveBeenCalledWith('/library', undefined); + }); +}); diff --git a/apps/readest-app/src/__tests__/utils/window.test.ts b/apps/readest-app/src/__tests__/utils/window.test.ts new file mode 100644 index 00000000..bf148582 --- /dev/null +++ b/apps/readest-app/src/__tests__/utils/window.test.ts @@ -0,0 +1,115 @@ +import { describe, test, expect, beforeEach, vi } from 'vitest'; + +vi.mock('@tauri-apps/api/window', () => ({ + getCurrentWindow: vi.fn(), + getAllWindows: vi.fn(), +})); + +vi.mock('@tauri-apps/api/event', () => ({ + emitTo: vi.fn().mockResolvedValue(undefined), + TauriEvent: { WINDOW_FOCUS: 'tauri://focus' }, +})); + +vi.mock('@tauri-apps/plugin-process', () => ({ + exit: vi.fn(), +})); + +vi.mock('@tauri-apps/plugin-os', () => ({ + type: vi.fn(), +})); + +vi.mock('@/utils/event', () => ({ + eventDispatcher: { dispatch: vi.fn() }, +})); + +import { getCurrentWindow } from '@tauri-apps/api/window'; +import { type as osType } from '@tauri-apps/plugin-os'; +import { tauriHandleOnCloseWindow } from '@/utils/window'; + +type CloseHandler = (event: { preventDefault: () => void }) => Promise | void; + +function makeWindow(label: string) { + let registered: CloseHandler | undefined; + const win = { + label, + destroy: vi.fn().mockResolvedValue(undefined), + hide: vi.fn().mockResolvedValue(undefined), + onCloseRequested: vi.fn().mockImplementation((handler: CloseHandler) => { + registered = handler; + return Promise.resolve(() => {}); + }), + }; + const trigger = async () => { + if (!registered) throw new Error('no handler registered'); + await registered({ preventDefault: vi.fn() }); + }; + return { win, trigger }; +} + +beforeEach(() => { + vi.clearAllMocks(); + vi.useFakeTimers(); +}); + +describe('tauriHandleOnCloseWindow', () => { + test('on macOS, leaves the main window alone — no book cleanup, no destroy', async () => { + // Rust hide-on-close handler hides the window; the user expects the active + // book to still be loaded when they bring the window back. + vi.mocked(osType).mockReturnValue('macos'); + const { win, trigger } = makeWindow('main'); + vi.mocked(getCurrentWindow).mockReturnValue( + win as unknown as ReturnType, + ); + + const callback = vi.fn(); + await tauriHandleOnCloseWindow(callback); + await trigger(); + + expect(callback).not.toHaveBeenCalled(); + expect(win.destroy).not.toHaveBeenCalled(); + }); + + test('on Windows, destroys the main window', async () => { + vi.mocked(osType).mockReturnValue('windows'); + const { win, trigger } = makeWindow('main'); + vi.mocked(getCurrentWindow).mockReturnValue( + win as unknown as ReturnType, + ); + + const callback = vi.fn(); + await tauriHandleOnCloseWindow(callback); + await trigger(); + + expect(win.destroy).toHaveBeenCalled(); + }); + + test('on Linux, destroys the main window', async () => { + vi.mocked(osType).mockReturnValue('linux'); + const { win, trigger } = makeWindow('main'); + vi.mocked(getCurrentWindow).mockReturnValue( + win as unknown as ReturnType, + ); + + const callback = vi.fn(); + await tauriHandleOnCloseWindow(callback); + await trigger(); + + expect(win.destroy).toHaveBeenCalled(); + }); + + test('on macOS, dedicated reader windows still destroy after 300ms', async () => { + vi.mocked(osType).mockReturnValue('macos'); + const { win, trigger } = makeWindow('reader-0'); + vi.mocked(getCurrentWindow).mockReturnValue( + win as unknown as ReturnType, + ); + + const callback = vi.fn(); + await tauriHandleOnCloseWindow(callback); + await trigger(); + + expect(win.destroy).not.toHaveBeenCalled(); + vi.advanceTimersByTime(300); + expect(win.destroy).toHaveBeenCalled(); + }); +}); diff --git a/apps/readest-app/src/app/reader/components/ReaderContent.tsx b/apps/readest-app/src/app/reader/components/ReaderContent.tsx index 7da7df6c..3f479573 100644 --- a/apps/readest-app/src/app/reader/components/ReaderContent.tsx +++ b/apps/readest-app/src/app/reader/components/ReaderContent.tsx @@ -20,7 +20,11 @@ import { isTauriAppPlatform } from '@/services/environment'; import { uniqueId } from '@/utils/misc'; import { throttle } from '@/utils/throttle'; import { eventDispatcher } from '@/utils/event'; -import { ensureMainLibraryWindow, navigateToLibrary } from '@/utils/nav'; +import { + closeReaderWindowOrGoToLibrary, + ensureMainLibraryWindow, + navigateToLibrary, +} from '@/utils/nav'; import { clearDiscordPresence } from '@/utils/discord'; import { BOOK_IDS_SEPARATOR } from '@/services/constants'; import { BookDetailModal } from '@/components/metadata'; @@ -74,7 +78,10 @@ const ReaderContent: React.FC<{ ids?: string; settings: SystemSettings }> = ({ i setErrorLoading(true); eventDispatcher.dispatch('toast', { message: _('Unable to open book'), - callback: () => navigateBackToLibrary(), + callback: async () => { + const service = await envConfig.getAppService(); + await closeReaderWindowOrGoToLibrary(service, router); + }, timeout: 2000, type: 'error', }); diff --git a/apps/readest-app/src/utils/nav.ts b/apps/readest-app/src/utils/nav.ts index b7ef24e4..10e8db16 100644 --- a/apps/readest-app/src/utils/nav.ts +++ b/apps/readest-app/src/utils/nav.ts @@ -1,7 +1,7 @@ import { redirect, useRouter } from 'next/navigation'; import { getCurrentWindow, ScrollBarStyle } from '@tauri-apps/api/window'; import { WebviewWindow } from '@tauri-apps/api/webviewWindow'; -import { isPWA, isWebAppPlatform } from '@/services/environment'; +import { isPWA, isTauriAppPlatform, isWebAppPlatform } from '@/services/environment'; import { BOOK_IDS_SEPARATOR } from '@/services/constants'; import { AppService } from '@/types/system'; @@ -128,6 +128,27 @@ export const navigateToLibrary = ( router.replace(`/library${queryParams ? `?${queryParams}` : ''}`, navOptions); }; +// Recovery action when a reader has nothing to display — e.g. all books were +// closed, or a book failed to load in a freshly-opened reader window. +// In a dedicated reader window we close the window itself, ensuring the main +// library window is visible first; routing the reader window to /library +// instead would leave a leftover window the user has to close manually. +// In the main window or on web, fall back to /library navigation. +export const closeReaderWindowOrGoToLibrary = async ( + appService: AppService | null, + router: ReturnType, +) => { + if (isTauriAppPlatform() && appService?.hasWindow) { + const currentWindow = getCurrentWindow(); + if (currentWindow.label !== 'main') { + await ensureMainLibraryWindow(appService); + await currentWindow.close(); + return; + } + } + navigateToLibrary(router, '', undefined, true); +}; + export const redirectToLibrary = () => { redirect('/library'); }; diff --git a/apps/readest-app/src/utils/window.ts b/apps/readest-app/src/utils/window.ts index 1db36714..206bf7d0 100644 --- a/apps/readest-app/src/utils/window.ts +++ b/apps/readest-app/src/utils/window.ts @@ -51,6 +51,13 @@ export const tauriHandleOnCloseWindow = async (callback: () => void) => { const currentWindow = getCurrentWindow(); return await currentWindow.onCloseRequested(async (event) => { event.preventDefault(); + // On macOS, the main window's close is intercepted by the Rust backend + // to hide the window (close-to-hide), keeping the app in the dock. Skip + // the in-app cleanup — the user is just minimizing the window and + // expects the active book to still be there when the window reopens. + if (currentWindow.label === 'main' && (await osType()) === 'macos') { + return; + } await callback(); if (currentWindow.label.startsWith('reader')) { await emitTo('main', 'close-reader-window', { label: currentWindow.label });