fix(reader): make Shift+P toggle, exit, and resume paragraph mode reliably (#4717) (#4725)

Three paragraph-mode problems, all fixed:

- Shift+P inside paragraph mode flashed and re-entered (and pressing it
  repeatedly did nothing). A single keypress toggled twice: eventDispatcher
  .dispatch() iterated the live listener Set while awaiting each listener, and
  the exit's awaited dispatch('paragraph-mode-disabled') let React re-run
  useParagraphMode's subscription effect mid-loop, adding a handler the same
  dispatch then called. Snapshot the listeners before iterating (dispatchSync
  already did), so a listener added during a dispatch can't fire for the
  current event.

- Shift+P / Escape only worked when focus sat on the overlay. Handle the
  overlay's keys the way a dialog/alert does: focus the dialog element on open
  and handle Escape / the toggle shortcut / paragraph navigation in its own
  onKeyDown (stopping propagation so the global handler can't double-fire),
  instead of a global window listener. Suppress the focus ring on the
  programmatically-focused, non-tab-stop container.

- Resume jumped to the chapter start, and repeated enter/exit walked further
  back. Two causes: (a) entering/exiting scrolled the underlying view to the
  focused paragraph's start, which rewinds a page when that paragraph began on
  the previous page — don't scroll on resume/exit (the paragraph is already on
  screen); navigation still scrolls. (b) resume preferred the rAF-debounced
  store progress and a stored last-paragraph CFI that can come out malformed
  and resolve to an empty range, shadowing the correct candidate and sending
  findByRange to the first block. Resume from the view's live, foliate-
  generated lastLocation CFI first (set synchronously on every relocate,
  resolved against the current document so it survives iframe recreation).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

apps/readest-app/src/__tests__/paragraph-mode.test.tsx

@@ -208,6 +208,89 @@ describe('paragraph mode', () => {
     });
   });
 
