diff --git a/apps/readest-app/src/__tests__/hooks/useOpenAnnotationLink.test.ts b/apps/readest-app/src/__tests__/hooks/useOpenAnnotationLink.test.ts new file mode 100644 index 00000000..8c7605a9 --- /dev/null +++ b/apps/readest-app/src/__tests__/hooks/useOpenAnnotationLink.test.ts @@ -0,0 +1,137 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { cleanup, renderHook } from '@testing-library/react'; + +const navigateToReaderMock = vi.fn(); +const getCurrentMock = vi.fn(async () => [] as string[]); + +const libraryState = { + libraryLoaded: true, + getBookByHash: (hash: string) => (hash.startsWith('book') ? { hash } : undefined), +}; + +type ViewLike = { goTo: (cfi: string) => void }; +type ReaderState = { + bookKeys: string[]; + viewStates: Record; + setPreviewMode: ReturnType; +}; +// A reader is mounted; bookKeys lists only the currently-displayed book(s). +// viewStates may carry stale entries for books switched away from. +const readerState: ReaderState = { + bookKeys: ['bookA-1'], + viewStates: {}, + setPreviewMode: vi.fn(), +}; + +vi.mock('@tauri-apps/plugin-deep-link', () => ({ + getCurrent: () => getCurrentMock(), +})); +vi.mock('@/services/environment', async (orig) => { + const actual = await orig(); + return { ...actual, isTauriAppPlatform: () => true }; +}); +vi.mock('@/context/EnvContext', () => ({ useEnv: () => ({ appService: {} }) })); +vi.mock('@/hooks/useTranslation', () => ({ useTranslation: () => (k: string) => k })); +vi.mock('next/navigation', () => ({ useRouter: () => ({ push: vi.fn() }) })); +vi.mock('@/utils/nav', () => ({ + navigateToReader: (...a: unknown[]) => navigateToReaderMock(...a), +})); +vi.mock('@/store/libraryStore', () => { + const useLibraryStore = ((selector: (s: typeof libraryState) => unknown) => + selector(libraryState)) as unknown as { + (selector: (s: typeof libraryState) => unknown): unknown; + getState: () => typeof libraryState; + }; + useLibraryStore.getState = () => libraryState; + return { useLibraryStore }; +}); +vi.mock('@/store/readerStore', () => { + const useReaderStore = (() => readerState) as unknown as { + (): ReaderState; + getState: () => ReaderState; + }; + useReaderStore.getState = () => readerState; + return { useReaderStore }; +}); + +import { useOpenAnnotationLink } from '@/hooks/useOpenAnnotationLink'; +import { eventDispatcher } from '@/utils/event'; + +const CFI = 'epubcfi(/6/4!/4/2)'; +const urlFor = (hash: string) => + `readest://book/${hash}/annotation/note1?cfi=${encodeURIComponent(CFI)}`; + +const collectSwitch = () => { + const switched = vi.fn(); + const handler = (e: Event) => switched((e as CustomEvent).detail); + eventDispatcher.on('open-book-in-reader', handler); + return { + switched, + stop: () => eventDispatcher.off('open-book-in-reader', handler), + }; +}; + +describe('useOpenAnnotationLink — reader already mounted', () => { + beforeEach(() => { + navigateToReaderMock.mockReset(); + readerState.setPreviewMode.mockReset(); + readerState.bookKeys = ['bookA-1']; + readerState.viewStates = { 'bookA-1': { view: { goTo: vi.fn() }, inited: true } }; + window.history.replaceState({}, '', '/reader?ids=bookA'); + }); + afterEach(() => { + cleanup(); + window.history.replaceState({}, '', '/'); + }); + + it('switches the book in place instead of calling the no-op navigateToReader', async () => { + const { switched, stop } = collectSwitch(); + renderHook(() => useOpenAnnotationLink()); + await eventDispatcher.dispatch('app-incoming-url', { urls: [urlFor('bookB')] }); + await Promise.resolve(); + stop(); + + // navigateToReader('/reader?ids=bookB&cfi=...') does not re-init an + // already-mounted reader, so the reader would stay on bookA. The fix routes + // through the in-place switch event carrying the target cfi. + expect(navigateToReaderMock).not.toHaveBeenCalled(); + expect(switched).toHaveBeenCalledWith(expect.objectContaining({ bookHash: 'bookB', cfi: CFI })); + }); + + it('jumps in place when the target book is the currently displayed one', async () => { + const goTo = vi.fn(); + readerState.bookKeys = ['bookB-1']; + readerState.viewStates = { 'bookB-1': { view: { goTo }, inited: true } }; + + renderHook(() => useOpenAnnotationLink()); + await eventDispatcher.dispatch('app-incoming-url', { urls: [urlFor('bookB')] }); + await Promise.resolve(); + + expect(goTo).toHaveBeenCalledWith(CFI); + expect(readerState.setPreviewMode).toHaveBeenCalledWith('bookB-1', true); + expect(navigateToReaderMock).not.toHaveBeenCalled(); + }); + + it('switches back to a book that was opened before but is no longer displayed (#4887)', async () => { + // Simulates opening A -> B -> A all by deep link. After A -> B, bookA-1 is + // still in viewStates with a now-detached view (never cleared on switch), + // while bookKeys reflects only bookB-2. A deep link back to book A must + // switch in place, NOT goTo the stale detached bookA view. + const staleGoTo = vi.fn(); + readerState.bookKeys = ['bookB-2']; + readerState.viewStates = { + 'bookA-1': { view: { goTo: staleGoTo }, inited: true }, + 'bookB-2': { view: { goTo: vi.fn() }, inited: true }, + }; + + const { switched, stop } = collectSwitch(); + renderHook(() => useOpenAnnotationLink()); + await eventDispatcher.dispatch('app-incoming-url', { urls: [urlFor('bookA')] }); + await Promise.resolve(); + stop(); + + expect(staleGoTo).not.toHaveBeenCalled(); + expect(navigateToReaderMock).not.toHaveBeenCalled(); + expect(switched).toHaveBeenCalledWith(expect.objectContaining({ bookHash: 'bookA', cfi: CFI })); + }); +}); diff --git a/apps/readest-app/src/app/reader/hooks/useBooksManager.ts b/apps/readest-app/src/app/reader/hooks/useBooksManager.ts index 7211831b..e98ae054 100644 --- a/apps/readest-app/src/app/reader/hooks/useBooksManager.ts +++ b/apps/readest-app/src/app/reader/hooks/useBooksManager.ts @@ -42,16 +42,45 @@ const useBooksManager = () => { setShouldUpdateSearchParams(true); }; - // Open a book in-place when the widget taps a book while a reader is already - // mounted. REPLACE the open book(s) with the tapped one (single ids=) - // rather than appending: appending produced ids=a+b which, with the OS - // re-delivering the launch deep link, looped. The store update renders the - // new book immediately; closing the previous key follows the same path as - // dismissBook. - const openBookInReader = (bookHash: string) => { + // Jump the switched-in book to a deep-link cfi (#4887) once its view has + // finished initing. The freshly-opened FoliateView first lands on the saved + // reading position, so wait for `inited` and then goTo; mark it a preview so + // the saved position is not overwritten. The subscription cleans itself up on + // success or on load failure. + const goToCfiWhenReady = (bookKey: string, cfi: string) => { + const jump = () => { + const { getView, setPreviewMode } = useReaderStore.getState(); + getView(bookKey)?.goTo(cfi); + setPreviewMode(bookKey, true); + }; + const ready = (state: ReturnType) => { + const vs = state.viewStates[bookKey]; + return { done: !!vs?.error || (!!vs?.inited && !!vs?.view), ok: !!vs?.inited && !!vs?.view }; + }; + const initial = ready(useReaderStore.getState()); + if (initial.done) { + if (initial.ok) jump(); + return; + } + const unsub = useReaderStore.subscribe((state) => { + const { done, ok } = ready(state); + if (!done) return; + unsub(); + if (ok) jump(); + }); + }; + + // Open a book in-place when a widget/deep link targets a book while a reader + // is already mounted. REPLACE the open book(s) with the target one (single + // ids=) rather than appending: appending produced ids=a+b which, with + // the OS re-delivering the launch deep link, looped. The store update renders + // the new book immediately; closing the previous key follows the same path as + // dismissBook. An optional cfi (annotation deep link) is applied once ready. + const openBookInReader = (bookHash: string, cfi?: string) => { const existing = bookKeys.find((key) => key.startsWith(bookHash)); if (existing) { setSideBarBookKey(existing); + if (cfi) goToCfiWhenReady(existing, cfi); return; } const newKey = `${bookHash}-${uniqueId()}`; @@ -59,6 +88,7 @@ const useBooksManager = () => { setBookKeys([newKey]); setSideBarBookKey(newKey); setShouldUpdateSearchParams(true); + if (cfi) goToCfiWhenReady(newKey, cfi); }; // Stable ref so the listener calls the latest closure without re-subscribing. @@ -66,8 +96,8 @@ const useBooksManager = () => { openBookRef.current = openBookInReader; useEffect(() => { const handle = (event: CustomEvent) => { - const { bookHash } = event.detail as { bookHash: string }; - openBookRef.current(bookHash); + const { bookHash, cfi } = event.detail as { bookHash: string; cfi?: string }; + openBookRef.current(bookHash, cfi); }; eventDispatcher.on('open-book-in-reader', handle); return () => eventDispatcher.off('open-book-in-reader', handle); diff --git a/apps/readest-app/src/hooks/useOpenAnnotationLink.ts b/apps/readest-app/src/hooks/useOpenAnnotationLink.ts index 0fe511df..059614d4 100644 --- a/apps/readest-app/src/hooks/useOpenAnnotationLink.ts +++ b/apps/readest-app/src/hooks/useOpenAnnotationLink.ts @@ -62,19 +62,34 @@ export function useOpenAnnotationLink() { return; } - const { viewStates, setPreviewMode } = useReaderStore.getState(); - const openEntry = Object.entries(viewStates).find( - ([key, state]) => key.startsWith(bookHash) && state.view, - ); - if (openEntry) { - const [bookKey, state] = openEntry; + // Only books in the current bookKeys are actually displayed. viewStates + // accumulates stale entries for books switched away from in place (their + // views are detached from the DOM, never cleared on switch), so matching + // against all viewStates would goTo a dead view and the reader would stay + // stuck on the current book when switching back to a previously-open one + // (#4887). Scope the "already open" check to the displayed bookKeys. + const { viewStates, bookKeys, setPreviewMode } = useReaderStore.getState(); + const openKey = bookKeys.find((key) => key.startsWith(bookHash) && viewStates[key]?.view); + if (openKey) { if (cfi) { - state.view!.goTo(cfi); - setPreviewMode(bookKey, true); + viewStates[openKey]!.view!.goTo(cfi); + setPreviewMode(openKey, true); } return; } + // A reader is already mounted showing a different book. router.push to the + // same /reader route does NOT re-run the reader's one-shot init effect, so + // navigateToReader is a no-op and the reader stays on the current book + // (#4887). Switch the book in place via useBooksManager, carrying the cfi + // so the freshly-opened view jumps to the annotation once it is ready. + if (window.location.pathname.startsWith('/reader')) { + eventDispatcher.dispatch('open-book-in-reader', { bookHash, cfi }); + return; + } + + // No reader mounted (library / cold start) - navigate fresh; FoliateViewer + // reads ?cfi= at init. const queryParams = cfi ? `cfi=${encodeURIComponent(cfi)}` : undefined; navigateToReader(router, [bookHash], queryParams); },