From 9cbcbdc9ab03b0202dcffad708dd6298e90eee73 Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Wed, 6 May 2026 12:07:04 -0300 Subject: [PATCH 1/4] fix(presentation-editor): bail on clicks in inter-page gap (SD-2749) Clicking in the vertical gap between two pages caused the viewer to jump to a different page. PointerNormalization returns pageIndex=undefined when elementsFromPoint finds no .superdoc-page under the cursor (the gap area), but the existing isOutsidePageBodyContent guard from SD-2356 only fires when pageIndex is finite. The pointer flow then fell through to the geometry fallback, which always snaps to the nearest fragment, moving the selection and triggering a scrollIntoView. Treat "no page under cursor" the same as "click in page margin": prevent default, focus the editor, do not change selection or scroll. Also harden isOutsidePageBodyContent against a missing layout.pages array. Tests: - New regression test in EditorInputManager.pageMarginClick.test.ts asserts that a click with undefined pageIndex does not call into the position resolver or set a selection. - Existing pointer tests had incomplete normalizeClientPoint mocks that did not return pageIndex. Updated them to mirror the production contract so in-page click paths still exercise position resolution. - Two PresentationEditor integration tests stub elementsFromPoint to simulate a .superdoc-page under the click point, since happy-dom does not compute layout. Verified end-to-end: clicking in the 24px gap between pages no longer moves selection or scroll, while in-page text clicks still place the caret normally. --- .../pointer-events/EditorInputManager.ts | 24 +++++++---- ...torInputManager.activeCommentClick.test.ts | 7 +++- .../EditorInputManager.dragAutoScroll.test.ts | 7 +++- .../EditorInputManager.footnoteClick.test.ts | 7 +++- ...EditorInputManager.pageMarginClick.test.ts | 19 +++++++++ ...itorInputManager.structuredContent.test.ts | 7 +++- .../tests/PresentationEditor.test.ts | 42 +++++++++++++++++++ 7 files changed, 101 insertions(+), 12 deletions(-) diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/EditorInputManager.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/EditorInputManager.ts index debb3a0736..f4c7d945c5 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/EditorInputManager.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/EditorInputManager.ts @@ -132,7 +132,7 @@ function isOutsidePageBodyContent(layout: Layout, x: number, pageIndex?: number, return false; } - const page = layout.pages[pageIndex]; + const page = layout?.pages?.[pageIndex]; if (!page) { return false; } @@ -1480,13 +1480,21 @@ export class EditorInputManager { } } - if ( - !useActiveSurfaceHitTest && - isOutsidePageBodyContent(layoutState.layout, x, normalizedPoint.pageIndex, normalizedPoint.pageLocalY) - ) { - event.preventDefault(); - this.#focusEditor(); - return; + // Bail when the click did not land on any page body. Two cases: + // - SD-2356: click inside a page's bounding box but in the margin/header/footer area. + // - SD-2749: click in the gap between pages (no .superdoc-page under the cursor), + // in which case normalizeClientPoint leaves pageIndex undefined. + // Both should preserve the current selection and scroll position. + if (!useActiveSurfaceHitTest) { + const pointerOffAnyPage = !Number.isFinite(normalizedPoint.pageIndex); + if ( + pointerOffAnyPage || + isOutsidePageBodyContent(layoutState.layout, x, normalizedPoint.pageIndex, normalizedPoint.pageLocalY) + ) { + event.preventDefault(); + this.#focusEditor(); + return; + } } const { rawHit, hit } = this.#resolveSelectionPointerHit({ diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.activeCommentClick.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.activeCommentClick.test.ts index a312d39902..09ff40385b 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.activeCommentClick.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.activeCommentClick.test.ts @@ -144,7 +144,12 @@ describe('EditorInputManager - single-thread comment highlight clicks', () => { }; mockCallbacks = { - normalizeClientPoint: vi.fn((clientX: number, clientY: number) => ({ x: clientX, y: clientY })), + normalizeClientPoint: vi.fn((clientX: number, clientY: number) => ({ + x: clientX, + y: clientY, + pageIndex: 0, + pageLocalY: clientY, + })), scheduleSelectionUpdate: vi.fn(), updateSelectionDebugHud: vi.fn(), }; diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.dragAutoScroll.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.dragAutoScroll.test.ts index e315831327..7346169ab4 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.dragAutoScroll.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.dragAutoScroll.test.ts @@ -132,7 +132,12 @@ describe('EditorInputManager - Drag Auto Scroll', () => { }; mockCallbacks = { - normalizeClientPoint: vi.fn((clientX: number, clientY: number) => ({ x: clientX, y: clientY })), + normalizeClientPoint: vi.fn((clientX: number, clientY: number) => ({ + x: clientX, + y: clientY, + pageIndex: 0, + pageLocalY: clientY, + })), updateSelectionVirtualizationPins: vi.fn(), scheduleSelectionUpdate: vi.fn(), notifyDragSelectionEnded: vi.fn(), diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.footnoteClick.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.footnoteClick.test.ts index 40bca24de0..22625445b6 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.footnoteClick.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.footnoteClick.test.ts @@ -149,7 +149,12 @@ describe('EditorInputManager - Footnote click selection behavior', () => { mockCallbacks = { activateRenderedNoteSession: vi.fn(() => true), - normalizeClientPoint: vi.fn((clientX: number, clientY: number) => ({ x: clientX, y: clientY })), + normalizeClientPoint: vi.fn((clientX: number, clientY: number) => ({ + x: clientX, + y: clientY, + pageIndex: 0, + pageLocalY: clientY, + })), scheduleSelectionUpdate: vi.fn(), updateSelectionDebugHud: vi.fn(), }; diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.pageMarginClick.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.pageMarginClick.test.ts index 74435888f1..9c32636967 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.pageMarginClick.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.pageMarginClick.test.ts @@ -204,4 +204,23 @@ describe('EditorInputManager - page margin clicks', () => { expect(resolvePointerPositionHit).toHaveBeenCalled(); }); + + // SD-2749: clicking in the inter-page gap caused the viewer to jump to a + // different page. normalizeClientPoint returns pageIndex undefined when + // elementsFromPoint finds no .superdoc-page under the cursor (e.g. the gap + // between two pages). The pointer-down handler must bail out the same way it + // does for in-page margin clicks (SD-2356) — no selection change, no scroll. + it('does not resolve a position hit for clicks in the gap between pages', () => { + (mockCallbacks.normalizeClientPoint as Mock).mockReturnValueOnce({ x: 200, y: 410 }); + + const target = document.createElement('span'); + viewportHost.appendChild(target); + + dispatchPointerDown(target, { clientX: 200, clientY: 410 }); + + expect(resolvePointerPositionHit).not.toHaveBeenCalled(); + expect(TextSelection.create as unknown as Mock).not.toHaveBeenCalled(); + expect(mockEditor.state.tr.setSelection).not.toHaveBeenCalled(); + expect(mockEditor.view.focus).toHaveBeenCalled(); + }); }); diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.structuredContent.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.structuredContent.test.ts index e5647ac539..2d8cba0125 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.structuredContent.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.structuredContent.test.ts @@ -216,7 +216,12 @@ describe('EditorInputManager structuredContentBlock table exception', () => { isSelectionAwareVirtualizationEnabled: vi.fn(() => false), }); manager.setCallbacks({ - normalizeClientPoint: vi.fn((clientX: number, clientY: number) => ({ x: clientX, y: clientY })), + normalizeClientPoint: vi.fn((clientX: number, clientY: number) => ({ + x: clientX, + y: clientY, + pageIndex: 0, + pageLocalY: clientY, + })), scheduleSelectionUpdate: vi.fn(), updateSelectionDebugHud: vi.fn(), hitTestTable: (mockHitTestTable = vi.fn(() => null)), diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts index 8d4474b90a..648ea073fc 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts @@ -2305,6 +2305,29 @@ describe('PresentationEditor', () => { toJSON: () => ({}), } as DOMRect); + // SD-2749: PointerNormalization uses elementsFromPoint to detect the + // .superdoc-page under the cursor; happy-dom doesn't compute layout, so + // simulate a page element under the click point. + const pagesHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + const fakePage = document.createElement('div'); + fakePage.classList.add('superdoc-page'); + fakePage.setAttribute('data-page-index', '0'); + pagesHost.appendChild(fakePage); + vi.spyOn(fakePage, 'getBoundingClientRect').mockReturnValue({ + left: 0, + top: 0, + width: 612, + height: 792, + right: 612, + bottom: 792, + x: 0, + y: 0, + toJSON: () => ({}), + } as DOMRect); + (document as unknown as { elementsFromPoint: (x: number, y: number) => Element[] }).elementsFromPoint = () => [ + fakePage, + ]; + // Clear mock to track fresh calls mockClickToPosition.mockClear(); mockResolvePointerPositionHit.mockClear(); @@ -4847,6 +4870,7 @@ describe('PresentationEditor', () => { // Mark page 0 as mounted for the drag anchor. const pagesHost = container.querySelector('.presentation-editor__pages') as HTMLElement; const page0 = document.createElement('div'); + page0.classList.add('superdoc-page'); page0.setAttribute('data-page-index', '0'); pagesHost.appendChild(page0); @@ -4863,6 +4887,24 @@ describe('PresentationEditor', () => { toJSON: () => ({}), } as DOMRect); + // SD-2749: PointerNormalization uses elementsFromPoint to detect the + // .superdoc-page under the cursor; happy-dom doesn't compute layout, so + // simulate the page element under each click point. + vi.spyOn(page0, 'getBoundingClientRect').mockReturnValue({ + left: 0, + top: 0, + width: 612, + height: 792, + right: 612, + bottom: 792, + x: 0, + y: 0, + toJSON: () => ({}), + } as DOMRect); + (document as unknown as { elementsFromPoint: (x: number, y: number) => Element[] }).elementsFromPoint = () => [ + page0, + ]; + // pointerdown: page 0 (mounted), pointermove: page 1 (unmounted), pointerup finalize: page 1 mockClickToPosition.mockReset(); mockClickToPosition From 57c46e5a80c8bbe3d10669c574638fd841f2acce Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Wed, 6 May 2026 13:13:23 -0300 Subject: [PATCH 2/4] fix(presentation-editor): refresh session state when body click exits H/F (SD-2749) After double-clicking a header/footer to enter editing mode and then clicking back into a body paragraph, ProseMirror was dispatching the new selection on the now-stale active (header/footer) editor. With the H/F surface scrolled out of view, ProseMirror's scrollIntoView pulled the viewport back to the H/F surface, jumping the user away from where they clicked. The captured `sessionMode`, `useActiveSurfaceHitTest`, and `editor` locals in #handlePointerDown were captured before #handleClickInHeaderFooterMode exited the session for a body-content click. Re-derive them from the fresh getHeaderFooterSession() reading once the H/F handler returns false (meaning it didn't claim the click), so subsequent hit testing and selection dispatch target the body editor. Adds EditorInputManager.headerFooterBodyClick.test.ts covering the regression. Verified end-to-end in the browser: scrollTop and selection are preserved when clicking body after editing a header. --- .../pointer-events/EditorInputManager.ts | 18 +- ...InputManager.headerFooterBodyClick.test.ts | 198 ++++++++++++++++++ 2 files changed, 213 insertions(+), 3 deletions(-) create mode 100644 packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.headerFooterBodyClick.test.ts diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/EditorInputManager.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/EditorInputManager.ts index f4c7d945c5..adc0f77e89 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/EditorInputManager.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/EditorInputManager.ts @@ -1459,11 +1459,23 @@ export class EditorInputManager { } const isNoteEditing = activeNoteSession != null; - const useActiveSurfaceHitTest = sessionMode !== 'body' || activeStorySession != null; - const editor = sessionMode === 'body' && !isNoteEditing ? bodyEditor : this.#deps.getActiveEditor(); - if (sessionMode !== 'body') { + let currentSessionMode = sessionMode; + let useActiveSurfaceHitTest = currentSessionMode !== 'body' || activeStorySession != null; + let editor = currentSessionMode === 'body' && !isNoteEditing ? bodyEditor : this.#deps.getActiveEditor(); + if (currentSessionMode !== 'body') { if (this.#handleClickInHeaderFooterMode(event, x, y, normalizedPoint.pageIndex, normalizedPoint.pageLocalY)) return; + // SD-2749: clicking on body content from inside a header/footer session + // exits the session synchronously. Re-derive local mode so the click + // dispatches its selection on the body editor instead of the now-stale + // active editor — otherwise ProseMirror's scrollIntoView would pull the + // viewport back to the header/footer the user just exited. + const refreshedSessionMode = this.#deps.getHeaderFooterSession()?.session?.mode ?? 'body'; + if (refreshedSessionMode === 'body' && !isNoteEditing) { + currentSessionMode = 'body'; + useActiveSurfaceHitTest = activeStorySession != null; + editor = bodyEditor; + } } // Check for header/footer region hit diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.headerFooterBodyClick.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.headerFooterBodyClick.test.ts new file mode 100644 index 0000000000..2854a8c573 --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.headerFooterBodyClick.test.ts @@ -0,0 +1,198 @@ +import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest'; + +import { + EditorInputManager, + type EditorInputDependencies, + type EditorInputCallbacks, +} from '../pointer-events/EditorInputManager.js'; + +vi.mock('../input/PositionHitResolver.js', () => ({ + resolvePointerPositionHit: vi.fn(() => ({ + pos: 50, + layoutEpoch: 1, + pageIndex: 0, + blockId: 'paragraph-1', + column: 0, + lineIndex: -1, + })), +})); + +vi.mock('@superdoc/layout-bridge', () => ({ + clickToPosition: vi.fn(() => ({ pos: 50, layoutEpoch: 1, pageIndex: 0, blockId: 'paragraph-1' })), + getFragmentAtPosition: vi.fn(() => null), +})); + +vi.mock('prosemirror-state', async (importOriginal) => { + const original = await importOriginal(); + // Spy on create to return a stub, but keep the real class so `instanceof + // TextSelection` checks in the production code don't throw. + vi.spyOn(original.TextSelection, 'create').mockImplementation( + () => + ({ + empty: true, + $from: { parent: { inlineContent: true } }, + }) as unknown as InstanceType, + ); + return original; +}); + +// SD-2749: clicking on body content while a header/footer editing session is +// active was dispatching the resulting selection on the header editor. When +// the header was scrolled out of view, ProseMirror's scrollIntoView pulled +// the viewport back to the header, jumping the user away from where they +// clicked. The fix re-derives the local sessionMode/editor flags after +// #handleClickInHeaderFooterMode exits the session for a body-content click. + +describe('EditorInputManager - body click during header/footer session', () => { + let manager: EditorInputManager; + let viewportHost: HTMLElement; + let visibleHost: HTMLElement; + let bodyEditor: ReturnType; + let headerEditor: ReturnType; + let sessionMode: 'body' | 'header'; + let mockDeps: EditorInputDependencies; + let mockCallbacks: EditorInputCallbacks; + + function buildEditor(docSize: number) { + return { + isEditable: true, + state: { + doc: { + content: { size: docSize }, + nodesBetween: vi.fn((_from, _to, cb) => { + cb({ isTextblock: true }, 0); + }), + }, + tr: { + setSelection: vi.fn().mockReturnThis(), + setStoredMarks: vi.fn().mockReturnThis(), + }, + selection: { $anchor: null }, + storedMarks: null, + }, + view: { + dispatch: vi.fn(), + dom: document.createElement('div'), + focus: vi.fn(), + hasFocus: vi.fn(() => false), + }, + on: vi.fn(), + off: vi.fn(), + emit: vi.fn(), + }; + } + + beforeEach(() => { + viewportHost = document.createElement('div'); + viewportHost.className = 'presentation-editor__viewport'; + visibleHost = document.createElement('div'); + visibleHost.className = 'presentation-editor__visible'; + visibleHost.appendChild(viewportHost); + + const container = document.createElement('div'); + container.className = 'presentation-editor'; + container.appendChild(visibleHost); + document.body.appendChild(container); + + bodyEditor = buildEditor(10_000); + headerEditor = buildEditor(20); + sessionMode = 'header'; + + mockDeps = { + getEditor: vi.fn(() => bodyEditor as unknown as ReturnType), + getActiveEditor: vi.fn(() => + sessionMode === 'header' + ? (headerEditor as unknown as ReturnType) + : (bodyEditor as unknown as ReturnType), + ), + getLayoutState: vi.fn(() => ({ + layout: { + pageSize: { w: 600, h: 800 }, + pages: [ + { + number: 1, + size: { w: 600, h: 800 }, + margins: { top: 72, right: 72, bottom: 72, left: 72 }, + fragments: [], + }, + ], + } as any, + blocks: [], + measures: [], + })), + getEpochMapper: vi.fn(() => ({ + mapPosFromLayoutToCurrentDetailed: vi.fn(() => ({ ok: true, pos: 50, toEpoch: 1 })), + })) as unknown as EditorInputDependencies['getEpochMapper'], + getViewportHost: vi.fn(() => viewportHost), + getVisibleHost: vi.fn(() => visibleHost), + getLayoutMode: vi.fn(() => 'vertical'), + getHeaderFooterSession: vi.fn(() => + sessionMode === 'header' + ? ({ session: { mode: 'header' } } as ReturnType) + : null, + ), + getPageGeometryHelper: vi.fn(() => null), + getZoom: vi.fn(() => 1), + isViewLocked: vi.fn(() => false), + getDocumentMode: vi.fn(() => 'editing'), + getPageElement: vi.fn(() => null), + isSelectionAwareVirtualizationEnabled: vi.fn(() => false), + }; + + mockCallbacks = { + normalizeClientPoint: vi.fn((clientX: number, clientY: number) => ({ + x: clientX, + y: clientY, + pageIndex: 0, + pageLocalY: clientY, + })), + scheduleSelectionUpdate: vi.fn(), + updateSelectionDebugHud: vi.fn(), + hitTestHeaderFooterRegion: vi.fn(() => null), + // Simulate the runtime: clicking on body content exits the header session synchronously. + exitHeaderFooterMode: vi.fn(() => { + sessionMode = 'body'; + }), + }; + + manager = new EditorInputManager(); + manager.setDependencies(mockDeps); + manager.setCallbacks(mockCallbacks); + manager.bind(); + }); + + afterEach(() => { + manager.destroy(); + document.body.innerHTML = ''; + vi.clearAllMocks(); + }); + + function getPointerEventImpl(): typeof PointerEvent | typeof MouseEvent { + return ( + (globalThis as unknown as { PointerEvent?: typeof PointerEvent; MouseEvent: typeof MouseEvent }).PointerEvent ?? + globalThis.MouseEvent + ); + } + + it('dispatches the click selection on the body editor, not the header editor', () => { + // Body content under the cursor — outside the header region. + const target = document.createElement('span'); + viewportHost.appendChild(target); + + const PointerEventImpl = getPointerEventImpl(); + target.dispatchEvent( + new PointerEventImpl('pointerdown', { + bubbles: true, + cancelable: true, + button: 0, + buttons: 1, + clientX: 200, + clientY: 400, + } as PointerEventInit), + ); + + expect(mockCallbacks.exitHeaderFooterMode as Mock).toHaveBeenCalled(); + expect(headerEditor.view.dispatch).not.toHaveBeenCalled(); + expect(bodyEditor.view.dispatch).toHaveBeenCalled(); + }); +}); From 8ddc44efbeed44c96a8f0976c8a5993d6d2a3837 Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Wed, 6 May 2026 14:01:34 -0300 Subject: [PATCH 3/4] fix(presentation-editor): also re-read story session after H/F exit Address two P2 review findings on PR #3181: 1. Refresh activeStorySession after exiting H/F (codex-connector inline review). exitHeaderFooterMode synchronously clears the backing StoryPresentationSessionManager, but the previous patch recomputed useActiveSurfaceHitTest from the pre-exit activeStorySession variable. In production H/F sessions are story-backed, so that stale value kept the first body click on the active-surface path, skipping the new page-gap guard. Re-read getActiveStorySession() after the exit before recomputing the locals. The headerFooterBodyClick test now mocks a story-backed session so it would fail without the re-read, and a second test asserts that an inter-page gap click (pageIndex undefined) after exiting an H/F session still preserves the selection. 2. Restore document.elementsFromPoint after stubbing (Codex review). The two PresentationEditor.test.ts tests that fake elementsFromPoint now save and restore the original (or delete the stub if there was none) via try/finally, so subsequent tests do not inherit the fake hit chain. --- .../pointer-events/EditorInputManager.ts | 9 +- ...InputManager.headerFooterBodyClick.test.ts | 38 +++++ .../tests/PresentationEditor.test.ts | 142 ++++++++++-------- 3 files changed, 124 insertions(+), 65 deletions(-) diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/EditorInputManager.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/EditorInputManager.ts index adc0f77e89..4f309058a1 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/EditorInputManager.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/EditorInputManager.ts @@ -1466,12 +1466,13 @@ export class EditorInputManager { if (this.#handleClickInHeaderFooterMode(event, x, y, normalizedPoint.pageIndex, normalizedPoint.pageLocalY)) return; // SD-2749: clicking on body content from inside a header/footer session - // exits the session synchronously. Re-derive local mode so the click - // dispatches its selection on the body editor instead of the now-stale - // active editor — otherwise ProseMirror's scrollIntoView would pull the - // viewport back to the header/footer the user just exited. + // exits the session synchronously, which also clears the backing story + // session. Re-read both so subsequent hit testing and selection dispatch + // target the body editor — otherwise ProseMirror's scrollIntoView would + // pull the viewport back to the header/footer the user just exited. const refreshedSessionMode = this.#deps.getHeaderFooterSession()?.session?.mode ?? 'body'; if (refreshedSessionMode === 'body' && !isNoteEditing) { + activeStorySession = this.#deps.getActiveStorySession?.() ?? null; currentSessionMode = 'body'; useActiveSurfaceHitTest = activeStorySession != null; editor = bodyEditor; diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.headerFooterBodyClick.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.headerFooterBodyClick.test.ts index 2854a8c573..d7f25fadcc 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.headerFooterBodyClick.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.headerFooterBodyClick.test.ts @@ -131,6 +131,16 @@ describe('EditorInputManager - body click during header/footer session', () => { ? ({ session: { mode: 'header' } } as ReturnType) : null, ), + // Production H/F sessions are story-backed; mirror that here so the test + // would catch a fix that recomputes useActiveSurfaceHitTest from the + // pre-exit story session instead of re-reading after the exit. + getActiveStorySession: vi.fn(() => + sessionMode === 'header' + ? ({ kind: 'headerFooter' } as unknown as ReturnType< + NonNullable + >) + : null, + ), getPageGeometryHelper: vi.fn(() => null), getZoom: vi.fn(() => 1), isViewLocked: vi.fn(() => false), @@ -195,4 +205,32 @@ describe('EditorInputManager - body click during header/footer session', () => { expect(headerEditor.view.dispatch).not.toHaveBeenCalled(); expect(bodyEditor.view.dispatch).toHaveBeenCalled(); }); + + // Re-reading getActiveStorySession after the H/F session exits is what keeps + // useActiveSurfaceHitTest correctly false. Without it, the inter-page gap + // guard (clicks where pageIndex is undefined) is skipped and the click + // resolves a position, moving the selection. + it('preserves selection on inter-page gap click after exiting an H/F session', () => { + (mockCallbacks.normalizeClientPoint as Mock).mockReturnValueOnce({ x: 200, y: 400 }); + + const target = document.createElement('span'); + viewportHost.appendChild(target); + + const PointerEventImpl = getPointerEventImpl(); + target.dispatchEvent( + new PointerEventImpl('pointerdown', { + bubbles: true, + cancelable: true, + button: 0, + buttons: 1, + clientX: 200, + clientY: 400, + } as PointerEventInit), + ); + + expect(mockCallbacks.exitHeaderFooterMode as Mock).toHaveBeenCalled(); + expect(headerEditor.view.dispatch).not.toHaveBeenCalled(); + expect(bodyEditor.view.dispatch).not.toHaveBeenCalled(); + expect(bodyEditor.state.tr.setSelection).not.toHaveBeenCalled(); + }); }); diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts index 648ea073fc..ad48a3b48e 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts @@ -2324,25 +2324,34 @@ describe('PresentationEditor', () => { y: 0, toJSON: () => ({}), } as DOMRect); + const originalElementsFromPoint = (document as unknown as { elementsFromPoint?: unknown }).elementsFromPoint; (document as unknown as { elementsFromPoint: (x: number, y: number) => Element[] }).elementsFromPoint = () => [ fakePage, ]; - // Clear mock to track fresh calls - mockClickToPosition.mockClear(); - mockResolvePointerPositionHit.mockClear(); + try { + // Clear mock to track fresh calls + mockClickToPosition.mockClear(); + mockResolvePointerPositionHit.mockClear(); - const clickEvent = new MouseEvent('pointerdown', { - bubbles: true, - clientX: 100, - clientY: 100, - button: 0, - }); + const clickEvent = new MouseEvent('pointerdown', { + bubbles: true, + clientX: 100, + clientY: 100, + button: 0, + }); - viewport.dispatchEvent(clickEvent); + viewport.dispatchEvent(clickEvent); - // Verify resolvePointerPositionHit was called (normal flow) - expect(mockResolvePointerPositionHit).toHaveBeenCalled(); + // Verify resolvePointerPositionHit was called (normal flow) + expect(mockResolvePointerPositionHit).toHaveBeenCalled(); + } finally { + if (originalElementsFromPoint === undefined) { + delete (document as unknown as { elementsFromPoint?: unknown }).elementsFromPoint; + } else { + (document as unknown as { elementsFromPoint: unknown }).elementsFromPoint = originalElementsFromPoint; + } + } }); it('should handle case where editor view DOM is not available when layout is not ready', () => { @@ -4901,59 +4910,70 @@ describe('PresentationEditor', () => { y: 0, toJSON: () => ({}), } as DOMRect); + const originalElementsFromPoint = (document as unknown as { elementsFromPoint?: unknown }).elementsFromPoint; (document as unknown as { elementsFromPoint: (x: number, y: number) => Element[] }).elementsFromPoint = () => [ page0, ]; - // pointerdown: page 0 (mounted), pointermove: page 1 (unmounted), pointerup finalize: page 1 - mockClickToPosition.mockReset(); - mockClickToPosition - .mockReturnValueOnce({ pos: 1, layoutEpoch: 0, pageIndex: 0 }) - .mockReturnValueOnce({ pos: 10, layoutEpoch: 0, pageIndex: 1 }) - .mockReturnValueOnce({ pos: 12, layoutEpoch: 0, pageIndex: 1 }); - mockResolvePointerPositionHit.mockReset(); - mockResolvePointerPositionHit - .mockReturnValueOnce({ pos: 1, layoutEpoch: 0, pageIndex: 0, blockId: '', column: 0, lineIndex: -1 }) - .mockReturnValueOnce({ pos: 10, layoutEpoch: 0, pageIndex: 1, blockId: '', column: 0, lineIndex: -1 }) - .mockReturnValueOnce({ pos: 12, layoutEpoch: 0, pageIndex: 1, blockId: '', column: 0, lineIndex: -1 }); - - viewport.dispatchEvent( - new MouseEvent('pointerdown', { - bubbles: true, - clientX: 120, - clientY: 200, - button: 0, - }), - ); - - viewport.dispatchEvent( - new MouseEvent('pointermove', { - bubbles: true, - clientX: 120, - clientY: 900, - buttons: 1, - }), - ); - - const lastPinsBeforePointerUp = setPins.mock.calls[setPins.mock.calls.length - 1]?.[0] as number[] | undefined; - expect(lastPinsBeforePointerUp).toEqual([0, 1, 2]); - - // Simulate virtualization mounting the endpoint page before pointerup finalization. - const page1 = document.createElement('div'); - page1.setAttribute('data-page-index', '1'); - pagesHost.appendChild(page1); - - viewport.dispatchEvent( - new MouseEvent('pointerup', { - bubbles: true, - clientX: 120, - clientY: 900, - button: 0, - }), - ); - - // pointerup should attempt a DOM-refined finalize after using geometry fallback. - expect(mockResolvePointerPositionHit).toHaveBeenCalledTimes(3); + try { + // pointerdown: page 0 (mounted), pointermove: page 1 (unmounted), pointerup finalize: page 1 + mockClickToPosition.mockReset(); + mockClickToPosition + .mockReturnValueOnce({ pos: 1, layoutEpoch: 0, pageIndex: 0 }) + .mockReturnValueOnce({ pos: 10, layoutEpoch: 0, pageIndex: 1 }) + .mockReturnValueOnce({ pos: 12, layoutEpoch: 0, pageIndex: 1 }); + mockResolvePointerPositionHit.mockReset(); + mockResolvePointerPositionHit + .mockReturnValueOnce({ pos: 1, layoutEpoch: 0, pageIndex: 0, blockId: '', column: 0, lineIndex: -1 }) + .mockReturnValueOnce({ pos: 10, layoutEpoch: 0, pageIndex: 1, blockId: '', column: 0, lineIndex: -1 }) + .mockReturnValueOnce({ pos: 12, layoutEpoch: 0, pageIndex: 1, blockId: '', column: 0, lineIndex: -1 }); + + viewport.dispatchEvent( + new MouseEvent('pointerdown', { + bubbles: true, + clientX: 120, + clientY: 200, + button: 0, + }), + ); + + viewport.dispatchEvent( + new MouseEvent('pointermove', { + bubbles: true, + clientX: 120, + clientY: 900, + buttons: 1, + }), + ); + + const lastPinsBeforePointerUp = setPins.mock.calls[setPins.mock.calls.length - 1]?.[0] as + | number[] + | undefined; + expect(lastPinsBeforePointerUp).toEqual([0, 1, 2]); + + // Simulate virtualization mounting the endpoint page before pointerup finalization. + const page1 = document.createElement('div'); + page1.setAttribute('data-page-index', '1'); + pagesHost.appendChild(page1); + + viewport.dispatchEvent( + new MouseEvent('pointerup', { + bubbles: true, + clientX: 120, + clientY: 900, + button: 0, + }), + ); + + // pointerup should attempt a DOM-refined finalize after using geometry fallback. + expect(mockResolvePointerPositionHit).toHaveBeenCalledTimes(3); + } finally { + if (originalElementsFromPoint === undefined) { + delete (document as unknown as { elementsFromPoint?: unknown }).elementsFromPoint; + } else { + (document as unknown as { elementsFromPoint: unknown }).elementsFromPoint = originalElementsFromPoint; + } + } }); }); From 09b3162b4520a54b8ae7ec95a6469f6c68298470 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Wed, 6 May 2026 14:43:17 -0300 Subject: [PATCH 4/4] test(presentation-editor): cover SD-2749 gap click in story session Pins the implicit invariant that an active note session is exited before useActiveSurfaceHitTest is computed, so the new pointerOffAnyPage bail also catches inter-page gap clicks during footnote/endnote editing. Fails on main, passes on this branch. --- ...rInputManager.storySessionGapClick.test.ts | 230 ++++++++++++++++++ 1 file changed, 230 insertions(+) create mode 100644 packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.storySessionGapClick.test.ts diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.storySessionGapClick.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.storySessionGapClick.test.ts new file mode 100644 index 0000000000..cb83f36d00 --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.storySessionGapClick.test.ts @@ -0,0 +1,230 @@ +import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest'; + +import { resolvePointerPositionHit } from '../input/PositionHitResolver.js'; +import { TextSelection } from 'prosemirror-state'; + +import { + EditorInputManager, + type EditorInputDependencies, + type EditorInputCallbacks, +} from '../pointer-events/EditorInputManager.js'; + +vi.mock('../input/PositionHitResolver.js', () => ({ + resolvePointerPositionHit: vi.fn(() => ({ + pos: 24, + layoutEpoch: 1, + pageIndex: 0, + blockId: 'note-1', + column: 0, + lineIndex: -1, + })), +})); + +vi.mock('@superdoc/layout-bridge', () => ({ + clickToPosition: vi.fn(() => ({ pos: 24, layoutEpoch: 1, pageIndex: 0, blockId: 'note-1' })), + getFragmentAtPosition: vi.fn(() => null), +})); + +vi.mock('prosemirror-state', async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + TextSelection: { + ...original.TextSelection, + create: vi.fn(() => ({ + empty: true, + $from: { parent: { inlineContent: true } }, + })), + }, + }; +}); + +// SD-2749 regression lock: when a footnote/endnote story session is active and +// the user clicks in the inter-page gap (no .superdoc-page under the cursor, +// pageIndex undefined), the click must NOT dispatch a selection on the story +// editor. The new pointerOffAnyPage bail is gated behind !useActiveSurfaceHitTest, +// which would otherwise leave the gap-click path unprotected for story +// sessions. In practice the active note session is exited earlier in the +// pointer-down handler whenever the click target isn't a note element (see +// EditorInputManager.ts handlePointerDown, ~line 1442), which flips +// activeStorySession to null before useActiveSurfaceHitTest is computed and +// lets the new bail apply. This test pins that invariant: future refactors +// that delay or skip the session-exit step would re-open the gap-jump bug +// inside story sessions, and this test would fail. + +describe('EditorInputManager - inter-page gap click while story session active', () => { + let manager: EditorInputManager; + let viewportHost: HTMLElement; + let visibleHost: HTMLElement; + let bodyEditor: ReturnType; + let storyEditor: ReturnType; + let mockDeps: EditorInputDependencies; + let mockCallbacks: EditorInputCallbacks; + + function buildEditor(docSize: number) { + return { + isEditable: true, + state: { + doc: { + content: { size: docSize }, + nodesBetween: vi.fn((_from, _to, cb) => { + cb({ isTextblock: true }, 0); + }), + }, + tr: { + setSelection: vi.fn().mockReturnThis(), + setStoredMarks: vi.fn().mockReturnThis(), + }, + selection: { $anchor: null }, + storedMarks: null, + }, + view: { + dispatch: vi.fn(), + dom: document.createElement('div'), + focus: vi.fn(), + hasFocus: vi.fn(() => false), + }, + on: vi.fn(), + off: vi.fn(), + emit: vi.fn(), + }; + } + + beforeEach(() => { + viewportHost = document.createElement('div'); + viewportHost.className = 'presentation-editor__viewport'; + visibleHost = document.createElement('div'); + visibleHost.className = 'presentation-editor__visible'; + visibleHost.appendChild(viewportHost); + + const container = document.createElement('div'); + container.className = 'presentation-editor'; + container.appendChild(visibleHost); + document.body.appendChild(container); + + bodyEditor = buildEditor(10_000); + storyEditor = buildEditor(80); + + mockDeps = { + // Active story session means getActiveEditor() returns the story editor. + getActiveEditor: vi.fn(() => storyEditor as unknown as ReturnType), + getEditor: vi.fn(() => bodyEditor as unknown as ReturnType), + // The piece under test: a footnote/endnote session is active. + getActiveStorySession: vi.fn( + () => + ({ + kind: 'note', + locator: { storyType: 'footnote', noteId: '1' }, + }) as unknown as ReturnType>, + ), + getLayoutState: vi.fn(() => ({ + layout: { + pageSize: { w: 600, h: 800 }, + pages: [ + { + number: 1, + size: { w: 600, h: 800 }, + margins: { top: 72, right: 72, bottom: 72, left: 72 }, + fragments: [], + }, + ], + } as any, + blocks: [], + measures: [], + })), + getEpochMapper: vi.fn(() => ({ + mapPosFromLayoutToCurrentDetailed: vi.fn(() => ({ ok: true, pos: 24, toEpoch: 1 })), + })) as unknown as EditorInputDependencies['getEpochMapper'], + getViewportHost: vi.fn(() => viewportHost), + getVisibleHost: vi.fn(() => visibleHost), + getLayoutMode: vi.fn(() => 'vertical'), + getHeaderFooterSession: vi.fn(() => null), + getPageGeometryHelper: vi.fn(() => null), + getZoom: vi.fn(() => 1), + isViewLocked: vi.fn(() => false), + getDocumentMode: vi.fn(() => 'editing'), + getPageElement: vi.fn(() => null), + isSelectionAwareVirtualizationEnabled: vi.fn(() => false), + }; + + mockCallbacks = { + // Default: behave like the production normalizer with a real page under + // the cursor. Tests that exercise the gap override this with a + // pageIndex-undefined return. + normalizeClientPoint: vi.fn((clientX: number, clientY: number) => ({ + x: clientX, + y: clientY, + pageIndex: 0, + pageLocalY: clientY, + })), + scheduleSelectionUpdate: vi.fn(), + updateSelectionDebugHud: vi.fn(), + hitTestHeaderFooterRegion: vi.fn(() => null), + // Simulates PresentationEditor.hitTest -> clickToPositionGeometry on the + // active note context. Returns *some* fragment for any coordinate, the + // same way snapToNearestFragment does in production. + hitTest: vi.fn(() => ({ + pos: 24, + layoutEpoch: 1, + pageIndex: 0, + blockId: 'note-1-0', + column: 0, + lineIndex: -1, + })), + }; + + manager = new EditorInputManager(); + manager.setDependencies(mockDeps); + manager.setCallbacks(mockCallbacks); + manager.bind(); + }); + + afterEach(() => { + manager.destroy(); + document.body.innerHTML = ''; + vi.clearAllMocks(); + }); + + function getPointerEventImpl(): typeof PointerEvent | typeof MouseEvent { + return ( + (globalThis as unknown as { PointerEvent?: typeof PointerEvent; MouseEvent: typeof MouseEvent }).PointerEvent ?? + globalThis.MouseEvent + ); + } + + function dispatchPointerDown( + target: HTMLElement, + { clientX = 200, clientY = 410 }: { clientX?: number; clientY?: number } = {}, + ): void { + const PointerEventImpl = getPointerEventImpl(); + target.dispatchEvent( + new PointerEventImpl('pointerdown', { + bubbles: true, + cancelable: true, + button: 0, + buttons: 1, + clientX, + clientY, + } as PointerEventInit), + ); + } + + it('does not dispatch a selection on the story editor for clicks in the gap between pages', () => { + // normalizeClientPoint mirrors PointerNormalization when no .superdoc-page + // is under the cursor: pageIndex/pageLocalY are undefined. + (mockCallbacks.normalizeClientPoint as Mock).mockReturnValueOnce({ x: 200, y: 410 }); + + const target = document.createElement('span'); + viewportHost.appendChild(target); + + dispatchPointerDown(target, { clientX: 200, clientY: 410 }); + + // No selection on either editor + no PM TextSelection means no + // scrollIntoView, so the footnote pane stays put. + expect(TextSelection.create as unknown as Mock).not.toHaveBeenCalled(); + expect(storyEditor.state.tr.setSelection).not.toHaveBeenCalled(); + expect(storyEditor.view.dispatch).not.toHaveBeenCalled(); + expect(bodyEditor.state.tr.setSelection).not.toHaveBeenCalled(); + expect(bodyEditor.view.dispatch).not.toHaveBeenCalled(); + }); +});