+  it('resumes without scrolling the underlying view so repeated enter/exit cannot rewind (#4717)', async () => {
+    const doc = createDoc('<p>Para A</p><p>Para B</p><p>Para C</p>');
+    const { view, renderer } = createMockView([doc], 0);
+    const viewRef = { current: view } as React.RefObject<FoliateView | null>;
+
+    render(<HookHarness view={viewRef} />);
+    await waitFor(() => {
+      expect(hookApi?.paragraphState.currentRange).toBeTruthy();
+    });
+
+    // Resuming/entering focuses the paragraph already at the reading position.
+    // Scrolling the underlying view to that paragraph's start rewinds whenever it
+    // began on an earlier page, so the view must NOT be moved on resume (#4717).
+    expect(renderer.goTo).not.toHaveBeenCalled();
+    expect(renderer.scrollToAnchor).not.toHaveBeenCalled();
+  });
+
+  it('resumes at the view live CFI even when the store progress is stale (#4717)', async () => {
+    const doc = createDoc('<p>Block zero</p><p>Block one</p><p>Block two</p>');
+    const { view } = createMockView([doc], 0);
+    // The rAF-debounced store (mockGetProgress) returns null/stale; the view's
+    // live lastLocation CFI points at the third paragraph. Resume must follow the
+    // live CFI (resolved against the current doc), not fall back to chapter start.
+    const thirdParagraph = doc.querySelectorAll('p')[2]!;
+    (view as unknown as { lastLocation: { cfi: string } }).lastLocation = { cfi: 'cfi-live' };
+    (view.resolveCFI as ReturnType<typeof vi.fn>).mockImplementation((cfi: string) =>
+      cfi === 'cfi-live'
+        ? {
+            index: 0,
+            anchor: () => {
+              const r = doc.createRange();
+              r.selectNodeContents(thirdParagraph);
+              return r;
+            },
+          }
+        : null,
+    );
+    const viewRef = { current: view } as React.RefObject<FoliateView | null>;
+
+    render(<HookHarness view={viewRef} />);
+    await waitFor(() => {
+      expect(hookApi?.paragraphState.currentRange?.toString()).toContain('Block two');
+    });
+  });
+
+  it('does not scroll the underlying view when exiting paragraph mode (#4717)', async () => {
+    const doc = createDoc('<p>Para A</p><p>Para B</p>');
+    const { view, renderer } = createMockView([doc], 0);
+    const viewRef = { current: view } as React.RefObject<FoliateView | null>;
+
+    render(<HookHarness view={viewRef} />);
+    await waitFor(() => {
+      expect(hookApi?.paragraphState.currentRange).toBeTruthy();
+    });
+
+    await act(async () => {
+      await hookApi?.toggleParagraphMode();
+    });
+
+    expect(renderer.scrollToAnchor).not.toHaveBeenCalled();
+  });
+
+  it('still scrolls the underlying view when navigating paragraphs', async () => {
+    const doc = createDoc('<p>Para A</p><p>Para B</p><p>Para C</p>');
+    const { view, renderer } = createMockView([doc], 0);
+    const viewRef = { current: view } as React.RefObject<FoliateView | null>;
+
+    render(<HookHarness view={viewRef} />);
+    await waitFor(() => {
+      expect(hookApi?.paragraphState.currentRange).toBeTruthy();
+    });
+
+    await act(async () => {
+      await hookApi?.goToNextParagraph();
+    });
+
+    // Navigation to another paragraph must move the underlying view (the goTo
+    // runs after a rAF inside focusCurrentParagraph, so wait for it).
+    await waitFor(() => {
+      expect(renderer.goTo).toHaveBeenCalled();
+    });
+  });
+
   it('renders preserved presentation and layout-aware click zones in the overlay', async () => {
     const dispatchSpy = vi.spyOn(eventDispatcher, 'dispatch');
     const overlayBookKey = 'overlay-book';
@@ -382,20 +465,52 @@ describe('paragraph mode', () => {
     expect(onClose).toHaveBeenCalledTimes(1);
   });
 
+  const getDialog = (container: HTMLElement) =>
+    container.querySelector('[role="dialog"]') as HTMLDivElement;
+
+  it('focuses the dialog when it opens so it receives keys directly (#4717)', async () => {
+    const { container } = await renderVisibleOverlay(vi.fn());
+    const dialog = getDialog(container);
+    expect(document.activeElement).toBe(dialog);
+  });
+
   it('exits when the toggle paragraph mode shortcut (Shift+P) is pressed (#4717)', async () => {
     const onClose = vi.fn();
-    await renderVisibleOverlay(onClose);
+    const { container } = await renderVisibleOverlay(onClose);
 
-    fireEvent.keyDown(document.body, { key: 'P', shiftKey: true });
+    fireEvent.keyDown(getDialog(container), { key: 'P', shiftKey: true });
 
     expect(onClose).toHaveBeenCalledTimes(1);
   });
 
+  it('exits when Escape is pressed on the dialog (#4717)', async () => {
+    const onClose = vi.fn();
+    const { container } = await renderVisibleOverlay(onClose);
+
+    fireEvent.keyDown(getDialog(container), { key: 'Escape' });
+
+    expect(onClose).toHaveBeenCalledTimes(1);
+  });
+
+  it('stops the toggle key from propagating so it cannot fire twice (#4717)', async () => {
+    const onClose = vi.fn();
+    const { container } = await renderVisibleOverlay(onClose);
+    const windowSpy = vi.fn();
+    window.addEventListener('keydown', windowSpy);
+
+    fireEvent.keyDown(getDialog(container), { key: 'P', shiftKey: true });
+
+    // The dialog handler must stop propagation so the global useShortcuts
+    // handler never receives the same keypress (which would re-toggle).
+    expect(windowSpy).not.toHaveBeenCalled();
+    window.removeEventListener('keydown', windowSpy);
+  });
+
   it('does not exit on an unrelated key while visible', async () => {
     const onClose = vi.fn();
-    await renderVisibleOverlay(onClose);
+    const { container } = await renderVisibleOverlay(onClose);
 
-    fireEvent.keyDown(document.body, { key: 'x' });
+    fireEvent.keyDown(getDialog(container), { key: 'x' });
 
     expect(onClose).not.toHaveBeenCalled();
   });

apps/readest-app/src/__tests__/utils/event.test.ts

@@ -68,6 +68,31 @@ describe('EventDispatcher', () => {
       eventDispatcher.off('no-detail', fn);
     });
 
+    it('does not call listeners added during an in-progress dispatch (snapshot semantics)', async () => {
+      // Regression: a listener that (re)subscribes another listener mid-dispatch
+      // must not have that new listener fire for the *current* event. The live
+      // Set used to be iterated directly, so a handler added during an awaited
+      // listener got invoked in the same dispatch — causing paragraph mode to
+      // toggle twice per keypress (#4717).
+      const added = vi.fn();
+      const adder = vi.fn(async () => {
+        await Promise.resolve();
+        eventDispatcher.on('reentrant', added);
+      });
+      eventDispatcher.on('reentrant', adder);
+
+      await eventDispatcher.dispatch('reentrant');
+      expect(adder).toHaveBeenCalledOnce();
+      expect(added).not.toHaveBeenCalled();
+
+      // The newly-added listener takes effect on the next dispatch.
+      await eventDispatcher.dispatch('reentrant');
+      expect(added).toHaveBeenCalledOnce();
+
+      eventDispatcher.off('reentrant', adder);
+      eventDispatcher.off('reentrant', added);
+    });
+
     it('awaits async listeners sequentially', async () => {
       const order: string[] = [];
       const slowFn = vi.fn(async () => {

apps/readest-app/src/app/reader/components/paragraph/ParagraphOverlay.tsx

@@ -331,12 +331,21 @@ const ParagraphOverlay: React.FC<ParagraphOverlayProps> = ({
     };
   }, [bookKey, addParagraph]);
 
+  // Focus the dialog when it opens (the dialog/alert pattern) so it receives
+  // keydowns directly via its own onKeyDown handler, regardless of where focus
+  // sat before — fixes Shift+P/Escape not toggling when focus was still inside
+  // the book iframe (#4717).
   useEffect(() => {
     if (!isVisible) return;
+    containerRef.current?.focus({ preventScroll: true });
+  }, [isVisible]);
 
-    const handleKeyDown = (e: KeyboardEvent) => {
+  // Keydown handler bound to the dialog element. Keeps every key inside the
+  // overlay (stopPropagation) so the global shortcut handler never sees it — the
+  // toggle therefore fires exactly once.
+  const handleKeyDown = useCallback(
+    (e: React.KeyboardEvent) => {
       e.stopPropagation();
-      e.stopImmediatePropagation();
 
       if (e.key === 'Escape' || e.key === 'Backspace') {
         e.preventDefault();
@@ -344,10 +353,6 @@ const ParagraphOverlay: React.FC<ParagraphOverlayProps> = ({
         return;
       }
 
-      // The overlay swallows every keydown in the capture phase, so the global
-      // toggle shortcut (Shift+P by default) never reaches useShortcuts. Honor
-      // it here so the same shortcut that enters paragraph mode also exits it
-      // (#4717).
       if (matchesShortcut(e, loadShortcuts().onToggleParagraphMode.keys)) {
         e.preventDefault();
         onCloseRef.current?.();
@@ -358,18 +363,13 @@ const ParagraphOverlay: React.FC<ParagraphOverlayProps> = ({
       if (action === 'next') {
         e.preventDefault();
         eventDispatcher.dispatch('paragraph-next', { bookKey });
-        return;
-      }
-
-      if (action === 'prev') {
+      } else if (action === 'prev') {
         e.preventDefault();
         eventDispatcher.dispatch('paragraph-prev', { bookKey });
       }
-    };
-
-    window.addEventListener('keydown', handleKeyDown, true);
-    return () => window.removeEventListener('keydown', handleKeyDown, true);
-  }, [activePresentation, bookKey, isVisible, viewSettings]);
+    },
+    [activePresentation, bookKey, viewSettings],
+  );
 
   useEffect(() => {
     if (!isVisible) return;
@@ -473,6 +473,9 @@ const ParagraphOverlay: React.FC<ParagraphOverlayProps> = ({
 
   const handleBackdropClick = useCallback(
     (e: React.MouseEvent) => {
+      // Keep keyboard focus on the dialog so Escape/Shift+P keep working after a
+      // tap moved focus elsewhere (e.g. into the book iframe).
+      containerRef.current?.focus({ preventScroll: true });
       // Tapping the empty area around the paragraph used to exit, which made it
       // easy to leave paragraph mode by accident. Reveal the controls instead so
       // exiting stays an explicit action (the bar's exit button or Escape).
@@ -487,6 +490,8 @@ const ParagraphOverlay: React.FC<ParagraphOverlayProps> = ({
   const handleContentClick = useCallback(
     (e: React.MouseEvent) => {
       e.stopPropagation();
+      // Keep keyboard focus on the dialog so it keeps receiving keys after a tap.
+      containerRef.current?.focus({ preventScroll: true });
 
       const now = Date.now();
       if (now - lastTapTimeRef.current < 300) {
@@ -543,6 +548,10 @@ const ParagraphOverlay: React.FC<ParagraphOverlayProps> = ({
       className={clsx(
         'fixed inset-0 z-40',
         'flex flex-col items-center justify-center',
+        // The dialog is focused programmatically (so it receives keys); it is not
+        // a tab stop, so suppress the focus ring that would otherwise outline the
+        // whole viewport.
+        'outline-none',
         'transition-opacity duration-300 ease-out',
         isOverlayMounted ? 'opacity-100' : 'opacity-0',
       )}
@@ -555,7 +564,7 @@ const ParagraphOverlay: React.FC<ParagraphOverlayProps> = ({
       }}
       onClick={handleBackdropClick}
       onTouchStart={handleTouchStart}
-      onKeyDown={(e) => e.stopPropagation()}
+      onKeyDown={handleKeyDown}
     >
       {/* TTS "following audio" indicator, pinned top-center. Anchored below the
           top safe-area inset the overlay already accounts for; idle/unsupported

apps/readest-app/src/app/reader/hooks/useParagraphMode.ts

@@ -205,10 +205,14 @@ export const useParagraphMode = ({ bookKey, viewRef }: UseParagraphModeProps) =>
         const isSameDoc = progressRange?.startContainer?.ownerDocument === doc;
         const lastParagraph = lastParagraphRef.current;
 
-        const resolveRangeFromLocation = (): Range | null => {
-          if (!progressLocation) return null;
+        // Resolve a CFI against the CURRENT document. A CFI is document-instance
+        // independent, so this survives the section iframe being recreated when
+        // paragraph mode toggles — unlike a stored Range, which is bound to the
+        // document it was created in.
+        const resolveRangeFromCfi = (cfi?: string | null): Range | null => {
+          if (!cfi) return null;
           try {
-            const resolved = view.resolveCFI(progressLocation);
+            const resolved = view.resolveCFI(cfi);
             if (!resolved || resolved.index !== docIndex) return null;
             const anchor = resolved.anchor(doc);
             // Realm-safe: iframe-realm Range fails top-realm `instanceof Range`.
@@ -228,26 +232,22 @@ export const useParagraphMode = ({ bookKey, viewRef }: UseParagraphModeProps) =>
           if (!lastParagraph || !progressLocation) return null;
           if (lastParagraph.progressLocation !== progressLocation) return null;
           if (lastParagraph.docIndex !== docIndex) return null;
-          try {
-            const resolved = view.resolveCFI(lastParagraph.paragraphCfi);
-            if (!resolved || resolved.index !== docIndex) return null;
-            const anchor = resolved.anchor(doc);
-            // Realm-safe: iframe-realm Range fails top-realm `instanceof Range`.
-            if (isRangeLike(anchor)) return anchor;
-            if (anchor) {
-              const range = doc.createRange();
-              range.selectNodeContents(anchor);
-              return range;
-            }
-          } catch {
-            return null;
-          }
-          return null;
+          return resolveRangeFromCfi(lastParagraph.paragraphCfi);
         };
 
+        // Resume from the freshest current position. The view's live lastLocation
+        // CFI is set synchronously by foliate on every relocate; the readerStore
+        // progress is rAF-debounced and can lag, which sent resume to a stale page
+        // (the chapter start) and let repeated enter/exit walk further back (#4717).
+        // The view's live lastLocation CFI is foliate-generated and well-formed,
+        // so it's the most reliable resume target. The stored last-paragraph CFI
+        // is `view.getCFI` of a paragraph block range, which can come out malformed
+        // and resolve to an empty range — so it must NOT take priority (#4717).
         const targetRange =
+          resolveRangeFromCfi(view.lastLocation?.cfi) ??
           resolveRangeFromLastParagraph() ??
-          (isSameDoc ? progressRange : resolveRangeFromLocation());
+          (isSameDoc ? progressRange : null) ??
+          resolveRangeFromCfi(progressLocation);
 
         if (targetRange && iteratorRef.current) {
           try {
@@ -289,48 +289,59 @@ export const useParagraphMode = ({ bookKey, viewRef }: UseParagraphModeProps) =>
     return initPromise;
   }, [getPrimaryContent, viewRef, getProgress, updateStateFromIterator]);
 
-  const focusCurrentParagraph = useCallback(async () => {
-    const view = viewRef.current;
-    const iterator = iteratorRef.current;
-    if (!view || !iterator) return;
+  // `align` scrolls the underlying view so the focused paragraph sits at the page
+  // top. Pass `false` when entering/resuming paragraph mode: the focused paragraph
+  // is already on screen, and scrolling to its start rewinds the reading position
+  // whenever that paragraph began on an earlier page — repeated enter/exit would
+  // then walk back to the chapter start (#4717). Navigation passes `true` so the
+  // view follows to off-screen paragraphs.
+  const focusCurrentParagraph = useCallback(
+    async (align = true) => {
+      const view = viewRef.current;
+      const iterator = iteratorRef.current;
+      if (!view || !iterator) return;
 
-    const range = iterator.current();
-    if (!range) return;
+      const range = iterator.current();
+      if (!range) return;
 
-    await new Promise((r) => requestAnimationFrame(r));
+      await new Promise((r) => requestAnimationFrame(r));
 
-    if (focusResetTimerRef.current) {
-      clearTimeout(focusResetTimerRef.current);
-    }
+      if (focusResetTimerRef.current) {
+        clearTimeout(focusResetTimerRef.current);
+      }
 
-    const presentation = getParagraphPresentation(
-      range.startContainer.ownerDocument,
-      range,
-      getViewSettings(bookKeyRef.current),
-    );
+      const presentation = getParagraphPresentation(
+        range.startContainer.ownerDocument,
+        range,
+        getViewSettings(bookKeyRef.current),
+      );
 
-    isFocusingRef.current = true;
-    const docIndex = currentDocIndexRef.current;
-    const renderer = view.renderer as FoliateView['renderer'] & {
-      goTo?: (target: { index: number; anchor: Range }) => Promise<void>;
-    };
-    if (docIndex !== undefined && renderer.goTo) {
-      renderer.goTo({ index: docIndex, anchor: range });
-    } else {
-      view.renderer.scrollToAnchor?.(range);
-    }
-    focusResetTimerRef.current = setTimeout(() => {
-      isFocusingRef.current = false;
-    }, 200);
+      if (align) {
+        isFocusingRef.current = true;
+        const docIndex = currentDocIndexRef.current;
+        const renderer = view.renderer as FoliateView['renderer'] & {
+          goTo?: (target: { index: number; anchor: Range }) => Promise<void>;
+        };
+        if (docIndex !== undefined && renderer.goTo) {
+          renderer.goTo({ index: docIndex, anchor: range });
+        } else {
+          view.renderer.scrollToAnchor?.(range);
+        }
+        focusResetTimerRef.current = setTimeout(() => {
+          isFocusingRef.current = false;
+        }, 200);
+      }
 
-    eventDispatcher.dispatch('paragraph-focus', {
-      bookKey: bookKeyRef.current,
-      range,
-      index: iterator.currentIndex,
-      total: iterator.length,
-      presentation,
-    });
-  }, [getViewSettings, viewRef]);
+      eventDispatcher.dispatch('paragraph-focus', {
+        bookKey: bookKeyRef.current,
+        range,
+        index: iterator.currentIndex,
+        total: iterator.length,
+        presentation,
+      });
+    },
+    [getViewSettings, viewRef],
+  );
 
   // Sync-focus path for TTS following. Moves focus to `index` and scrolls/emits
   // exactly like focusCurrentParagraph but WITHOUT arming isFocusingRef. The
@@ -640,7 +651,8 @@ export const useParagraphMode = ({ bookKey, viewRef }: UseParagraphModeProps) =>
 
         const success = await initIterator();
         if (success) {
-          await focusCurrentParagraph();
+          // Resume: don't scroll the underlying view (would rewind, #4717).
+          await focusCurrentParagraph(false);
         }
       } else {
         setViewSettings(bookKeyRef.current, { ...settings, paragraphMode: newConfig });
@@ -661,7 +673,9 @@ export const useParagraphMode = ({ bookKey, viewRef }: UseParagraphModeProps) =>
                 docIndex,
               };
             }
-            view.renderer.scrollToAnchor?.(range);
+            // Don't scroll to the paragraph on exit: navigation already moved the
+            // view to the focused paragraph, and re-anchoring to its start rewinds
+            // the reading position when it began on an earlier page (#4717).
           }
         }
         eventDispatcher.dispatch('paragraph-mode-disabled', { bookKey: bookKeyRef.current });
@@ -690,7 +704,8 @@ export const useParagraphMode = ({ bookKey, viewRef }: UseParagraphModeProps) =>
       const init = async () => {
         const success = await initIterator();
         if (success) {
-          await focusCurrentParagraph();
+          // Resume on open: don't scroll the underlying view (would rewind, #4717).
+          await focusCurrentParagraph(false);
         }
       };
       const timer = setTimeout(init, 100);

apps/readest-app/src/types/view.ts

@@ -102,6 +102,9 @@ export interface FoliateView extends HTMLElement {
   ) => Promise<void>;
   book: BookDoc;
   tts: TTS | null;
+  // The most recent relocate location, set synchronously by foliate on every
+  // relocate — fresher than the rAF-debounced readerStore progress.
+  lastLocation?: { cfi?: string; range?: Range | null };
   isFixedLayout: boolean;
   language: {
     locale?: LocaleWithTextInfo;

apps/readest-app/src/utils/event.ts

@@ -22,7 +22,13 @@ class EventDispatcher {
     const listeners = this.asyncListeners.get(event);
     if (listeners) {
       const customEvent = new CustomEvent(event, { detail });
-      for (const listener of listeners) {
+      // Snapshot the listeners before iterating: an awaited listener may
+      // (re)subscribe another handler for the same event — e.g. a React effect
+      // re-running on a state change it triggered. Iterating the live Set would
+      // invoke that freshly-added handler in the same dispatch, double-firing
+      // the event (paragraph mode toggled twice per keypress, #4717). dispatchSync
+      // already snapshots for the same reason.
+      for (const listener of [...listeners]) {
         await listener(customEvent);
       }
     }