diff --git a/packages/document-api/src/footnotes/footnotes.types.ts b/packages/document-api/src/footnotes/footnotes.types.ts index d4d5dacfc3..ab70f9ce3f 100644 --- a/packages/document-api/src/footnotes/footnotes.types.ts +++ b/packages/document-api/src/footnotes/footnotes.types.ts @@ -49,7 +49,11 @@ export interface FootnoteGetInput { } export interface FootnoteInsertInput { - at: TextTarget; + /** + * Where to place the reference marker. Omit to insert at the current + * selection (caret position) — the natural target for toolbar actions. + */ + at?: TextTarget; type: 'footnote' | 'endnote'; content: string; } diff --git a/packages/layout-engine/layout-bridge/src/incrementalLayout.ts b/packages/layout-engine/layout-bridge/src/incrementalLayout.ts index 4412344af4..7a681a6022 100644 --- a/packages/layout-engine/layout-bridge/src/incrementalLayout.ts +++ b/packages/layout-engine/layout-bridge/src/incrementalLayout.ts @@ -1878,6 +1878,38 @@ export async function incrementalLayout( // just inflated dead reserve. Overflow now propagates naturally: // any continuation beyond next-page capacity stays in // pendingByColumn and lands on page+2, page+3, etc. + // Tallest per-column cluster demand for a page's anchored footnotes. + // The carry-forward bump counts only the FIRST LINE of the last entry + // (the rest continues onto the following page); the terminal-page bump + // needs full heights because there is nowhere to continue. + const clusterDemandFor = (targetPageIndex: number, lastEntryFirstLineOnly: boolean): number => { + let demand = 0; + for (let cIdx = 0; cIdx < columnCount; cIdx += 1) { + const ids = idsByColumn.get(targetPageIndex)?.get(cIdx) ?? []; + if (ids.length === 0) continue; + let columnCluster = 0; + for (let i = 0; i < ids.length; i += 1) { + const isLast = i === ids.length - 1; + columnCluster += lastEntryFirstLineOnly && isLast ? firstLineOf(ids[i]) : fullHeightOf(ids[i]); + if (i > 0) columnCluster += safeGap; + } + if (columnCluster > demand) demand = columnCluster; + } + return demand; + }; + + // Physical band cap for a page: content height minus a minimum body strip. + const maxBandFor = (targetPageIndex: number): number => { + const page = layoutForPages.pages?.[targetPageIndex]; + const size = page?.size ?? layoutForPages.pageSize ?? DEFAULT_PAGE_SIZE; + const top = normalizeMargin(page?.margins?.top, DEFAULT_MARGINS.top); + const bottom = normalizeMargin(page?.margins?.bottom, DEFAULT_MARGINS.bottom); + const physicalContentHeight = Math.max(0, size.h - top - bottom); + return Math.max(0, physicalContentHeight - MIN_FOOTNOTE_BODY_HEIGHT * 20); + }; + + const bandOverhead = safeSeparatorSpacingBefore + continuationDividerHeight + safeTopPadding; + if (pageIndex + 1 < pageCount) { let continuationDemand = 0; pendingByColumn.forEach((entries) => { @@ -1889,30 +1921,12 @@ export async function incrementalLayout( }); }); // Next page's mandatory cluster demand (ordered minimum). - let nextClusterDemand = 0; - for (let cIdx = 0; cIdx < columnCount; cIdx += 1) { - const idsNext = idsByColumn.get(pageIndex + 1)?.get(cIdx) ?? []; - if (idsNext.length === 0) continue; - let columnCluster = 0; - for (let i = 0; i < idsNext.length; i += 1) { - const isLast = i === idsNext.length - 1; - columnCluster += isLast ? firstLineOf(idsNext[i]) : fullHeightOf(idsNext[i]); - if (i > 0) columnCluster += safeGap; - } - if (columnCluster > nextClusterDemand) nextClusterDemand = columnCluster; - } + const nextClusterDemand = clusterDemandFor(pageIndex + 1, true); if (continuationDemand > 0 || nextClusterDemand > 0) { - const overhead = safeSeparatorSpacingBefore + continuationDividerHeight + safeTopPadding; - const nextPage = layoutForPages.pages?.[pageIndex + 1]; - const nextPageSize = nextPage?.size ?? layoutForPages.pageSize ?? DEFAULT_PAGE_SIZE; - const nextTop = normalizeMargin(nextPage?.margins?.top, DEFAULT_MARGINS.top); - const nextBottomRaw = normalizeMargin(nextPage?.margins?.bottom, DEFAULT_MARGINS.bottom); - const physicalContentHeight = Math.max(0, nextPageSize.h - nextTop - nextBottomRaw); - const minBodyHeight = MIN_FOOTNOTE_BODY_HEIGHT * 20; - const nextPageMaxBand = Math.max(0, physicalContentHeight - minBodyHeight); + const nextPageMaxBand = maxBandFor(pageIndex + 1); // The band has a single overhead block (separator + padding) // whether or not we have a cluster. - const overheadForBand = nextClusterDemand > 0 || continuationDemand > 0 ? overhead : 0; + const overheadForBand = nextClusterDemand > 0 || continuationDemand > 0 ? bandOverhead : 0; // Mandatory cluster room (cluster slices only, no overhead). const clusterRoomPx = nextClusterDemand > 0 ? Math.min(nextClusterDemand, Math.max(0, nextPageMaxBand - overheadForBand)) : 0; @@ -1924,6 +1938,24 @@ export async function incrementalLayout( const finalReserve = Math.min(clusterRoomPx + continuationToReservePx + overheadForBand, nextPageMaxBand); reserves[pageIndex + 1] = Math.max(reserves[pageIndex + 1] ?? 0, Math.ceil(finalReserve)); } + } else { + // SD-3400: terminal-page footnote reserve bump. + // The carry-forward bump above only runs when there is a next page to + // drain onto. On the LAST page a footnote anchored here has nowhere to + // continue, so once the body fills the page the bodyMaxY-derived + // maxReserve collapses to ~0, placeFootnote can place nothing, and + // reserves[pageIndex] stays 0 — the body never yields and the footnote + // is silently dropped. When the placed reserve is short of the anchored + // demand, bump this page's reserve to that demand (capped at the + // physical band) so the next relayout pass shrinks the body and the + // footnote renders on its anchor page (matching Word). Guarded on + // `< clusterDemand` so pages whose footnote already placed fully are + // untouched — no gap/regression on non-dense pages. + const clusterDemand = clusterDemandFor(pageIndex, false); + if (clusterDemand > 0 && (reserves[pageIndex] ?? 0) < clusterDemand) { + const finalReserve = Math.min(clusterDemand + bandOverhead, maxBandFor(pageIndex)); + reserves[pageIndex] = Math.max(reserves[pageIndex] ?? 0, Math.ceil(finalReserve)); + } } } diff --git a/packages/layout-engine/layout-bridge/test/footnoteDensePageRender.test.ts b/packages/layout-engine/layout-bridge/test/footnoteDensePageRender.test.ts new file mode 100644 index 0000000000..3533734979 --- /dev/null +++ b/packages/layout-engine/layout-bridge/test/footnoteDensePageRender.test.ts @@ -0,0 +1,108 @@ +/** + * SD-3400 (prerequisite): footnotes must still render when the body nearly fills + * a single (terminal) page. + * + * Root cause: the SD-2656 bodyMaxY-anchored reserve (`computeMaxFootnoteReserve`) + * makes the planner's max reserve equal the leftover body-region space + * (`pageH - bottomMargin - bodyMaxY`). When the body fills the page that leftover + * is ~0, so the footnote can't be placed; on a single-page document there is no + * continuation page to overflow onto, and the footnote is silently dropped. + * + * Reproduced live: `Footnote tests.docx` (body fills ~98% of the body region, + * leaving ~17px) renders 0 of its 6 footnotes. `basic-footnotes.docx` (tiny body) + * and multi-page docs render fine — so this is specifically the dense terminal-page + * case. + * + * Invariant: a footnote anchored on a page must render its body. The body must + * yield space (break earlier / grow a page) so the footnote fits on its anchor + * page, rather than being dropped. + */ + +import { describe, it, expect, vi } from 'vitest'; +import type { FlowBlock, Measure } from '@superdoc/contracts'; +import { incrementalLayout } from '../src/incrementalLayout'; + +const makeParagraph = (id: string, text: string, pmStart: number): FlowBlock => ({ + kind: 'paragraph', + id, + runs: [{ text, fontFamily: 'Arial', fontSize: 12, pmStart, pmEnd: pmStart + text.length }], +}); + +const makeMeasure = (lineHeight: number, lineCount: number): Measure => ({ + kind: 'paragraph', + lines: Array.from({ length: lineCount }, (_, i) => ({ + fromRun: 0, + fromChar: i, + toRun: 0, + toChar: i + 1, + width: 200, + ascent: lineHeight * 0.8, + descent: lineHeight * 0.2, + lineHeight, + })), + totalHeight: lineCount * lineHeight, +}); + +const countFootnoteFragments = (layout: { pages: Array<{ fragments: Array<{ blockId?: string }> }> }, idPrefix: string) => { + let count = 0; + for (const page of layout.pages) { + for (const f of page.fragments) { + if (String(f.blockId).startsWith(idPrefix)) count += 1; + } + } + return count; +}; + +describe('SD-3400 prerequisite: footnote render on a dense terminal page', () => { + it('renders the footnote body even when the body nearly fills the only page', async () => { + // Page geometry: body region = 600px (h 744, margins 72/72), line height 20. + // 30 body lines × 20 = 600px → the body fills the region exactly, leaving ~0 + // reserve. The footnote ref is mid-body (line 10), so it is anchored on page 1. + const BODY_LINES = 30; + const LINE_H = 20; + const FOOTNOTE_LINES = 5; + const FOOTNOTE_LINE_H = 12; + + let pos = 0; + const blocks: FlowBlock[] = []; + for (let i = 0; i < BODY_LINES; i += 1) { + const text = `Body line ${i + 1}.`; + blocks.push(makeParagraph(`body-${i}`, text, pos)); + pos += text.length + 1; + } + const refBlock = blocks[9]; + const refPos = (refBlock.kind === 'paragraph' ? (refBlock.runs?.[0]?.pmStart ?? 0) : 0) + 2; + const ftBlock = makeParagraph('footnote-1-0-paragraph', 'Footnote body content here.', 0); + + const measureBlock = vi.fn(async (b: FlowBlock) => { + if (b.id.startsWith('footnote-')) return makeMeasure(FOOTNOTE_LINE_H, FOOTNOTE_LINES); + return makeMeasure(LINE_H, 1); + }); + + const margins = { top: 72, right: 72, bottom: 72, left: 72 }; + const result = await incrementalLayout( + [], + null, + blocks, + { + pageSize: { w: 612, h: 600 + margins.top + margins.bottom }, + margins, + footnotes: { + refs: [{ id: '1', pos: refPos }], + blocksById: new Map([['1', [ftBlock]]]), + topPadding: 6, + dividerHeight: 6, + }, + }, + measureBlock, + ); + + // The footnote body must render somewhere (it is currently dropped → 0). + const noteFragments = countFootnoteFragments(result.layout, 'footnote-1'); + expect(noteFragments).toBeGreaterThan(0); + + // And its separator should render too, confirming the band exists. + const sepFragments = countFootnoteFragments(result.layout, 'footnote-separator'); + expect(sepFragments).toBeGreaterThan(0); + }); +}); diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index 0ad87bf7e7..7cd70ee97b 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -44,6 +44,7 @@ import type { ListBlock, } from '@superdoc/contracts'; import { + computeLinePmRange, LAYOUT_BOUNDARY_SCHEMA, buildLayoutSourceIdentityForFragment, expandRunsForInlineNewlines, @@ -65,6 +66,7 @@ import { containerStyles, containerStylesHorizontal, ensureFieldAnnotationStyles, + ensureFootnoteStyles, ensureFormattingMarksStyles, ensureImageSelectionStyles, ensureLinkStyles, @@ -1301,6 +1303,7 @@ export class DomPainter { ensureSdtContainerStyles(doc); ensureImageSelectionStyles(doc); ensureMathMencloseStyles(doc); + ensureFootnoteStyles(doc); if (!this.isSemanticFlow && this.options.ruler?.enabled) { ensureRulerStyles(doc); } @@ -2440,6 +2443,14 @@ export class DomPainter { pageEl.replaceChild(replacement, current.element); current.element = replacement; current.signature = resolvedSig; + } else if (isNonBodyStoryBlockId(fragment.blockId)) { + // Story fragments (notes, headers/footers) use story-local positions: + // the body transaction mapping does not apply, but the resolved item + // carries FRESH story positions every paint. Shift the painted + // attributes by the fresh-vs-painted delta so reused fragments never + // serve stale positions (SD-3400: stale note ranges broke caret, + // selection, and arrow navigation downstream). + this.updateStoryPositionAttributes(current.element, resolvedItem); } else if (this.currentMapping) { // Fragment NOT rebuilt - update position attributes to reflect document changes this.updatePositionAttributes(current.element, this.currentMapping); @@ -2487,6 +2498,64 @@ export class DomPainter { * using the transaction's mapping. Skips header/footer content (separate PM coordinate space). * Also skips fragments that end before the edit point (their positions don't change). */ + /** + * Refreshes data-pm-start/data-pm-end on a REUSED story fragment from the + * fresh resolved item. Story positions are local to their story document, + * so the body transaction mapping cannot update them; instead the uniform + * shift between the fresh first position and the painted one is applied. + * Exact for unchanged blocks (positions inside one block shift uniformly). + */ + private updateStoryPositionAttributes(fragmentEl: HTMLElement, resolvedItem: ResolvedPaintItem | undefined): void { + if (!resolvedItem || resolvedItem.kind !== 'fragment') return; + + // Fragment-scoped fresh landmark: the pm start of THIS fragment's first + // line (matches what render-line stamps as the first painted attribute, + // including continuation fragments that start mid-block). + let freshStart: number | undefined; + const block = 'block' in resolvedItem ? resolvedItem.block : undefined; + const firstLine = 'content' in resolvedItem ? resolvedItem.content?.lines?.[0]?.line : undefined; + if (block && firstLine) { + const range = computeLinePmRange(block, firstLine); + if (typeof range.pmStart === 'number' && Number.isFinite(range.pmStart)) { + freshStart = range.pmStart; + } + } + if (freshStart == null && block) { + const runs = (block as { runs?: Array<{ pmStart?: number | null }> }).runs; + if (Array.isArray(runs)) { + for (const run of runs) { + if (typeof run?.pmStart === 'number' && Number.isFinite(run.pmStart)) { + freshStart = run.pmStart; + break; + } + } + } + } + if (freshStart == null || !Number.isFinite(freshStart)) return; + + const elements = [fragmentEl, ...Array.from(fragmentEl.querySelectorAll('[data-pm-start], [data-pm-end]'))]; + let paintedStart = Infinity; + for (const el of elements) { + const start = Number(el.dataset.pmStart); + if (Number.isFinite(start)) paintedStart = Math.min(paintedStart, start); + } + if (!Number.isFinite(paintedStart)) return; + + const delta = freshStart - paintedStart; + if (delta === 0) return; + + for (const el of elements) { + const start = Number(el.dataset.pmStart); + if (el.dataset.pmStart !== undefined && el.dataset.pmStart !== '' && Number.isFinite(start)) { + el.dataset.pmStart = String(start + delta); + } + const end = Number(el.dataset.pmEnd); + if (el.dataset.pmEnd !== undefined && el.dataset.pmEnd !== '' && Number.isFinite(end)) { + el.dataset.pmEnd = String(end + delta); + } + } + } + private updatePositionAttributes(fragmentEl: HTMLElement, mapping: PositionMapping): void { // Skip header/footer elements (they use a separate PM coordinate space) if (fragmentEl.closest('.superdoc-page-header, .superdoc-page-footer')) { diff --git a/packages/layout-engine/painters/dom/src/styles.test.ts b/packages/layout-engine/painters/dom/src/styles.test.ts index b8eb27442e..fc0f5d1cd4 100644 --- a/packages/layout-engine/painters/dom/src/styles.test.ts +++ b/packages/layout-engine/painters/dom/src/styles.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { ensureSdtContainerStyles, ensureTrackChangeStyles, lineStyles } from './styles.js'; +import { ensureFootnoteStyles, ensureSdtContainerStyles, ensureTrackChangeStyles, lineStyles } from './styles.js'; describe('lineStyles', () => { it('sets height and lineHeight from the argument', () => { @@ -14,6 +14,34 @@ describe('lineStyles', () => { }); }); +describe('ensureFootnoteStyles', () => { + it('renders a text cursor over footnote and endnote note content (SD-3400)', () => { + ensureFootnoteStyles(document); + + const styleEl = document.querySelector('[data-superdoc-footnote-styles="true"]'); + const cssText = styleEl?.textContent ?? ''; + + // Note fragments are generic .superdoc-fragment elements keyed by block-id prefix. + expect(cssText).toContain('[data-block-id^="footnote-"]'); + expect(cssText).toContain('[data-block-id^="endnote-"]'); + expect(cssText).toContain('[data-block-id^="__sd_semantic_footnote-"]'); + expect(cssText).toContain('cursor: text;'); + }); + + it('signals clickability on body reference markers and highlights the active note (SD-3400)', () => { + ensureFootnoteStyles(document); + + const styleEl = document.querySelector('[data-superdoc-footnote-styles="true"]'); + const cssText = styleEl?.textContent ?? ''; + + expect(cssText).toContain('[data-note-reference]'); + expect(cssText).toContain('cursor: pointer;'); + expect(cssText).toContain('[data-note-reference]:hover'); + expect(cssText).toContain('.sd-note-session-active'); + expect(cssText).toContain('sd-note-activate-pulse'); + }); +}); + describe('ensureSdtContainerStyles', () => { it('exposes hover border tokens for structured content overrides', () => { ensureSdtContainerStyles(document); diff --git a/packages/layout-engine/painters/dom/src/styles.ts b/packages/layout-engine/painters/dom/src/styles.ts index f84115e6f6..4258e4d75d 100644 --- a/packages/layout-engine/painters/dom/src/styles.ts +++ b/packages/layout-engine/painters/dom/src/styles.ts @@ -1154,6 +1154,64 @@ menclose::after { } `; +/** + * SD-3400: footnote/endnote note content uses a text (I-beam) cursor like body + * text, not the default arrow. Note fragments are painted as generic + * `.superdoc-fragment` elements distinguished only by their block-id prefix + * (footnote-/endnote-/__sd_semantic_footnote-/__sd_semantic_endnote-), so the + * cursor rule keys off `data-block-id`. The renderer marks these fragments + * contenteditable=false, so without this rule the browser shows a default arrow + * over editable note text. + */ +const FOOTNOTE_STYLES = ` +[data-block-id^="footnote-"], +[data-block-id^="endnote-"], +[data-block-id^="__sd_semantic_footnote-"], +[data-block-id^="__sd_semantic_endnote-"] { + cursor: text; +} + +/* SD-3400: body reference markers are interactive (double-click opens the + * note). Pointer cursor + a hover pill signal clickability without affecting + * layout (background/box-shadow are paint-only). */ +[data-note-reference] { + cursor: pointer; + border-radius: 2px; + position: relative; +} +/* The painted digit is ~6x11px — far too small to hover or double-click + * reliably. An invisible pseudo-element halo expands the interactive target + * (hover, cursor, clicks all hit the marker span) without moving any text. */ +[data-note-reference]::after { + content: ''; + position: absolute; + inset: -4px -5px; +} +[data-note-reference]:hover { + background-color: var(--sd-content-controls-block-hover-bg, #d3e3fd); + box-shadow: 0 0 0 2px var(--sd-content-controls-block-hover-bg, #d3e3fd); +} + +/* SD-3400: while a note session is open, highlight the note's fragments at the + * page bottom so the focus change is visible. Applied by PresentationEditor on + * activation, re-applied after each paint, removed on session exit. The pulse + * draws the eye when focus jumps from the body reference to the note. */ +.sd-note-session-active { + background-color: rgba(98, 155, 231, 0.07); + /* Thin accent bar with breathing room: the first shadow masks a 3px gap with + * the page background, the second paints a 1px bar beyond it. Box-shadows + * paint outside the box, so the note line itself is untouched. */ + box-shadow: + -3px 0 0 0 var(--sd-page-bg, #ffffff), + -4px 0 0 0 rgba(98, 155, 231, 0.55); + animation: sd-note-activate-pulse 0.6s ease-out 1; +} +@keyframes sd-note-activate-pulse { + 0% { background-color: rgba(98, 155, 231, 0.22); } + 100% { background-color: rgba(98, 155, 231, 0.07); } +} +`; + let printStylesInjected = false; let linkStylesInjected = false; let trackChangeStylesInjected = false; @@ -1162,6 +1220,7 @@ let sdtContainerStylesInjected = false; let fieldAnnotationStylesInjected = false; let imageSelectionStylesInjected = false; let mathMencloseStylesInjected = false; +let footnoteStylesInjected = false; export const ensurePrintStyles = (doc: Document | null | undefined) => { if (printStylesInjected || !doc) return; @@ -1245,3 +1304,16 @@ export const ensureMathMencloseStyles = (doc: Document | null | undefined) => { doc.head?.appendChild(styleEl); mathMencloseStylesInjected = true; }; + +/** + * Injects footnote/endnote interaction styles (text cursor over note content) + * into the document head. Injected once per document lifecycle. (SD-3400) + */ +export const ensureFootnoteStyles = (doc: Document | null | undefined) => { + if (footnoteStylesInjected || !doc) return; + const styleEl = doc.createElement('style'); + styleEl.setAttribute('data-superdoc-footnote-styles', 'true'); + styleEl.textContent = FOOTNOTE_STYLES; + doc.head?.appendChild(styleEl); + footnoteStylesInjected = true; +}; diff --git a/packages/super-editor/src/editors/v1/core/commands/clearNodes.js b/packages/super-editor/src/editors/v1/core/commands/clearNodes.js index eb7950ac20..d0b4de09f1 100644 --- a/packages/super-editor/src/editors/v1/core/commands/clearNodes.js +++ b/packages/super-editor/src/editors/v1/core/commands/clearNodes.js @@ -1,4 +1,5 @@ import { liftTarget } from 'prosemirror-transform'; +import { isNoteStorySession } from './linkedStyleSplitHelpers.js'; /** * Normalize nodes to the default node (paragraph by default). @@ -8,7 +9,7 @@ import { liftTarget } from 'prosemirror-transform'; * it has the highest priority (priority: 1000) and it's loaded first. */ // prettier-ignore -export const clearNodes = () => ({ state, tr, dispatch }) => { +export const clearNodes = () => ({ state, tr, dispatch, editor }) => { const { selection } = tr; const { ranges } = selection; @@ -28,7 +29,12 @@ export const clearNodes = () => ({ state, tr, dispatch }) => { if (node.type.isTextblock) { const { defaultType } = $mappedFrom.parent.contentMatchAt($mappedFrom.index()); - tr.setNodeMarkup(nodeRange.start, defaultType); + // SD-3400: note paragraphs keep their note style (FootnoteText) even + // when normalized — clearing it makes them render at the body size. + const preservedAttrs = isNoteStorySession(editor) + ? { paragraphProperties: node.attrs?.paragraphProperties } + : undefined; + tr.setNodeMarkup(nodeRange.start, defaultType, preservedAttrs); } if (targetLiftDepth || targetLiftDepth === 0) { diff --git a/packages/super-editor/src/editors/v1/core/commands/createParagraphNear.js b/packages/super-editor/src/editors/v1/core/commands/createParagraphNear.js index 7f291c1545..9fc3cb5bd2 100644 --- a/packages/super-editor/src/editors/v1/core/commands/createParagraphNear.js +++ b/packages/super-editor/src/editors/v1/core/commands/createParagraphNear.js @@ -1,9 +1,23 @@ import { createParagraphNear as originalCreateParagraphNear } from 'prosemirror-commands'; +import { isNoteStorySession } from './linkedStyleSplitHelpers.js'; +import { findParagraphDepth, restoreParagraphPropertiesAfterDispatch } from './noteParagraphStyle.js'; /** * Create a paragraph nearby. + * + * SD-3400: the ProseMirror base command creates the new paragraph with + * default (empty) attributes. In a note story session the new paragraph must + * keep the note's paragraph style (FootnoteText/EndnoteText), otherwise it + * renders at the document default size. */ //prettier-ignore -export const createParagraphNear = () => ({ state, dispatch }) => { - return originalCreateParagraphNear(state, dispatch); +export const createParagraphNear = () => ({ state, dispatch, editor }) => { + if (!dispatch || !isNoteStorySession(editor)) { + return originalCreateParagraphNear(state, dispatch); + } + + const sourceDepth = findParagraphDepth(state.selection.$from); + const sourceProps = sourceDepth ? state.selection.$from.node(sourceDepth).attrs?.paragraphProperties : null; + + return originalCreateParagraphNear(state, restoreParagraphPropertiesAfterDispatch(dispatch, sourceProps)); }; diff --git a/packages/super-editor/src/editors/v1/core/commands/index.js b/packages/super-editor/src/editors/v1/core/commands/index.js index 8e8a325d8d..3205344db4 100644 --- a/packages/super-editor/src/editors/v1/core/commands/index.js +++ b/packages/super-editor/src/editors/v1/core/commands/index.js @@ -53,6 +53,7 @@ export * from './backspaceNextToRun.js'; export * from './backspaceAcrossRuns.js'; export * from './backspaceAtomBefore.js'; export * from './selectInlineSdtBeforeRunStart.js'; +export * from './selectFootnoteMarkerBefore.js'; export * from './selectBlockSdtAtTextBlockBoundary.js'; export * from './deleteBlockSdtAtTextBlockStart.js'; export * from './moveIntoBlockSdtBeforeTextBlockStart.js'; diff --git a/packages/super-editor/src/editors/v1/core/commands/joinBackward.js b/packages/super-editor/src/editors/v1/core/commands/joinBackward.js index 983a8233da..a5a66fe194 100644 --- a/packages/super-editor/src/editors/v1/core/commands/joinBackward.js +++ b/packages/super-editor/src/editors/v1/core/commands/joinBackward.js @@ -1,4 +1,6 @@ import { joinBackward as originalJoinBackward } from 'prosemirror-commands'; +import { isNoteStorySession } from './linkedStyleSplitHelpers.js'; +import { findParagraphDepth, restoreParagraphPropertiesAfterDispatch } from './noteParagraphStyle.js'; /** * Join two nodes backward. @@ -14,10 +16,16 @@ import { joinBackward as originalJoinBackward } from 'prosemirror-commands'; * https://prosemirror.net/docs/ref/#commands.joinBackward */ //prettier-ignore -export const joinBackward = () => ({ state, dispatch }) => { +export const joinBackward = () => ({ state, dispatch, editor }) => { const { selection, doc } = state; const { $from } = selection; + // SD-3400: in note sessions, the merged paragraph must keep the note + // paragraph style. PM's join keeps the first paragraph's attrs in simple + // joins, but the deleteBarrier restructuring path can drop them. + const guardedDispatch = (paragraphProps) => + isNoteStorySession(editor) ? restoreParagraphPropertiesAfterDispatch(dispatch, paragraphProps) : dispatch; + if ( !$from.parent.isTextblock || $from.parentOffset > 0 @@ -36,5 +44,11 @@ export const joinBackward = () => ({ state, dispatch }) => { return false; } - return originalJoinBackward(state, dispatch); + // The join survivor is the paragraph BEFORE the cut; its style wins (Word). + const survivorProps = + nodeBefore?.type?.name === 'paragraph' + ? nodeBefore.attrs?.paragraphProperties + : (findParagraphDepth($from) ? $from.node(findParagraphDepth($from)).attrs?.paragraphProperties : null); + + return originalJoinBackward(state, dispatch ? guardedDispatch(survivorProps) : dispatch); }; diff --git a/packages/super-editor/src/editors/v1/core/commands/joinForward.js b/packages/super-editor/src/editors/v1/core/commands/joinForward.js index de7cb81b57..58cac1105a 100644 --- a/packages/super-editor/src/editors/v1/core/commands/joinForward.js +++ b/packages/super-editor/src/editors/v1/core/commands/joinForward.js @@ -1,4 +1,6 @@ import { joinForward as originalJoinForward } from 'prosemirror-commands'; +import { isNoteStorySession } from './linkedStyleSplitHelpers.js'; +import { findParagraphDepth, restoreParagraphPropertiesAfterDispatch } from './noteParagraphStyle.js'; /** * Join two nodes forward. @@ -12,7 +14,13 @@ import { joinForward as originalJoinForward } from 'prosemirror-commands'; * https://prosemirror.net/docs/ref/#commands.joinForward */ //prettier-ignore -export const joinForward = () => ({ state, dispatch }) => { +export const joinForward = () => ({ state, dispatch, editor }) => { + // SD-3400: keep the current paragraph's note style on forward joins. + if (dispatch && isNoteStorySession(editor)) { + const depth = findParagraphDepth(state.selection.$from); + const props = depth ? state.selection.$from.node(depth).attrs?.paragraphProperties : null; + dispatch = restoreParagraphPropertiesAfterDispatch(dispatch, props); + } const { selection, doc } = state; const { $from } = selection; diff --git a/packages/super-editor/src/editors/v1/core/commands/liftEmptyBlock.js b/packages/super-editor/src/editors/v1/core/commands/liftEmptyBlock.js index e2e8d7213a..6c9cff9f57 100644 --- a/packages/super-editor/src/editors/v1/core/commands/liftEmptyBlock.js +++ b/packages/super-editor/src/editors/v1/core/commands/liftEmptyBlock.js @@ -1,9 +1,24 @@ import { liftEmptyBlock as originalLiftEmptyBlock } from 'prosemirror-commands'; +import { isNoteStorySession } from './linkedStyleSplitHelpers.js'; +import { findParagraphDepth, restoreParagraphPropertiesAfterDispatch } from './noteParagraphStyle.js'; /** * If the cursor is in an empty textblock that can be lifted, lift the block. * + * SD-3400: when the base command splits instead of lifting, the resulting + * paragraph can lose its attributes; in note story sessions re-stamp the + * note paragraph style. + * * https://prosemirror.net/docs/ref/#commands.liftEmptyBlock */ //prettier-ignore -export const liftEmptyBlock = () => ({ state, dispatch }) => originalLiftEmptyBlock(state, dispatch); +export const liftEmptyBlock = () => ({ state, dispatch, editor }) => { + if (!dispatch || !isNoteStorySession(editor)) { + return originalLiftEmptyBlock(state, dispatch); + } + + const sourceDepth = findParagraphDepth(state.selection.$from); + const sourceProps = sourceDepth ? state.selection.$from.node(sourceDepth).attrs?.paragraphProperties : null; + + return originalLiftEmptyBlock(state, restoreParagraphPropertiesAfterDispatch(dispatch, sourceProps)); +}; diff --git a/packages/super-editor/src/editors/v1/core/commands/linkedStyleSplitHelpers.js b/packages/super-editor/src/editors/v1/core/commands/linkedStyleSplitHelpers.js index 03c65bbdc6..0202770995 100644 --- a/packages/super-editor/src/editors/v1/core/commands/linkedStyleSplitHelpers.js +++ b/packages/super-editor/src/editors/v1/core/commands/linkedStyleSplitHelpers.js @@ -1,5 +1,14 @@ import { readTranslatedLinkedStyles } from '@core/parts/adapters/styles-read.js'; +/** + * SD-3400: note story sessions (footnote/endnote editing) keep their + * paragraph style across structural edits — Word's FootnoteText/EndnoteText + * have no w:next, so new and merged paragraphs stay note-styled. Header and + * footer stories keep body behavior. + */ +export const isNoteStorySession = (editor) => + Boolean(editor?.options?.parentEditor && !editor?.options?.isHeaderOrFooter); + export const isLinkedParagraphStyleId = (editor, styleId) => { if (!styleId) return false; @@ -18,6 +27,11 @@ export const isLinkedCharacterStyleId = (editor, styleId) => { export const clearInheritedLinkedStyleId = (attrs, editor, { emptyParagraph = false } = {}) => { if (!emptyParagraph) return attrs; + // SD-3400: pressing Enter in a footnote continues with the note style; + // clearing it here made new note paragraphs render at the document default + // size. The clearing heuristic targets body heading-like flows, so it stays + // active for the body and header/footer. + if (isNoteStorySession(editor)) return attrs; if (!attrs || typeof attrs !== 'object') return attrs; const paragraphProperties = attrs.paragraphProperties; const styleId = paragraphProperties?.styleId; diff --git a/packages/super-editor/src/editors/v1/core/commands/linkedStyleSplitHelpers.test.js b/packages/super-editor/src/editors/v1/core/commands/linkedStyleSplitHelpers.test.js index 048f11f05a..4013d655d4 100644 --- a/packages/super-editor/src/editors/v1/core/commands/linkedStyleSplitHelpers.test.js +++ b/packages/super-editor/src/editors/v1/core/commands/linkedStyleSplitHelpers.test.js @@ -104,6 +104,30 @@ describe('linkedStyleSplitHelpers', () => { }); describe('clearInheritedLinkedStyleId', () => { + it('keeps the linked paragraph style inside a note story session (SD-3400)', () => { + // FootnoteText is a linked style (w:link FootnoteTextChar) but has no + // w:next: Word keeps it on Enter inside a footnote. Clearing it made + // new note paragraphs render at the document default size once the + // linked-styles cache was populated. + const editor = { + options: { parentEditor: {}, isHeaderOrFooter: false }, + converter: { + translatedLinkedStyles: { + styles: { + FootnoteText: { styleId: 'FootnoteText', type: 'paragraph', link: 'FootnoteTextChar' }, + }, + }, + }, + }; + const attrs = { + paragraphProperties: { styleId: 'FootnoteText' }, + }; + + const result = clearInheritedLinkedStyleId(attrs, editor, { emptyParagraph: true }); + + expect(result).toBe(attrs); + }); + it('removes styleId when it belongs to a linked paragraph style', () => { const editor = { converter: { diff --git a/packages/super-editor/src/editors/v1/core/commands/noteParagraphStyle.js b/packages/super-editor/src/editors/v1/core/commands/noteParagraphStyle.js new file mode 100644 index 0000000000..ac18b185f2 --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/commands/noteParagraphStyle.js @@ -0,0 +1,42 @@ +/** + * SD-3400: paragraph-style preservation for note story sessions. + * + * Several ProseMirror base commands (createParagraphNear, liftEmptyBlock, + * joinBackward/joinForward via deleteBarrier) can produce paragraphs with + * default attributes. Inside a footnote/endnote session that drops the + * FootnoteText/EndnoteText style, so the paragraph renders at the document + * default size instead of the note size. These helpers re-stamp the source + * paragraph's `paragraphProperties` onto the affected paragraph after the + * base command runs, in the same transaction. + */ + +/** Depth of the nearest paragraph ancestor at a resolved position, or null. */ +export function findParagraphDepth($pos) { + for (let depth = $pos.depth; depth >= 1; depth -= 1) { + if ($pos.node(depth).type.name === 'paragraph') return depth; + } + return null; +} + +/** + * Wraps a dispatch so that, after the base command builds its transaction, + * the paragraph holding the selection gets `sourceProps` re-stamped when the + * command left it without a styleId. No-op when sourceProps carries no + * styleId or the paragraph kept its own. + */ +export function restoreParagraphPropertiesAfterDispatch(dispatch, sourceProps) { + if (!sourceProps?.styleId) return dispatch; + + return (tr) => { + const $head = tr.selection.$head; + const depth = findParagraphDepth($head); + if (depth) { + const paragraph = $head.node(depth); + if (paragraph.attrs?.paragraphProperties?.styleId == null) { + const pos = $head.before(depth); + tr.setNodeMarkup(pos, undefined, { ...paragraph.attrs, paragraphProperties: sourceProps }); + } + } + dispatch(tr); + }; +} diff --git a/packages/super-editor/src/editors/v1/core/commands/noteParagraphStyle.test.js b/packages/super-editor/src/editors/v1/core/commands/noteParagraphStyle.test.js new file mode 100644 index 0000000000..436310869c --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/commands/noteParagraphStyle.test.js @@ -0,0 +1,96 @@ +/** + * SD-3400: paragraphs inside a note story session must NEVER lose their + * paragraph style (FootnoteText), no matter which Enter/Backspace path + * created or merged them. The user-reported corruption appeared after bursts + * of Enter/typing/Backspace once the linked-styles cache populated. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { TextSelection } from 'prosemirror-state'; +import { initTestEditor } from '../../tests/helpers/helpers.js'; +import { handleEnter, handleBackspace } from '../extensions/keymap.js'; + +const NOTE_DOC = { + type: 'doc', + content: [ + { + type: 'paragraph', + attrs: { paragraphProperties: { styleId: 'FootnoteText' } }, + content: [{ type: 'run', content: [{ type: 'text', text: 'First note line' }] }], + }, + ], +}; + +function makeNoteSessionEditor() { + const { editor } = initTestEditor({ loadFromSchema: true, content: NOTE_DOC }); + // Mark as a note story session (what createStoryEditor sets up). + editor.options.parentEditor = {}; + editor.options.isHeaderOrFooter = false; + // Arm the linked-styles cache: FootnoteText IS a linked style in real + // documents (w:link FootnoteTextChar), which is what made the clearing + // heuristic fire once the cache populated mid-session. + editor.converter = { + ...(editor.converter ?? {}), + translatedLinkedStyles: { + styles: { + FootnoteText: { styleId: 'FootnoteText', type: 'paragraph', link: 'FootnoteTextChar' }, + }, + }, + }; + return editor; +} + +function paragraphStyleIds(editor) { + const ids = []; + editor.state.doc.forEach((node) => { + ids.push(node.attrs?.paragraphProperties?.styleId ?? null); + }); + return ids; +} + +function typeText(editor, text) { + editor.dispatch(editor.state.tr.insertText(text)); +} + +describe('note session paragraph style preservation (SD-3400)', () => { + let editor; + + afterEach(() => { + editor?.destroy(); + editor = null; + }); + + it('keeps FootnoteText through a burst of Enters, typing, and Backspaces', () => { + editor = makeNoteSessionEditor(); + // caret to end of content + const endPos = editor.state.doc.content.size - 2; + editor.dispatch(editor.state.tr.setSelection(TextSelection.create(editor.state.doc, endPos))); + + // Burst through the REAL keymap chains: Enter x3, type, Enter x2, type, + // Backspace x4, type. + handleEnter(editor); + handleEnter(editor); + handleEnter(editor); + typeText(editor, 'first words'); + handleEnter(editor); + handleEnter(editor); + typeText(editor, 'second words'); + handleBackspace(editor); + handleBackspace(editor); + handleBackspace(editor); + handleBackspace(editor); + typeText(editor, 'tail'); + + const ids = paragraphStyleIds(editor); + expect(ids.length).toBeGreaterThan(1); + expect(ids.every((id) => id === 'FootnoteText')).toBe(true); + }); + + it('keeps FootnoteText when clearNodes normalizes a note paragraph', () => { + editor = makeNoteSessionEditor(); + + editor.commands.clearNodes(); + + expect(paragraphStyleIds(editor)).toEqual(['FootnoteText']); + }); +}); diff --git a/packages/super-editor/src/editors/v1/core/commands/selectFootnoteMarkerBefore.js b/packages/super-editor/src/editors/v1/core/commands/selectFootnoteMarkerBefore.js new file mode 100644 index 0000000000..4eb6c67d71 --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/commands/selectFootnoteMarkerBefore.js @@ -0,0 +1,117 @@ +import { TextSelection } from 'prosemirror-state'; + +export const SELECT_FOOTNOTE_MARKER_META = 'selectFootnoteMarker'; + +const isNoteReference = (node) => + node?.type.name === 'footnoteReference' || node?.type.name === 'endnoteReference'; + +/** + * Resolves the note marker ending at `boundaryPos` (the position right after it). + * Real documents wrap each reference in its own run, so the sibling at the + * boundary is usually that run wrapper, not the marker — look at its last child. + */ +function markerEndingAt(node, boundaryPos) { + if (isNoteReference(node)) { + return { node, pos: boundaryPos - node.nodeSize }; + } + if (node?.type.name === 'run' && isNoteReference(node.lastChild)) { + const marker = node.lastChild; + // Marker sits at the end of the run's content, just inside the closing token. + return { node: marker, pos: boundaryPos - 1 - marker.nodeSize }; + } + return null; +} + +/** Forward mirror of {@link markerEndingAt}: marker starting at `boundaryPos`. */ +function markerStartingAt(node, boundaryPos) { + if (isNoteReference(node)) { + return { node, pos: boundaryPos }; + } + if (node?.type.name === 'run' && isNoteReference(node.firstChild)) { + // Marker sits at the start of the run's content, just inside the opening token. + return { node: node.firstChild, pos: boundaryPos + 1 }; + } + return null; +} + +function getPreviousNoteMarker(state) { + const { $from } = state.selection; + + // Caret at the start of a run: the marker (or its run wrapper) precedes the run. + if ($from.parent.type.name === 'run' && $from.parentOffset === 0) { + const runStart = $from.before($from.depth); + return markerEndingAt(state.doc.resolve(runStart).nodeBefore, runStart); + } + + return markerEndingAt($from.nodeBefore, $from.pos); +} + +function getNextNoteMarker(state) { + const { $from } = state.selection; + + // Caret at the end of a run: the marker (or its run wrapper) follows the run. + if ($from.parent.type.name === 'run' && $from.parentOffset === $from.parent.content.size) { + const runEnd = $from.after($from.depth); + return markerStartingAt(state.doc.resolve(runEnd).nodeAfter, runEnd); + } + + return markerStartingAt($from.nodeAfter, $from.pos); +} + +function selectNoteMarker(state, dispatch, marker) { + if (dispatch) { + const from = marker.pos; + const to = marker.pos + marker.node.nodeSize; + dispatch( + state.tr.setMeta(SELECT_FOOTNOTE_MARKER_META, true).setSelection(TextSelection.create(state.doc, from, to)), + ); + } + + return true; +} + +/** + * SD-3400: Word-like staged delete of footnote/endnote markers. + * + * When Backspace is pressed with a collapsed caret immediately after a + * footnote/endnote reference marker, select the marker instead of deleting it. + * The next Backspace sees a non-empty selection, so this command returns false + * and the chain falls through to `deleteSelection`, which removes the marker and + * lets the footnote renumber (and drop from the note area, since the renderer + * only paints notes that still have a body reference). + * + * `footnoteReference` is `selectable: false`, so a `TextSelection` spanning the + * atom is used as the highlight (a `NodeSelection` is unavailable). + * + * @returns {import('@core/commands/types').Command} + */ +export const selectFootnoteMarkerBefore = + () => + ({ state, dispatch }) => { + const { selection } = state; + if (!selection.empty) return false; + + const marker = getPreviousNoteMarker(state); + if (!marker) return false; + + return selectNoteMarker(state, dispatch, marker); + }; + +/** + * SD-3400: forward (Delete-key) mirror of {@link selectFootnoteMarkerBefore}. + * Selects a footnote/endnote marker immediately after the caret on the first + * Delete; the second Delete removes it via the selection fall-through. + * + * @returns {import('@core/commands/types').Command} + */ +export const selectFootnoteMarkerAfter = + () => + ({ state, dispatch }) => { + const { selection } = state; + if (!selection.empty) return false; + + const marker = getNextNoteMarker(state); + if (!marker) return false; + + return selectNoteMarker(state, dispatch, marker); + }; diff --git a/packages/super-editor/src/editors/v1/core/commands/selectFootnoteMarkerBefore.test.js b/packages/super-editor/src/editors/v1/core/commands/selectFootnoteMarkerBefore.test.js new file mode 100644 index 0000000000..ae7f5c8491 --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/commands/selectFootnoteMarkerBefore.test.js @@ -0,0 +1,165 @@ +import { describe, it, expect, vi } from 'vitest'; +import { Schema } from 'prosemirror-model'; +import { EditorState, NodeSelection, TextSelection } from 'prosemirror-state'; +import { selectFootnoteMarkerBefore, selectFootnoteMarkerAfter } from './selectFootnoteMarkerBefore.js'; + +// Mirrors the real document shape: each footnote/endnote reference is an inline +// atom wrapped in its own run (verified against live docs — parentType 'run', +// parentOffset 0). +const makeSchema = () => + new Schema({ + nodes: { + doc: { content: 'block+' }, + paragraph: { group: 'block', content: 'inline*' }, + run: { inline: true, group: 'inline', content: 'inline*' }, + footnoteReference: { + inline: true, + group: 'inline', + atom: true, + selectable: false, + attrs: { id: { default: null } }, + }, + endnoteReference: { + inline: true, + group: 'inline', + atom: true, + selectable: false, + attrs: { id: { default: null } }, + }, + text: { group: 'inline' }, + }, + marks: {}, + }); + +const makeDoc = (schema, refType = 'footnoteReference') => { + const beforeRun = schema.nodes.run.create(null, schema.text('Before')); + const markerRun = schema.nodes.run.create(null, schema.nodes[refType].create({ id: '1' })); + const afterRun = schema.nodes.run.create(null, schema.text('After')); + return schema.node('doc', null, [schema.node('paragraph', null, [beforeRun, markerRun, afterRun])]); +}; + +const findNode = (doc, typeName, predicate = () => true) => { + let result = null; + doc.descendants((node, pos) => { + if (!result && node.type.name === typeName && predicate(node)) { + result = { node, pos, end: pos + node.nodeSize }; + return false; + } + return true; + }); + return result; +}; + +describe('selectFootnoteMarkerBefore', () => { + it.each(['footnoteReference', 'endnoteReference'])('selects a %s marker immediately before the caret', (refType) => { + const schema = makeSchema(); + const doc = makeDoc(schema, refType); + const marker = findNode(doc, refType); + const state = EditorState.create({ schema, doc, selection: TextSelection.create(doc, marker.end) }); + + let dispatched; + const ok = selectFootnoteMarkerBefore()({ state, dispatch: (tr) => (dispatched = tr) }); + + expect(ok).toBe(true); + // TextSelection (not NodeSelection — the marker is selectable:false) spanning the atom. + expect(dispatched.selection).toBeInstanceOf(TextSelection); + expect(dispatched.selection).not.toBeInstanceOf(NodeSelection); + expect(dispatched.selection.from).toBe(marker.pos); + expect(dispatched.selection.to).toBe(marker.end); + }); + + it('selects the marker when the caret is at the start of the FOLLOWING run (marker wrapped in its own run)', () => { + // Real documents wrap each reference in its own run. Clicking just after the + // superscript places the caret at the start of the next text run, where + // nodeBefore is the marker's run wrapper — not the marker itself. The command + // must look inside the wrapper. (Manual-testing regression: first Backspace + // deleted the letter before the marker instead of selecting the marker.) + const schema = makeSchema(); + const doc = makeDoc(schema); + const marker = findNode(doc, 'footnoteReference'); + const afterRun = findNode(doc, 'run', (n) => n.textContent === 'After'); + const state = EditorState.create({ schema, doc, selection: TextSelection.create(doc, afterRun.pos + 1) }); + + let dispatched; + const ok = selectFootnoteMarkerBefore()({ state, dispatch: (tr) => (dispatched = tr) }); + + expect(ok).toBe(true); + expect(dispatched.selection.from).toBe(marker.pos); + expect(dispatched.selection.to).toBe(marker.end); + }); + + it('returns true without dispatching when no dispatch is provided (first-press select is allowed)', () => { + const schema = makeSchema(); + const doc = makeDoc(schema); + const marker = findNode(doc, 'footnoteReference'); + const state = EditorState.create({ schema, doc, selection: TextSelection.create(doc, marker.end) }); + + expect(selectFootnoteMarkerBefore()({ state })).toBe(true); + }); + + it('returns false on the second press (selection already spans the marker) so deleteSelection runs', () => { + const schema = makeSchema(); + const doc = makeDoc(schema); + const marker = findNode(doc, 'footnoteReference'); + const state = EditorState.create({ schema, doc, selection: TextSelection.create(doc, marker.pos, marker.end) }); + const dispatch = vi.fn(); + + expect(selectFootnoteMarkerBefore()({ state, dispatch })).toBe(false); + expect(dispatch).not.toHaveBeenCalled(); + }); + + it('returns false when the node before the caret is not a note marker', () => { + const schema = makeSchema(); + const doc = makeDoc(schema); + const beforeRun = findNode(doc, 'run'); + // Caret in the middle of the leading "Before" run — nothing to stage. + const state = EditorState.create({ schema, doc, selection: TextSelection.create(doc, beforeRun.pos + 3) }); + const dispatch = vi.fn(); + + expect(selectFootnoteMarkerBefore()({ state, dispatch })).toBe(false); + expect(dispatch).not.toHaveBeenCalled(); + }); +}); + +describe('selectFootnoteMarkerAfter', () => { + it.each(['footnoteReference', 'endnoteReference'])('selects a %s marker immediately after the caret', (refType) => { + const schema = makeSchema(); + const doc = makeDoc(schema, refType); + const marker = findNode(doc, refType); + const state = EditorState.create({ schema, doc, selection: TextSelection.create(doc, marker.pos) }); + + let dispatched; + const ok = selectFootnoteMarkerAfter()({ state, dispatch: (tr) => (dispatched = tr) }); + + expect(ok).toBe(true); + expect(dispatched.selection).toBeInstanceOf(TextSelection); + expect(dispatched.selection.from).toBe(marker.pos); + expect(dispatched.selection.to).toBe(marker.end); + }); + + it('selects the marker when the caret is at the end of the PRECEDING run (marker wrapped in its own run)', () => { + const schema = makeSchema(); + const doc = makeDoc(schema); + const marker = findNode(doc, 'footnoteReference'); + const beforeRun = findNode(doc, 'run', (n) => n.textContent === 'Before'); + const state = EditorState.create({ schema, doc, selection: TextSelection.create(doc, beforeRun.end - 1) }); + + let dispatched; + const ok = selectFootnoteMarkerAfter()({ state, dispatch: (tr) => (dispatched = tr) }); + + expect(ok).toBe(true); + expect(dispatched.selection.from).toBe(marker.pos); + expect(dispatched.selection.to).toBe(marker.end); + }); + + it('returns false when the node after the caret is not a note marker', () => { + const schema = makeSchema(); + const doc = makeDoc(schema); + const afterRun = findNode(doc, 'run'); + const state = EditorState.create({ schema, doc, selection: TextSelection.create(doc, afterRun.pos + 1) }); + const dispatch = vi.fn(); + + expect(selectFootnoteMarkerAfter()({ state, dispatch })).toBe(false); + expect(dispatch).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/super-editor/src/editors/v1/core/extensions/keymap-backspace-chain.test.js b/packages/super-editor/src/editors/v1/core/extensions/keymap-backspace-chain.test.js index 40ba420e99..e3848b1d8b 100644 --- a/packages/super-editor/src/editors/v1/core/extensions/keymap-backspace-chain.test.js +++ b/packages/super-editor/src/editors/v1/core/extensions/keymap-backspace-chain.test.js @@ -38,6 +38,8 @@ describe('handleBackspace chain ordering', () => { undoInputRule: make('undoInputRule'), deleteBlockSdtAtTextBlockStart: make('deleteBlockSdtAtTextBlockStart'), selectInlineSdtBeforeRunStart: make('selectInlineSdtBeforeRunStart'), + selectFootnoteMarkerBefore: make('selectFootnoteMarkerBefore'), + deleteSelectedNoteMarker: make('deleteSelectedNoteMarker'), selectBlockSdtBeforeTextBlockStart: make('selectBlockSdtBeforeTextBlockStart'), moveIntoBlockSdtBeforeTextBlockStart: make('moveIntoBlockSdtBeforeTextBlockStart'), backspaceEmptyRunParagraph: make('backspaceEmptyRunParagraph'), @@ -77,6 +79,8 @@ describe('handleBackspace chain ordering', () => { // step 2 sets inputType meta and returns false (no command call) 'deleteBlockSdtAtTextBlockStart', 'selectInlineSdtBeforeRunStart', + 'selectFootnoteMarkerBefore', + 'deleteSelectedNoteMarker', 'selectBlockSdtBeforeTextBlockStart', 'moveIntoBlockSdtBeforeTextBlockStart', 'backspaceEmptyRunParagraph', @@ -109,8 +113,10 @@ describe('handleBackspace chain ordering', () => { expect(callLog[0]).toBe('undoInputRule'); expect(callLog[1]).toBe('deleteBlockSdtAtTextBlockStart'); expect(callLog[2]).toBe('selectInlineSdtBeforeRunStart'); - expect(callLog[3]).toBe('selectBlockSdtBeforeTextBlockStart'); - expect(callLog[4]).toBe('moveIntoBlockSdtBeforeTextBlockStart'); + expect(callLog[3]).toBe('selectFootnoteMarkerBefore'); + expect(callLog[4]).toBe('deleteSelectedNoteMarker'); + expect(callLog[5]).toBe('selectBlockSdtBeforeTextBlockStart'); + expect(callLog[6]).toBe('moveIntoBlockSdtBeforeTextBlockStart'); }); it('places mixedBidiBackspace after backspaceAcrossRuns and before deleteSelection', () => { @@ -182,6 +188,8 @@ describe('handleDelete chain ordering', () => { const commands = { deleteBlockSdtAtTextBlockStart: make('deleteBlockSdtAtTextBlockStart'), selectInlineSdtAfterRunEnd: make('selectInlineSdtAfterRunEnd'), + selectFootnoteMarkerAfter: make('selectFootnoteMarkerAfter'), + deleteSelectedNoteMarker: make('deleteSelectedNoteMarker'), selectBlockSdtAfterTextBlockEnd: make('selectBlockSdtAfterTextBlockEnd'), moveIntoBlockSdtAfterTextBlockEnd: make('moveIntoBlockSdtAfterTextBlockEnd'), deleteSkipEmptyRun: make('deleteSkipEmptyRun'), @@ -216,6 +224,8 @@ describe('handleDelete chain ordering', () => { expect(callLog).toEqual([ 'deleteBlockSdtAtTextBlockStart', 'selectInlineSdtAfterRunEnd', + 'selectFootnoteMarkerAfter', + 'deleteSelectedNoteMarker', 'selectBlockSdtAfterTextBlockEnd', 'moveIntoBlockSdtAfterTextBlockEnd', 'deleteSkipEmptyRun', diff --git a/packages/super-editor/src/editors/v1/core/extensions/keymap.js b/packages/super-editor/src/editors/v1/core/extensions/keymap.js index 2867757f1a..a57821d438 100644 --- a/packages/super-editor/src/editors/v1/core/extensions/keymap.js +++ b/packages/super-editor/src/editors/v1/core/extensions/keymap.js @@ -39,6 +39,8 @@ export const handleBackspace = (editor) => { }, () => commands.deleteBlockSdtAtTextBlockStart(), () => commands.selectInlineSdtBeforeRunStart(), + () => commands.selectFootnoteMarkerBefore?.() ?? false, + () => commands.deleteSelectedNoteMarker?.() ?? false, () => commands.selectBlockSdtBeforeTextBlockStart(), () => commands.moveIntoBlockSdtBeforeTextBlockStart(), () => commands.backspaceEmptyRunParagraph(), @@ -62,6 +64,8 @@ export const handleDelete = (editor) => { return editor.commands.first(({ commands }) => [ () => commands.deleteBlockSdtAtTextBlockStart(), () => commands.selectInlineSdtAfterRunEnd(), + () => commands.selectFootnoteMarkerAfter?.() ?? false, + () => commands.deleteSelectedNoteMarker?.() ?? false, () => commands.selectBlockSdtAfterTextBlockEnd(), () => commands.moveIntoBlockSdtAfterTextBlockEnd(), () => commands.deleteSkipEmptyRun(), diff --git a/packages/super-editor/src/editors/v1/core/layout-adapter/converters/inline-converters/footnote-reference.test.ts b/packages/super-editor/src/editors/v1/core/layout-adapter/converters/inline-converters/footnote-reference.test.ts index 960d446fe1..eee1294963 100644 --- a/packages/super-editor/src/editors/v1/core/layout-adapter/converters/inline-converters/footnote-reference.test.ts +++ b/packages/super-editor/src/editors/v1/core/layout-adapter/converters/inline-converters/footnote-reference.test.ts @@ -49,6 +49,14 @@ describe('footnoteReferenceToBlock', () => { expect(run.text).toBe('1'); }); + it('stamps interaction data attributes on the painted marker (SD-3400)', () => { + const run = footnoteReferenceToBlock(makeParams()); + + // Drives the hover/clickability affordance and active-note matching. + expect(run.dataAttrs?.['data-note-reference']).toBe('footnote'); + expect(run.dataAttrs?.['data-note-id']).toBe('1'); + }); + it('does not emit Unicode superscript glyphs', () => { const run = footnoteReferenceToBlock(makeParams()); diff --git a/packages/super-editor/src/editors/v1/core/layout-adapter/converters/inline-converters/reference-marker.ts b/packages/super-editor/src/editors/v1/core/layout-adapter/converters/inline-converters/reference-marker.ts index c43db55344..fdda95dfcb 100644 --- a/packages/super-editor/src/editors/v1/core/layout-adapter/converters/inline-converters/reference-marker.ts +++ b/packages/super-editor/src/editors/v1/core/layout-adapter/converters/inline-converters/reference-marker.ts @@ -60,6 +60,27 @@ const stripVerticalPositioningFromRunProperties = ( return sanitizedRunProperties; }; +/** + * SD-3400: stamp the painted marker with interaction attributes so the DOM + * painter output can signal clickability (hover affordance) and so the active + * note can be visually matched. `data-note-reference` carries the story type, + * `data-note-id` the note id. + */ +const withReferenceDataAttrs = (run: TextRun, params: InlineConverterParams): TextRun => { + const rawType = (params.node as { type?: string | { name?: string } }).type; + const typeName = typeof rawType === 'string' ? rawType : rawType?.name; + const kind = typeName === 'endnoteReference' ? 'endnote' : 'footnote'; + const id = (params.node.attrs as Record | undefined)?.id; + return { + ...run, + dataAttrs: { + ...run.dataAttrs, + 'data-note-reference': kind, + ...(id != null ? { 'data-note-id': String(id) } : {}), + }, + }; +}; + const copyReferencePmPositions = (run: TextRun, params: InlineConverterParams): TextRun => { const refPos = params.positions.get(params.node); if (!refPos) { @@ -119,19 +140,22 @@ export function buildReferenceMarkerRun(displayText: string, params: InlineConve const originalRun = buildOriginalReferenceRun(displayText, params); if (hasExplicitBaselineShift(originalRun.baselineShift)) { - return copyReferencePmPositions(originalRun, params); + return withReferenceDataAttrs(copyReferencePmPositions(originalRun, params), params); } const runWithoutVerticalPositioning = buildReferenceRunWithoutVerticalPositioning(displayText, params); const baseFontSize = resolveReferenceBaseFontSize(runWithoutVerticalPositioning, originalRun, params.defaultSize); - return copyReferencePmPositions( - { - ...originalRun, - vertAlign: 'superscript', - baselineShift: undefined, - fontSize: baseFontSize * SUBSCRIPT_SUPERSCRIPT_SCALE, - }, + return withReferenceDataAttrs( + copyReferencePmPositions( + { + ...originalRun, + vertAlign: 'superscript', + baselineShift: undefined, + fontSize: baseFontSize * SUBSCRIPT_SUPERSCRIPT_SCALE, + }, + params, + ), params, ); } diff --git a/packages/super-editor/src/editors/v1/core/parts/adapters/notes-part-descriptor.ts b/packages/super-editor/src/editors/v1/core/parts/adapters/notes-part-descriptor.ts index eeb3c3475b..98abbdfe28 100644 --- a/packages/super-editor/src/editors/v1/core/parts/adapters/notes-part-descriptor.ts +++ b/packages/super-editor/src/editors/v1/core/parts/adapters/notes-part-descriptor.ts @@ -128,13 +128,25 @@ export function getNoteElements(part: unknown, childElementName: string): OoxmlE * Used during insert/update to write text content directly into the * canonical OOXML part. The result is valid w:p elements that can be * re-imported by the standard footnote importer. + * + * Each paragraph carries the FootnoteText/EndnoteText paragraph style — + * Word always stamps it on note body paragraphs, and without it exported + * new notes render at the Normal style's size in Word. */ -export function textToNoteOoxmlParagraphs(text: string): OoxmlElement[] { +export function textToNoteOoxmlParagraphs(text: string, childElementName: string): OoxmlElement[] { + const styleName = childElementName === 'w:endnote' ? 'EndnoteText' : 'FootnoteText'; + const pPr: OoxmlElement = { + type: 'element', + name: 'w:pPr', + elements: [{ type: 'element', name: 'w:pStyle', attributes: { 'w:val': styleName } }], + }; + return text.split(/\r?\n/).map((line) => ({ type: 'element', name: 'w:p', - elements: - line.length > 0 + elements: [ + structuredClone(pPr), + ...(line.length > 0 ? [ { type: 'element', @@ -149,7 +161,8 @@ export function textToNoteOoxmlParagraphs(text: string): OoxmlElement[] { ], }, ] - : [], + : []), + ], })); } @@ -220,7 +233,7 @@ export function addNoteElement(part: unknown, config: NotePartConfig, noteId: st throw new Error(`addNoteElement: note id "${noteId}" already exists in ${config.partId}`); } - const paragraphs = textToNoteOoxmlParagraphs(text); + const paragraphs = textToNoteOoxmlParagraphs(text, config.childElementName); ensureFootnoteRefRun(paragraphs, config.childElementName); const noteElement: OoxmlElement = { @@ -245,7 +258,7 @@ export function updateNoteElement(part: unknown, config: NotePartConfig, noteId: const target = notes.find((el) => el.attributes?.['w:id'] === noteId); if (!target) return false; - const paragraphs = textToNoteOoxmlParagraphs(text); + const paragraphs = textToNoteOoxmlParagraphs(text, config.childElementName); ensureFootnoteRefRun(paragraphs, config.childElementName); target.elements = paragraphs; return true; @@ -457,4 +470,35 @@ export function bootstrapNotesPart(editor: Editor, type: 'footnote' | 'endnote') if (converter.convertedXml[config.partId] !== undefined) return; converter.convertedXml[config.partId] = createInitialNotesPart(config); + ensureSpecialNotesListInSettings(converter.convertedXml, config); +} + +/** + * Write the special-note list to `word/settings.xml` alongside a freshly + * bootstrapped notes part (§17.11.9): `w:footnotePr`/`w:endnotePr` listing + * the separator (-1) and continuation separator (0) ids. Strict consumers + * do not load separators that are not listed here. + * + * Imported documents own their settings — this runs only on bootstrap, and + * preserves any existing properties element (it just adds the missing ids). + */ +function ensureSpecialNotesListInSettings(convertedXml: Record, config: NotePartConfig): void { + const settingsRoot = getRootElement(convertedXml['word/settings.xml']); + if (!settingsRoot) return; + if (!settingsRoot.elements) settingsRoot.elements = []; + + const prName = config.childElementName === 'w:endnote' ? 'w:endnotePr' : 'w:footnotePr'; + let pr = settingsRoot.elements.find((el) => el.name === prName); + if (!pr) { + pr = { type: 'element', name: prName, elements: [] }; + settingsRoot.elements.push(pr); + } + if (!pr.elements) pr.elements = []; + + for (const id of ['-1', '0']) { + const listed = pr.elements.some((el) => el.name === config.childElementName && el.attributes?.['w:id'] === id); + if (!listed) { + pr.elements.push({ type: 'element', name: config.childElementName, attributes: { 'w:id': id } }); + } + } } diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts index 56360c4f6a..d8ba0042f2 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts @@ -112,7 +112,9 @@ import { renderCaretOverlay, renderSelectionRects } from './selection/LocalSelec import { computeCaretLayoutRectGeometry as computeCaretLayoutRectGeometryFromHelper } from './selection/CaretGeometry.js'; import { shouldUseNativeCaretFallback } from './selection/native-caret-fallback.js'; import { + computeCaretRectFromPmPosition as computeCaretRectFromPmPositionFromHelper, computeCaretRectFromVisibleTextOffset as computeCaretRectFromVisibleTextOffsetFromHelper, + computeSelectionRectsFromPmRange as computeSelectionRectsFromPmRangeFromHelper, computeSelectionRectsFromVisibleTextOffsets as computeSelectionRectsFromVisibleTextOffsetsFromHelper, measureVisibleTextOffset as measureVisibleTextOffsetFromHelper, measureVisibleTextOffsetInContainers as measureVisibleTextOffsetInContainersFromHelper, @@ -141,6 +143,8 @@ import type { } from './story-session/StoryPresentationSessionManager.js'; import type { StoryPresentationSession } from './story-session/types.js'; import { resolveStoryRuntime } from '../../document-api-adapters/story-runtime/resolve-story-runtime.js'; +import { parseRenderedNoteTarget, type RenderedNoteTarget } from './notes/note-target.js'; +import { NoteSessionCoordinator } from './notes/NoteSessionCoordinator.js'; import { BODY_STORY_KEY, buildStoryKey, parseStoryKey } from '../../document-api-adapters/story-runtime/story-key.js'; import { createStoryEditor } from '../story-editor-factory.js'; import { buildEndnoteBlocks } from './layout/EndnotesBuilder.js'; @@ -238,11 +242,6 @@ type ThreadAnchorScrollPlan = { applyScroll: (behavior: ScrollBehavior) => void; }; -type RenderedNoteTarget = { - storyType: 'footnote' | 'endnote'; - noteId: string; -}; - type UnifiedHistoryDebugGlobal = typeof globalThis & { __SD_DEBUG_UNIFIED_HISTORY__?: boolean; }; @@ -283,28 +282,6 @@ type RenderedNoteFragmentHit = { pageIndex: number; }; -function parseRenderedNoteTarget(blockId: string): RenderedNoteTarget | null { - if (typeof blockId !== 'string' || blockId.length === 0) { - return null; - } - - if (blockId.startsWith('footnote-')) { - const noteId = blockId.slice('footnote-'.length).split('-')[0] ?? ''; - return noteId ? { storyType: 'footnote', noteId } : null; - } - - if (blockId.startsWith('__sd_semantic_footnote-')) { - const noteId = blockId.slice('__sd_semantic_footnote-'.length).split('-')[0] ?? ''; - return noteId ? { storyType: 'footnote', noteId } : null; - } - - if (blockId.startsWith('endnote-')) { - const noteId = blockId.slice('endnote-'.length).split('-')[0] ?? ''; - return noteId ? { storyType: 'endnote', noteId } : null; - } - - return null; -} import { splitRunsAtDecorationBoundaries } from './layout/SplitRunsAtDecorationBoundaries.js'; import { DOM_CLASS_NAMES } from '@superdoc/dom-contract'; import { @@ -510,6 +487,8 @@ export class PresentationEditor extends EventEmitter { #painterHost: HTMLElement; #selectionOverlay: HTMLElement; #permissionOverlay: HTMLElement | null = null; + /** SD-3400: highlight + smart-scroll + emptied-note commit for the open note session. */ + #noteSessionCoordinator: NoteSessionCoordinator | null = null; #hiddenHost: HTMLElement; /** Scroll-isolating wrapper around #hiddenHost. Append/remove this from the DOM. */ #hiddenHostWrapper: HTMLElement; @@ -5183,7 +5162,7 @@ export class PresentationEditor extends EventEmitter { this.#editorInputManager?.clearCellAnchor(); } }; - const handleSelection = () => { + const handleSelection = ({ transaction }: { transaction?: Transaction } = {}) => { // User-initiated selection change — scroll caret/head into view once, except during // pointer drag: EditorInputManager edge auto-scroll must not fight #scrollActiveEndIntoView. if (!this.#editorInputManager?.isDragging) { @@ -5195,7 +5174,13 @@ export class PresentationEditor extends EventEmitter { // setDocEpoch → cancelScheduledRender. Immediate rendering is safe here: // if layout is updating (due to a concurrent doc change), flushNow() // is a no-op and the render will be picked up after layout completes. - this.#scheduleSelectionUpdate({ immediate: true }); + // + // SD-3400: NOT safe for doc-changing transactions. 'selectionUpdate' + // fires BEFORE 'update', so the epoch/layout gates are not armed yet + // and an immediate flush renders the caret against the PRE-change + // paint (visibly stale caret on every Enter/Backspace). Defer those to + // the post-paint flush. + this.#scheduleSelectionUpdate({ immediate: !transaction?.docChanged }); // Update local cursor in awareness for collaboration // This bypasses y-prosemirror's focus check which may fail for hidden PM views this.#updateLocalAwarenessCursor(); @@ -7319,6 +7304,10 @@ export class PresentationEditor extends EventEmitter { this.emit('layoutUpdated', payload); this.emit('paginationUpdate', payload); + // SD-3400: fragments are rebuilt on every paint — re-apply the active + // note highlight and complete any pending scroll-to-note. + this.#noteSessionCoordinator?.onPaint(); + // Emit fresh comment positions after layout completes. // Always emit — even when empty — so the store can clear stale positions // (e.g. when undo removes the last tracked-change mark). @@ -8992,7 +8981,7 @@ export class PresentationEditor extends EventEmitter { #activateRenderedNoteSession( target: RenderedNoteTarget, - options: { clientX: number; clientY: number; pageIndex?: number }, + options: { clientX?: number; clientY?: number; pageIndex?: number }, ): boolean { if ((this.#headerFooterSession?.session?.mode ?? 'body') !== 'body') { this.#headerFooterSession?.exitMode(); @@ -9028,30 +9017,63 @@ export class PresentationEditor extends EventEmitter { }, ); - const hit = this.hitTest(options.clientX, options.clientY); const doc = session.editor.state?.doc; - if (hit && doc) { - try { - const selection = this.#createCollapsedSelectionNearInlineContent(doc, hit.pos); - const tr = session.editor.state.tr.setSelection(selection); - session.editor.view?.dispatch(tr); - } catch { - // Ignore stale pointer hits during activation races. + // SD-3400: pointer activation places the caret at the click position; + // programmatic activation (no coords, e.g. insert-footnote focus) leaves the + // caret at the note's default start so the user can type from the beginning. + if (typeof options.clientX === 'number' && typeof options.clientY === 'number' && doc) { + const hit = this.hitTest(options.clientX, options.clientY); + if (hit) { + try { + const selection = this.#createCollapsedSelectionNearInlineContent(doc, hit.pos); + const tr = session.editor.state.tr.setSelection(selection); + session.editor.view?.dispatch(tr); + } catch { + // Ignore stale pointer hits during activation races. + } } } session.editor.view?.focus(); this.#shouldScrollSelectionIntoView = true; this.#scheduleSelectionUpdate({ immediate: true }); + + // SD-3400: highlight the note, watch for the user emptying it, and bring + // it into view (see NoteSessionCoordinator for the full UX contract). + this.#ensureNoteSessionCoordinator().onActivated(target, session); return true; } + #ensureNoteSessionCoordinator(): NoteSessionCoordinator { + if (!this.#noteSessionCoordinator) { + this.#noteSessionCoordinator = new NoteSessionCoordinator({ + getHost: () => this.#painterHost ?? this.#visibleHost, + getScrollContainer: () => this.#scrollContainer, + hasActiveSession: () => Boolean(this.#getActiveStorySession()), + exitActiveSession: () => this.#exitActiveStorySession(), + }); + } + return this.#noteSessionCoordinator; + } + + /** + * SD-3400: programmatically open a footnote/endnote note session without a + * pointer. Focuses the note and scrolls it into view with the caret at the + * note's start. Used by insert-footnote (and any non-pointer navigation) so + * the user can immediately type in the new note. + */ + activateNoteSession(target: RenderedNoteTarget): boolean { + return this.#activateRenderedNoteSession(target, {}); + } + #exitActiveStorySession(): void { const session = this.#getActiveStorySession(); if (!session) { return; } + this.#noteSessionCoordinator?.onExit(); + this.#storySessionManager?.exit(); this.#pendingDocChange = true; this.#scheduleRerender(); @@ -10245,14 +10267,36 @@ export class PresentationEditor extends EventEmitter { return null; } - const startOffset = this.#measureActiveEditorVisibleTextOffset(Math.min(from, to)); - const endOffset = this.#measureActiveEditorVisibleTextOffset(Math.max(from, to)); - if (startOffset == null || endOffset == null) { + const noteFragments = this.#getRenderedNoteFragmentElements(this.#collectNoteBlockIds(context)); + if (!noteFragments.length) { return null; } - const noteFragments = this.#getRenderedNoteFragmentElements(this.#collectNoteBlockIds(context)); - if (!noteFragments.length) { + const geometryOptions = { + containers: noteFragments, + zoom: this.#layoutOptions.zoom ?? 1, + pageHeight: this.#getBodyPageHeight(), + pageGap: layout.pageGap ?? this.#getEffectivePageGap(), + }; + + // Same block-anchored pm-first strategy as #computeNoteDomCaretRect (SD-3400). + const pmRects = computeSelectionRectsFromPmRangeFromHelper(geometryOptions, from, to, { + from: this.#resolveNoteBlockAnchor(from), + to: this.#resolveNoteBlockAnchor(to), + }); + if (pmRects != null) { + return pmRects; + } + + // Same in-flight-rerender guard as #computeNoteDomCaretRect (SD-3400). + if (this.#renderScheduled || this.#isRerendering || this.#pendingDocChange) { + this.#scheduleSelectionUpdate({ immediate: false }); + return null; + } + + const startOffset = this.#measureActiveEditorVisibleTextOffset(Math.min(from, to)); + const endOffset = this.#measureActiveEditorVisibleTextOffset(Math.max(from, to)); + if (startOffset == null || endOffset == null) { return null; } @@ -10260,12 +10304,7 @@ export class PresentationEditor extends EventEmitter { const renderedEndOffset = this.#toRenderedNoteVisibleTextOffset(noteFragments, endOffset); return computeSelectionRectsFromVisibleTextOffsetsFromHelper( - { - containers: noteFragments, - zoom: this.#layoutOptions.zoom ?? 1, - pageHeight: this.#getBodyPageHeight(), - pageGap: layout.pageGap ?? this.#getEffectivePageGap(), - }, + geometryOptions, renderedStartOffset, renderedEndOffset, ); @@ -10286,6 +10325,46 @@ export class PresentationEditor extends EventEmitter { return selectionToRects(layout, context.blocks, context.measures, from, to, this.#pageGeometryHelper ?? undefined); } + /** + * Anchors a session position to its paragraph block for stale-tolerant + * caret resolution (SD-3400): painted pm ranges of unchanged note + * paragraphs drift after edits, but block identity (sdBlockId) plus the + * block's current first-leaf position let the geometry helper translate + * into the fragment's coordinate space. + */ + #resolveNoteBlockAnchor(pos: number): { sdBlockId: string; currentStart: number } | null { + const doc = this.getActiveEditor()?.state?.doc; + if (!doc || !Number.isFinite(pos)) return null; + try { + const clamped = Math.max(0, Math.min(pos, doc.content.size)); + const $pos = doc.resolve(clamped); + let blockDepth = 0; + for (let depth = $pos.depth; depth >= 1; depth -= 1) { + if ($pos.node(depth).isBlock) blockDepth = depth; + } + if (!blockDepth) return null; + const blockNode = $pos.node(blockDepth); + const sdBlockId = blockNode.attrs?.sdBlockId; + if (typeof sdBlockId !== 'string' || !sdBlockId) return null; + const blockPos = $pos.before(blockDepth); + let currentStart: number | null = null; + doc.nodesBetween(blockPos, blockPos + blockNode.nodeSize, (node, nodePos) => { + if (currentStart != null) return false; + if (node.isInline && (node.isLeaf || node.isText)) { + currentStart = nodePos; + return false; + } + return true; + }); + // Empty paragraph: no inline leaf exists, its only caret position is + // the block's content start. The painted placeholder line anchors there. + if (currentStart == null) currentStart = blockPos + 1; + return { sdBlockId, currentStart }; + } catch { + return null; + } + } + #computeNoteDomCaretRect(context: NoteLayoutContext, pos: number): LayoutRect | null { const layout = this.#layoutState.layout; if (!layout) { @@ -10297,27 +10376,48 @@ export class PresentationEditor extends EventEmitter { return null; } - const textOffset = this.#measureActiveEditorVisibleTextOffset(pos); - if (textOffset == null) { + const noteFragments = this.#getRenderedNoteFragmentElements(noteBlockIds); + if (!noteFragments.length) { return null; } - const noteFragments = this.#getRenderedNoteFragmentElements(noteBlockIds); - if (!noteFragments.length) { + const geometryOptions = { + containers: noteFragments, + zoom: this.#layoutOptions.zoom ?? 1, + pageHeight: this.#getBodyPageHeight(), + pageGap: layout.pageGap ?? this.#getEffectivePageGap(), + }; + + // Resolve by block identity first (stale-tolerant), then by global pm + // ranges. Painted pm ranges of unchanged note paragraphs drift after + // edits, so absolute resolution alone picks wrong lines (SD-3400). + const anchor = this.#resolveNoteBlockAnchor(pos); + const anchoredRect = anchor ? computeCaretRectFromPmPositionFromHelper(geometryOptions, pos, anchor) : null; + if (anchoredRect) { + return anchoredRect; + } + const pmRect = computeCaretRectFromPmPositionFromHelper(geometryOptions, pos); + if (pmRect) { + return pmRect; + } + + // Position not painted yet (fresh paragraph) while a rerender is in + // flight: bridging now would measure STALE paint and the wrong caret + // would stick until the next selection change. Defer to the post-paint + // flush instead (SD-3400). + if (this.#renderScheduled || this.#isRerendering || this.#pendingDocChange) { + this.#scheduleSelectionUpdate({ immediate: false }); + return null; + } + + const textOffset = this.#measureActiveEditorVisibleTextOffset(pos); + if (textOffset == null) { return null; } const renderedTextOffset = this.#toRenderedNoteVisibleTextOffset(noteFragments, textOffset); - return computeCaretRectFromVisibleTextOffsetFromHelper( - { - containers: noteFragments, - zoom: this.#layoutOptions.zoom ?? 1, - pageHeight: this.#getBodyPageHeight(), - pageGap: layout.pageGap ?? this.#getEffectivePageGap(), - }, - renderedTextOffset, - ); + return computeCaretRectFromVisibleTextOffsetFromHelper(geometryOptions, renderedTextOffset); } #computeNoteCaretRect(pos: number): LayoutRect | null { diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/layout/FootnotesBuilder.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/layout/FootnotesBuilder.ts index e70baf8602..586031139e 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/layout/FootnotesBuilder.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/layout/FootnotesBuilder.ts @@ -135,6 +135,7 @@ export function buildFootnotesInput( // Build blocks for each footnote const blocksById = new Map(); + const docDefaultRunProps = resolveDocDefaultRunProps(converterContext); idsInUse.forEach((id) => { try { @@ -154,7 +155,7 @@ export function buildFootnotesInput( if (!customMarkIds.has(id)) { // §17.11.11 — per-id format from section override wins over document default. const numFmtForId = footnoteFormatById?.[id] ?? footnoteNumberFormat; - ensureFootnoteMarker(result.blocks, id, footnoteNumberById, numFmtForId); + ensureFootnoteMarker(result.blocks, id, footnoteNumberById, numFmtForId, docDefaultRunProps); } blocksById.set(id, result.blocks); } @@ -203,11 +204,31 @@ function resolveDisplayNumber(id: string, footnoteNumberById: Record 0) return ascii; + return DEFAULT_MARKER_FONT_FAMILY; +} + +function resolveMarkerBaseFontSize(firstTextRun: Run | undefined, docDefaults: DocDefaultRunProps | undefined): number { if ( typeof firstTextRun?.fontSize === 'number' && Number.isFinite(firstTextRun.fontSize) && @@ -216,10 +237,20 @@ function resolveMarkerBaseFontSize(firstTextRun: Run | undefined): number { return firstTextRun.fontSize; } + // docDefaults fontSize is in half-points; runs use px (1pt = 1/0.75 px). + const halfPoints = docDefaults?.fontSize; + if (typeof halfPoints === 'number' && Number.isFinite(halfPoints) && halfPoints > 0) { + return halfPoints / 2 / 0.75; + } + return DEFAULT_MARKER_FONT_SIZE; } -function buildMarkerRun(markerText: string, firstTextRun: Run | undefined): Run { +function buildMarkerRun( + markerText: string, + firstTextRun: Run | undefined, + docDefaults: DocDefaultRunProps | undefined, +): Run { // Word renders the FootnoteReference rStyle as a plain superscript, independent // of the following run's formatting. Inheriting bold/italic/letterSpacing from // the first body text run would render "³**NTD**" with a bold marker — visibly @@ -229,8 +260,8 @@ function buildMarkerRun(markerText: string, firstTextRun: Run | undefined): Run kind: 'text', text: `${markerText}\u00A0`, dataAttrs: { [FOOTNOTE_MARKER_DATA_ATTR]: 'true' }, - fontFamily: resolveMarkerFontFamily(firstTextRun), - fontSize: resolveMarkerBaseFontSize(firstTextRun) * SUBSCRIPT_SUPERSCRIPT_SCALE, + fontFamily: resolveMarkerFontFamily(firstTextRun, docDefaults), + fontSize: resolveMarkerBaseFontSize(firstTextRun, docDefaults) * SUBSCRIPT_SUPERSCRIPT_SCALE, vertAlign: 'superscript', }; @@ -304,6 +335,7 @@ function ensureFootnoteMarker( id: string, footnoteNumberById: Record | undefined, footnoteNumberFormat: string | undefined, + docDefaults: DocDefaultRunProps | undefined, ): void { const firstParagraph = blocks.find((b) => b?.kind === 'paragraph') as ParagraphBlock | undefined; if (!firstParagraph) return; @@ -314,7 +346,7 @@ function ensureFootnoteMarker( // leading marker matches the inline reference (single source of truth). const markerText = formatFootnoteCardinal(displayNumber, footnoteNumberFormat); const firstTextRun = runs.find((run) => typeof run.text === 'string' && !isFootnoteMarker(run)); - const normalizedMarkerRun = buildMarkerRun(markerText, firstTextRun); + const normalizedMarkerRun = buildMarkerRun(markerText, firstTextRun, docDefaults); // Check if marker already exists const existingMarker = runs.find(isFootnoteMarker); diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/notes/NoteSessionCoordinator.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/notes/NoteSessionCoordinator.test.ts new file mode 100644 index 0000000000..621b32737d --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/notes/NoteSessionCoordinator.test.ts @@ -0,0 +1,170 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { NoteSessionCoordinator } from './NoteSessionCoordinator.js'; +import type { RenderedNoteTarget } from './note-target.js'; + +// Minimal PM-doc fakes for isNoteContentEmpty (walks descendants for text/atoms). +const emptyDoc = () => + ({ + descendants: (cb: (node: unknown) => boolean | void) => { + cb({ isText: false, isAtom: false, type: { name: 'paragraph' } }); + }, + }) as never; + +const docWithText = () => + ({ + descendants: (cb: (node: unknown) => boolean | void) => { + cb({ isText: false, isAtom: false, type: { name: 'paragraph' } }); + cb({ isText: true, isAtom: true, text: 'note text', type: { name: 'text' } }); + }, + }) as never; + +type FakeSessionEditor = { + state: { doc: never }; + on: (event: string, cb: () => void) => void; + off: (event: string, cb: () => void) => void; + emitUpdate: () => void; +}; + +const makeSessionEditor = (doc: never): FakeSessionEditor => { + const handlers = new Set<() => void>(); + const editor: FakeSessionEditor = { + state: { doc }, + on: (event, cb) => { + if (event === 'update') handlers.add(cb); + }, + off: (event, cb) => { + if (event === 'update') handlers.delete(cb); + }, + emitUpdate: () => { + handlers.forEach((cb) => cb()); + }, + }; + return editor; +}; + +const TARGET: RenderedNoteTarget = { storyType: 'footnote', noteId: '2' }; + +describe('NoteSessionCoordinator', () => { + let host: HTMLElement; + let scroller: HTMLElement; + let hasActiveSession: ReturnType; + let exitActiveSession: ReturnType; + let coordinator: NoteSessionCoordinator; + let scrollIntoViewSpy: ReturnType; + + const addFragment = (blockId: string): HTMLElement => { + const el = document.createElement('div'); + el.setAttribute('data-block-id', blockId); + host.appendChild(el); + return el; + }; + + beforeEach(() => { + host = document.createElement('div'); + scroller = document.createElement('div'); + document.body.append(host, scroller); + hasActiveSession = vi.fn(() => true); + exitActiveSession = vi.fn(); + scrollIntoViewSpy = vi.fn(); + Element.prototype.scrollIntoView = scrollIntoViewSpy as never; + coordinator = new NoteSessionCoordinator({ + getHost: () => host, + getScrollContainer: () => scroller, + hasActiveSession, + exitActiveSession, + }); + }); + + afterEach(() => { + document.body.innerHTML = ''; + vi.restoreAllMocks(); + }); + + it('highlights only the active note fragments and clears them on exit', () => { + const noteFrag = addFragment('footnote-2-ABC123'); + const otherNote = addFragment('footnote-9-DEF456'); + const bodyFrag = addFragment('para-uuid'); + + coordinator.onActivated(TARGET, { editor: makeSessionEditor(docWithText()) }); + + expect(noteFrag.classList.contains('sd-note-session-active')).toBe(true); + expect(otherNote.classList.contains('sd-note-session-active')).toBe(false); + expect(bodyFrag.classList.contains('sd-note-session-active')).toBe(false); + + coordinator.onExit(); + expect(noteFrag.classList.contains('sd-note-session-active')).toBe(false); + }); + + it('re-applies the highlight after a repaint rebuilds the fragments', () => { + addFragment('footnote-2-ABC123'); + coordinator.onActivated(TARGET, { editor: makeSessionEditor(docWithText()) }); + + host.innerHTML = ''; + const rebuilt = addFragment('footnote-2-NEWHASH'); + coordinator.onPaint(); + + expect(rebuilt.classList.contains('sd-note-session-active')).toBe(true); + }); + + it('self-heals when the session ended through another path', () => { + const frag = addFragment('footnote-2-ABC123'); + coordinator.onActivated(TARGET, { editor: makeSessionEditor(docWithText()) }); + expect(frag.classList.contains('sd-note-session-active')).toBe(true); + + hasActiveSession.mockReturnValue(false); + coordinator.onPaint(); + expect(frag.classList.contains('sd-note-session-active')).toBe(false); + }); + + it('keeps the scroll pending until the fragment paints, then scrolls once', () => { + // Activated before the (inserted) note has painted — nothing to scroll yet. + coordinator.onActivated(TARGET, { editor: makeSessionEditor(docWithText()) }); + expect(scrollIntoViewSpy).not.toHaveBeenCalled(); + + addFragment('footnote-2-ABC123'); + coordinator.onPaint(); + expect(scrollIntoViewSpy).toHaveBeenCalledTimes(1); + + coordinator.onPaint(); + expect(scrollIntoViewSpy).toHaveBeenCalledTimes(1); // not re-scrolled + }); + + it('does not scroll when the note is already fully visible', () => { + const frag = addFragment('footnote-2-ABC123'); + frag.getBoundingClientRect = vi.fn(() => ({ top: 100, bottom: 160 }) as DOMRect); + scroller.getBoundingClientRect = vi.fn(() => ({ top: 0, bottom: 500 }) as DOMRect); + + coordinator.onActivated(TARGET, { editor: makeSessionEditor(docWithText()) }); + + expect(scrollIntoViewSpy).not.toHaveBeenCalled(); + }); + + it('exits the session when a note that had content is emptied', async () => { + addFragment('footnote-2-ABC123'); + const sessionEditor = makeSessionEditor(docWithText()); + coordinator.onActivated(TARGET, { editor: sessionEditor }); + + sessionEditor.state.doc = emptyDoc(); + sessionEditor.emitUpdate(); + await Promise.resolve(); // exit is deferred past the transaction lifecycle + + expect(exitActiveSession).toHaveBeenCalledTimes(1); + }); + + it('does not remove a freshly inserted note until it has held content', async () => { + addFragment('footnote-2-ABC123'); + const sessionEditor = makeSessionEditor(emptyDoc()); + coordinator.onActivated(TARGET, { editor: sessionEditor }); + + sessionEditor.emitUpdate(); // still empty — fresh note, no removal + await Promise.resolve(); + expect(exitActiveSession).not.toHaveBeenCalled(); + + sessionEditor.state.doc = docWithText(); + sessionEditor.emitUpdate(); // user typed + sessionEditor.state.doc = emptyDoc(); + sessionEditor.emitUpdate(); // then cleared — now it removes + await Promise.resolve(); + expect(exitActiveSession).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/notes/NoteSessionCoordinator.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/notes/NoteSessionCoordinator.ts new file mode 100644 index 0000000000..2a076cf8e1 --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/notes/NoteSessionCoordinator.ts @@ -0,0 +1,153 @@ +/** + * NoteSessionCoordinator — UX for an open footnote/endnote session (SD-3400). + * + * Owns the three paint-level behaviors that make a note session feel focused: + * + * 1. Highlight: the active note's painted fragments get the + * `sd-note-session-active` class (tint + accent bar via painter CSS). + * Fragments are rebuilt on every paint, so the class is re-applied from + * {@link onPaint} and self-heals when the session ends through any path. + * 2. Smart scroll: bring the note into view on activation. No-op when the + * fragment is already fully visible; freshly inserted notes only paint + * after the next relayout, so the request stays pending until the fragment + * exists. + * 3. Emptied-note commit: when the user clears ALL content of a note that + * previously had content, exit the session immediately so the commit + * (which removes the footnote on both sides) runs without an extra click. + * Freshly inserted notes open empty and are exempt until they have held + * content, so insert-and-type is unaffected. + * + * Everything here is paint-only (classList + scroll) — no layout impact. + * PresentationEditor delegates via three calls: onActivated / onPaint / onExit. + */ + +import type { Node as ProseMirrorNode } from 'prosemirror-model'; +import { isNoteContentEmpty } from '../../../document-api-adapters/story-runtime/note-story-runtime.js'; +import { renderedNoteBlockIdPrefixes, type RenderedNoteTarget } from './note-target.js'; + +const ACTIVE_NOTE_CLASS = 'sd-note-session-active'; + +type NoteSessionEditorLike = { + state: { doc: ProseMirrorNode }; + on?: (event: string, callback: () => void) => void; + off?: (event: string, callback: () => void) => void; +}; + +export type NoteSessionLike = { + editor: NoteSessionEditorLike; +}; + +export interface NoteSessionCoordinatorDeps { + /** Painted-pages host to query for note fragments. */ + getHost: () => HTMLElement | null; + /** The element (or window) that actually scrolls the document. */ + getScrollContainer: () => Element | Window | null; + /** Whether a story session is currently open (self-healing guard). */ + hasActiveSession: () => boolean; + /** Exits the active story session, committing its content. */ + exitActiveSession: () => void; +} + +export class NoteSessionCoordinator { + #deps: NoteSessionCoordinatorDeps; + #activeTarget: RenderedNoteTarget | null = null; + #pendingScrollIntoView = false; + #emptyWatchCleanup: (() => void) | null = null; + + constructor(deps: NoteSessionCoordinatorDeps) { + this.#deps = deps; + } + + /** A note session opened: highlight it, watch for emptying, scroll to it. */ + onActivated(target: RenderedNoteTarget, session: NoteSessionLike): void { + this.#activeTarget = target; + this.#refreshHighlight(); + this.#bindEmptyWatch(session); + this.#pendingScrollIntoView = true; + this.#scrollIntoView(); + } + + /** A paint completed: re-apply the highlight, complete any pending scroll. */ + onPaint(): void { + this.#refreshHighlight(); + this.#scrollIntoView(); + } + + /** The session is exiting: unbind and clear all visual state. */ + onExit(): void { + this.#emptyWatchCleanup?.(); + this.#activeTarget = null; + this.#pendingScrollIntoView = false; + this.#refreshHighlight(); + } + + #findActiveFragments(host: HTMLElement): Element[] { + const target = this.#activeTarget; + if (!target) return []; + const prefixes = renderedNoteBlockIdPrefixes(target); + return Array.from(host.querySelectorAll('[data-block-id]')).filter((el) => { + const id = el.getAttribute('data-block-id') ?? ''; + return prefixes.some((prefix) => id.startsWith(prefix)); + }); + } + + #refreshHighlight(): void { + const host = this.#deps.getHost(); + if (!host) return; + if (this.#activeTarget && !this.#deps.hasActiveSession()) { + this.#activeTarget = null; + } + host.querySelectorAll(`.${ACTIVE_NOTE_CLASS}`).forEach((el) => el.classList.remove(ACTIVE_NOTE_CLASS)); + this.#findActiveFragments(host).forEach((el) => el.classList.add(ACTIVE_NOTE_CLASS)); + } + + #scrollIntoView(): void { + if (!this.#pendingScrollIntoView) return; + if (!this.#activeTarget) { + this.#pendingScrollIntoView = false; + return; + } + const host = this.#deps.getHost(); + if (!host) return; + const fragment = this.#findActiveFragments(host)[0]; + if (!fragment) return; // not painted yet — retried from the next onPaint + + this.#pendingScrollIntoView = false; + const rect = fragment.getBoundingClientRect(); + const scroller = this.#deps.getScrollContainer(); + const viewport = + scroller instanceof Window + ? { top: 0, bottom: scroller.innerHeight } + : scroller instanceof Element + ? (() => { + const r = scroller.getBoundingClientRect(); + return { top: r.top, bottom: r.bottom }; + })() + : { top: 0, bottom: window.innerHeight }; + const fullyVisible = rect.top >= viewport.top + 8 && rect.bottom <= viewport.bottom - 8; + if (fullyVisible) return; + fragment.scrollIntoView({ block: 'center', behavior: 'smooth' }); + } + + #bindEmptyWatch(session: NoteSessionLike): void { + this.#emptyWatchCleanup?.(); + const sessionEditor = session.editor; + let hadContent = !isNoteContentEmpty(sessionEditor.state.doc); + const onUpdate = () => { + const empty = isNoteContentEmpty(sessionEditor.state.doc); + if (!empty) { + hadContent = true; + return; + } + if (!hadContent) return; + this.#emptyWatchCleanup?.(); + // Defer past the session editor's own transaction lifecycle. + queueMicrotask(() => this.#deps.exitActiveSession()); + }; + sessionEditor.on?.('update', onUpdate); + this.#emptyWatchCleanup = () => { + sessionEditor.off?.('update', onUpdate); + this.#emptyWatchCleanup = null; + }; + } +} diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/notes/note-target.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/notes/note-target.ts new file mode 100644 index 0000000000..f6f4af2ae0 --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/notes/note-target.ts @@ -0,0 +1,70 @@ +/** + * Shared identity for rendered footnote/endnote targets (SD-3400). + * + * A note painted at the page bottom is addressed by `{ storyType, noteId }`, + * parsed from its fragment block id. These helpers were previously duplicated + * in EditorInputManager and PresentationEditor; this module is the single + * source of truth for the block-id ↔ note-target mapping. + * + * Block-id shapes: + * - `footnote-{id}-{hash}` / `endnote-{id}-{hash}` — regular note fragments + * - `__sd_semantic_footnote-{id}-{hash}` — semantic-flow footnote blocks + */ + +import { isSemanticFootnoteBlockId } from '../semantic-flow-constants.js'; + +export type RenderedNoteTarget = { + storyType: 'footnote' | 'endnote'; + noteId: string; +}; + +/** True when a fragment block id belongs to rendered note content. */ +export function isRenderedNoteBlockId(blockId: string): boolean { + return ( + typeof blockId === 'string' && + (blockId.startsWith('footnote-') || blockId.startsWith('endnote-') || isSemanticFootnoteBlockId(blockId)) + ); +} + +/** Parses a fragment block id into its note target, or null for non-note ids. */ +export function parseRenderedNoteTarget(blockId: string): RenderedNoteTarget | null { + if (typeof blockId !== 'string' || blockId.length === 0) { + return null; + } + + if (blockId.startsWith('footnote-')) { + const noteId = blockId.slice('footnote-'.length).split('-')[0] ?? ''; + return noteId ? { storyType: 'footnote', noteId } : null; + } + + if (blockId.startsWith('__sd_semantic_footnote-')) { + const noteId = blockId.slice('__sd_semantic_footnote-'.length).split('-')[0] ?? ''; + return noteId ? { storyType: 'footnote', noteId } : null; + } + + if (blockId.startsWith('endnote-')) { + const noteId = blockId.slice('endnote-'.length).split('-')[0] ?? ''; + return noteId ? { storyType: 'endnote', noteId } : null; + } + + return null; +} + +export function isSameRenderedNoteTarget( + left: RenderedNoteTarget | null | undefined, + right: RenderedNoteTarget | null | undefined, +): boolean { + if (!left || !right) { + return false; + } + + return left.storyType === right.storyType && left.noteId === right.noteId; +} + +/** + * Fragment block-id prefixes that belong to a specific note target. Used to + * match a target against painted `[data-block-id]` elements. + */ +export function renderedNoteBlockIdPrefixes(target: RenderedNoteTarget): string[] { + return [`${target.storyType}-${target.noteId}-`, `__sd_semantic_${target.storyType}-${target.noteId}-`]; +} 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 adfa044657..57b7f3ab4b 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 @@ -41,7 +41,13 @@ import { import { debugLog } from '../selection/SelectionDebug.js'; import { DOM_CLASS_NAMES, buildAnnotationSelector, DRAGGABLE_SELECTOR } from '@superdoc/dom-contract'; import { applyEditableSlotAtInlineBoundary } from '@helpers/ensure-editable-slot-inline-boundary.js'; -import { isSemanticFootnoteBlockId } from '../semantic-flow-constants.js'; +import { + isRenderedNoteBlockId, + isSameRenderedNoteTarget, + parseRenderedNoteTarget, + type RenderedNoteTarget, +} from '../notes/note-target.js'; +import { resolveNoteReferenceAtPointer } from './note-reference-hit.js'; import { CommentsPluginKey } from '@extensions/comment/comments-plugin.js'; import { findStructuredContentBlockAtPos, @@ -88,57 +94,6 @@ type CommentThreadHit = { threadId: string | null; }; -/** - * Block IDs for note content use `footnote-{id}-` / `endnote-{id}-` prefixes. - * Semantic footnote blocks use the {@link isSemanticFootnoteBlockId} helper from - * shared constants — it matches both heading and body footnote block IDs. - */ -function isRenderedNoteBlockId(blockId: string): boolean { - return ( - typeof blockId === 'string' && - (blockId.startsWith('footnote-') || blockId.startsWith('endnote-') || isSemanticFootnoteBlockId(blockId)) - ); -} - -type RenderedNoteTarget = { - storyType: 'footnote' | 'endnote'; - noteId: string; -}; - -function parseRenderedNoteTarget(blockId: string): RenderedNoteTarget | null { - if (typeof blockId !== 'string' || blockId.length === 0) { - return null; - } - - if (blockId.startsWith('footnote-')) { - const noteId = blockId.slice('footnote-'.length).split('-')[0] ?? ''; - return noteId ? { storyType: 'footnote', noteId } : null; - } - - if (blockId.startsWith('__sd_semantic_footnote-')) { - const noteId = blockId.slice('__sd_semantic_footnote-'.length).split('-')[0] ?? ''; - return noteId ? { storyType: 'footnote', noteId } : null; - } - - if (blockId.startsWith('endnote-')) { - const noteId = blockId.slice('endnote-'.length).split('-')[0] ?? ''; - return noteId ? { storyType: 'endnote', noteId } : null; - } - - return null; -} - -function isSameRenderedNoteTarget( - left: RenderedNoteTarget | null | undefined, - right: RenderedNoteTarget | null | undefined, -): boolean { - if (!left || !right) { - return false; - } - - return left.storyType === right.storyType && left.noteId === right.noteId; -} - function isOutsidePageBodyContent(layout: Layout, x: number, pageIndex?: number, pageLocalY?: number): boolean { if (!Number.isFinite(x) || !Number.isFinite(pageIndex) || !Number.isFinite(pageLocalY)) { return false; @@ -1892,6 +1847,25 @@ export class EditorInputManager { this.#callbacks.clearHoverRegion?.(); } + /** + * SD-3400: resolve a double-clicked BODY footnote/endnote reference marker to + * its note target so navigation can open the corresponding note. Delegates to + * the pure {@link resolveNoteReferenceAtPointer} helper. + */ + #resolveFootnoteReferenceTargetAtPointer( + target: HTMLElement | null, + clientX: number, + clientY: number, + ): RenderedNoteTarget | null { + return resolveNoteReferenceAtPointer({ + target, + clientX, + clientY, + doc: this.#deps?.getEditor()?.state?.doc, + ownerDocument: this.#deps?.getViewportHost()?.ownerDocument ?? document, + }); + } + #handleDoubleClick(event: MouseEvent): void { if (!this.#deps) return; if (event.button !== 0) return; @@ -1914,6 +1888,21 @@ export class EditorInputManager { const normalized = this.#callbacks.normalizeClientPoint?.(event.clientX, event.clientY); if (!normalized) return; + // SD-3400: double-clicking a BODY footnote/endnote reference marker navigates + // to its note content. Activating the note session focuses the note and scrolls + // its selection into view, so the user lands on the corresponding note. + const footnoteRefTarget = this.#resolveFootnoteReferenceTargetAtPointer(target, event.clientX, event.clientY); + if (footnoteRefTarget) { + event.preventDefault(); + event.stopPropagation(); + this.#callbacks.activateRenderedNoteSession?.(footnoteRefTarget, { + clientX: event.clientX, + clientY: event.clientY, + pageIndex: normalized.pageIndex, + }); + return; + } + const clickedNoteTarget = this.#resolveRenderedNoteTargetAtPointer(target, event.clientX, event.clientY); if (clickedNoteTarget) { if (isSameRenderedNoteTarget(this.#getActiveRenderedNoteTarget(), clickedNoteTarget)) { diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/note-reference-hit.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/note-reference-hit.test.ts new file mode 100644 index 0000000000..065f8a8d8c --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/note-reference-hit.test.ts @@ -0,0 +1,237 @@ +import { describe, it, expect } from 'vitest'; +import { Schema, type Node as ProseMirrorNode } from 'prosemirror-model'; +import { resolveNoteReferenceAtPointer } from './note-reference-hit.js'; + +// Mirrors the real document shape: note references are inline atoms wrapped in +// runs. Word's cross-reference bookmark (`_RefXXXX`) wraps the original note +// reference; the importer emits it as a FLAT bookmarkStart/bookmarkEnd marker +// pair (matched by id, both empty) with the reference between them — verified +// against the NVCA fixture. The schema also permits bookmarkStart to hold +// content, so the resolver supports both shapes. +const schema = new Schema({ + nodes: { + doc: { content: 'block+' }, + paragraph: { group: 'block', content: 'inline*' }, + run: { inline: true, group: 'inline', content: 'inline*' }, + bookmarkStart: { + inline: true, + group: 'inline', + content: 'inline*', + attrs: { name: { default: null }, id: { default: null } }, + }, + bookmarkEnd: { + inline: true, + group: 'inline', + atom: true, + attrs: { id: { default: null } }, + }, + footnoteReference: { + inline: true, + group: 'inline', + atom: true, + selectable: false, + attrs: { id: { default: null } }, + }, + endnoteReference: { + inline: true, + group: 'inline', + atom: true, + selectable: false, + attrs: { id: { default: null } }, + }, + crossReference: { + inline: true, + group: 'inline', + atom: true, + selectable: false, + attrs: { target: { default: '' }, resolvedText: { default: '' } }, + }, + text: { group: 'inline' }, + }, + marks: {}, +}); + +/** + * Builds a doc shaped like the NVCA fixture's cross-reference pattern: + * p1: "See" + bookmarkStart(_Ref1)[ run[ ] ] + "below" + * p2: "as noted in" + crossReference(target=_Ref1, "footnote 8") + */ +function makeDoc(noteRefType: 'footnoteReference' | 'endnoteReference' = 'footnoteReference', bookmarkContent?: ProseMirrorNode[]) { + const noteRef = schema.nodes[noteRefType].create({ id: '8' }); + const bookmark = schema.nodes.bookmarkStart.create( + { name: '_Ref1', id: '1' }, + bookmarkContent ?? [schema.nodes.run.create(null, noteRef)], + ); + const p1 = schema.node('paragraph', null, [ + schema.nodes.run.create(null, schema.text('See')), + bookmark, + schema.nodes.run.create(null, schema.text('below')), + ]); + const crossRef = schema.nodes.crossReference.create({ target: '_Ref1', resolvedText: 'footnote 8' }); + const p2 = schema.node('paragraph', null, [schema.nodes.run.create(null, schema.text('as noted in')), crossRef]); + return schema.node('doc', null, [p1, p2]); +} + +function findPos(doc: ProseMirrorNode, typeName: string): number { + let found = -1; + doc.descendants((node, pos) => { + if (found < 0 && node.type.name === typeName) { + found = pos; + return false; + } + return true; + }); + return found; +} + +function makeRefSpan(pmStart: number): HTMLElement { + const el = document.createElement('span'); + el.setAttribute('data-pm-start', String(pmStart)); + document.body.appendChild(el); + return el; +} + +function resolveAt(doc: ProseMirrorNode, pmStart: number) { + return resolveNoteReferenceAtPointer({ + target: makeRefSpan(pmStart), + clientX: 5, + clientY: 5, + doc, + ownerDocument: document, + }); +} + +/** + * The shape real imports produce (NVCA fixture): + * p1: "Dividends." + bookmarkStart(_Ref1, id=69) + run[] + bookmarkEnd(id=69) + * p2: "as noted in" + crossReference(target=_Ref1) + */ +function makeFlatMarkerDoc(noteRefType: 'footnoteReference' | 'endnoteReference' = 'footnoteReference') { + const noteRef = schema.nodes[noteRefType].create({ id: '8' }); + const p1 = schema.node('paragraph', null, [ + schema.nodes.run.create(null, schema.text('Dividends.')), + schema.nodes.bookmarkStart.create({ name: '_Ref1', id: '69' }), + schema.nodes.run.create(null, noteRef), + schema.nodes.bookmarkEnd.create({ id: '69' }), + ]); + const crossRef = schema.nodes.crossReference.create({ target: '_Ref1', resolvedText: '1' }); + const p2 = schema.node('paragraph', null, [schema.nodes.run.create(null, schema.text('as noted in')), crossRef]); + return schema.node('doc', null, [p1, p2]); +} + +describe('resolveNoteReferenceAtPointer — cross-reference navigation (SD-3400)', () => { + it.each([ + ['footnoteReference', 'footnote'], + ['endnoteReference', 'endnote'], + ])('resolves a crossReference across a flat %s bookmark marker pair (real import shape)', (refType, storyType) => { + const doc = makeFlatMarkerDoc(refType as 'footnoteReference' | 'endnoteReference'); + + const target = resolveAt(doc, findPos(doc, 'crossReference')); + + expect(target).toEqual({ storyType, noteId: '8' }); + }); + + it('does not resolve a note that sits OUTSIDE the flat marker pair', () => { + // The footnote ref here precedes the bookmarkStart — the bookmark range + // holds plain text only, so the cross-ref is not a note reference. + const noteRef = schema.nodes.footnoteReference.create({ id: '8' }); + const p1 = schema.node('paragraph', null, [ + schema.nodes.run.create(null, noteRef), + schema.nodes.bookmarkStart.create({ name: '_Ref1', id: '69' }), + schema.nodes.run.create(null, schema.text('Section 2')), + schema.nodes.bookmarkEnd.create({ id: '69' }), + ]); + const crossRef = schema.nodes.crossReference.create({ target: '_Ref1', resolvedText: 'Section 2' }); + const p2 = schema.node('paragraph', null, [crossRef]); + const doc = schema.node('doc', null, [p1, p2]); + + const target = resolveAt(doc, findPos(doc, 'crossReference')); + + expect(target).toBeNull(); + }); + + it('resolves a crossReference to the footnote wrapped by its target bookmark', () => { + const doc = makeDoc('footnoteReference'); + + const target = resolveAt(doc, findPos(doc, 'crossReference')); + + expect(target).toEqual({ storyType: 'footnote', noteId: '8' }); + }); + + it('resolves a crossReference to an endnote wrapped by its target bookmark', () => { + const doc = makeDoc('endnoteReference'); + + const target = resolveAt(doc, findPos(doc, 'crossReference')); + + expect(target).toEqual({ storyType: 'endnote', noteId: '8' }); + }); + + it('returns null when the target bookmark holds no note reference (plain bookmark cross-ref)', () => { + const doc = makeDoc('footnoteReference', [schema.nodes.run.create(null, schema.text('Section 2'))]); + + const target = resolveAt(doc, findPos(doc, 'crossReference')); + + expect(target).toBeNull(); + }); + + it('returns null for a crossReference with no target', () => { + const crossRef = schema.nodes.crossReference.create({ target: '', resolvedText: 'dangling' }); + const doc = schema.node('doc', null, [schema.node('paragraph', null, [crossRef])]); + + const target = resolveAt(doc, findPos(doc, 'crossReference')); + + expect(target).toBeNull(); + }); + + it('still resolves a plain body footnote reference (regression)', () => { + const doc = makeDoc('footnoteReference'); + + const target = resolveAt(doc, findPos(doc, 'footnoteReference')); + + expect(target).toEqual({ storyType: 'footnote', noteId: '8' }); + }); +}); + +describe('resolveNoteReferenceAtPointer — story-space guards (footer dblclick regression)', () => { + function resolveTarget(doc: ProseMirrorNode, target: HTMLElement) { + return resolveNoteReferenceAtPointer({ target, clientX: 5, clientY: 5, doc, ownerDocument: document }); + } + + it.each(['superdoc-page-header', 'superdoc-page-footer'])( + 'ignores pm-start elements inside a %s container (header/footer story space)', + (containerClass) => { + const doc = makeDoc('footnoteReference'); + const container = document.createElement('div'); + container.className = containerClass; + const span = document.createElement('span'); + // A footer-local position that HAPPENS to resolve to a body note ref. + span.setAttribute('data-pm-start', String(findPos(doc, 'footnoteReference'))); + container.appendChild(span); + document.body.appendChild(container); + + expect(resolveTarget(doc, span)).toBeNull(); + }, + ); + + it('ignores pm-start elements inside a rendered-note fragment (note story space)', () => { + const doc = makeDoc('footnoteReference'); + const fragment = document.createElement('div'); + fragment.setAttribute('data-block-id', 'footnote-8-p0'); + const span = document.createElement('span'); + span.setAttribute('data-pm-start', String(findPos(doc, 'footnoteReference'))); + fragment.appendChild(span); + document.body.appendChild(fragment); + + expect(resolveTarget(doc, span)).toBeNull(); + }); + + it('returns null instead of throwing for a pm-start beyond the body document', () => { + const doc = makeDoc('footnoteReference'); + const span = document.createElement('span'); + // Story-local offsets can exceed the body size; nodeAt would throw. + span.setAttribute('data-pm-start', String(doc.content.size + 50)); + document.body.appendChild(span); + + expect(resolveTarget(doc, span)).toBeNull(); + }); +}); diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/note-reference-hit.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/note-reference-hit.ts new file mode 100644 index 0000000000..98e9969322 --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/note-reference-hit.ts @@ -0,0 +1,125 @@ +/** + * Resolves a pointer event over a painted BODY footnote/endnote reference to + * its note target (SD-3400 double-click navigation). + * + * The painted reference is a superscript run carrying `data-pm-start` (the PM + * position of the footnoteReference/endnoteReference node) but no note id, so + * the PM node at that position supplies the story type and id. Real pointer + * events usually land on the selection overlay above the pages — when the + * event target has no `data-pm-start` ancestor, the full `elementsFromPoint` + * hit chain is walked (same strategy as the rendered-note resolver). + */ + +import type { Node as ProseMirrorNode } from 'prosemirror-model'; +import { isRenderedNoteBlockId, type RenderedNoteTarget } from '../notes/note-target.js'; + +/** + * Painted header/footer content lives inside these containers and carries + * `data-pm-start` values in the header/footer part's OWN coordinate space. + * Resolving those against the body document is meaningless and, for positions + * past the body size, makes `nodeAt` throw — which would abort the caller's + * double-click handling before header/footer activation runs. + */ +const HEADER_FOOTER_CONTAINER_SELECTOR = '.superdoc-page-header, .superdoc-page-footer'; + +export type NoteReferenceHitOptions = { + /** The pointer event's target. */ + target: HTMLElement | null; + clientX: number; + clientY: number; + /** The body editor's PM document (resolves pm-start → reference node). */ + doc: ProseMirrorNode | null | undefined; + /** Document used for the elementsFromPoint fallback. */ + ownerDocument: Document; +}; + +export function resolveNoteReferenceAtPointer(options: NoteReferenceHitOptions): RenderedNoteTarget | null { + const { target, clientX, clientY, doc, ownerDocument } = options; + + const fromTarget = noteTargetFromPmStartElement(target?.closest?.('[data-pm-start]') as HTMLElement | null, doc); + if (fromTarget) return fromTarget; + + if (typeof ownerDocument.elementsFromPoint !== 'function') return null; + for (const element of ownerDocument.elementsFromPoint(clientX, clientY)) { + if (!(element instanceof HTMLElement)) continue; + const resolved = noteTargetFromPmStartElement(element.closest('[data-pm-start]') as HTMLElement | null, doc); + if (resolved) return resolved; + } + return null; +} + +function noteTargetFromPmStartElement( + refEl: HTMLElement | null, + doc: ProseMirrorNode | null | undefined, +): RenderedNoteTarget | null { + if (!refEl || !doc) return null; + // Only BODY fragments carry body-space pm positions. Header/footer and + // rendered-note fragments use their own story's coordinate space. + if (refEl.closest(HEADER_FOOTER_CONTAINER_SELECTOR)) return null; + const blockId = refEl.closest('[data-block-id]')?.getAttribute('data-block-id') ?? ''; + if (isRenderedNoteBlockId(blockId)) return null; + const pmStart = Number(refEl.getAttribute('data-pm-start')); + if (!Number.isFinite(pmStart) || pmStart < 0 || pmStart >= doc.content.size) return null; + const node = doc.nodeAt(pmStart); + if (node?.type?.name === 'crossReference') { + return noteTargetFromCrossReference(doc, node.attrs?.target); + } + return noteTargetFromReferenceNode(node); +} + +function noteTargetFromReferenceNode(node: ProseMirrorNode | null | undefined): RenderedNoteTarget | null { + const nodeType = node?.type?.name; + if (nodeType !== 'footnoteReference' && nodeType !== 'endnoteReference') return null; + const noteId = node?.attrs?.id; + if (noteId == null || String(noteId).length === 0) return null; + return { + storyType: nodeType === 'endnoteReference' ? 'endnote' : 'footnote', + noteId: String(noteId), + }; +} + +/** + * Resolves a REF/NOTEREF cross-reference to the note it points at. Word's + * cross-reference bookmark (`_RefXXXX`) wraps the ORIGINAL note reference in + * the body. The importer emits the bookmark as a flat bookmarkStart/bookmarkEnd + * marker pair matched by id, so the note is found by scanning the document + * range from the named bookmarkStart to its matching bookmarkEnd. (The schema + * also permits bookmarkStart to hold content; scanning to at least the end of + * the start node covers that shape too.) Returns null for cross-references to + * anything other than a note (headings, tables), letting the double-click fall + * through to default text behavior. + */ +function noteTargetFromCrossReference(doc: ProseMirrorNode, bookmarkName: unknown): RenderedNoteTarget | null { + if (typeof bookmarkName !== 'string' || bookmarkName.length === 0) return null; + + let startPos = -1; + let rangeEnd = -1; + let bookmarkId: unknown = null; + let foundEnd = false; + doc.descendants((node, pos) => { + if (foundEnd) return false; + if (startPos < 0) { + if (node.type?.name === 'bookmarkStart' && node.attrs?.name === bookmarkName) { + startPos = pos; + rangeEnd = pos + node.nodeSize; + bookmarkId = node.attrs?.id; + } + return true; + } + if (node.type?.name === 'bookmarkEnd' && bookmarkId != null && node.attrs?.id === bookmarkId) { + rangeEnd = pos; + foundEnd = true; + return false; + } + return true; + }); + if (startPos < 0) return null; + + let result: RenderedNoteTarget | null = null; + doc.nodesBetween(startPos, rangeEnd, (node) => { + if (result) return false; + result = noteTargetFromReferenceNode(node); + return !result; + }); + return result; +} diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/selection/VisibleTextOffsetGeometry.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/selection/VisibleTextOffsetGeometry.ts index 62853cc7a6..e5abe5b87d 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/selection/VisibleTextOffsetGeometry.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/selection/VisibleTextOffsetGeometry.ts @@ -29,6 +29,22 @@ export type VisibleTextOffsetGeometryOptions = { pageGap: number; }; +/** + * Anchors a ProseMirror position to its paragraph block so resolution can + * survive stale painted pm ranges (SD-3400). The painter skips repainting + * unchanged note paragraphs, so their data-pm-* attributes drift after + * upstream edits shift positions. Within one unchanged block the ranges stay + * internally consistent, so translating the position by the block-start + * delta (current first-leaf position minus the fragment's first painted + * pmStart) makes resolution exact per block. + */ +export type PmBlockAnchor = { + /** The paragraph's sdBlockId — painted fragment block ids end with it. */ + sdBlockId: string; + /** Current document position of the block's first inline leaf. */ + currentStart: number; +}; + /** * Measures a visible-text offset within a DOM root from a concrete DOM boundary. * @@ -282,6 +298,275 @@ export function computeSelectionRectsFromVisibleTextOffsets( return layoutRects; } +type ResolvedPmPoint = { + node: Text; + offset: number; + pageElement: HTMLElement; + lineElement: HTMLElement; +}; + +/** + * Resolves a ProseMirror position directly against the painted lines' pm + * ranges (`data-pm-start`/`data-pm-end`), bypassing the visible-text-offset + * bridge. Painted note fragments carry SESSION-coordinate pm ranges, so this + * is exact across paragraph boundaries — the offset bridge counts structural + * paragraph tokens on the painted side but not on the hidden-editor side, + * which drifted the caret backwards in multi-paragraph notes (SD-3400). + * + * Returns null when no painted line covers the position (e.g., a structural + * gap or unpainted content) so callers can fall back to the offset bridge. + */ +function resolvePmPoint( + containers: readonly HTMLElement[], + pos: number, + anchor?: PmBlockAnchor | null, +): ResolvedPmPoint | null { + if (!Number.isFinite(pos)) { + return null; + } + + // Block-anchored resolution: scope to the paragraph's own fragment(s) and + // translate the position into the fragment's painted coordinate space. + if (anchor?.sdBlockId) { + const blockContainers = containers.filter((el) => + (el.getAttribute('data-block-id') ?? '').endsWith(anchor.sdBlockId), + ); + if (blockContainers.length) { + const blockLines = collectRenderedLineElements(blockContainers) + .map((line) => ({ line, pmStart: getPmStart(line), pmEnd: getPmEnd(line) })) + .filter((entry): entry is { line: HTMLElement; pmStart: number; pmEnd: number } => + entry.pmStart != null && entry.pmEnd != null) + .sort((a, b) => a.pmStart - b.pmStart || a.pmEnd - b.pmEnd); + if (blockLines.length) { + const delta = anchor.currentStart - blockLines[0].pmStart; + const translated = Math.max( + blockLines[0].pmStart, + Math.min(pos - delta, blockLines[blockLines.length - 1].pmEnd), + ); + const resolved = resolvePmPoint(blockContainers, translated); + if (resolved) { + return resolved; + } + } + } + return null; + } + + // Painted fragments come back in DOM insertion order, which after + // incremental repaints is NOT document order — sort by pm range so the + // forward-affinity scan and the gap snap pick the right line (SD-3400). + const lines = collectRenderedLineElements(containers) + .map((line) => ({ line, pmStart: getPmStart(line), pmEnd: getPmEnd(line) })) + .filter((entry): entry is { line: HTMLElement; pmStart: number; pmEnd: number } => + entry.pmStart != null && entry.pmEnd != null) + .sort((a, b) => a.pmStart - b.pmStart || a.pmEnd - b.pmEnd); + let lineElement: HTMLElement | null = null; + let resolvedPos = pos; + let sawEarlierLine = false; + for (const { line, pmStart, pmEnd } of lines) { + if (pos > pmEnd) { + sawEarlierLine = true; + continue; + } + if (pos < pmStart) { + // Interior structural gap (paragraph boundary tokens between painted + // lines): the position is a valid caret position in the doc, so snap + // forward to this line's start instead of failing into the offset + // bridge. Positions BEFORE the first painted line stay unresolved. + if (sawEarlierLine && !lineElement) { + lineElement = line; + resolvedPos = pmStart; + } + break; + } + lineElement = line; + resolvedPos = pos; + // Forward affinity: a position at this line's end that also starts the + // next line belongs to the next line, so keep scanning while pos == pmEnd. + if (pos < pmEnd) { + break; + } + } + if (!lineElement) { + return null; + } + + const pageElement = lineElement.closest(`.${DOM_CLASS_NAMES.PAGE}[data-page-index]`); + if (!pageElement) { + return null; + } + + const leaves = collectLeafPmElements(lineElement); + let leaf: HTMLElement | null = null; + for (const candidate of leaves) { + const pmStart = getPmStart(candidate); + const pmEnd = getPmEnd(candidate); + if (pmStart == null || pmEnd == null || resolvedPos < pmStart || resolvedPos > pmEnd) { + continue; + } + leaf = candidate; + if (resolvedPos < pmEnd) { + break; + } + } + if (!leaf) { + return null; + } + + const leafPmStart = getPmStart(leaf) ?? 0; + const doc = leaf.ownerDocument ?? document; + const walker = doc.createTreeWalker(leaf, NodeFilter.SHOW_TEXT); + let remaining = Math.max(0, resolvedPos - leafPmStart); + let lastTextNode: Text | null = null; + + let currentNode = walker.nextNode(); + while (currentNode) { + const textNode = currentNode as Text; + const textLength = textNode.textContent?.length ?? 0; + if (textLength > 0) { + lastTextNode = textNode; + if (remaining <= textLength) { + return { node: textNode, offset: remaining, pageElement, lineElement }; + } + remaining -= textLength; + } + currentNode = walker.nextNode(); + } + + if (!lastTextNode) { + return null; + } + // Position past the leaf's painted text (pm range wider than visible text, + // e.g. tracked wrapper structure): clamp to the leaf's end. + return { + node: lastTextNode, + offset: lastTextNode.textContent?.length ?? 0, + pageElement, + lineElement, + }; +} + +/** + * Caret rect for a ProseMirror position resolved via painted pm ranges. + * See {@link resolvePmPoint}; returns null so callers can fall back. + */ +export function computeCaretRectFromPmPosition( + options: VisibleTextOffsetGeometryOptions, + pos: number, + anchor?: PmBlockAnchor | null, +): LayoutRect | null { + const point = resolvePmPoint(options.containers, pos, anchor); + if (!point) { + return null; + } + + const doc = point.node.ownerDocument ?? document; + const range = doc.createRange(); + range.setStart(point.node, point.offset); + range.setEnd(point.node, point.offset); + + const rangeRect = range.getBoundingClientRect(); + const lineRect = point.lineElement.getBoundingClientRect(); + const pageRect = point.pageElement.getBoundingClientRect(); + const pageIndex = Number(point.pageElement.dataset.pageIndex ?? 'NaN'); + if (!Number.isFinite(pageIndex)) { + return null; + } + + const localX = (rangeRect.left - pageRect.left) / options.zoom; + const localY = (lineRect.top - pageRect.top) / options.zoom; + if (!Number.isFinite(localX) || !Number.isFinite(localY)) { + return null; + } + + return { + pageIndex, + x: localX, + y: pageIndex * (options.pageHeight + options.pageGap) + localY, + width: 1, + height: Math.max(1, lineRect.height / options.zoom), + }; +} + +/** + * Selection rects for a ProseMirror range resolved via painted pm ranges. + * See {@link resolvePmPoint}; returns null so callers can fall back. + */ +export function computeSelectionRectsFromPmRange( + options: VisibleTextOffsetGeometryOptions, + from: number, + to: number, + anchors?: { from?: PmBlockAnchor | null; to?: PmBlockAnchor | null }, +): LayoutRect[] | null { + if (!Number.isFinite(from) || !Number.isFinite(to)) { + return null; + } + + const startPos = Math.min(from, to); + const endPos = Math.max(from, to); + if (startPos === endPos) { + return []; + } + + const startPoint = resolvePmPoint(options.containers, startPos, from <= to ? anchors?.from : anchors?.to); + const endPoint = resolvePmPoint(options.containers, endPos, from <= to ? anchors?.to : anchors?.from); + if (!startPoint || !endPoint) { + return null; + } + + const doc = startPoint.node.ownerDocument ?? document; + const range = doc.createRange(); + try { + range.setStart(startPoint.node, startPoint.offset); + range.setEnd(endPoint.node, endPoint.offset); + } catch { + return null; + } + + const rawRects = Array.from(range.getClientRects()) as unknown as DOMRect[]; + const pageElements: HTMLElement[] = []; + for (const pageElement of [startPoint.pageElement, endPoint.pageElement]) { + if (!pageElements.includes(pageElement)) { + pageElements.push(pageElement); + } + } + const rects = deduplicateOverlappingRects(rawRects); + const layoutRects: LayoutRect[] = []; + + for (const rect of rects) { + if (!Number.isFinite(rect.width) || !Number.isFinite(rect.height) || rect.width <= 0 || rect.height <= 0) { + continue; + } + + const pageElement = findPageElementForRect(rect, pageElements); + if (!pageElement) { + continue; + } + + const pageRect = pageElement.getBoundingClientRect(); + const pageIndex = Number(pageElement.dataset.pageIndex ?? 'NaN'); + if (!Number.isFinite(pageIndex)) { + continue; + } + + const localX = (rect.left - pageRect.left) / options.zoom; + const localY = (rect.top - pageRect.top) / options.zoom; + if (!Number.isFinite(localX) || !Number.isFinite(localY)) { + continue; + } + + layoutRects.push({ + pageIndex, + x: localX, + y: pageIndex * (options.pageHeight + options.pageGap) + localY, + width: Math.max(1, rect.width / options.zoom), + height: Math.max(1, rect.height / options.zoom), + }); + } + + return layoutRects; +} + function collectVisibleTextModel(containers: readonly HTMLElement[]): VisibleTextModel { const lines = collectRenderedLineElements(containers); if (!lines.length) { 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 22625445b6..c4087f816e 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 @@ -443,6 +443,147 @@ describe('EditorInputManager - Footnote click selection behavior', () => { expect(activateRenderedNoteSession).not.toHaveBeenCalled(); }); + describe('SD-3400: double-click a body footnote/endnote reference navigates to the note', () => { + const makeRefSpan = (pmStart: number, text: string) => { + const refEl = document.createElement('span'); + refEl.setAttribute('data-pm-start', String(pmStart)); + refEl.setAttribute('data-pm-end', String(pmStart + 1)); + refEl.textContent = text; + viewportHost.appendChild(refEl); + return refEl; + }; + + it('activates the footnote session for the referenced note', () => { + (mockEditor.state.doc as unknown as { nodeAt: (pos: number) => unknown }).nodeAt = (pos: number) => + pos === 38 ? { type: { name: 'footnoteReference' }, attrs: { id: '3' } } : null; + const refEl = makeRefSpan(38, '3'); + + refEl.dispatchEvent(new MouseEvent('dblclick', { bubbles: true, cancelable: true, button: 0, clientX: 5, clientY: 5 })); + + expect(activateRenderedNoteSession).toHaveBeenCalledWith( + { storyType: 'footnote', noteId: '3' }, + expect.objectContaining({ clientX: 5, clientY: 5 }), + ); + }); + + it('activates the endnote session for a body endnote reference', () => { + (mockEditor.state.doc as unknown as { nodeAt: (pos: number) => unknown }).nodeAt = (pos: number) => + pos === 50 ? { type: { name: 'endnoteReference' }, attrs: { id: '2' } } : null; + const refEl = makeRefSpan(50, 'ii'); + + refEl.dispatchEvent(new MouseEvent('dblclick', { bubbles: true, cancelable: true, button: 0, clientX: 7, clientY: 9 })); + + expect(activateRenderedNoteSession).toHaveBeenCalledWith( + { storyType: 'endnote', noteId: '2' }, + expect.objectContaining({ clientX: 7, clientY: 9 }), + ); + }); + + it('activates via elementsFromPoint when the event target is the selection overlay (real pointer path)', () => { + // Real double-clicks land on the transparent selection overlay above the + // pages, so event.target has no data-pm-start ancestor. The resolver must + // fall back to the elementsFromPoint hit chain. (Manual-testing regression: + // double-click on a body reference did nothing.) + (mockEditor.state.doc as unknown as { nodeAt: (pos: number) => unknown }).nodeAt = (pos: number) => + pos === 38 ? { type: { name: 'footnoteReference' }, attrs: { id: '4' } } : null; + const refEl = makeRefSpan(38, '4'); + + const overlay = document.createElement('div'); + overlay.className = 'presentation-editor__selection-overlay'; + viewportHost.appendChild(overlay); + + const originalElementsFromPoint = document.elementsFromPoint?.bind(document); + Object.defineProperty(document, 'elementsFromPoint', { + configurable: true, + value: () => [overlay, refEl], + }); + try { + overlay.dispatchEvent( + new MouseEvent('dblclick', { bubbles: true, cancelable: true, button: 0, clientX: 21, clientY: 33 }), + ); + + expect(activateRenderedNoteSession).toHaveBeenCalledWith( + { storyType: 'footnote', noteId: '4' }, + expect.objectContaining({ clientX: 21, clientY: 33 }), + ); + } finally { + Object.defineProperty(document, 'elementsFromPoint', { + configurable: true, + value: originalElementsFromPoint, + }); + } + }); + + it('does not activate when double-clicking ordinary body text', () => { + (mockEditor.state.doc as unknown as { nodeAt: (pos: number) => unknown }).nodeAt = (pos: number) => + pos === 12 ? { type: { name: 'text' }, attrs: {} } : null; + const refEl = makeRefSpan(12, 'word'); + + refEl.dispatchEvent(new MouseEvent('dblclick', { bubbles: true, cancelable: true, button: 0, clientX: 5, clientY: 5 })); + + expect(activateRenderedNoteSession).not.toHaveBeenCalled(); + }); + }); + + it('keeps backward (right-to-left) drag selection symmetric inside an active note (SD-3400)', () => { + // Pins the ticket's "selection consistent in both directions" requirement. + // Browser verification on footnote-tests.docx showed LTR and RTL drags + // produce the same range with anchor/head swapped; this test keeps the + // drag path direction-agnostic: anchor stays at the mousedown hit, head + // follows the pointer even when it moves backward. + const activeNoteEditor = createActiveSessionEditor(); + (mockDeps.getActiveStorySession as Mock).mockReturnValue({ + kind: 'note', + locator: { kind: 'story', storyType: 'footnote', noteId: '6' }, + editor: activeNoteEditor, + }); + (mockDeps.getActiveEditor as Mock).mockReturnValue(activeNoteEditor); + // Story-surface hit test: right side of the note resolves to pos 40, + // left side to pos 10. + mockCallbacks.hitTest = vi.fn((clientX: number) => ({ + pos: clientX > 100 ? 40 : 10, + layoutEpoch: 7, + pageIndex: 0, + blockId: 'footnote-6-0', + column: 0, + lineIndex: -1, + })); + manager.setCallbacks(mockCallbacks); + + const fragmentEl = document.createElement('span'); + fragmentEl.setAttribute('data-block-id', 'footnote-6-0'); + viewportHost.appendChild(fragmentEl); + + const PointerEventImpl = getPointerEventImpl(); + // Mouse down at the END of the text (pos 40)… + fragmentEl.dispatchEvent( + new PointerEventImpl('pointerdown', { + bubbles: true, + cancelable: true, + button: 0, + buttons: 1, + clientX: 120, + clientY: 16, + pointerId: 1, + } as PointerEventInit), + ); + // …then drag LEFT past the threshold to the start (pos 10). + viewportHost.dispatchEvent( + new PointerEventImpl('pointermove', { + bubbles: true, + cancelable: true, + buttons: 1, + clientX: 20, + clientY: 16, + pointerId: 1, + } as PointerEventInit), + ); + + // The selection extends backward: anchor stays at 40, head moves to 10. + expect(TextSelection.create as unknown as Mock).toHaveBeenCalledWith(activeNoteEditor.state.doc, 40, 10); + expect(activeNoteEditor.view.dispatch).toHaveBeenCalled(); + }); + it('does not activate a note session on semantic footnotes heading click', () => { (resolvePointerPositionHit as unknown as Mock).mockReturnValue(null); diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/FootnotesBuilder.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/FootnotesBuilder.test.ts index 2a7c6648bc..2715391c87 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/FootnotesBuilder.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/FootnotesBuilder.test.ts @@ -352,6 +352,48 @@ describe('buildFootnotesInput', () => { expect(firstRun?.vertAlign).toBe('superscript'); }); + it('falls back to the document default font when the first run carries none (SD-3400)', () => { + // Fallback chain: explicit first-run size -> document default -> constant. + // The mock toFlowBlocks produces runs without fontSize/fontFamily, so the + // marker must pick up the docDefaults run properties (half-points -> px). + const editorState = createMockEditorState([{ id: '1', pos: 10 }]); + const converter = createMockConverter([ + { id: '1', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Note' }] }] }, + ]); + const context = { + footnoteNumberById: { '1': 1 }, + translatedNumbering: {}, + translatedLinkedStyles: { + docDefaults: { runProperties: { fontSize: 22, fontFamily: { ascii: 'Times New Roman' } } }, + latentStyles: {}, + styles: {}, + }, + } as unknown as ConverterContext; + + const result = buildFootnotesInput(editorState, converter, context, undefined); + + const marker = (blocksFromResult(result)?.[0] as { runs?: Array<{ fontSize?: number; fontFamily?: string }> }) + ?.runs?.[0]; + // 22 half-points = 11pt = 11 / 0.75 px, then superscript-scaled. + expect(marker?.fontSize).toBeCloseTo((11 / 0.75) * SUBSCRIPT_SUPERSCRIPT_SCALE, 5); + expect(marker?.fontFamily).toBe('Times New Roman'); + }); + + it('keeps the constant marker fallback when neither run nor doc defaults carry a size', () => { + const editorState = createMockEditorState([{ id: '1', pos: 10 }]); + const converter = createMockConverter([ + { id: '1', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Note' }] }] }, + ]); + const context = createMockConverterContext({ '1': 1 }); + + const result = buildFootnotesInput(editorState, converter, context, undefined); + + const marker = (blocksFromResult(result)?.[0] as { runs?: Array<{ fontSize?: number; fontFamily?: string }> }) + ?.runs?.[0]; + expect(marker?.fontSize).toBe(12 * SUBSCRIPT_SUPERSCRIPT_SCALE); + expect(marker?.fontFamily).toBe('Arial'); + }); + it('uses correct display number from context', () => { const editorState = createMockEditorState([{ id: '5', pos: 10 }]); const converter = createMockConverter([ 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 7535fc4c9c..21ab9961a8 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 @@ -109,6 +109,13 @@ const { size: 10, }, textBetween: vi.fn(() => 'Lazy note session'), + // Mirror the real PM doc contract: this stub doc reports text via + // textBetween, so descendants must walk a matching text node (the + // note-session empty watch inspects it via isNoteContentEmpty). + descendants: vi.fn((cb: (node: unknown) => boolean | void) => { + cb({ isText: false, isAtom: false, type: { name: 'paragraph' } }); + cb({ isText: true, isAtom: true, text: 'Lazy note session', type: { name: 'text' } }); + }), }, }, options: {}, @@ -4731,6 +4738,48 @@ describe('PresentationEditor', () => { rafSpy.mockRestore(); }); + it('defers the selection overlay render for doc-changing transactions (SD-3400)', async () => { + // Editor emits 'selectionUpdate' BEFORE 'update', so for a transaction + // that changed the doc the epoch/layout gates are not armed yet: an + // immediate flush renders the caret against the PRE-change paint + // (stale caret on every Enter/Backspace). Doc-changing transactions + // must defer to the post-paint flush; selection-only changes keep the + // immediate path (collab-cancellation rationale). + const layoutResult = { + layout: { pages: [] }, + measures: [], + }; + mockIncrementalLayout.mockResolvedValue(layoutResult); + + editor = new PresentationEditor({ + element: container, + documentId: 'test-doc', + }); + + const mockEditorInstance = (Editor as unknown as MockedEditor).mock.results[ + (Editor as unknown as MockedEditor).mock.results.length - 1 + ].value; + await new Promise((resolve) => setTimeout(resolve, 100)); + + const rafSpy = vi.spyOn(window, 'requestAnimationFrame'); + const onCalls = mockEditorInstance.on as unknown as Mock; + const selectionUpdateCall = onCalls.mock.calls.find((call) => call[0] === 'selectionUpdate'); + const handleSelection = selectionUpdateCall![1] as (payload?: { + transaction?: { docChanged?: boolean }; + }) => void; + + // Doc-changing transaction: must NOT render synchronously (RAF-deferred). + handleSelection({ transaction: { docChanged: true } }); + expect(rafSpy).toHaveBeenCalled(); + + // Selection-only transaction: immediate path, no RAF needed. + rafSpy.mockClear(); + handleSelection({ transaction: { docChanged: false } }); + expect(rafSpy).not.toHaveBeenCalled(); + + rafSpy.mockRestore(); + }); + it('should skip scheduling during rerender (#isRerendering flag)', async () => { const layoutResult = { layout: { pages: [] }, diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/VisibleTextOffsetGeometry.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/VisibleTextOffsetGeometry.test.ts index 822be59436..bb01b3bcea 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/VisibleTextOffsetGeometry.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/VisibleTextOffsetGeometry.test.ts @@ -2,6 +2,8 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { computeCaretRectFromVisibleTextOffset, + computeCaretRectFromPmPosition, + computeSelectionRectsFromPmRange, computeSelectionRectsFromVisibleTextOffsets, measureVisibleTextOffset, type VisibleTextOffsetGeometryOptions, @@ -94,6 +96,173 @@ describe('computeCaretRectFromVisibleTextOffset', () => { }); }); +describe('computeCaretRectFromPmPosition (SD-3400 multi-paragraph note caret)', () => { + /** + * Mirrors the painted shape of a 3-paragraph note ("here is the footnote i + * am adding" / "thank you for this" / "ddd"): one fragment per paragraph, + * lines carrying SESSION-coordinate pm ranges with +4 token gaps at the + * paragraph boundaries (34->38, 56->60). The visible-text bridge mapped the + * caret for pm 60 into paragraph 2 ("thank you for thi|s"); pm resolution + * must place it on paragraph 3. + */ + function buildThreeParagraphNote() { + const page = document.createElement('div'); + page.className = 'superdoc-page'; + page.dataset.pageIndex = '0'; + page.innerHTML = ` +
+
+ + here is the footnote i am adding +
+
+
+
+ thank you for this +
+
+
+
+ ddd +
+
+ `; + document.body.appendChild(page); + + const fragments = Array.from(page.querySelectorAll('[data-block-id]')); + const textNodeOf = (text: string) => + Array.from(page.querySelectorAll('span')).find((el) => el.textContent === text)?.firstChild as Text; + + page.getBoundingClientRect = vi.fn(() => createRect(0, 0, 612, 792)); + const lines = Array.from(page.querySelectorAll('.superdoc-line')); + lines[0]!.getBoundingClientRect = vi.fn(() => createRect(10, 500, 300, 15)); + lines[1]!.getBoundingClientRect = vi.fn(() => createRect(10, 515, 120, 15)); + lines[2]!.getBoundingClientRect = vi.fn(() => createRect(10, 530, 30, 15)); + + return { page, fragments, textNodeOf }; + } + + it('places the caret on the THIRD paragraph for a position after two paragraph breaks', () => { + const { fragments, textNodeOf } = buildThreeParagraphNote(); + const dddTextNode = textNodeOf('ddd'); + + vi.spyOn(Range.prototype, 'getBoundingClientRect').mockImplementation(function () { + if (this.startContainer === dddTextNode && this.startOffset === 0) { + return createRect(10, 530, 0, 15); + } + return createRect(0, 0, 0, 0); + }); + + const rect = computeCaretRectFromPmPosition(createGeometryOptions(fragments), 60); + + expect(rect).toMatchObject({ pageIndex: 0, x: 10, y: 530, height: 15 }); + }); + + it('places a mid-paragraph caret at the pm offset within the leaf text', () => { + const { fragments, textNodeOf } = buildThreeParagraphNote(); + const thankTextNode = textNodeOf('thank you for this'); + + vi.spyOn(Range.prototype, 'getBoundingClientRect').mockImplementation(function () { + if (this.startContainer === thankTextNode && this.startOffset === 6) { + return createRect(50, 515, 0, 15); + } + return createRect(0, 0, 0, 0); + }); + + // pm 44 = 6 chars into "thank you for this" (leaf pmStart 38). + const rect = computeCaretRectFromPmPosition(createGeometryOptions(fragments), 44); + + expect(rect).toMatchObject({ pageIndex: 0, x: 50, y: 515, height: 15 }); + }); + + it('snaps a position inside an interior structural gap forward to the next line (SD-3400)', () => { + // Positions on paragraph-boundary tokens (e.g. 36 in the 34->38 gap) are + // valid caret positions in the session doc. Returning null here forced the + // drift-prone visible-text bridge; snap forward to the next painted line. + const { fragments, textNodeOf } = buildThreeParagraphNote(); + const thankTextNode = textNodeOf('thank you for this'); + + vi.spyOn(Range.prototype, 'getBoundingClientRect').mockImplementation(function () { + if (this.startContainer === thankTextNode && this.startOffset === 0) { + return createRect(10, 515, 0, 15); + } + return createRect(0, 0, 0, 0); + }); + + const rect = computeCaretRectFromPmPosition(createGeometryOptions(fragments), 36); + + expect(rect).toMatchObject({ pageIndex: 0, x: 10, y: 515, height: 15 }); + }); + + it('returns null for a position beyond the painted lines so callers can retry after paint', () => { + // A brand-new paragraph that has not painted yet has positions past every + // painted line: that must stay null (the caller reschedules post-paint). + const { fragments } = buildThreeParagraphNote(); + + expect(computeCaretRectFromPmPosition(createGeometryOptions(fragments), 70)).toBeNull(); + }); + + it('ignores the pm-less synthetic marker text', () => { + const { fragments, textNodeOf } = buildThreeParagraphNote(); + const firstTextNode = textNodeOf('here is the footnote i am adding'); + + vi.spyOn(Range.prototype, 'getBoundingClientRect').mockImplementation(function () { + if (this.startContainer === firstTextNode && this.startOffset === 0) { + return createRect(22, 500, 0, 15); + } + return createRect(0, 0, 0, 0); + }); + + const rect = computeCaretRectFromPmPosition(createGeometryOptions(fragments), 2); + + expect(rect).toMatchObject({ pageIndex: 0, x: 22, y: 500, height: 15 }); + }); +}); + +describe('computeSelectionRectsFromPmRange (SD-3400)', () => { + it('builds selection rects across paragraph boundaries from pm positions', () => { + const page = document.createElement('div'); + page.className = 'superdoc-page'; + page.dataset.pageIndex = '0'; + page.innerHTML = ` +
+
+ first +
+
+
+
+ second +
+
+ `; + document.body.appendChild(page); + + const fragments = Array.from(page.querySelectorAll('[data-block-id]')); + const firstTextNode = Array.from(page.querySelectorAll('span')).find((el) => el.textContent === 'first') + ?.firstChild as Text; + const secondTextNode = Array.from(page.querySelectorAll('span')).find((el) => el.textContent === 'second') + ?.firstChild as Text; + + page.getBoundingClientRect = vi.fn(() => createRect(0, 0, 612, 792)); + + vi.spyOn(Range.prototype, 'getClientRects').mockImplementation(function () { + if (this.startContainer === firstTextNode && this.startOffset === 2 && this.endContainer === secondTextNode) { + return [createRect(20, 500, 60, 15), createRect(10, 515, 30, 15)] as unknown as DOMRectList; + } + return [] as unknown as DOMRectList; + }); + + // pm 4 (inside "first") to pm 15 (inside "second"). + const rects = computeSelectionRectsFromPmRange(createGeometryOptions(fragments), 4, 15); + + expect(rects).toEqual([ + { pageIndex: 0, x: 20, y: 500, width: 60, height: 15 }, + { pageIndex: 0, x: 10, y: 515, width: 30, height: 15 }, + ]); + }); +}); + describe('computeSelectionRectsFromVisibleTextOffsets', () => { it('maps later-word selection offsets after an inserted run to the correct painted range', () => { const page = document.createElement('div'); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/footnote-wrappers.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/footnote-wrappers.test.ts index c5c29553f2..c6005d3ea7 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/footnote-wrappers.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/footnote-wrappers.test.ts @@ -73,6 +73,8 @@ import { footnotesUpdateWrapper, footnotesRemoveWrapper, footnotesConfigureWrapper, + removeNoteEverywhere, + removeNoteReferenceAt, } from './footnote-wrappers.js'; // --------------------------------------------------------------------------- @@ -153,6 +155,7 @@ function makeEditor( state: { doc: makeDocWithFootnoteRefs(refs), tr, + selection: { head: 1, from: 1, to: 1 }, }, schema: { nodes: { @@ -217,6 +220,148 @@ describe('footnote-wrappers', () => { expect(noteElements[0].attributes['w:id']).toBe('1'); }); + it('removeNoteEverywhere deletes ALL references to the note and the OOXML element (SD-3400)', () => { + // Two references to footnote id '2' (multi-ref note emptied in the area): + // both markers and the element must go. + const editor = makeEditor([{ id: '2', text: 'Shared note' }], ['2', '2']); + + const result = removeNoteEverywhere(editor, { noteId: '2', type: 'footnote' }); + + expect(result.success).toBe(true); + expect(editor.state.tr.delete).toHaveBeenCalledTimes(2); + expect(getFootnoteElements(editor)).toHaveLength(0); + }); + + it('removeNoteEverywhere is type-aware: endnote id N never touches footnote id N (SD-3400)', () => { + const editor = makeEditor([{ id: '2', text: 'Footnote two' }], []); + // Document carries BOTH a footnote ref and an endnote ref with id '2'. + const mixedDoc = { + descendants: (cb: (node: unknown, pos: number) => boolean | void) => { + cb({ type: { name: 'footnoteReference' }, attrs: { id: '2' } }, 1); + cb({ type: { name: 'endnoteReference' }, attrs: { id: '2' } }, 5); + return true; + }, + nodeAt: vi.fn(() => ({ nodeSize: 1 })), + }; + (editor.state as unknown as { doc: unknown }).doc = mixedDoc; + (editor.state.tr as unknown as { doc: unknown }).doc = mixedDoc; + + const result = removeNoteEverywhere(editor, { noteId: '2', type: 'footnote' }); + + expect(result.success).toBe(true); + // Only the footnote reference (pos 1) is deleted; the endnote ref survives. + expect(editor.state.tr.delete).toHaveBeenCalledTimes(1); + expect(editor.state.tr.delete).toHaveBeenCalledWith(1, 2); + expect(getFootnoteElements(editor)).toHaveLength(0); + }); + + it('removeNoteReferenceAt deletes the reference at the exact position, not the first id match (SD-3400)', () => { + // Two references to footnote id '2' at positions 1 and 2; the staged + // delete targets the SECOND one. The element survives because the first + // reference still exists. + const editor = makeEditor([{ id: '2', text: 'Shared note' }], ['2', '2'], { refsAfterDispatch: ['2'] }); + + const removed = removeNoteReferenceAt(editor, { pos: 2, noteId: '2', type: 'footnote' }); + + expect(removed).toBe(true); + expect(editor.state.tr.delete).toHaveBeenCalledTimes(1); + expect(editor.state.tr.delete).toHaveBeenCalledWith(2, 3); + expect(getFootnoteElements(editor)).toHaveLength(1); + }); + + it('removeNoteReferenceAt prunes the OOXML element when the last reference is deleted (SD-3400)', () => { + // Body-side staged delete symmetry: the second Backspace must not leave + // an orphaned w:footnote element behind. + const editor = makeEditor([{ id: '2', text: 'Note 2' }], ['2'], { refsAfterDispatch: [] }); + + const removed = removeNoteReferenceAt(editor, { pos: 1, noteId: '2', type: 'footnote' }); + + expect(removed).toBe(true); + expect(getFootnoteElements(editor)).toHaveLength(0); + }); + + it('removeNoteEverywhere is a NO_OP failure when no reference of that type exists', () => { + const editor = makeEditor([{ id: '3', text: 'Orphan' }], []); + + const result = removeNoteEverywhere(editor, { noteId: '3', type: 'footnote' }); + + expect(result.success).toBe(false); + expect(editor.state.tr.delete).not.toHaveBeenCalled(); + expect(getFootnoteElements(editor)).toHaveLength(1); + }); + + it('rejects insertion from a story editor (footnote inside a note is non-conformant, SD-3400)', () => { + // §17.11.14: a footnoteReference inside a footnote/endnote makes the + // document non-conformant. Story editors carry options.parentEditor. + const editor = makeEditor([], []); + (editor as unknown as { options: Record }).options = { parentEditor: makeEditor([], []) }; + + const result = footnotesInsertWrapper(editor, { type: 'footnote', content: '' }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.failure.code).toBe('INVALID_TARGET'); + } + // Nothing was inserted anywhere. + expect(editor.state.tr.insert).not.toHaveBeenCalled(); + expect(getFootnoteElements(editor)).toHaveLength(0); + }); + + it('inserts at the current selection head when at is omitted (SD-3400 toolbar path)', () => { + const editor = makeEditor([], []); + + const result = footnotesInsertWrapper(editor, { type: 'footnote', content: '' }); + + expect(result.success).toBe(true); + // The reference node lands at the selection head, no TextTarget required. + expect(editor.state.tr.insert).toHaveBeenCalledWith(1, expect.anything()); + expect(getFootnoteElements(editor)).toHaveLength(1); + }); + + it('stamps w:pStyle FootnoteText on generated note paragraphs (Word fidelity, SD-3400)', () => { + // Word always styles footnote body paragraphs with FootnoteText; without + // it, exported new footnotes render at Normal/11pt in Word. + const editor = makeEditor([], []); + + footnotesInsertWrapper(editor, { type: 'footnote', content: 'Styled note' }); + + const note = getFootnoteElements(editor)[0] as unknown as { + elements: Array<{ name: string; elements?: Array<{ name: string; attributes?: Record }> }>; + }; + const paragraph = note.elements.find((el) => el.name === 'w:p'); + const pPr = paragraph?.elements?.find((el) => el.name === 'w:pPr'); + const pStyle = (pPr as { elements?: Array<{ name: string; attributes?: Record }> })?.elements?.find( + (el) => el.name === 'w:pStyle', + ); + expect(pStyle?.attributes?.['w:val']).toBe('FootnoteText'); + }); + + it('bootstrap writes the special-footnote list to settings.xml (17.11.9, SD-3400)', () => { + const editor = makeEditor([], [], { omitFootnotesPart: true }); + + footnotesInsertWrapper(editor, { type: 'footnote', content: 'First footnote' }); + + const converter = (editor as unknown as { converter: { convertedXml: Record } }).converter; + const settingsRoot = (converter.convertedXml['word/settings.xml'] as XmlDoc).elements[0]; + const pr = settingsRoot.elements.find((el) => el.name === 'w:footnotePr') as unknown as { + elements: Array<{ name: string; attributes: Record }>; + }; + const ids = pr.elements.filter((el) => el.name === 'w:footnote').map((el) => el.attributes['w:id']); + expect(ids).toEqual(['-1', '0']); + }); + + it('bootstrap leaves settings.xml untouched when the notes part already exists', () => { + // Imported documents own their settings; the special list is only seeded + // alongside a freshly bootstrapped notes part. + const editor = makeEditor([{ id: '1', text: 'Existing' }], ['1']); + + footnotesInsertWrapper(editor, { type: 'footnote', content: 'Second' }); + + const converter = (editor as unknown as { converter: { convertedXml: Record } }).converter; + const settingsRoot = (converter.convertedXml['word/settings.xml'] as XmlDoc).elements[0]; + expect(settingsRoot.elements.find((el) => el.name === 'w:footnotePr')).toBeUndefined(); + }); + it('allocates a note id that avoids all existing ids', () => { const editor = makeEditor([], ['7', '3']); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/footnote-wrappers.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/footnote-wrappers.ts index 75cd5ead45..85d9cd469b 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/footnote-wrappers.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/footnote-wrappers.ts @@ -192,6 +192,18 @@ export function footnotesInsertWrapper( rejectTrackedMode('footnotes.insert', options); checkRevision(editor, options?.expectedRevision); + // §17.11.14: a footnote reference inside a footnote or endnote makes the + // document non-conformant (and Word also forbids footnotes in headers and + // footers). Story editors carry options.parentEditor — reject insertion + // there so toolbar actions wired to the ACTIVE editor cannot write a + // footnoteReference into a note story; callers must use the host editor. + if ((editor.options as { parentEditor?: unknown } | undefined)?.parentEditor) { + return footnoteFailure( + 'INVALID_TARGET', + 'footnotes.insert: footnotes can only be inserted into the document body, not inside a footnote, endnote, header, or footer.', + ); + } + const converter = getConverter(editor); const notesConfig = getNotesConfig(input.type); const noteId = allocateNextNoteId(editor, converter, input.type); @@ -210,7 +222,11 @@ export function footnotesInsertWrapper( ); } - const resolved = resolveInlineInsertPosition(editor, input.at, 'footnotes.insert'); + // SD-3400: omitting `at` inserts at the current selection head — the natural + // target for toolbar actions ("place a marker at the current cursor location"). + const resolved = input.at + ? resolveInlineInsertPosition(editor, input.at, 'footnotes.insert') + : { from: editor.state.selection.head, to: editor.state.selection.head }; const { success } = compoundMutation({ editor, @@ -309,34 +325,58 @@ export function footnotesRemoveWrapper( return footnoteSuccess(address); } - const notesConfig = getNotesConfig(resolved.type); + const removed = removeNoteReferenceAt(editor, { + pos: resolved.pos, + noteId: resolved.noteId, + type: resolved.type, + }); + + if (!removed) { + return footnoteFailure('NO_OP', 'Remove operation produced no change.'); + } + + return footnoteSuccess(address); +} + +/** + * Remove the single note reference at an exact document position, pruning the + * OOXML note element when no other reference of the same type remains. + * + * Position-addressed (not id-addressed) so callers that already hold the node + * — the staged Backspace/Delete on a selected marker (SD-3400) — remove + * exactly that reference even when the same id appears multiple times. + * {@link footnotesRemoveWrapper} delegates here after resolving its target. + */ +export function removeNoteReferenceAt( + editor: Editor, + ref: { pos: number; noteId: string; type: 'footnote' | 'endnote' }, +): boolean { + const notesConfig = getNotesConfig(ref.type); const { success } = compoundMutation({ editor, - source: `footnotes.remove:${resolved.type}`, + source: `footnotes.remove:${ref.type}`, affectedParts: [notesConfig.partId], execute: () => { // 1. Delete the reference node from the PM document const { tr } = editor.state; - const node = tr.doc.nodeAt(resolved.pos); + const node = tr.doc.nodeAt(ref.pos); if (!node) return false; - tr.delete(resolved.pos, resolved.pos + node.nodeSize); + tr.delete(ref.pos, ref.pos + node.nodeSize); editor.dispatch(tr); // 2. Remove from the OOXML part if no other references remain - const stillReferenced = findAllFootnotes(editor.state.doc, resolved.type).some( - (f) => f.noteId === resolved.noteId, - ); + const stillReferenced = findAllFootnotes(editor.state.doc, ref.type).some((f) => f.noteId === ref.noteId); if (!stillReferenced) { mutatePart({ editor, partId: notesConfig.partId, operation: 'mutate', - source: `footnotes.remove:${resolved.type}`, + source: `footnotes.remove:${ref.type}`, mutate({ part }) { - removeNoteElement(part, notesConfig, resolved.noteId); + removeNoteElement(part, notesConfig, ref.noteId); }, }); } @@ -346,6 +386,62 @@ export function footnotesRemoveWrapper( }, }); + return success; +} + +/** + * SD-3400: remove a note and EVERY body reference to it ("remove on both + * sides"). Used by the note-area emptied-note commit, where the whole footnote + * ceases to exist — including multi-reference notes, whose surviving markers + * would otherwise keep the old (un-emptied) content. + * + * Type-aware: footnote and endnote ids are independent OOXML namespaces, so + * resolution filters by note type — emptying endnote "2" must never touch + * footnote "2". The address-based {@link footnotesRemoveWrapper} keeps its + * single-reference semantics for the document API. + */ +export function removeNoteEverywhere( + editor: Editor, + input: { noteId: string; type: 'footnote' | 'endnote' }, +): FootnoteMutationResult { + const refs = findAllFootnotes(editor.state.doc, input.type).filter((f) => f.noteId === input.noteId); + if (refs.length === 0) { + return footnoteFailure('NO_OP', `No ${input.type} reference with id "${input.noteId}" found.`); + } + + const notesConfig = getNotesConfig(input.type); + const address: FootnoteAddress = { kind: 'entity', entityType: 'footnote', noteId: input.noteId }; + + const { success } = compoundMutation({ + editor, + source: `footnotes.removeEverywhere:${input.type}`, + affectedParts: [notesConfig.partId], + execute: () => { + const { tr } = editor.state; + // Descending positions keep earlier offsets valid as later refs go. + [...refs] + .sort((a, b) => b.pos - a.pos) + .forEach((ref) => { + const node = tr.doc.nodeAt(ref.pos); + if (node) tr.delete(ref.pos, ref.pos + node.nodeSize); + }); + editor.dispatch(tr); + + mutatePart({ + editor, + partId: notesConfig.partId, + operation: 'mutate', + source: `footnotes.removeEverywhere:${input.type}`, + mutate({ part }) { + removeNoteElement(part, notesConfig, input.noteId); + }, + }); + + clearIndexCache(editor); + return true; + }, + }); + if (!success) { return footnoteFailure('NO_OP', 'Remove operation produced no change.'); } diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/story-runtime/note-story-runtime.integration.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/story-runtime/note-story-runtime.integration.test.ts new file mode 100644 index 0000000000..b101b15185 --- /dev/null +++ b/packages/super-editor/src/editors/v1/document-api-adapters/story-runtime/note-story-runtime.integration.test.ts @@ -0,0 +1,159 @@ +/** + * SD-3400 area-delete integration: commit of an EMPTIED note runs the REAL + * removal pipeline (removeNoteEverywhere -> removeNoteElement) against a real + * convertedXml part. Unlike note-story-runtime.test.ts, the footnote-wrappers + * module is NOT mocked here — only the story-editor factory (DOM-bound) and + * the part-mutation transaction plumbing are shimmed, with mutatePart applying + * the mutation directly to the part. + */ + +import { describe, it, expect, vi } from 'vitest'; +import type { Editor } from '../../core/Editor.js'; + +const mockCreateStoryEditor = vi.fn(); + +vi.mock('../../core/story-editor-factory.js', () => ({ + createStoryEditor: (...args: unknown[]) => mockCreateStoryEditor(...args), +})); + +vi.mock('../../core/parts/mutation/mutate-part.js', () => ({ + mutatePart: vi.fn( + (request: { mutate?: (ctx: { part: unknown; dryRun: boolean }) => unknown; editor: Editor; partId: string }) => { + const converter = (request.editor as unknown as { converter?: { convertedXml?: Record } }) + .converter; + const part = converter?.convertedXml?.[request.partId] ?? {}; + request.mutate?.({ part, dryRun: false }); + if (converter?.convertedXml) converter.convertedXml[request.partId] = part; + return { changed: true, changedPaths: [], degraded: false, result: undefined }; + }, + ), +})); + +vi.mock('../../core/parts/mutation/compound-mutation.js', () => ({ + compoundMutation: vi.fn((request: { execute: () => boolean }) => ({ success: request.execute() })), +})); + +vi.mock('../helpers/index-cache.js', () => ({ + clearIndexCache: vi.fn(), +})); + +import { resolveNoteRuntime } from './note-story-runtime.js'; + +function makeFootnotesXml(entries: Array<{ id: string }>) { + return { + declaration: { attributes: { version: '1.0', encoding: 'UTF-8', standalone: 'yes' } }, + elements: [ + { + type: 'element', + name: 'w:footnotes', + attributes: { 'xmlns:w': 'http://schemas.openxmlformats.org/wordprocessingml/2006/main' }, + elements: entries.map((e) => ({ + type: 'element', + name: 'w:footnote', + attributes: { 'w:id': e.id }, + elements: [], + })), + }, + ], + }; +} + +function footnoteElementIds(host: { converter: { convertedXml: Record } }): string[] { + const xml = host.converter.convertedXml['word/footnotes.xml'] as { + elements: Array<{ elements: Array<{ name: string; attributes: Record }> }>; + }; + return xml.elements[0].elements.filter((el) => el.name === 'w:footnote').map((el) => el.attributes['w:id']); +} + +/** Host whose body has footnoteReference markers for the given ids. */ +function makeHost(refIds: string[]) { + const doc = { + descendants: (cb: (node: unknown, pos: number) => boolean | void) => { + refIds.forEach((id, index) => { + cb({ type: { name: 'footnoteReference' }, attrs: { id } }, index + 1); + }); + return true; + }, + nodeAt: vi.fn(() => ({ nodeSize: 1 })), + }; + const tr = { delete: vi.fn(), doc }; + return { + converter: { + footnotes: [{ id: '1', content: [{ type: 'paragraph' }] }], + endnotes: [], + convertedXml: { 'word/footnotes.xml': makeFootnotesXml([{ id: '1' }, { id: '2' }]) }, + }, + state: { doc, tr }, + dispatch: vi.fn(), + on: vi.fn(), + emit: vi.fn(), + safeEmit: vi.fn(() => []), + options: {}, + } as unknown as Editor & { converter: { convertedXml: Record } }; +} + +const footnoteLocator = { kind: 'story' as const, storyType: 'footnote' as const, noteId: '1' }; + +/** Story editor whose doc is empty (only an empty paragraph). */ +const emptiedStoryEditor = () => ({ + state: { + doc: { + content: { size: 4 }, + textBetween: () => '', + descendants: (cb: (n: unknown, p: number) => boolean | void) => { + cb({ isText: false, isAtom: false, type: { name: 'paragraph' } }, 0); + }, + }, + }, + schema: {}, + getJSON: () => ({ type: 'doc', content: [{ type: 'paragraph' }] }), + getUpdatedJson: () => ({ type: 'doc', content: [{ type: 'paragraph' }] }), + destroy: vi.fn(), + on: vi.fn(), +}); + +describe('SD-3400 area-delete integration (real removal pipeline)', () => { + it('committing an emptied note deletes every body marker and the w:footnote element', () => { + mockCreateStoryEditor.mockReturnValueOnce(emptiedStoryEditor() as never); + // Two references to note 1 (multi-ref) plus an unrelated note 2 marker. + const host = makeHost(['1', '2', '1']); + + const runtime = resolveNoteRuntime(host, footnoteLocator); + runtime.commit?.(host); + + // Both markers for note 1 deleted, descending positions (3 then 1). + expect(host.state.tr.delete).toHaveBeenCalledTimes(2); + expect((host.state.tr.delete as ReturnType).mock.calls).toEqual([ + [3, 4], + [1, 2], + ]); + // The w:footnote element is physically gone from the part; note 2 survives. + expect(footnoteElementIds(host)).toEqual(['2']); + }); + + it('committing a note that still has content leaves markers and the element alone', () => { + mockCreateStoryEditor.mockReturnValueOnce({ + ...emptiedStoryEditor(), + state: { + doc: { + content: { size: 10 }, + textBetween: () => 'still here', + descendants: (cb: (n: unknown, p: number) => boolean | void) => { + cb({ isText: true, isAtom: true, text: 'still here', type: { name: 'text' } }, 1); + }, + }, + }, + getUpdatedJson: () => ({ + type: 'doc', + content: [{ type: 'paragraph', content: [{ type: 'text', text: 'still here' }] }], + }), + } as never); + const host = makeHost(['1', '2']); + + const runtime = resolveNoteRuntime(host, footnoteLocator); + runtime.commit?.(host); + + expect(host.state.tr.delete).not.toHaveBeenCalled(); + expect(footnoteElementIds(host)).toEqual(['1', '2']); + }); +}); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/story-runtime/note-story-runtime.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/story-runtime/note-story-runtime.test.ts index ece7557c3b..d47e2765e2 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/story-runtime/note-story-runtime.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/story-runtime/note-story-runtime.test.ts @@ -5,7 +5,7 @@ * empty or blank notes to be misclassified as missing. */ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; import { DocumentApiAdapterError } from '../errors.js'; // --------------------------------------------------------------------------- @@ -36,6 +36,14 @@ vi.mock('../../core/parts/adapters/notes-part-descriptor.js', () => ({ updateNoteElement: vi.fn(), })); +// SD-3400: mock the removal boundary so the commit-on-empty wiring can be +// asserted without exercising removeNoteEverywhere's internals (covered by +// footnote-wrappers.test.ts). +const mockRemoveNoteEverywhere = vi.fn(() => ({ success: true })); +vi.mock('../plan-engine/footnote-wrappers.js', () => ({ + removeNoteEverywhere: (...args: unknown[]) => mockRemoveNoteEverywhere(...args), +})); + // Import after mocks are set up import { resolveNoteRuntime } from './note-story-runtime.js'; @@ -254,3 +262,107 @@ describe('resolveNoteRuntime — empty note content', () => { expect(mockCreateStoryEditor).toHaveBeenCalledWith(hostEditor, doc, expect.any(Object)); }); }); + +describe('SD-3400: note commits strip footnote references (17.11.14)', () => { + it('removes pasted footnoteReference nodes from the exported note content', () => { + const exportToXmlJson = vi.fn(() => ({ + result: { elements: [{ elements: [{ type: 'element', name: 'w:p' }] }] }, + })); + // Story doc has real text (not empty) plus a pasted footnoteReference node. + mockCreateStoryEditor.mockReturnValueOnce({ + state: { + doc: { + content: { size: 8 }, + textBetween: () => 'kept', + descendants: (cb: (n: unknown) => boolean | void) => { + cb({ isText: true, isAtom: true, text: 'kept', type: { name: 'text' } }); + }, + }, + }, + schema: {}, + getJSON: () => ({ type: 'doc', content: [] }), + getUpdatedJson: () => ({ + type: 'doc', + content: [ + { + type: 'paragraph', + content: [ + { type: 'text', text: 'kept' }, + { type: 'footnoteReference', attrs: { id: '9' } }, + ], + }, + ], + }), + destroy: vi.fn(), + on: vi.fn(), + } as never); + const host = { + converter: { footnotes: [{ id: '1', content: [{ type: 'paragraph' }] }], endnotes: [], exportToXmlJson }, + state: { + doc: { + descendants: (cb: (n: unknown, p: number) => void) => + cb({ type: { name: 'footnoteReference' }, attrs: { id: '1' } }, 5), + }, + }, + on: vi.fn(), + } as any; + + const runtime = resolveNoteRuntime(host, footnoteLocator); + runtime.commit?.(host); + + expect(exportToXmlJson).toHaveBeenCalledTimes(1); + const exported = JSON.stringify(exportToXmlJson.mock.calls[0][0].data); + expect(exported).not.toContain('footnoteReference'); + expect(exported).toContain('kept'); + }); +}); + +describe('SD-3400: clearing a note in the area removes the footnote on both sides', () => { + beforeEach(() => mockRemoveNoteEverywhere.mockClear()); + + const makeHost = () => + ({ + converter: { footnotes: [{ id: '1', content: [{ type: 'paragraph' }] }], endnotes: [] }, + state: { doc: { descendants: (cb: (n: unknown, p: number) => void) => cb({ type: { name: 'footnoteReference' }, attrs: { id: '1' } }, 5) } }, + on: vi.fn(), + }) as any; + + const storyEditorWith = (descendants: (cb: (n: unknown, p: number) => boolean | void) => void) => ({ + state: { doc: { content: { size: 4 }, textBetween: () => '', descendants } }, + schema: {}, + getJSON: () => ({ type: 'doc', content: [{ type: 'paragraph' }] }), + getUpdatedJson: () => ({ type: 'doc', content: [{ type: 'paragraph' }] }), + destroy: vi.fn(), + on: vi.fn(), + }); + + it('removes both the body reference and the note element when the committed content is empty', () => { + // Story doc holds only an empty paragraph — no text, no atoms. + mockCreateStoryEditor.mockReturnValueOnce( + storyEditorWith((cb) => { + cb({ isText: false, isAtom: false, type: { name: 'paragraph' } }, 0); + }) as never, + ); + const host = makeHost(); + const runtime = resolveNoteRuntime(host, footnoteLocator); + + runtime.commit?.(host); + + expect(mockRemoveNoteEverywhere).toHaveBeenCalledWith(host, { noteId: '1', type: 'footnote' }); + }); + + it('does not remove the footnote when the committed note still has content', () => { + mockCreateStoryEditor.mockReturnValueOnce( + storyEditorWith((cb) => { + cb({ isText: false, isAtom: false, type: { name: 'paragraph' } }, 0); + cb({ isText: true, isAtom: true, text: 'kept', type: { name: 'text' } }, 1); + }) as never, + ); + const host = makeHost(); + const runtime = resolveNoteRuntime(host, footnoteLocator); + + runtime.commit?.(host); + + expect(mockRemoveNoteEverywhere).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/story-runtime/note-story-runtime.ts b/packages/super-editor/src/editors/v1/document-api-adapters/story-runtime/note-story-runtime.ts index 488797a3ce..14f27e330c 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/story-runtime/note-story-runtime.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/story-runtime/note-story-runtime.ts @@ -20,9 +20,30 @@ import { updateNoteElement, } from '../../core/parts/adapters/notes-part-descriptor.js'; import { normalizeNotePmJson } from '../helpers/note-pm-json.js'; +import { removeNoteEverywhere } from '../plan-engine/footnote-wrappers.js'; +import type { Node as ProseMirrorNode } from 'prosemirror-model'; type NoteStoryLocator = FootnoteStoryLocator | EndnoteStoryLocator; +/** + * SD-3400: a note is "empty" once it holds no text and no embedded atoms + * (images, etc.). Whitespace-only content counts as empty — the user cleared it. + * Exported so PresentationEditor's note-session watcher applies the same rule. + */ +export function isNoteContentEmpty(doc: ProseMirrorNode): boolean { + let hasContent = false; + doc.descendants((node) => { + if (hasContent) return false; + if (node.isText) { + if ((node.text ?? '').trim().length > 0) hasContent = true; + } else if (node.isAtom && node.type.name !== 'text') { + hasContent = true; + } + return !hasContent; + }); + return !hasContent; +} + interface NoteExportToXmlJsonResult { result?: { elements?: Array<{ @@ -97,6 +118,8 @@ export function resolveNoteRuntime(hostEditor: Editor, locator: NoteStoryLocator }; } +type NotesConfig = ReturnType; + function commitNoteRuntime( hostEditor: Editor, storyEditor: Editor, @@ -106,46 +129,105 @@ function commitNoteRuntime( const noteType = isFootnote ? 'footnote' : 'endnote'; const notesConfig = getNotesConfig(noteType); - // Try rich export via converter's exportToXmlJson (preserves formatting) + if (isNoteContentEmpty(storyEditor.state.doc)) { + removeEmptiedNote(hostEditor, locator); + return; + } + + if (commitRichNoteContent(hostEditor, storyEditor, locator, notesConfig)) { + return; + } + + commitPlainTextNoteContent(hostEditor, storyEditor, locator, notesConfig); +} + +/** + * SD-3400: clearing all content in the note area deletes the footnote on BOTH + * sides — the note element in the notes part AND every body reference — and + * the document renumbers. This mirrors the body-side staged delete; deleting + * from either side removes the whole footnote. Multi-reference notes lose all + * their markers (the emptied note no longer exists for any of them), and + * resolution is type-aware so emptying endnote "2" never touches footnote "2". + */ +function removeEmptiedNote(hostEditor: Editor, locator: NoteStoryLocator): void { + removeNoteEverywhere(hostEditor, { noteId: locator.noteId, type: locator.storyType }); +} + +const NOTE_REFERENCE_NODE_TYPES = new Set(['footnoteReference', 'endnoteReference']); + +/** + * §17.11.14: a footnote reference inside a footnote or endnote makes the + * document non-conformant. Reference nodes can reach a note story through + * paste (HTML containing `sup[data-footnote-id]` parses to footnoteReference); + * strip them before the note content is exported to the OOXML part. + */ +function stripNoteReferenceNodes(node: T): T { + if (!Array.isArray(node.content)) return node; + return { + ...node, + content: node.content + .filter((child) => !NOTE_REFERENCE_NODE_TYPES.has(child?.type ?? '')) + .map((child) => stripNoteReferenceNodes(child)), + }; +} + +/** + * Rich commit via the converter's exportToXmlJson (preserves formatting). + * Returns false when the converter is unavailable or export produced nothing, + * so the caller can fall back to plain text. + */ +function commitRichNoteContent( + hostEditor: Editor, + storyEditor: Editor, + locator: NoteStoryLocator, + notesConfig: NotesConfig, +): boolean { const conv = (hostEditor as unknown as { converter?: ConverterWithNoteExport }).converter; - const pmJson = + const rawPmJson = typeof storyEditor.getUpdatedJson === 'function' ? storyEditor.getUpdatedJson() : storyEditor.getJSON(); + if (!conv?.exportToXmlJson || !rawPmJson) return false; + const pmJson = stripNoteReferenceNodes(rawPmJson); - if (conv?.exportToXmlJson && pmJson) { - let ooxmlElements: unknown[] | null = null; - try { - const { result } = conv.exportToXmlJson({ - data: pmJson, - editor: storyEditor, - editorSchema: storyEditor.schema, - isHeaderFooter: true, - comments: [], - commentDefinitions: [], - }); - // result.elements[0] is the body wrapper; its children are all - // content elements (paragraphs, tables, etc.). Keep all of them - // so tables and other non-paragraph content survive the commit. - const body = result?.elements?.[0] as { elements?: unknown[] } | undefined; - ooxmlElements = body?.elements ?? null; - } catch { - // Fall through to plain-text fallback - } - - if (ooxmlElements && ooxmlElements.length > 0) { - mutatePart({ - editor: hostEditor, - partId: notesConfig.partId, - operation: 'mutate', - source: `story-runtime:commit:${locator.storyType}`, - mutate({ part }) { - updateNoteContentFromOoxml(part, notesConfig, locator.noteId, ooxmlElements!); - }, - }); - return; - } + let ooxmlElements: unknown[] | null = null; + try { + const { result } = conv.exportToXmlJson({ + data: pmJson, + editor: storyEditor, + editorSchema: storyEditor.schema, + isHeaderFooter: true, + comments: [], + commentDefinitions: [], + }); + // result.elements[0] is the body wrapper; its children are all + // content elements (paragraphs, tables, etc.). Keep all of them + // so tables and other non-paragraph content survive the commit. + const body = result?.elements?.[0] as { elements?: unknown[] } | undefined; + ooxmlElements = body?.elements ?? null; + } catch { + // Fall through to plain-text fallback } + if (!ooxmlElements || ooxmlElements.length === 0) return false; + + const elements = ooxmlElements; + mutatePart({ + editor: hostEditor, + partId: notesConfig.partId, + operation: 'mutate', + source: `story-runtime:commit:${locator.storyType}`, + mutate({ part }) { + updateNoteContentFromOoxml(part, notesConfig, locator.noteId, elements); + }, + }); + return true; +} - // Fallback: plain-text export (loses formatting) +/** Fallback: plain-text export (loses formatting). */ +function commitPlainTextNoteContent( + hostEditor: Editor, + storyEditor: Editor, + locator: NoteStoryLocator, + notesConfig: NotesConfig, +): void { const doc = storyEditor.state.doc; const text = doc.textBetween(0, doc.content.size, '\n', '\n'); diff --git a/packages/super-editor/src/editors/v1/extensions/footnote/delete-note-marker.js b/packages/super-editor/src/editors/v1/extensions/footnote/delete-note-marker.js new file mode 100644 index 0000000000..7143b859c3 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/footnote/delete-note-marker.js @@ -0,0 +1,45 @@ +import { removeNoteReferenceAt } from '../../document-api-adapters/plan-engine/footnote-wrappers.js'; + +const NOTE_TYPE_BY_NODE = { + footnoteReference: 'footnote', + endnoteReference: 'endnote', +}; + +/** + * SD-3400: detects the staged-delete selection produced by + * `selectFootnoteMarkerBefore`/`selectFootnoteMarkerAfter` — a TextSelection + * spanning exactly one footnote/endnote reference atom. + * + * @param {import('prosemirror-state').EditorState} state + * @returns {{ pos: number, noteId: string, type: 'footnote' | 'endnote' } | null} + */ +export function getSelectedNoteMarker(state) { + const { from, to, empty } = state.selection; + if (empty) return null; + + const node = state.doc.nodeAt(from); + const type = NOTE_TYPE_BY_NODE[node?.type?.name]; + if (!type || from + node.nodeSize !== to) return null; + + return { pos: from, noteId: String(node.attrs?.id ?? ''), type }; +} + +/** + * SD-3400: second stage of the Word-like staged delete, symmetric with the + * note-area delete ("remove on both sides"). Where plain `deleteSelection` + * would only remove the body marker — leaving an orphaned `w:footnote` + * element in the notes part — this routes the removal through the document + * API wrapper, which also prunes the OOXML element when the deleted marker + * was the note's last reference. + * + * Plain orchestrator (no PM command plumbing) so any caller can use it; the + * `deleteSelectedNoteMarker` editor command is a thin shim over this function. + * + * @param {import('@core/Editor.js').Editor} editor + * @returns {boolean} True when a staged-selected marker was removed. + */ +export function deleteSelectedNoteMarker(editor) { + const marker = getSelectedNoteMarker(editor.state); + if (!marker) return false; + return removeNoteReferenceAt(editor, marker); +} diff --git a/packages/super-editor/src/editors/v1/extensions/footnote/delete-note-marker.test.js b/packages/super-editor/src/editors/v1/extensions/footnote/delete-note-marker.test.js new file mode 100644 index 0000000000..2bd23f9485 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/footnote/delete-note-marker.test.js @@ -0,0 +1,129 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { Schema } from 'prosemirror-model'; +import { EditorState, TextSelection } from 'prosemirror-state'; + +// SD-3400: mock the removal boundary — the wrapper's delete/prune internals +// are covered by footnote-wrappers.test.ts. This suite verifies detection of +// a staged-selected marker and the handoff to the wrapper. +const mockRemoveNoteReferenceAt = vi.fn(() => true); +vi.mock('../../document-api-adapters/plan-engine/footnote-wrappers.js', () => ({ + removeNoteReferenceAt: (...args) => mockRemoveNoteReferenceAt(...args), +})); + +import { getSelectedNoteMarker, deleteSelectedNoteMarker } from './delete-note-marker.js'; + +const makeSchema = () => + new Schema({ + nodes: { + doc: { content: 'block+' }, + paragraph: { group: 'block', content: 'inline*' }, + run: { inline: true, group: 'inline', content: 'inline*' }, + footnoteReference: { + inline: true, + group: 'inline', + atom: true, + selectable: false, + attrs: { id: { default: null } }, + }, + endnoteReference: { + inline: true, + group: 'inline', + atom: true, + selectable: false, + attrs: { id: { default: null } }, + }, + text: { group: 'inline' }, + }, + marks: {}, + }); + +const makeDoc = (schema, refType = 'footnoteReference') => { + const beforeRun = schema.nodes.run.create(null, schema.text('Before')); + const markerRun = schema.nodes.run.create(null, schema.nodes[refType].create({ id: '7' })); + const afterRun = schema.nodes.run.create(null, schema.text('After')); + return schema.node('doc', null, [schema.node('paragraph', null, [beforeRun, markerRun, afterRun])]); +}; + +const findNode = (doc, typeName) => { + let result = null; + doc.descendants((node, pos) => { + if (!result && node.type.name === typeName) { + result = { node, pos, end: pos + node.nodeSize }; + return false; + } + return true; + }); + return result; +}; + +const makeState = (schema, doc, from, to) => + EditorState.create({ schema, doc, selection: TextSelection.create(doc, from, to) }); + +beforeEach(() => mockRemoveNoteReferenceAt.mockClear()); + +describe('getSelectedNoteMarker', () => { + it.each([ + ['footnoteReference', 'footnote'], + ['endnoteReference', 'endnote'], + ])('detects a selection spanning exactly one %s atom', (refType, noteType) => { + const schema = makeSchema(); + const doc = makeDoc(schema, refType); + const marker = findNode(doc, refType); + const state = makeState(schema, doc, marker.pos, marker.end); + + expect(getSelectedNoteMarker(state)).toEqual({ pos: marker.pos, noteId: '7', type: noteType }); + }); + + it('returns null for a collapsed selection', () => { + const schema = makeSchema(); + const doc = makeDoc(schema); + const marker = findNode(doc, 'footnoteReference'); + const state = makeState(schema, doc, marker.end, marker.end); + + expect(getSelectedNoteMarker(state)).toBeNull(); + }); + + it('returns null when the selection spans regular text', () => { + const schema = makeSchema(); + const doc = makeDoc(schema); + const state = makeState(schema, doc, 2, 5); + + expect(getSelectedNoteMarker(state)).toBeNull(); + }); + + it('returns null when the selection extends beyond the marker', () => { + const schema = makeSchema(); + const doc = makeDoc(schema); + const marker = findNode(doc, 'footnoteReference'); + const state = makeState(schema, doc, marker.pos, marker.end + 2); + + expect(getSelectedNoteMarker(state)).toBeNull(); + }); +}); + +describe('deleteSelectedNoteMarker', () => { + it('removes the selected marker through the wrapper (element pruned when last reference)', () => { + const schema = makeSchema(); + const doc = makeDoc(schema); + const marker = findNode(doc, 'footnoteReference'); + const editor = { state: makeState(schema, doc, marker.pos, marker.end) }; + + const handled = deleteSelectedNoteMarker(editor); + + expect(handled).toBe(true); + expect(mockRemoveNoteReferenceAt).toHaveBeenCalledWith(editor, { + pos: marker.pos, + noteId: '7', + type: 'footnote', + }); + }); + + it('does nothing when the selection is not a staged marker selection', () => { + const schema = makeSchema(); + const doc = makeDoc(schema); + const editor = { state: makeState(schema, doc, 2, 5) }; + + expect(deleteSelectedNoteMarker(editor)).toBe(false); + expect(mockRemoveNoteReferenceAt).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/super-editor/src/editors/v1/extensions/footnote/footnote.js b/packages/super-editor/src/editors/v1/extensions/footnote/footnote.js index 576afa6b9c..d7f046b3e0 100644 --- a/packages/super-editor/src/editors/v1/extensions/footnote/footnote.js +++ b/packages/super-editor/src/editors/v1/extensions/footnote/footnote.js @@ -1,5 +1,7 @@ import { Node } from '@core/Node.js'; import { Attribute } from '@core/Attribute.js'; +import { insertFootnoteAtCursor } from './insert-footnote.js'; +import { getSelectedNoteMarker, deleteSelectedNoteMarker } from './delete-note-marker.js'; const toSuperscriptDigits = (value) => { const map = { @@ -120,6 +122,39 @@ export const FootnoteReference = Node.create({ }; }, + addCommands() { + return { + /** + * SD-3400: thin command shim over {@link insertFootnoteAtCursor} so any + * custom toolbar can call `editor.commands.insertFootnote()`. + * Intentionally NOT registered in the default toolbar (per SD-3400). + */ + insertFootnote: + () => + ({ editor, tr }) => { + // The document API dispatches its own (compound) transactions, which + // would leave the CommandService transaction stale — suppress it. + tr.setMeta('preventDispatch', true); + return insertFootnoteAtCursor(editor); + }, + + /** + * SD-3400: thin command shim over {@link deleteSelectedNoteMarker}. + * Runs before `deleteSelection` in the Backspace/Delete chains so the + * second stage of the staged marker delete also prunes the OOXML note + * element ("remove on both sides"). + */ + deleteSelectedNoteMarker: + () => + ({ editor, state, tr }) => { + if (!getSelectedNoteMarker(state)) return false; + // Same preventDispatch reason as insertFootnote above. + tr.setMeta('preventDispatch', true); + return deleteSelectedNoteMarker(editor); + }, + }; + }, + parseDOM() { return [{ tag: 'sup[data-footnote-id]' }]; }, diff --git a/packages/super-editor/src/editors/v1/extensions/footnote/insert-footnote.js b/packages/super-editor/src/editors/v1/extensions/footnote/insert-footnote.js new file mode 100644 index 0000000000..ddd78fac48 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/footnote/insert-footnote.js @@ -0,0 +1,27 @@ +/** + * SD-3400: insert a footnote at the current cursor and focus the new note. + * + * Plain orchestrator over two existing capabilities: + * 1. `editor.doc.footnotes.insert` (document API) — allocates the note id, + * creates the body reference at the selection head, writes the OOXML note + * element, and bootstraps the footnotes part (with separators) when the + * document has none. + * 2. `presentationEditor.activateNoteSession` — opens the note session with + * the caret at the note's start and smart-scrolls it into view. + * + * Lives outside the ProseMirror extension so any caller (custom toolbar + * actions, tests, tooling) can use it without PM command plumbing. The + * `insertFootnote` editor command is a thin shim over this function. + * + * @param {import('@core/Editor.js').Editor} editor + * @returns {boolean} True when the footnote was inserted. + */ +export function insertFootnoteAtCursor(editor) { + const result = editor.doc?.footnotes?.insert({ type: 'footnote', content: '' }); + if (!result?.success) return false; + const noteId = result.footnote?.noteId; + if (noteId != null) { + editor.presentationEditor?.activateNoteSession?.({ storyType: 'footnote', noteId: String(noteId) }); + } + return true; +} diff --git a/packages/super-editor/src/editors/v1/extensions/footnote/insert-footnote.test.js b/packages/super-editor/src/editors/v1/extensions/footnote/insert-footnote.test.js new file mode 100644 index 0000000000..5d94528de3 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/footnote/insert-footnote.test.js @@ -0,0 +1,41 @@ +import { describe, it, expect, vi } from 'vitest'; +import { insertFootnoteAtCursor } from './insert-footnote.js'; + +const makeEditor = ({ insertResult, presentationEditor } = {}) => ({ + doc: { footnotes: { insert: vi.fn(() => insertResult) } }, + presentationEditor, +}); + +describe('insertFootnoteAtCursor', () => { + it('inserts at the cursor and focuses the new note session', () => { + const activateNoteSession = vi.fn(() => true); + const editor = makeEditor({ + insertResult: { success: true, footnote: { kind: 'entity', entityType: 'footnote', noteId: 7 } }, + presentationEditor: { activateNoteSession }, + }); + + expect(insertFootnoteAtCursor(editor)).toBe(true); + expect(editor.doc.footnotes.insert).toHaveBeenCalledWith({ type: 'footnote', content: '' }); + expect(activateNoteSession).toHaveBeenCalledWith({ storyType: 'footnote', noteId: '7' }); + }); + + it('returns false and does not activate when the insert fails', () => { + const activateNoteSession = vi.fn(); + const editor = makeEditor({ + insertResult: { success: false, failure: { code: 'NO_OP', message: 'nope' } }, + presentationEditor: { activateNoteSession }, + }); + + expect(insertFootnoteAtCursor(editor)).toBe(false); + expect(activateNoteSession).not.toHaveBeenCalled(); + }); + + it('still succeeds when no presentation editor is attached (headless)', () => { + const editor = makeEditor({ + insertResult: { success: true, footnote: { kind: 'entity', entityType: 'footnote', noteId: '3' } }, + presentationEditor: undefined, + }); + + expect(insertFootnoteAtCursor(editor)).toBe(true); + }); +}); diff --git a/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.js b/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.js index 8cdca87b5b..3606d4afb6 100644 --- a/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.js +++ b/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.js @@ -11,6 +11,7 @@ export const VerticalNavigationPluginKey = new PluginKey('verticalNavigation'); */ const createDefaultState = () => ({ goalX: null, + goalClientX: null, }); /** @@ -50,24 +51,28 @@ export const VerticalNavigation = Extension.create({ if (meta?.type === 'vertical-move') { return { goalX: meta.goalX ?? value.goalX ?? null, + goalClientX: meta.goalClientX ?? value.goalClientX ?? null, }; } if (meta?.type === 'set-goal-x') { return { ...value, goalX: meta.goalX ?? null, + goalClientX: meta.goalClientX ?? null, }; } if (meta?.type === 'reset-goal-x') { return { ...value, goalX: null, + goalClientX: null, }; } if (tr.selectionSet) { return { ...value, goalX: null, + goalClientX: null, }; } return value; @@ -114,15 +119,18 @@ export const VerticalNavigation = Extension.create({ // 3. Perform hit test at (goal X, adjacent line center Y) to find target position. // 4. Move selection to target position, extending if Shift is held. - // 1. Get or set goal X + // 1. Get or set goal X (layout space for the body fallback, client + // space for hit testing — the two only coincide for body surfaces). const pluginState = VerticalNavigationPluginKey.getState(view.state); let goalX = pluginState?.goalX; + let goalClientX = pluginState?.goalClientX; const coords = getCurrentCoords(editor, view.state.selection); if (!coords) return false; - if (goalX == null) { + if (goalX == null || goalClientX == null) { goalX = coords?.x; - if (!Number.isFinite(goalX)) return false; - view.dispatch(view.state.tr.setMeta(VerticalNavigationPluginKey, { type: 'set-goal-x', goalX })); + goalClientX = coords?.clientX; + if (!Number.isFinite(goalX) || !Number.isFinite(goalClientX)) return false; + view.dispatch(view.state.tr.setMeta(VerticalNavigationPluginKey, { type: 'set-goal-x', goalX, goalClientX })); } // 2. Find adjacent line @@ -134,7 +142,20 @@ export const VerticalNavigation = Extension.create({ // a page boundary), hit testing with screen coordinates produces incorrect // positions. In that case, fall back to layout-based position resolution // using the line's PM position range and computeCaretLayoutRect. - let hit = getHitFromLayoutCoords(editor, goalX, adjacent.clientY, coords, adjacent.pageIndex); + // SD-3400: note sessions resolve the goal column natively on the + // painted adjacent line via caretRangeFromPoint — pure client space, + // no layout conversions (which mix coordinate systems for notes). + const isNoteSession = Boolean(editor?.options?.parentEditor && !editor?.options?.isHeaderOrFooter); + let hit = null; + if (isNoteSession && adjacent.lineElement) { + const ownerDoc = editor.presentationEditor?.visibleHost?.ownerDocument ?? document; + hit = resolvePositionAtClientPoint(ownerDoc, adjacent.lineElement, goalClientX); + } + if (!hit) { + // Hit test directly in client space — the goal column came from the + // painted caret, so no layout-to-client conversion is needed. + hit = editor.presentationEditor.hitTest(goalClientX, adjacent.clientY); + } // Check if the hit test result is plausible: if the adjacent line has PM // position data, the hit should land within or very close to that range. @@ -145,7 +166,7 @@ export const VerticalNavigation = Extension.create({ // lands on the current line's fragment start instead of the adjacent // line — this causes the cursor to appear stuck since the "new" position // equals the current one. - if (adjacent.pmStart != null && adjacent.pmEnd != null) { + if (!isNoteSession && adjacent.pmStart != null && adjacent.pmEnd != null) { const TOLERANCE = 5; const hitPos = hit?.pos; if ( @@ -167,7 +188,7 @@ export const VerticalNavigation = Extension.create({ if (!selection) return false; view.dispatch( view.state.tr - .setMeta(VerticalNavigationPluginKey, { type: 'vertical-move', goalX }) + .setMeta(VerticalNavigationPluginKey, { type: 'vertical-move', goalX, goalClientX }) .setSelection(selection), ); return true; @@ -229,6 +250,23 @@ function isPresenting(editor) { function getCurrentCoords(editor, selection) { const presentationEditor = editor.presentationEditor; const layoutSpaceCoords = presentationEditor.computeCaretLayoutRect(selection.head); + + // SD-3400: the painted caret overlay is the ground truth in client space. + // computeCaretLayoutRect + denormalizeClientPoint disagree on coordinate + // spaces for note sessions (stacked vs page-local y), which broke goal-x + // and produced off-screen client points for arrows inside footnotes. + const doc = presentationEditor?.visibleHost?.ownerDocument ?? document; + const caretRect = doc.querySelector('.presentation-editor__selection-caret')?.getBoundingClientRect?.(); + if (caretRect && caretRect.height > 0) { + return { + clientX: caretRect.left, + clientY: caretRect.top, + height: caretRect.height, + x: layoutSpaceCoords?.x ?? caretRect.left, + y: layoutSpaceCoords?.y ?? caretRect.top, + }; + } + if (!layoutSpaceCoords) return null; const clientCoords = presentationEditor.denormalizeClientPoint( layoutSpaceCoords.x, @@ -262,12 +300,147 @@ function resolveLineBoundaryPosition(editor, selection, key) { const lineEl = findLineElementAtPoint(doc, caretX, caretY); if (!lineEl) return null; - const pmStart = Number(lineEl.dataset?.pmStart); - const pmEnd = Number(lineEl.dataset?.pmEnd); + let pmStart = Number(lineEl.dataset?.pmStart); + let pmEnd = Number(lineEl.dataset?.pmEnd); if (!Number.isFinite(pmStart) || !Number.isFinite(pmEnd)) return null; + // SD-3400: translate stale note ranges into current coordinates. + ({ pmStart, pmEnd } = translateStaleNoteLineRange(editor, lineEl, pmStart, pmEnd)); return key === 'Home' ? pmStart : pmEnd; } +/** + * Browser-portable caret-from-point: WebKit/Blink expose caretRangeFromPoint, + * Firefox exposes caretPositionFromPoint. Normalizes both to {node, offset}. + * Without the Firefox branch, note sessions silently fell back to the + * mixed-coordinate hitTest path and the goal column drifted. + * + * @param {Document} ownerDoc + * @param {number} x + * @param {number} y + * @returns {{ node: Node, offset: number } | null} + */ +function caretPointFromClientPoint(ownerDoc, x, y) { + if (typeof ownerDoc.caretRangeFromPoint === 'function') { + const range = ownerDoc.caretRangeFromPoint(x, y); + if (range?.startContainer) return { node: range.startContainer, offset: range.startOffset }; + } + if (typeof ownerDoc.caretPositionFromPoint === 'function') { + const caret = ownerDoc.caretPositionFromPoint(x, y); + if (caret?.offsetNode) return { node: caret.offsetNode, offset: caret.offset }; + } + return null; +} + +/** + * Resolves the ProseMirror position at a client X on a painted line using the + * browser's native point-to-text mapping ({@link caretPointFromClientPoint}) + * and the line's leaf pm attributes. Pure client space — no layout/client + * conversions. + * + * @param {Document} ownerDoc + * @param {Element} lineEl + * @param {number} clientX + * @returns {{ pos: number } | null} + */ +function resolvePositionAtClientPoint(ownerDoc, lineEl, clientX) { + const lineRect = lineEl.getBoundingClientRect?.(); + if (!lineRect || lineRect.height === 0) return null; + const y = lineRect.top + lineRect.height / 2; + const x = Math.max(lineRect.left, Math.min(clientX, lineRect.right - 1)); + + // Browser globals via the document's own window (also keeps the file free + // of DOM globals for lint environments without them). + const win = ownerDoc.defaultView; + if (!win) return null; + + const hit = caretPointFromClientPoint(ownerDoc, x, y); + if (hit?.node) { + const node = hit.node; + const host = node.nodeType === win.Node.TEXT_NODE ? node.parentElement : node; + const leaf = host?.closest?.('[data-pm-start][data-pm-end]'); + if (leaf && lineEl.contains(leaf)) { + const pmStart = Number(leaf.dataset?.pmStart); + const pmEnd = Number(leaf.dataset?.pmEnd); + if (Number.isFinite(pmStart)) { + let offset = 0; + const walker = ownerDoc.createTreeWalker(leaf, win.NodeFilter.SHOW_TEXT); + let current = walker.nextNode(); + while (current) { + if (current === node) { + offset += hit.offset; + break; + } + offset += current.textContent?.length ?? 0; + current = walker.nextNode(); + } + const pos = pmStart + offset; + return { pos: Number.isFinite(pmEnd) ? Math.min(pos, pmEnd) : pos }; + } + } + } + + // Point fell outside text (margins): clamp to the line's edges. + const lineStart = Number(lineEl.dataset?.pmStart); + const lineEnd = Number(lineEl.dataset?.pmEnd); + if (clientX <= lineRect.left && Number.isFinite(lineStart)) return { pos: lineStart }; + if (Number.isFinite(lineEnd)) return { pos: lineEnd }; + return null; +} + +/** + * SD-3400: painted pm ranges of unchanged note paragraphs drift after edits + * (the painter skips repainting them), so the adjacent line's data-pm range + * can be stale. Translate it into CURRENT session coordinates by anchoring on + * the line's paragraph block: find the block in the live doc by sdBlockId and + * shift the range by (current block content start - fragment first pmStart). + * Returns the input range unchanged when translation is not applicable. + * + * @param {Object} editor + * @param {Element} lineEl + * @param {number} pmStart + * @param {number} pmEnd + * @returns {{ pmStart: number, pmEnd: number }} + */ +function translateStaleNoteLineRange(editor, lineEl, pmStart, pmEnd) { + const isNoteSession = Boolean(editor?.options?.parentEditor && !editor?.options?.isHeaderOrFooter); + if (!isNoteSession) return { pmStart, pmEnd }; + + const fragEl = lineEl.closest?.('[data-block-id]'); + const blockIdAttr = fragEl?.getAttribute?.('data-block-id') ?? ''; + const doc = editor.state?.doc; + if (!fragEl || !blockIdAttr || !doc) return { pmStart, pmEnd }; + + // Anchor: the smallest painted pmStart across the fragment's lines. + let fragmentFirstStart = Infinity; + for (const line of fragEl.querySelectorAll('.superdoc-line[data-pm-start]')) { + const start = Number(line.dataset?.pmStart); + if (Number.isFinite(start)) fragmentFirstStart = Math.min(fragmentFirstStart, start); + } + if (!Number.isFinite(fragmentFirstStart)) return { pmStart, pmEnd }; + + let delta = null; + doc.descendants((node, pos) => { + if (delta != null) return false; + if (!node.isBlock) return true; + const id = node.attrs?.sdBlockId; + if (typeof id !== 'string' || !id || !blockIdAttr.endsWith(id)) return true; + let firstLeaf = null; + node.descendants((child, childPos) => { + if (firstLeaf != null) return false; + if (child.isInline && (child.isLeaf || child.isText)) { + firstLeaf = pos + 1 + childPos; + return false; + } + return true; + }); + const currentStart = firstLeaf ?? pos + 1; + delta = currentStart - fragmentFirstStart; + return false; + }); + if (delta == null || delta === 0) return { pmStart, pmEnd }; + return { pmStart: pmStart + delta, pmEnd: pmEnd + delta }; +} + /** * Finds the adjacent line center Y in client space and associated page index. * Also returns the PM position range from the line's data attributes so that @@ -295,9 +468,13 @@ function getAdjacentLineClientTarget(editor, coords, direction) { const clientY = rect.top + rect.height / 2; if (!Number.isFinite(clientY)) return null; - // Read PM position range from data attributes for layout-based fallback - const pmStart = Number(adjacentLine.dataset?.pmStart); - const pmEnd = Number(adjacentLine.dataset?.pmEnd); + // Read PM position range from data attributes for layout-based fallback. + // SD-3400: translate stale note ranges into current coordinates. + let pmStart = Number(adjacentLine.dataset?.pmStart); + let pmEnd = Number(adjacentLine.dataset?.pmEnd); + if (Number.isFinite(pmStart) && Number.isFinite(pmEnd)) { + ({ pmStart, pmEnd } = translateStaleNoteLineRange(editor, adjacentLine, pmStart, pmEnd)); + } // Read direction from the visual DOM — DomPainter sets dir="rtl" on RTL lines // using fully resolved properties (style cascade, not just inline attrs). @@ -309,26 +486,10 @@ function getAdjacentLineClientTarget(editor, coords, direction) { pmStart: Number.isFinite(pmStart) ? pmStart : undefined, pmEnd: Number.isFinite(pmEnd) ? pmEnd : undefined, isRtl, + lineElement: adjacentLine, }; } -/** - * Converts layout coords to client coords and performs a hit test. - * @param {Object} editor - * @param {number} goalX - * @param {number} clientY - * @param {{ y: number }} coords - * @param {number | undefined} pageIndex - * @returns {{ pos: number } | null} - */ -function getHitFromLayoutCoords(editor, goalX, clientY, coords, pageIndex) { - const presentationEditor = editor.presentationEditor; - const clientPoint = presentationEditor.denormalizeClientPoint(goalX, coords.y, pageIndex); - const clientX = clientPoint?.x; - if (!Number.isFinite(clientX)) return null; - return presentationEditor.hitTest(clientX, clientY); -} - /** * Builds a text selection for the target position, optionally extending. * @param {import('prosemirror-state').EditorState} state diff --git a/packages/super-editor/src/editors/v1/tests/data/footnote-tests-B.docx b/packages/super-editor/src/editors/v1/tests/data/footnote-tests-B.docx new file mode 100644 index 0000000000..2c8850d757 Binary files /dev/null and b/packages/super-editor/src/editors/v1/tests/data/footnote-tests-B.docx differ diff --git a/packages/super-editor/src/editors/v1/tests/import-export/footnotes-roundtrip.test.js b/packages/super-editor/src/editors/v1/tests/import-export/footnotes-roundtrip.test.js index a46a996fcd..b7f22f27f9 100644 --- a/packages/super-editor/src/editors/v1/tests/import-export/footnotes-roundtrip.test.js +++ b/packages/super-editor/src/editors/v1/tests/import-export/footnotes-roundtrip.test.js @@ -187,6 +187,64 @@ describe('footnotes import/export roundtrip', () => { }); }); + it('preserves separator config and footnote linkage in the variant fixture (SD-3400)', async () => { + // footnote-tests-B carries explicit separator/continuationSeparator notes + // and the settings.xml special-footnote list — the configuration variants + // from the SD-3400 Observatory check. + const docxPath = join(__dirname, '../data', 'footnote-tests-B.docx'); + const docxBuffer = await fs.readFile(docxPath); + + const originalZipper = new DocxZipper(); + const originalFiles = await originalZipper.getDocxData(docxBuffer, true); + const originalFootnotesJson = parseXmlToJson( + originalFiles.find((f) => f.name === 'word/footnotes.xml').content, + ); + const originalRoot = findFootnotesRoot(originalFootnotesJson); + const regularIds = collectFootnoteIds(originalRoot).filter((id) => { + const type = findFootnoteById(originalRoot, id)?.attributes?.['w:type']; + return !type || (type !== 'separator' && type !== 'continuationSeparator'); + }); + expect(regularIds.length).toBeGreaterThan(0); + + const [docx, media, mediaFiles, fonts] = await Editor.loadXmlData(docxBuffer, true); + const { editor: testEditor } = initTestEditor({ content: docx, media, mediaFiles, fonts, isHeadless: true }); + editor = testEditor; + + const exportedBuffer = await editor.exportDocx({ isFinalDoc: false }); + const exportedZipper = new DocxZipper(); + const exportedFiles = await exportedZipper.getDocxData(exportedBuffer, true); + const exportedRoot = findFootnotesRoot( + parseXmlToJson(exportedFiles.find((f) => f.name === 'word/footnotes.xml').content), + ); + + // Every regular footnote survives with its id. + regularIds.forEach((id) => { + expect(findFootnoteById(exportedRoot, id)).toBeTruthy(); + }); + // Separator notes survive with their types and reserved ids. + expect(findFootnotesByType(exportedRoot, 'separator').map((el) => el.attributes['w:id'])).toEqual(['-1']); + expect(findFootnotesByType(exportedRoot, 'continuationSeparator').map((el) => el.attributes['w:id'])).toEqual([ + '0', + ]); + // The settings.xml special-footnote list (§17.11.9) survives export. + const settingsPr = findFootnotePrInSettings(findSettingsXml(exportedFiles)); + expect(settingsPr).toBeTruthy(); + const listedIds = (settingsPr.elements ?? []) + .filter((el) => el.name === 'w:footnote') + .map((el) => el.attributes?.['w:id']); + expect(listedIds).toContain('-1'); + expect(listedIds).toContain('0'); + // Body references survive in document order. + const exportedBody = parseXmlToJson(exportedFiles.find((f) => f.name === 'word/document.xml').content); + const refIds = []; + const walk = (node) => { + if (node?.name === 'w:footnoteReference') refIds.push(node.attributes?.['w:id']); + (node?.elements ?? []).forEach(walk); + }; + walk(exportedBody.elements?.[0]); + expect(refIds).toEqual(regularIds); + }); + it('preserves footnoteReference nodes in document body', async () => { const docxPath = join(__dirname, '../data', DOCX_FIXTURE_NAME); const docxBuffer = await fs.readFile(docxPath); diff --git a/packages/superdoc/src/dev/components/SuperdocDev.vue b/packages/superdoc/src/dev/components/SuperdocDev.vue index c1d67e8a37..058f0a1de9 100644 --- a/packages/superdoc/src/dev/components/SuperdocDev.vue +++ b/packages/superdoc/src/dev/components/SuperdocDev.vue @@ -1244,6 +1244,12 @@ const toggleShowBookmarks = () => { superdoc.value?.setShowBookmarks?.(showBookmarks.value); }; +// SD-3400: demo wiring for the custom insert-footnote action. Consumer apps +// register this on their own toolbar; it is intentionally not a default item. +const handleInsertFootnote = () => { + activeEditor.value?.commands?.insertFootnote?.(); +}; + const toggleViewLayout = () => { const nextValue = !useWebLayout.value; const url = new URL(window.location.href); @@ -1544,6 +1550,7 @@ if (scrollTestMode.value) { + diff --git a/tests/behavior/tests/footnotes/footnote-interactions.spec.ts b/tests/behavior/tests/footnotes/footnote-interactions.spec.ts new file mode 100644 index 0000000000..39772f0194 --- /dev/null +++ b/tests/behavior/tests/footnotes/footnote-interactions.spec.ts @@ -0,0 +1,318 @@ +/** + * SD-3400 footnote interactions, end to end through the presentation surface: + * + * 1. Double-click a BODY reference marker navigates to (and focuses) the note. + * 2. Staged Backspace: first press selects the marker, second press removes it + * AND prunes the w:footnote element ("remove on both sides"), renumbering + * the remaining notes. + * 3. Area-delete: clearing all note content removes the footnote everywhere + * (body marker + OOXML element) and exits the session. + * 4. insertFootnote command: inserts the marker at the cursor, creates the + * note, and focuses the new note session, on a document without footnotes. + */ + +import type { Page } from '@playwright/test'; +import { test, expect } from '../../fixtures/superdoc.js'; +import { BASIC_FOOTNOTES_DOC_PATH, H_F_NORMAL_DOC_PATH } from '../../helpers/story-fixtures.js'; + +test.use({ config: { showCaret: true, showSelection: true } }); + +async function getActiveStorySession(page: Page) { + return page.evaluate(() => { + const session = (window as any).editor?.presentationEditor?.getStorySessionManager?.()?.getActiveSession?.(); + return session?.locator ?? null; + }); +} + +/** Body footnoteReference positions and ids, in document order. */ +async function getBodyNoteRefs(page: Page): Promise> { + return page.evaluate(() => { + const refs: Array<{ pos: number; id: string }> = []; + (window as any).editor.state.doc.descendants((node: any, pos: number) => { + if (node.type?.name === 'footnoteReference') refs.push({ pos, id: String(node.attrs?.id ?? '') }); + return true; + }); + return refs; + }); +} + +/** w:footnote ids present in the canonical footnotes part (separators excluded). */ +async function getFootnoteElementIds(page: Page): Promise { + return page.evaluate(() => { + const xml = (window as any).editor?.converter?.convertedXml?.['word/footnotes.xml']; + const root = xml?.elements?.[0]; + if (!root?.elements) return []; + return root.elements + .filter( + (el: any) => + el.name === 'w:footnote' && + el.attributes?.['w:type'] !== 'separator' && + el.attributes?.['w:type'] !== 'continuationSeparator', + ) + .map((el: any) => String(el.attributes?.['w:id'] ?? '')); + }); +} + +async function focusBodyAt(page: Page, pos: number) { + await page.evaluate((position) => { + const editor = (window as any).editor; + const TextSelection = editor.state.selection.constructor; + editor.view.dispatch(editor.state.tr.setSelection(TextSelection.create(editor.state.doc, position))); + editor.view.focus(); + }, pos); +} + +test('double-click a body reference marker opens the referenced note session', async ({ superdoc }) => { + await superdoc.loadDocument(BASIC_FOOTNOTES_DOC_PATH); + await superdoc.waitForStable(); + + const marker = superdoc.page.locator('[data-note-reference][data-note-id="1"]').first(); + await marker.scrollIntoViewIfNeeded(); + await marker.waitFor({ state: 'visible', timeout: 15_000 }); + + const box = await marker.boundingBox(); + expect(box).toBeTruthy(); + await superdoc.page.mouse.dblclick(box!.x + box!.width / 2, box!.y + box!.height / 2); + await superdoc.waitForStable(); + + await expect + .poll(() => getActiveStorySession(superdoc.page)) + .toEqual({ kind: 'story', storyType: 'footnote', noteId: '1' }); +}); + +test('staged Backspace removes the marker and prunes the note on both sides', async ({ superdoc }) => { + await superdoc.loadDocument(BASIC_FOOTNOTES_DOC_PATH); + await superdoc.waitForStable(); + + const refsBefore = await getBodyNoteRefs(superdoc.page); + expect(refsBefore.map((r) => r.id)).toEqual(['1', '2']); + expect(await getFootnoteElementIds(superdoc.page)).toEqual(['1', '2']); + + // Caret immediately after footnote 1's marker (inside the body editor). + const firstRef = refsBefore[0]; + await superdoc.page + .locator('[data-block-id]:not([data-block-id^="footnote-"])') + .filter({ hasText: 'Simple text' }) + .first() + .click(); + await superdoc.waitForStable(); + await focusBodyAt(superdoc.page, firstRef.pos + 1); + + // First Backspace: the marker is SELECTED, not deleted (Word-like staging). + await superdoc.page.keyboard.press('Backspace'); + await expect.poll(() => getBodyNoteRefs(superdoc.page).then((r) => r.length)).toBe(2); + const selection = await superdoc.page.evaluate(() => { + const sel = (window as any).editor.state.selection; + return { from: sel.from, to: sel.to }; + }); + expect(selection).toEqual({ from: firstRef.pos, to: firstRef.pos + 1 }); + + // Second Backspace: marker gone, w:footnote element pruned, note renumbered. + await superdoc.page.keyboard.press('Backspace'); + await superdoc.waitForStable(); + + await expect.poll(() => getBodyNoteRefs(superdoc.page).then((r) => r.map((x) => x.id))).toEqual(['2']); + await expect.poll(() => getFootnoteElementIds(superdoc.page)).toEqual(['2']); + // Former footnote 2 renumbers to display "1". + const note2 = superdoc.page.locator('[data-block-id^="footnote-2-"]').first(); + await note2.scrollIntoViewIfNeeded(); + await expect(note2).toContainText(/^\s*1/); +}); + +test('clearing all note content removes the footnote on both sides', async ({ superdoc }) => { + await superdoc.loadDocument(BASIC_FOOTNOTES_DOC_PATH); + await superdoc.waitForStable(); + + const note = superdoc.page.locator('[data-block-id^="footnote-1-"]').first(); + await note.scrollIntoViewIfNeeded(); + await note.waitFor({ state: 'visible', timeout: 15_000 }); + const box = await note.boundingBox(); + expect(box).toBeTruthy(); + await superdoc.page.mouse.dblclick(box!.x + box!.width / 2, box!.y + box!.height / 2); + await superdoc.waitForStable(); + await expect + .poll(() => getActiveStorySession(superdoc.page)) + .toEqual({ kind: 'story', storyType: 'footnote', noteId: '1' }); + + // Clear all content in the note area. + await superdoc.page.keyboard.press('ControlOrMeta+a'); + await superdoc.page.keyboard.press('Backspace'); + await superdoc.waitForStable(); + + // Emptied-note commit: session exits, marker AND element are removed. + await expect.poll(() => getActiveStorySession(superdoc.page)).toBeNull(); + await expect.poll(() => getBodyNoteRefs(superdoc.page).then((r) => r.map((x) => x.id))).toEqual(['2']); + await expect.poll(() => getFootnoteElementIds(superdoc.page)).toEqual(['2']); + await expect(superdoc.page.locator('[data-block-id^="footnote-1-"]')).toHaveCount(0); +}); + +test('insertFootnote places a marker at the cursor and focuses the new note', async ({ superdoc }) => { + // h_f-normal has no body footnote references. + await superdoc.loadDocument(H_F_NORMAL_DOC_PATH); + await superdoc.waitForStable(); + + expect(await getBodyNoteRefs(superdoc.page)).toEqual([]); + + await superdoc.page + .locator('[data-block-id]:not([data-block-id^="footnote-"])') + .filter({ hasText: 'This is a document' }) + .first() + .click(); + await superdoc.waitForStable(); + + const inserted = await superdoc.page.evaluate(() => (window as any).editor.commands.insertFootnote?.() ?? false); + expect(inserted).toBe(true); + await superdoc.waitForStable(); + + const refs = await getBodyNoteRefs(superdoc.page); + expect(refs).toHaveLength(1); + expect(await getFootnoteElementIds(superdoc.page)).toEqual([refs[0].id]); + + // The new note paints with its marker and the session is focused on it. + const noteFragment = superdoc.page.locator(`[data-block-id^="footnote-${refs[0].id}-"]`).first(); + await noteFragment.scrollIntoViewIfNeeded(); + await expect(noteFragment).toBeVisible(); + await expect + .poll(() => getActiveStorySession(superdoc.page)) + .toEqual({ kind: 'story', storyType: 'footnote', noteId: refs[0].id }); +}); + +test('arrow navigation and caret tracking across paragraphs in a note session', async ({ superdoc }) => { + // Regression for the SD-3400 caret drift: typing Enter inside a note made + // the caret render on the previous paragraph's line, so arrow movement + // looked broken even though the selection moved correctly. + await superdoc.loadDocument(BASIC_FOOTNOTES_DOC_PATH); + await superdoc.waitForStable(); + + const note = superdoc.page.locator('[data-block-id^="footnote-1-"]').first(); + await note.scrollIntoViewIfNeeded(); + const box = await note.boundingBox(); + expect(box).toBeTruthy(); + await superdoc.page.mouse.dblclick(box!.x + 60, box!.y + box!.height / 2); + await superdoc.waitForStable(); + await expect + .poll(() => getActiveStorySession(superdoc.page)) + .toEqual({ kind: 'story', storyType: 'footnote', noteId: '1' }); + + // Build: original line, empty paragraph, "tail". + await superdoc.page.keyboard.press('End'); + await superdoc.page.keyboard.press('Enter'); + await superdoc.page.keyboard.press('Enter'); + await superdoc.page.keyboard.type('tail'); + await superdoc.waitForStable(800); + + const readCaret = () => + superdoc.page.evaluate(() => { + const sed = (window as any).editor?.presentationEditor?.getActiveEditor?.(); + const caret = document.querySelector('.presentation-editor__selection-caret'); + const rect = caret?.getBoundingClientRect(); + const lineTops = Array.from(document.querySelectorAll('[data-block-id^="footnote-1-"] .superdoc-line')).map( + (line) => Math.round(line.getBoundingClientRect().top), + ); + return { sel: sed?.state?.selection?.head ?? -1, caretTop: rect ? Math.round(rect.top) : null, lineTops }; + }); + + const atTail = await readCaret(); + expect(atTail.caretTop).toBe(atTail.lineTops[2]); // caret on the "tail" line + + await superdoc.page.keyboard.press('ArrowUp'); + await superdoc.waitForStable(600); + const atEmpty = await readCaret(); + expect(atEmpty.sel).toBeLessThan(atTail.sel); // selection moved up + expect(atEmpty.caretTop).toBe(atEmpty.lineTops[1]); // caret on the empty line + + await superdoc.page.keyboard.press('ArrowUp'); + await superdoc.waitForStable(600); + const atFirst = await readCaret(); + expect(atFirst.sel).toBeLessThan(atEmpty.sel); + expect(atFirst.caretTop).toBe(atFirst.lineTops[0]); // caret on the first line + + // Typing where the caret points must land in the empty paragraph, not "tail". + await superdoc.page.keyboard.press('ArrowDown'); + await superdoc.waitForStable(600); + await superdoc.page.keyboard.type('middle'); + await superdoc.waitForStable(800); + const text = await superdoc.page.evaluate(() => { + const sed = (window as any).editor?.presentationEditor?.getActiveEditor?.(); + const paras: string[] = []; + sed?.state?.doc?.forEach((n: any) => paras.push(n.textContent || '')); + return paras; + }); + expect(text).toContain('middle'); + expect(text).toContain('tail'); + expect(text.find((t: string) => t.includes('middletail') || t.includes('tailmiddle'))).toBeUndefined(); +}); + +test('ArrowUp walks one visual line at a time after edits drift painted ranges', async ({ superdoc }) => { + // SD-3400: the painter skips repainting unchanged note paragraphs, so + // their painted pm ranges drift after edits above them. Vertical arrow + // navigation validated its hit against those stale ranges and skipped + // lines (user repro: ArrowUp jumped two lines). + await superdoc.loadDocument(BASIC_FOOTNOTES_DOC_PATH); + await superdoc.waitForStable(); + + const note = superdoc.page.locator('[data-block-id^="footnote-1-"]').first(); + await note.scrollIntoViewIfNeeded(); + const box = await note.boundingBox(); + await superdoc.page.mouse.dblclick(box!.x + 40, box!.y + box!.height / 2); + await superdoc.waitForStable(); + await expect + .poll(() => getActiveStorySession(superdoc.page)) + .toEqual({ kind: 'story', storyType: 'footnote', noteId: '1' }); + + // Build 6 paragraphs. + await superdoc.page.keyboard.press('End'); + for (const line of ['sdfasdfasdfdsfdas', 'sadfsadfsdafdsafasd dsfdasf dsf', 'asdfsadf sdaf sdfs', 'sdfsdf sdf sdfsd fsdfd', 'sdfsdafsadfssdfdsaf']) { + await superdoc.page.keyboard.press('Enter'); + await superdoc.page.keyboard.type(line); + } + await superdoc.waitForStable(800); + + // Edit paragraph 1 (shifts all later paragraphs' positions; the painter + // keeps their old painted ranges). + await superdoc.page.evaluate(() => { + const sed = (window as any).editor?.presentationEditor?.getActiveEditor?.(); + const TS = sed.state.selection.constructor; + sed.view.dispatch(sed.state.tr.setSelection(TS.create(sed.state.doc, 3))); + }); + await superdoc.page.keyboard.type('pri'); + await superdoc.waitForStable(800); + + // Caret to the very end, then walk up one line per press. + await superdoc.page.evaluate(() => { + const sed = (window as any).editor?.presentationEditor?.getActiveEditor?.(); + const TS = sed.state.selection.constructor; + sed.view.dispatch(sed.state.tr.setSelection(TS.create(sed.state.doc, sed.state.doc.content.size - 2))); + }); + await superdoc.waitForStable(600); + + const readCaret = () => + superdoc.page.evaluate(() => { + const cr = document.querySelector('.presentation-editor__selection-caret')?.getBoundingClientRect(); + return cr ? { top: Math.round(cr.top), left: Math.round(cr.left) } : null; + }); + const readCaretTop = async () => (await readCaret())?.top ?? null; + const lineTops = await superdoc.page.evaluate(() => + (Array.from(document.querySelectorAll('[data-block-id^="footnote-1-"] .superdoc-line')) as HTMLElement[]) + .map((l) => Math.round(l.getBoundingClientRect().top)) + .sort((a, b) => a - b), + ); + expect(lineTops.length).toBe(6); + + const start = await readCaret(); + expect(start?.top).toBe(lineTops[5]); + for (let expectedIndex = 4; expectedIndex >= 0; expectedIndex -= 1) { + await superdoc.page.keyboard.press('ArrowUp'); + await superdoc.waitForStable(500); + const caret = await readCaret(); + expect(caret?.top, `ArrowUp should land on line ${expectedIndex} (top ${lineTops[expectedIndex]}), got ${caret?.top}`).toBe( + lineTops[expectedIndex], + ); + // Goal column: the caret must stay near the starting column while + // arrowing vertically (SD-3400: notes drifted ~50px right per press). + expect( + Math.abs((caret?.left ?? 0) - (start?.left ?? 0)), + `goal-x drift on line ${expectedIndex}: ${start?.left} -> ${caret?.left}`, + ).toBeLessThan(12); + } +}); diff --git a/tests/behavior/tests/footnotes/note-typing-fuzz.spec.ts b/tests/behavior/tests/footnotes/note-typing-fuzz.spec.ts new file mode 100644 index 0000000000..67727fdd42 --- /dev/null +++ b/tests/behavior/tests/footnotes/note-typing-fuzz.spec.ts @@ -0,0 +1,208 @@ +/** + * SD-3400 fuzzer: drives a long randomized (seeded) sequence of REAL + * keystrokes inside a note session and asserts after every step that + * (a) no paragraph ever loses its FootnoteText style and + * (b) once paint settles, the caret overlay sits on the painted line that + * contains the selection head. + */ +import { test, expect } from '../../fixtures/superdoc.js'; +import { BASIC_FOOTNOTES_DOC_PATH } from '../../helpers/story-fixtures.js'; + +test.use({ config: { showCaret: true, showSelection: true } }); +test.setTimeout(240_000); + +// Deterministic PRNG so failures replay exactly. +function mulberry32(seed: number) { + return () => { + seed |= 0; + seed = (seed + 0x6d2b79f5) | 0; + let t = Math.imul(seed ^ (seed >>> 15), 1 | seed); + t = (t + Math.imul(t ^ (t >>> 7), 61 | t)) ^ t; + return ((t ^ (t >>> 14)) >>> 0) / 4294967296; + }; +} + +const WORDS = ['sdfsadfsd', 'asdf', 'sadf', 'dsfadsfad', 'fasd', 'sdafasdfasdfsdaf', 'x']; + +for (const seed of [0x5d3400, 0xbeef01, 0xc0ffee]) { +test(`fuzz(seed=${seed}): real keystroke ops never lose the note style or the caret`, async ({ superdoc }) => { + await superdoc.loadDocument(BASIC_FOOTNOTES_DOC_PATH); + await superdoc.waitForStable(); + + const note = superdoc.page.locator('[data-block-id^="footnote-1-"]').first(); + await note.scrollIntoViewIfNeeded(); + const box = await note.boundingBox(); + await superdoc.page.mouse.dblclick(box!.x + 40, box!.y + box!.height / 2); + await superdoc.waitForStable(); + await superdoc.page.keyboard.press('End'); + + // Seed a LONG paragraph so the painted note has wrapped lines (the + // user's screenshots show typing inside wrapped continuation lines). + await superdoc.page.keyboard.press('Enter'); + await superdoc.page.keyboard.type( + 'dsfsadfsdafasdfdasfdsafasdfasdfsdafdsafdsafsdfdasfdsafsdadsaf sdfsadfsdfsdf sdf sdfs dfsdf sdf sdf sdfaf dsfadsfad sfasdfasdfas dfdsf asdfasdfas sdafasdfasdfsdaf asdf sadf', + ); + await superdoc.waitForStable(600); + + const rand = mulberry32(seed); + const ops: string[] = []; + + const checkStyles = async (opLog: string) => { + const bad = await superdoc.page.evaluate(() => { + const sed = (window as any).editor?.presentationEditor?.getActiveEditor?.(); + const out: string[] = []; + sed?.state?.doc?.forEach((n: any) => { + if ((n.attrs?.paragraphProperties?.styleId ?? null) !== 'FootnoteText') { + out.push(`${n.attrs?.paragraphProperties?.styleId ?? 'NONE'}:${(n.textContent || '').slice(0, 12)}`); + } + }); + return out; + }); + expect(bad, `style lost after ops: ${opLog}`).toEqual([]); + }; + + // Root-fix invariant (SD-3400): painted note line pm ranges must never + // overlap across paragraphs once paint settles — the painter refreshes + // position attributes on reused story fragments. + const checkPaintedRanges = async (opLog: string) => { + const overlaps = await superdoc.page.evaluate(() => { + const ranges = (Array.from( + document.querySelectorAll('[data-block-id^="footnote-1-"] .superdoc-line[data-pm-start][data-pm-end]'), + ) as HTMLElement[]) + .map((l) => ({ s: Number(l.dataset.pmStart), e: Number(l.dataset.pmEnd) })) + .filter((r) => Number.isFinite(r.s) && Number.isFinite(r.e)) + .sort((a, b) => a.s - b.s); + const bad: string[] = []; + for (let i = 1; i < ranges.length; i += 1) { + if (ranges[i].s < ranges[i - 1].e) bad.push(`${ranges[i - 1].s}-${ranges[i - 1].e} overlaps ${ranges[i].s}-${ranges[i].e}`); + } + return bad; + }); + expect(overlaps, `stale painted ranges after ops: ${opLog}`).toEqual([]); + }; + + const checkCaret = async (opLog: string) => { + const evalCheck = () => + superdoc.page.evaluate(() => { + const sed = (window as any).editor?.presentationEditor?.getActiveEditor?.(); + const doc = sed?.state?.doc; + const head = sed?.state?.selection?.head ?? -1; + if (!doc || head < 0) return { ok: false, why: 'no editor', head }; + const caret = document.querySelector('.presentation-editor__selection-caret'); + const cr = caret?.getBoundingClientRect(); + if (!cr || cr.height === 0) return { ok: false, why: 'no caret', head }; + + // Ground truth from the SESSION doc: block identity + local text offset. + const $pos = doc.resolve(Math.min(head, doc.content.size)); + let blockDepth = 0; + for (let d = $pos.depth; d >= 1; d -= 1) if ($pos.node(d).isBlock) blockDepth = d; + if (!blockDepth) return { ok: true, why: 'no block', head }; + const blockNode = $pos.node(blockDepth); + const sdBlockId = blockNode.attrs?.sdBlockId ?? ''; + const blockPos = $pos.before(blockDepth); + let firstLeaf: number | null = null; + doc.nodesBetween(blockPos, blockPos + blockNode.nodeSize, (n: any, p: number) => { + if (firstLeaf != null) return false; + if (n.isInline && (n.isLeaf || n.isText)) { firstLeaf = p; return false; } + return true; + }); + const localOff = Math.max(0, head - (firstLeaf ?? head)); + + const fragments = Array.from( + document.querySelectorAll(`[data-block-id$="${sdBlockId}"]`), + ) as HTMLElement[]; + if (!fragments.length) return { ok: true, why: 'fragment unpainted', head }; + const lines = fragments + .flatMap((f) => Array.from(f.querySelectorAll('.superdoc-line')) as HTMLElement[]) + .map((l) => ({ top: Math.round(l.getBoundingClientRect().top), el: l })) + .sort((a, b) => a.top - b.top); + if (!lines.length) return { ok: true, why: 'no lines', head }; + + // cumulative visible text per line (pm-attr leaf spans only; the + // synthetic marker span carries no pm attrs). + let cum = 0; + const acceptableTops: number[] = []; + for (const { top, el } of lines) { + const len = (Array.from(el.querySelectorAll('[data-pm-start][data-pm-end]')) as HTMLElement[]) + .filter((sp) => !sp.querySelector('[data-pm-start]')) + .reduce((a, sp) => a + (sp.textContent?.length ?? 0), 0); + // boundary tolerance: wrap points trim a space, so allow +-1 char + if (localOff >= cum - 1 && localOff <= cum + len + 1) acceptableTops.push(top); + cum += len; + } + if (!acceptableTops.length) acceptableTops.push(lines[lines.length - 1].top); + const ok = acceptableTops.some((t) => Math.abs(cr.top - t) < 4); + const fragDump = fragments.map((f) => ({ + top: Math.round(f.getBoundingClientRect().top), + id: (f.getAttribute('data-block-id') ?? '').slice(9, 22), + lines: (Array.from(f.querySelectorAll('.superdoc-line')) as HTMLElement[]).map((l) => ({ + s: l.dataset.pmStart, + e: l.dataset.pmEnd, + top: Math.round(l.getBoundingClientRect().top), + text: (l.textContent ?? '').slice(0, 10), + })), + })); + return { + ok, + why: `caret ${Math.round(cr.top)} not in block ${sdBlockId.slice(0, 8)} lines [${acceptableTops.join(',')}] localOff=${localOff} head=${head} firstLeaf=${firstLeaf} frags=${JSON.stringify(fragDump)}`, + head, + }; + }); + + const res = await evalCheck(); + if (!res.ok) { + await superdoc.page.waitForTimeout(700); + const recheck = await evalCheck(); + expect(recheck.ok, `caret off after ops: ${opLog} :: first(${res.why}) recheck(PERSISTENT ${recheck.why}) head=${recheck.head}`).toBe(true); + console.log(`TRANSIENT caret mismatch after: ${opLog} :: ${res.why}`); + } + }; + + for (let i = 0; i < 120; i += 1) { + const r = rand(); + let op: string; + if (r < 0.35) { + const word = WORDS[Math.floor(rand() * WORDS.length)]; + op = `type:${word}`; + await superdoc.page.keyboard.type(word + (rand() < 0.5 ? ' ' : '')); + } else if (r < 0.55) { + op = 'Enter'; + await superdoc.page.keyboard.press('Enter'); + } else if (r < 0.7) { + op = 'Backspace'; + await superdoc.page.keyboard.press('Backspace'); + } else if (r < 0.78) { + op = 'ArrowUp'; + await superdoc.page.keyboard.press('ArrowUp'); + } else if (r < 0.86) { + op = 'ArrowDown'; + await superdoc.page.keyboard.press('ArrowDown'); + } else if (r < 0.92) { + op = 'ArrowLeft'; + await superdoc.page.keyboard.press('ArrowLeft'); + } else if (r < 0.96) { + op = 'End'; + await superdoc.page.keyboard.press('End'); + } else { + op = 'Home'; + await superdoc.page.keyboard.press('Home'); + } + ops.push(op); + + // Style integrity is checked EVERY step (cheap). + await checkStyles(ops.slice(-12).join(' > ')); + + // Caret correctness checked every 5 steps after settle (RAF paint). + if (i % 5 === 4) { + await superdoc.waitForStable(400); + await checkCaret(ops.slice(-12).join(' > ')); + await checkPaintedRanges(ops.slice(-12).join(' > ')); + } + } + + // Final full settle check. + await superdoc.waitForStable(800); + await checkCaret(ops.slice(-15).join(' > ')); + await checkStyles('final'); +}); +}