From 62179b6a5f8b43b19a1a7e4219003da4488d9f6e Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Tue, 21 Apr 2026 11:36:02 -0300 Subject: [PATCH 1/4] feat(super-editor): render imported heading/bookmark cross-references (SD-2495) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire the sd:crossReference v3 translator into the v2 importer entity list so REF / NOTEREF / STYLEREF fields imported from DOCX actually produce PM crossReference nodes instead of being silently dropped by the dispatch loop. Walk nested run wrappers when extracting the field's cached display text, and render the cross-reference as an internal hyperlink when the instruction carries the \\h switch. Route clicks on internal #bookmark anchors through goToAnchor so rendered cross-references navigate to their target in the document. Fixes IT-949 — Word cross-references (e.g. "Section 15") now appear in the viewer and are searchable, matching Word's output. --- .../inline-converters/cross-reference.test.ts | 88 +++++++++++++++++++ .../inline-converters/cross-reference.ts | 29 ++++-- .../pointer-events/EditorInputManager.ts | 7 +- .../v2/importer/crossReferenceImporter.js | 7 ++ .../v2/importer/docxImporter.js | 2 + .../crossReference-translator.js | 23 +++-- .../crossReference-translator.test.js | 77 ++++++++++++++++ 7 files changed, 219 insertions(+), 14 deletions(-) create mode 100644 packages/layout-engine/pm-adapter/src/converters/inline-converters/cross-reference.test.ts create mode 100644 packages/super-editor/src/editors/v1/core/super-converter/v2/importer/crossReferenceImporter.js diff --git a/packages/layout-engine/pm-adapter/src/converters/inline-converters/cross-reference.test.ts b/packages/layout-engine/pm-adapter/src/converters/inline-converters/cross-reference.test.ts new file mode 100644 index 0000000000..66cf1e30f9 --- /dev/null +++ b/packages/layout-engine/pm-adapter/src/converters/inline-converters/cross-reference.test.ts @@ -0,0 +1,88 @@ +import { describe, it, expect, vi } from 'vitest'; +import type { TextRun } from '@superdoc/contracts'; +import type { PMNode } from '../../types.js'; +import type { InlineConverterParams } from './common.js'; + +vi.mock('./text-run.js', () => ({ + textNodeToRun: vi.fn( + (params: InlineConverterParams): TextRun => ({ + text: params.node.text || '', + fontFamily: params.defaultFont, + fontSize: params.defaultSize, + }), + ), +})); + +import { crossReferenceNodeToRun } from './cross-reference.js'; + +function makeParams( + attrs: Record, + overrides: Partial = {}, +): InlineConverterParams { + const node: PMNode = { type: 'crossReference', attrs }; + return { + node, + positions: new WeakMap(), + defaultFont: 'Calibri', + defaultSize: 16, + inheritedMarks: [], + sdtMetadata: undefined, + hyperlinkConfig: { enableRichHyperlinks: false }, + themeColors: undefined, + runProperties: undefined, + paragraphProperties: undefined, + converterContext: {} as unknown as InlineConverterParams['converterContext'], + enableComments: false, + visitNode: vi.fn(), + bookmarks: undefined, + tabOrdinal: 0, + paragraphAttrs: {}, + nextBlockId: vi.fn(), + ...overrides, + } as InlineConverterParams; +} + +describe('crossReferenceNodeToRun (SD-2495)', () => { + it('emits a TextRun carrying the resolved display text', () => { + const run = crossReferenceNodeToRun( + makeParams({ resolvedText: '15', target: '_Ref506192326', instruction: 'REF _Ref506192326 \\w \\h' }), + ); + expect(run).not.toBeNull(); + expect(run!.text).toBe('15'); + }); + + it('synthesizes an internal link when the instruction has the \\h switch', () => { + const run = crossReferenceNodeToRun( + makeParams({ resolvedText: '15', target: '_Ref506192326', instruction: 'REF _Ref506192326 \\w \\h' }), + ); + expect(run!.link).toBeDefined(); + expect(run!.link?.anchor).toBe('_Ref506192326'); + }); + + it('does not attach a link when the \\h switch is absent', () => { + const run = crossReferenceNodeToRun( + makeParams({ resolvedText: '15', target: '_Ref506192326', instruction: 'REF _Ref506192326 \\w' }), + ); + expect(run!.link).toBeUndefined(); + }); + + it('still emits a TextRun (not null) when the cached text is empty', () => { + const run = crossReferenceNodeToRun( + makeParams({ resolvedText: '', target: '_Ref_missing', instruction: 'REF _Ref_missing \\h' }), + ); + expect(run).not.toBeNull(); + expect(run!.text).toBe(''); + // Still links to target so surrounding layout isn't broken and the click target + // is preserved if the text later becomes non-empty via a re-import. + expect(run!.link?.anchor).toBe('_Ref_missing'); + }); + + it('does not match a literal `h` character as the \\h switch', () => { + // Guards against naive substring check — instruction like `REF bh-target` + // must not produce a hyperlink just because `h` appears somewhere. + const run = crossReferenceNodeToRun( + makeParams({ resolvedText: 'label', target: 'bh-target', instruction: 'REF bh-target' }), + ); + expect(run!.link).toBeUndefined(); + }); +}); diff --git a/packages/layout-engine/pm-adapter/src/converters/inline-converters/cross-reference.ts b/packages/layout-engine/pm-adapter/src/converters/inline-converters/cross-reference.ts index c321a37099..1bc05ff8c7 100644 --- a/packages/layout-engine/pm-adapter/src/converters/inline-converters/cross-reference.ts +++ b/packages/layout-engine/pm-adapter/src/converters/inline-converters/cross-reference.ts @@ -1,25 +1,40 @@ import type { TextRun } from '@superdoc/contracts'; -import type { PMNode, PMMark } from '../../types.js'; +import type { PMNode } from '../../types.js'; import { textNodeToRun } from './text-run.js'; -import { applyMarksToRun } from '../../marks/index.js'; -import { applyInlineRunProperties, type InlineConverterParams } from './common.js'; +import { buildFlowRunLink } from '../../marks/links.js'; +import { type InlineConverterParams } from './common.js'; /** * Converts a crossReference PM node to a TextRun with the resolved display text. + * + * Renders Word REF / NOTEREF / STYLEREF fields imported from DOCX. Uses the + * cached result text from Word (`attrs.resolvedText`) — we do not recompute + * outline numbers for `\w`/`\r`/`\n` switches, we trust Word's cache. + * + * When the instruction carries the `\h` switch, the reference renders as an + * internal hyperlink pointing at `#` so clicks navigate to the + * corresponding bookmark via the existing anchor-link navigation path. */ export function crossReferenceNodeToRun(params: InlineConverterParams): TextRun | null { - const { node, positions, defaultFont, defaultSize, inheritedMarks, sdtMetadata, runProperties, converterContext } = - params; + const { node, positions, sdtMetadata } = params; const attrs = (node.attrs ?? {}) as Record; - const resolvedText = (attrs.resolvedText as string) || (attrs.target as string) || ''; - if (!resolvedText) return null; + const resolvedText = typeof attrs.resolvedText === 'string' ? attrs.resolvedText : ''; + const target = typeof attrs.target === 'string' ? attrs.target : ''; + const instruction = typeof attrs.instruction === 'string' ? attrs.instruction : ''; const run = textNodeToRun({ ...params, node: { type: 'text', text: resolvedText, marks: [...(node.marks ?? [])] } as PMNode, }); + if (target && /\\h\b/.test(instruction)) { + const synthesized = buildFlowRunLink({ anchor: target }); + if (synthesized) { + run.link = run.link ? { ...run.link, ...synthesized, anchor: target } : synthesized; + } + } + const pos = positions.get(node); if (pos) { run.pmStart = pos.start; 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 fb5d17840b..c1bb419f00 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 @@ -1532,9 +1532,12 @@ export class EditorInputManager { #handleLinkClick(event: MouseEvent, linkEl: HTMLAnchorElement): void { const href = linkEl.getAttribute('href') ?? ''; const isAnchorLink = href.startsWith('#') && href.length > 1; - const isTocLink = linkEl.closest('.superdoc-toc-entry') !== null; - if (isAnchorLink && isTocLink) { + // SD-2495: route any internal-anchor click (`#`) to in-document + // navigation. Covers TOC entries, heading/bookmark cross-references + // (REF fields with `\h`), and any other internal-hyperlink case — they all + // should scroll to the bookmark target instead of navigating the browser. + if (isAnchorLink) { event.preventDefault(); event.stopPropagation(); this.#callbacks.goToAnchor?.(href); diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/crossReferenceImporter.js b/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/crossReferenceImporter.js new file mode 100644 index 0000000000..6e896a236e --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/crossReferenceImporter.js @@ -0,0 +1,7 @@ +import { generateV2HandlerEntity } from '@core/super-converter/v3/handlers/utils'; +import { translator } from '../../v3/handlers/sd/crossReference/crossReference-translator.js'; + +/** + * @type {import("./docxImporter").NodeHandlerEntry} + */ +export const crossReferenceEntity = generateV2HandlerEntity('crossReferenceNodeHandler', translator); diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.js b/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.js index f6cb80864b..d49267ce09 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.js @@ -17,6 +17,7 @@ import { alternateChoiceHandler } from './alternateChoiceImporter.js'; import { autoPageHandlerEntity, autoTotalPageCountEntity } from './autoPageNumberImporter.js'; import { documentStatFieldHandlerEntity } from './documentStatFieldImporter.js'; import { pageReferenceEntity } from './pageReferenceImporter.js'; +import { crossReferenceEntity } from './crossReferenceImporter.js'; import { pictNodeHandlerEntity } from './pictNodeImporter.js'; import { importCommentData } from './documentCommentsImporter.js'; import { buildTrackedChangeIdMap } from './trackedChangeIdMapper.js'; @@ -249,6 +250,7 @@ export const defaultNodeListHandler = () => { autoTotalPageCountEntity, documentStatFieldHandlerEntity, pageReferenceEntity, + crossReferenceEntity, permStartHandlerEntity, permEndHandlerEntity, mathNodeHandlerEntity, diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/sd/crossReference/crossReference-translator.js b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/sd/crossReference/crossReference-translator.js index 343333bcc5..3f6419f00b 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/sd/crossReference/crossReference-translator.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/sd/crossReference/crossReference-translator.js @@ -108,16 +108,29 @@ function parseDisplay(instruction) { } /** - * Extracts resolved text from processed content. + * Extracts resolved text from processed content. Walks recursively because the + * cached result between w:fldChar separate/end is typically wrapped in a `run` + * node (or deeper: run -> text with marks), so a top-level text-only filter + * misses the field's display text. * @param {Array} content * @returns {string} */ function extractResolvedText(content) { if (!Array.isArray(content)) return ''; - return content - .filter((n) => n.type === 'text') - .map((n) => n.text || '') - .join(''); + let out = ''; + /** @param {Array} nodes */ + const walk = (nodes) => { + for (const node of nodes) { + if (!node) continue; + if (node.type === 'text') { + out += node.text || ''; + } else if (Array.isArray(node.content)) { + walk(node.content); + } + } + }; + walk(content); + return out; } /** @type {import('@translator').NodeTranslatorConfig} */ diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/sd/crossReference/crossReference-translator.test.js b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/sd/crossReference/crossReference-translator.test.js index 080024cc7c..9eb61b2d8a 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/sd/crossReference/crossReference-translator.test.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/sd/crossReference/crossReference-translator.test.js @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest'; import { exportSchemaToJson } from '../../../../exporter.js'; import { translator as runTranslator } from '../../w/r/r-translator.js'; +import { translator as crossReferenceTranslator } from './crossReference-translator.js'; const CROSS_REFERENCE_INSTRUCTION = 'REF bm-target \\h'; @@ -61,3 +62,79 @@ describe('crossReference export routing', () => { expect(exportedRuns.some((node) => hasFieldCharType(node, 'end'))).toBe(true); }); }); + +describe('crossReference import resolvedText extraction (SD-2495)', () => { + // Mirrors the Brillio-style REF cached payload: cached text lives inside a w:r + // wrapper, so a top-level-only `n.type === 'text'` filter returns empty. The + // recursive walk must descend through run wrappers to find the display text. + const buildRun = (innerElements) => ({ + type: 'element', + name: 'w:r', + elements: [{ type: 'element', name: 'w:rPr', elements: [{ type: 'element', name: 'w:i' }] }, ...innerElements], + }); + + const buildSdCrossReference = (instr, cachedRuns) => ({ + name: 'sd:crossReference', + type: 'element', + attributes: { instruction: instr, fieldType: 'REF' }, + elements: cachedRuns, + }); + + it('extracts cached text from runs wrapped around a w:t (Brillio shape)', () => { + const xmlNode = buildSdCrossReference('REF _Ref506192326 \\w \\h', [ + buildRun([]), // empty formatting-carrier run + buildRun([{ type: 'element', name: 'w:t', elements: [{ type: 'text', text: '15' }] }]), + ]); + const encoded = crossReferenceTranslator.encode({ + nodes: [xmlNode], + nodeListHandler: { + handler: ({ nodes }) => { + // Simulate w:r translator wrapping content in SuperDoc run nodes + return nodes + .map((run) => { + const textEl = run.elements?.find((el) => el?.name === 'w:t'); + if (!textEl) return null; + const text = (textEl.elements || []) + .map((child) => (typeof child?.text === 'string' ? child.text : '')) + .join(''); + if (!text) return null; + return { type: 'run', attrs: {}, content: [{ type: 'text', text }] }; + }) + .filter(Boolean); + }, + }, + }); + + expect(encoded.type).toBe('crossReference'); + expect(encoded.attrs.target).toBe('_Ref506192326'); + expect(encoded.attrs.resolvedText).toBe('15'); + expect(encoded.attrs.display).toBe('numberFullContext'); + }); + + it('concatenates cached text across multiple run wrappers', () => { + const xmlNode = buildSdCrossReference('REF _RefABC \\h', [ + buildRun([{ type: 'element', name: 'w:t', elements: [{ type: 'text', text: '4(b' }] }]), + buildRun([{ type: 'element', name: 'w:t', elements: [{ type: 'text', text: ')(2)' }] }]), + ]); + const encoded = crossReferenceTranslator.encode({ + nodes: [xmlNode], + nodeListHandler: { + handler: ({ nodes }) => + nodes.map((run) => ({ + type: 'run', + attrs: {}, + content: [ + { + type: 'text', + text: (run.elements?.find((el) => el?.name === 'w:t')?.elements ?? []) + .map((c) => c.text || '') + .join(''), + }, + ], + })), + }, + }); + + expect(encoded.attrs.resolvedText).toBe('4(b)(2)'); + }); +}); From 330b27fd581578a738ef805273c912657dde5914 Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Tue, 21 Apr 2026 12:12:23 -0300 Subject: [PATCH 2/4] fix(presentation-editor): anchor nav writes scrollTop to the real scroll container MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #scrollPageIntoView wrote scrollTop to #visibleHost, which is typically overflow: visible and therefore not the actual scroll target. Anchor navigation (TOC clicks and SD-2495 cross-reference click-to-navigate) silently did nothing whenever the bookmark target was outside the current viewport — the PM selection moved but the viewport never scrolled. Write to #scrollContainer (the resolved scrollable ancestor) as the primary target, plus #visibleHost for backward compatibility with legacy layouts and the existing test harness that mocks scrollTop on the host element. This unblocks SD-2495's cross-reference click-to-navigate on docs where cross-references and their targets live on different pages. --- .../presentation-editor/PresentationEditor.ts | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) 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 2c9d1dab9d..c7a3ef6eec 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 @@ -5839,8 +5839,24 @@ export class PresentationEditor extends EventEmitter { yPosition += pageHeight + virtualGap; } - // Scroll viewport to the calculated position - if (this.#visibleHost) { + // Scroll viewport to the calculated position. + // + // The authoritative scrollable ancestor is `#scrollContainer` — setting + // scrollTop on the visible host alone is a no-op when the host is + // `overflow: visible` (the standard layout). Without this, anchor + // navigation (TOC clicks, cross-reference click-to-navigate under + // SD-2495) silently does nothing whenever the target page is outside + // the current viewport. + // + // We also write to `#visibleHost` for backwards compatibility: legacy + // layouts may make the visible host itself scrollable, and tests mock + // scrollTop on the host element. + if (this.#scrollContainer instanceof Window) { + this.#scrollContainer.scrollTo({ top: yPosition }); + } else if (this.#scrollContainer) { + this.#scrollContainer.scrollTop = yPosition; + } + if (this.#visibleHost && this.#visibleHost !== this.#scrollContainer) { this.#visibleHost.scrollTop = yPosition; } } From c5c5b3ad43ba138f305ba32485ed5147970ca3ef Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Tue, 21 Apr 2026 12:35:59 -0300 Subject: [PATCH 3/4] feat(layout): show-bookmarks bracket indicators (SD-2454) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opt-in visual indicators for bookmark positions — mirrors Word's "Show bookmarks" (File > Options > Advanced). Off by default. - Pm-adapter bookmark-start and new bookmark-end converters emit gray `[` and `]` marker TextRuns when `layoutEngineOptions.showBookmarks` is true. Markers flow through pagination and line breaking as real characters, matching Word's own visual model. - Auto-generated bookmarks (`_Toc…`, `_Ref…`, `_GoBack`) are hidden even when the feature is on — matching Word. A `renderedBookmarkIds` set on the converter context pairs suppression so closing brackets don't orphan open ones. - PresentationEditor.setShowBookmarks toggles at runtime: clears the flow-block cache and schedules a re-render. - SuperDoc.setShowBookmarks is the public API passthrough. - Dev app gets a Show/Hide bookmarks toggle button in the header. - CSS: subtle gray, non-selectable so users don't include brackets in copied text. Bookmark name surfaces via the native title tooltip on the opening bracket. --- .../painters/dom/src/renderer.ts | 8 ++ .../layout-engine/painters/dom/src/styles.ts | 15 +++ .../pm-adapter/src/converter-context.ts | 17 +++ .../inline-converters/bookmark-end.ts | 44 +++++++ .../bookmark-markers.test.ts | 115 ++++++++++++++++++ .../inline-converters/bookmark-start.ts | 66 ++++++++-- .../pm-adapter/src/converters/paragraph.ts | 4 + .../layout-engine/pm-adapter/src/internal.ts | 6 + .../layout-engine/pm-adapter/src/types.ts | 7 ++ .../presentation-editor/PresentationEditor.ts | 20 +++ .../v1/core/presentation-editor/types.ts | 9 ++ packages/superdoc/src/core/SuperDoc.js | 19 +++ .../src/dev/components/SuperdocDev.vue | 10 ++ 13 files changed, 328 insertions(+), 12 deletions(-) create mode 100644 packages/layout-engine/pm-adapter/src/converters/inline-converters/bookmark-end.ts create mode 100644 packages/layout-engine/pm-adapter/src/converters/inline-converters/bookmark-markers.test.ts diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index 9dcaf5dda0..3b3989c6fe 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -5349,6 +5349,14 @@ export class DomPainter { elem.style.zIndex = '1'; applyRunDataAttributes(elem as HTMLElement, (run as TextRun).dataAttrs); + // SD-2454: bookmark marker runs carry a data-bookmark-name attribute. + // Surface the bookmark name as a native `title` tooltip so hovering the + // opening bracket identifies which bookmark is being marked. + const bookmarkName = (run as TextRun).dataAttrs?.['data-bookmark-name']; + if (bookmarkName) { + (elem as HTMLElement).title = bookmarkName; + } + // Assert PM positions are present for cursor fallback assertPmPositions(run, 'paragraph text run'); diff --git a/packages/layout-engine/painters/dom/src/styles.ts b/packages/layout-engine/painters/dom/src/styles.ts index b548d37107..302f1d60a6 100644 --- a/packages/layout-engine/painters/dom/src/styles.ts +++ b/packages/layout-engine/painters/dom/src/styles.ts @@ -197,6 +197,21 @@ const LINK_AND_TOC_STYLES = ` } } +/* SD-2454: bookmark bracket indicators. + * When the showBookmarks layout option is enabled, the pm-adapter emits + * [ and ] marker TextRuns at bookmark start/end positions. Mirror Word's + * visual treatment: subtle gray, non-selectable so users can't accidentally + * include the brackets in copied text. The bookmark name is surfaced via + * the native title tooltip on the opening bracket. */ +[data-bookmark-marker="start"], +[data-bookmark-marker="end"] { + color: #8b8b8b; + user-select: none; + cursor: default; + font-weight: normal; +} + + /* Reduced motion support */ @media (prefers-reduced-motion: reduce) { .superdoc-link { diff --git a/packages/layout-engine/pm-adapter/src/converter-context.ts b/packages/layout-engine/pm-adapter/src/converter-context.ts index 250b1237f6..89e75b5e76 100644 --- a/packages/layout-engine/pm-adapter/src/converter-context.ts +++ b/packages/layout-engine/pm-adapter/src/converter-context.ts @@ -54,6 +54,23 @@ export type ConverterContext = { * Used by table creation paths to determine which style to apply to new tables. */ defaultTableStyleId?: string; + /** + * When true, emit visible gray `[` and `]` marker TextRuns at bookmarkStart + * and bookmarkEnd positions — matching Word's "Show bookmarks" feature + * (File > Options > Advanced). Off by default because bookmarks are a + * structural concept, not a visual one. SD-2454. + */ + showBookmarks?: boolean; + + /** + * Populated by the bookmark-start inline converter during conversion: the + * set of bookmark numeric ids (as strings) that actually rendered a start + * marker. The bookmark-end converter reads this set to suppress emitting + * an orphan `]` for a start it also suppressed (e.g. `_Toc…` / `_Ref…` + * auto-generated bookmarks filtered out by the `showBookmarks` feature). + * SD-2454. + */ + renderedBookmarkIds?: Set; }; /** diff --git a/packages/layout-engine/pm-adapter/src/converters/inline-converters/bookmark-end.ts b/packages/layout-engine/pm-adapter/src/converters/inline-converters/bookmark-end.ts new file mode 100644 index 0000000000..5c68518d39 --- /dev/null +++ b/packages/layout-engine/pm-adapter/src/converters/inline-converters/bookmark-end.ts @@ -0,0 +1,44 @@ +import type { TextRun } from '@superdoc/contracts'; +import type { PMNode } from '../../types.js'; +import { textNodeToRun } from './text-run.js'; +import { type InlineConverterParams } from './common.js'; + +/** + * Converts a `bookmarkEnd` PM node. + * + * SD-2454: when `converterContext.showBookmarks` is true, emit a visible gray + * `]` marker at the bookmark end. Matches Word's "Show bookmarks" rendering. + * Returns void (no visual output) when the option is off, preserving today's + * behavior where bookmarkEnd is an invisible structural marker. + * + * The PM schema does not store the bookmark name on bookmarkEnd — only the + * numeric `id` that matches the corresponding bookmarkStart. We therefore + * don't set a tooltip on the closing bracket (Word also omits the name on + * the closing bracket's hover). Styling and identification happen on the + * opening bracket. + */ +export function bookmarkEndNodeToRun(params: InlineConverterParams): TextRun | void { + const { node, converterContext } = params; + if (converterContext?.showBookmarks !== true) return; + + const nodeAttrs = + typeof node.attrs === 'object' && node.attrs !== null ? (node.attrs as Record) : {}; + const bookmarkId = typeof nodeAttrs.id === 'string' || typeof nodeAttrs.id === 'number' ? String(nodeAttrs.id) : ''; + + // Only emit `]` if we emitted the matching `[`. Keeps brackets paired and + // prevents an orphan closing bracket for a suppressed auto-generated + // bookmark (`_Toc…`, `_Ref…`, `_GoBack`). + const rendered = converterContext?.renderedBookmarkIds; + if (rendered && bookmarkId && !rendered.has(bookmarkId)) return; + + const run = textNodeToRun({ + ...params, + node: { type: 'text', text: ']', marks: [...(node.marks ?? [])] } as PMNode, + }); + run.dataAttrs = { + ...(run.dataAttrs ?? {}), + 'data-bookmark-marker': 'end', + ...(bookmarkId ? { 'data-bookmark-id': bookmarkId } : {}), + }; + return run; +} diff --git a/packages/layout-engine/pm-adapter/src/converters/inline-converters/bookmark-markers.test.ts b/packages/layout-engine/pm-adapter/src/converters/inline-converters/bookmark-markers.test.ts new file mode 100644 index 0000000000..d0269be9ea --- /dev/null +++ b/packages/layout-engine/pm-adapter/src/converters/inline-converters/bookmark-markers.test.ts @@ -0,0 +1,115 @@ +import { describe, it, expect, vi } from 'vitest'; +import type { TextRun } from '@superdoc/contracts'; +import type { PMNode } from '../../types.js'; +import type { InlineConverterParams } from './common.js'; + +vi.mock('./text-run.js', () => ({ + textNodeToRun: vi.fn( + (params: InlineConverterParams): TextRun => ({ + text: params.node.text || '', + fontFamily: params.defaultFont, + fontSize: params.defaultSize, + }), + ), +})); + +import { bookmarkStartNodeToBlocks } from './bookmark-start.js'; +import { bookmarkEndNodeToRun } from './bookmark-end.js'; + +function makeParams( + node: PMNode, + opts: { showBookmarks?: boolean; bookmarks?: Map; renderedBookmarkIds?: Set } = {}, +): InlineConverterParams { + return { + node, + positions: new WeakMap(), + defaultFont: 'Calibri', + defaultSize: 16, + inheritedMarks: [], + sdtMetadata: undefined, + hyperlinkConfig: { enableRichHyperlinks: false }, + themeColors: undefined, + runProperties: undefined, + paragraphProperties: undefined, + converterContext: { + translatedNumbering: {}, + translatedLinkedStyles: { docDefaults: {}, latentStyles: {}, styles: {} }, + showBookmarks: opts.showBookmarks ?? false, + renderedBookmarkIds: opts.renderedBookmarkIds, + } as unknown as InlineConverterParams['converterContext'], + enableComments: false, + visitNode: vi.fn(), + bookmarks: opts.bookmarks, + tabOrdinal: 0, + paragraphAttrs: {}, + nextBlockId: vi.fn(), + } as InlineConverterParams; +} + +describe('bookmarkStartNodeToBlocks (SD-2454)', () => { + it('emits no visible run when showBookmarks is off (default)', () => { + const node: PMNode = { type: 'bookmarkStart', attrs: { name: 'chapter1', id: '1' } }; + const result = bookmarkStartNodeToBlocks(makeParams(node, { showBookmarks: false })); + expect(result).toBeUndefined(); + }); + + it('emits a `[` TextRun with bookmark-name data attr when showBookmarks is on', () => { + const node: PMNode = { type: 'bookmarkStart', attrs: { name: 'chapter1', id: '1' } }; + const result = bookmarkStartNodeToBlocks(makeParams(node, { showBookmarks: true })); + expect(result).toBeDefined(); + expect(result!.text).toBe('['); + expect(result!.dataAttrs).toEqual({ + 'data-bookmark-name': 'chapter1', + 'data-bookmark-marker': 'start', + }); + }); + + it('suppresses markers for auto-generated bookmarks (names starting with `_`)', () => { + // Matches Word behavior: `_Toc…`, `_Ref…`, `_GoBack` etc. are hidden from + // Show Bookmarks because they are internally generated for headings, + // fields, or navigation — showing them would clutter the document. + for (const name of ['_Toc1234', '_Ref506192326', '_GoBack']) { + const node: PMNode = { type: 'bookmarkStart', attrs: { name, id: '1' } }; + const result = bookmarkStartNodeToBlocks(makeParams(node, { showBookmarks: true })); + expect(result, `should suppress marker for "${name}"`).toBeUndefined(); + } + }); + + it('still records bookmark position for cross-reference resolution regardless of showBookmarks', () => { + const bookmarks = new Map(); + const node: PMNode = { type: 'bookmarkStart', attrs: { name: 'chapter1', id: '1' } }; + const params = makeParams(node, { showBookmarks: false, bookmarks }); + // Seed the position map + params.positions.set(node, { start: 42, end: 42 }); + bookmarkStartNodeToBlocks(params); + expect(bookmarks.get('chapter1')).toBe(42); + }); +}); + +describe('bookmarkEndNodeToRun (SD-2454)', () => { + it('emits no run when showBookmarks is off (default)', () => { + const node: PMNode = { type: 'bookmarkEnd', attrs: { id: '1' } }; + const result = bookmarkEndNodeToRun(makeParams(node, { showBookmarks: false })); + expect(result).toBeUndefined(); + }); + + it('emits a `]` TextRun when the matching start was rendered', () => { + const node: PMNode = { type: 'bookmarkEnd', attrs: { id: '1' } }; + const result = bookmarkEndNodeToRun(makeParams(node, { showBookmarks: true, renderedBookmarkIds: new Set(['1']) })); + expect(result).toBeDefined(); + expect(result!.text).toBe(']'); + expect(result!.dataAttrs).toEqual({ + 'data-bookmark-marker': 'end', + 'data-bookmark-id': '1', + }); + }); + + it('suppresses `]` when the matching start was also suppressed (no orphan brackets)', () => { + const node: PMNode = { type: 'bookmarkEnd', attrs: { id: '42' } }; + // Start with id 42 was suppressed — renderedBookmarkIds does not include it + const result = bookmarkEndNodeToRun( + makeParams(node, { showBookmarks: true, renderedBookmarkIds: new Set(['99']) }), + ); + expect(result).toBeUndefined(); + }); +}); diff --git a/packages/layout-engine/pm-adapter/src/converters/inline-converters/bookmark-start.ts b/packages/layout-engine/pm-adapter/src/converters/inline-converters/bookmark-start.ts index 779c95934b..a91ff1193a 100644 --- a/packages/layout-engine/pm-adapter/src/converters/inline-converters/bookmark-start.ts +++ b/packages/layout-engine/pm-adapter/src/converters/inline-converters/bookmark-start.ts @@ -1,26 +1,68 @@ -import { type InlineConverterParams } from './common'; +import type { TextRun } from '@superdoc/contracts'; +import type { PMNode } from '../../types.js'; +import { textNodeToRun } from './text-run.js'; +import { type InlineConverterParams } from './common.js'; -export function bookmarkStartNodeToBlocks({ - node, - positions, - bookmarks, - visitNode, - inheritedMarks, - sdtMetadata, - runProperties, -}: InlineConverterParams): void { - // Track bookmark position for cross-reference resolution +/** + * Converts a `bookmarkStart` PM node. + * + * Primary job: record the bookmark's PM position in the `bookmarks` Map so + * cross-reference navigation (goToAnchor) can resolve `#` + * hrefs to a document position. + * + * SD-2454: when `converterContext.showBookmarks` is true, also emit a visible + * gray `[` marker at the bookmark start, matching Word's opt-in "Show + * bookmarks" feature. The marker is a regular TextRun so it flows through + * pagination and line breaking like any other character; `dataAttrs` tag it + * so DomPainter can style it gray and set a tooltip with the bookmark name. + * + * When `showBookmarks` is false (the default), the converter still descends + * into any content inside the bookmark span but emits no visual output. + */ +export function bookmarkStartNodeToBlocks(params: InlineConverterParams): TextRun | void { + const { node, positions, bookmarks, visitNode, inheritedMarks, sdtMetadata, runProperties, converterContext } = + params; const nodeAttrs = typeof node.attrs === 'object' && node.attrs !== null ? (node.attrs as Record) : {}; const bookmarkName = typeof nodeAttrs.name === 'string' ? nodeAttrs.name : undefined; + if (bookmarkName && bookmarks) { const nodePos = positions.get(node); if (nodePos) { bookmarks.set(bookmarkName, nodePos.start); } } - // Process any content inside the bookmark (usually empty) + + // Word hides `_Toc…` / `_Ref…` / other `_`-prefixed bookmarks from its Show + // Bookmarks rendering because they're autogenerated (headings, fields). + // Mirror that so opt-in markers don't pollute every heading and xref target. + const shouldRender = + converterContext?.showBookmarks === true && typeof bookmarkName === 'string' && !bookmarkName.startsWith('_'); + + let run: TextRun | undefined; + if (shouldRender) { + run = textNodeToRun({ + ...params, + node: { type: 'text', text: '[', marks: [...(node.marks ?? [])] } as PMNode, + }); + run.dataAttrs = { + ...(run.dataAttrs ?? {}), + 'data-bookmark-name': bookmarkName!, + 'data-bookmark-marker': 'start', + }; + // Record the id so the matching bookmarkEnd converter knows to emit `]`. + // Without this, suppressing a `_`-prefixed start leaves an orphan `]`. + const bookmarkIdRaw = nodeAttrs.id; + const bookmarkId = + typeof bookmarkIdRaw === 'string' || typeof bookmarkIdRaw === 'number' ? String(bookmarkIdRaw) : ''; + if (bookmarkId && converterContext?.renderedBookmarkIds) { + converterContext.renderedBookmarkIds.add(bookmarkId); + } + } + if (Array.isArray(node.content)) { node.content.forEach((child) => visitNode(child, inheritedMarks, sdtMetadata, runProperties)); } + + return run; } diff --git a/packages/layout-engine/pm-adapter/src/converters/paragraph.ts b/packages/layout-engine/pm-adapter/src/converters/paragraph.ts index 0bc5a4d59b..58b887aa3a 100644 --- a/packages/layout-engine/pm-adapter/src/converters/paragraph.ts +++ b/packages/layout-engine/pm-adapter/src/converters/paragraph.ts @@ -49,6 +49,7 @@ import { structuredContentNodeToBlocks } from './inline-converters/structured-co import { pageReferenceNodeToBlock } from './inline-converters/page-reference.js'; import { fieldAnnotationNodeToRun } from './inline-converters/field-annotation.js'; import { bookmarkStartNodeToBlocks } from './inline-converters/bookmark-start.js'; +import { bookmarkEndNodeToRun } from './inline-converters/bookmark-end.js'; import { tabNodeToRun } from './inline-converters/tab.js'; import { tokenNodeToRun } from './inline-converters/generic-token.js'; import { imageNodeToRun } from './inline-converters/image.js'; @@ -927,6 +928,9 @@ const INLINE_CONVERTERS_REGISTRY: Record = { bookmarkStart: { inlineConverter: bookmarkStartNodeToBlocks, }, + bookmarkEnd: { + inlineConverter: bookmarkEndNodeToRun, + }, tab: { inlineConverter: tabNodeToRun, }, diff --git a/packages/layout-engine/pm-adapter/src/internal.ts b/packages/layout-engine/pm-adapter/src/internal.ts index 4ffd9da91d..f127e8fc20 100644 --- a/packages/layout-engine/pm-adapter/src/internal.ts +++ b/packages/layout-engine/pm-adapter/src/internal.ts @@ -155,6 +155,12 @@ export function toFlowBlocks(pmDoc: PMNode | object, options?: AdapterOptions): defaultFont, defaultSize, ); + if (options?.showBookmarks !== undefined) { + converterContext.showBookmarks = options.showBookmarks; + } + if (converterContext.showBookmarks) { + converterContext.renderedBookmarkIds = new Set(); + } const blocks: FlowBlock[] = []; const bookmarks = new Map(); diff --git a/packages/layout-engine/pm-adapter/src/types.ts b/packages/layout-engine/pm-adapter/src/types.ts index 5a98b205ed..cd9148ac1d 100644 --- a/packages/layout-engine/pm-adapter/src/types.ts +++ b/packages/layout-engine/pm-adapter/src/types.ts @@ -122,6 +122,13 @@ export interface AdapterOptions { */ emitSectionBreaks?: boolean; + /** + * When true, render visible gray `[` / `]` marker runs at bookmarkStart and + * bookmarkEnd positions (SD-2454). Matches Word's opt-in "Show bookmarks" + * behavior. Off by default because bookmarks are structural, not visual. + */ + showBookmarks?: boolean; + /** * Optional instrumentation hook for fidelity logging. */ 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 c7a3ef6eec..34ab45271d 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 @@ -468,6 +468,7 @@ export class PresentationEditor extends EventEmitter { emitCommentPositionsInViewing: options.layoutEngineOptions?.emitCommentPositionsInViewing, enableCommentsInViewing: options.layoutEngineOptions?.enableCommentsInViewing, presence: validatedPresence, + showBookmarks: options.layoutEngineOptions?.showBookmarks ?? false, }; this.#trackedChangesOverrides = options.layoutEngineOptions?.trackedChanges; @@ -2018,6 +2019,24 @@ export class PresentationEditor extends EventEmitter { this.#scheduleRerender(); } + /** + * Toggle the SD-2454 "Show bookmarks" bracket indicators at runtime. + * + * When enabled, the pm-adapter emits visible gray `[` / `]` marker runs at + * bookmarkStart / bookmarkEnd positions (mirroring Word's opt-in behavior). + * Because markers are real characters that participate in text measurement + * and line breaking, toggling invalidates the flow-block cache and triggers + * a full re-layout. + */ + setShowBookmarks(showBookmarks: boolean): void { + const next = !!showBookmarks; + if (this.#layoutOptions.showBookmarks === next) return; + this.#layoutOptions.showBookmarks = next; + this.#flowBlockCache?.clear(); + this.#pendingDocChange = true; + this.#scheduleRerender(); + } + /** * Convert a viewport coordinate into a document hit using the current layout. */ @@ -4194,6 +4213,7 @@ export class PresentationEditor extends EventEmitter { themeColors: this.#editor?.converter?.themeColors ?? undefined, converterContext, flowBlockCache: this.#flowBlockCache, + showBookmarks: this.#layoutOptions.showBookmarks ?? false, ...(positionMap ? { positions: positionMap } : {}), ...(atomNodeTypes.length > 0 ? { atomNodeTypes } : {}), }); diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/types.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/types.ts index e442a6f34e..a77100a932 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/types.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/types.ts @@ -165,6 +165,15 @@ export type LayoutEngineOptions = { ruler?: RulerOptions; /** Proofing / spellcheck configuration. */ proofing?: ProofingConfig; + /** + * Render visible gray `[` / `]` bracket markers at bookmark start/end + * positions — matching Word's opt-in "Show bookmarks" (File > Options > + * Advanced). Off by default because bookmarks are a structural concept, + * not a visual one. Auto-generated bookmarks (names starting with `_`, + * such as `_Toc…` or `_Ref…`) are hidden even when enabled, mirroring + * Word's behavior. SD-2454. + */ + showBookmarks?: boolean; }; export type PresentationEditorOptions = ConstructorParameters[0] & { diff --git a/packages/superdoc/src/core/SuperDoc.js b/packages/superdoc/src/core/SuperDoc.js index e1c59146ba..f92fe56295 100644 --- a/packages/superdoc/src/core/SuperDoc.js +++ b/packages/superdoc/src/core/SuperDoc.js @@ -1240,6 +1240,25 @@ export class SuperDoc extends EventEmitter { }); } + /** + * SD-2454: Toggle bookmark bracket indicators (opt-in, off by default). + * Matches Word's "Show bookmarks" option. Triggers a re-layout on change + * because the brackets are visible characters participating in text flow. + * @param {boolean} show + * @returns {void} + */ + setShowBookmarks(show = true) { + const nextValue = Boolean(show); + const layoutOptions = (this.config.layoutEngineOptions = this.config.layoutEngineOptions || {}); + if (layoutOptions.showBookmarks === nextValue) return; + layoutOptions.showBookmarks = nextValue; + + this.superdocStore?.documents?.forEach((doc) => { + const presentationEditor = doc.getPresentationEditor?.(); + presentationEditor?.setShowBookmarks?.(nextValue); + }); + } + /** * Set the document mode. * @param {DocumentMode} type diff --git a/packages/superdoc/src/dev/components/SuperdocDev.vue b/packages/superdoc/src/dev/components/SuperdocDev.vue index 1054f013c1..c9f7a5e51f 100644 --- a/packages/superdoc/src/dev/components/SuperdocDev.vue +++ b/packages/superdoc/src/dev/components/SuperdocDev.vue @@ -43,6 +43,7 @@ const testUserEmail = urlParams.get('email') || 'user@superdoc.com'; const testUserName = urlParams.get('name') || `SuperDoc ${Math.floor(1000 + Math.random() * 9000)}`; const userRole = urlParams.get('role') || 'editor'; const useLayoutEngine = ref(urlParams.get('layout') !== '0'); +const showBookmarks = ref(urlParams.get('bookmarks') === '1'); const useWebLayout = ref(urlParams.get('view') === 'web'); // Tracked-change replacement model. 'paired' groups ins+del into one change // (Google Docs model); 'independent' keeps each as its own revision (Word / ECMA-376). @@ -687,6 +688,7 @@ const init = async () => { layoutEngineOptions: { flowMode: useWebLayout.value ? 'semantic' : 'paginated', ...(useWebLayout.value ? { semanticOptions: { marginsMode: 'none' } } : {}), + showBookmarks: showBookmarks.value, }, rulers: true, rulerContainer: '#ruler-container', @@ -1204,6 +1206,11 @@ const toggleLayoutEngine = () => { window.location.href = url.toString(); }; +const toggleShowBookmarks = () => { + showBookmarks.value = !showBookmarks.value; + superdoc.value?.setShowBookmarks?.(showBookmarks.value); +}; + const toggleViewLayout = () => { const nextValue = !useWebLayout.value; const url = new URL(window.location.href); @@ -1501,6 +1508,9 @@ if (scrollTestMode.value) { @change="handleCompareFile" /> + From 2ab31d737e19881653ab24c1d882626dc6f40841 Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Tue, 21 Apr 2026 13:04:53 -0300 Subject: [PATCH 4/4] test: close regression gaps for SD-2495 / SD-2454 / anchor-nav fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fills the test gaps surfaced by the testing-excellence review of this PR: - crossReferenceImporter.integration.test.js (new, 4 tests): exercises the full v2 body pipeline (preprocessor -> dispatcher -> entity handler -> v3 translator). Asserts crossReferenceEntity is a member of the defaultNodeListHandler entities list, so the exact root cause that produced IT-949 ("Section 15" vanishing) fails loudly if a future refactor drops the wire-up. Unit tests of the translator alone cannot catch this — they bypass the dispatcher. - EditorInputManager.anchorClick.test.ts (new, 4 tests): pins the SD-2537 click-to-navigate routing. Clicking #bookmark hrefs routes through goToAnchor (was TOC-only before). External and empty-fragment hrefs are explicitly NOT routed. - cross-reference.test.ts: added marks-propagation test (node.marks flow into the emitted TextRun so italic/textStyle on xref text survives — SD-2537 "preserve surrounding run styling" AC). - bookmark-markers.test.ts: converted the `for` loop over auto-generated bookmark names into `it.each`. Each input now reports per-case on failure, complies with testing-excellence's "no control flow inside test bodies" guideline. - PresentationEditor.test.ts: documents why the scrollContainer-vs- visibleHost branch of the SD-2495 scrollPageIntoView fix isn't unit- testable here (happy-dom doesn't propagate inline overflow through getComputedStyle, which is what findScrollableAncestor uses). --- .../bookmark-markers.test.ts | 16 +- .../inline-converters/cross-reference.test.ts | 21 ++ .../EditorInputManager.anchorClick.test.ts | 150 ++++++++++++++ .../tests/PresentationEditor.test.ts | 15 ++ ...crossReferenceImporter.integration.test.js | 191 ++++++++++++++++++ 5 files changed, 384 insertions(+), 9 deletions(-) create mode 100644 packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.anchorClick.test.ts create mode 100644 packages/super-editor/src/editors/v1/core/super-converter/v2/importer/crossReferenceImporter.integration.test.js diff --git a/packages/layout-engine/pm-adapter/src/converters/inline-converters/bookmark-markers.test.ts b/packages/layout-engine/pm-adapter/src/converters/inline-converters/bookmark-markers.test.ts index d0269be9ea..21d5b1f0ef 100644 --- a/packages/layout-engine/pm-adapter/src/converters/inline-converters/bookmark-markers.test.ts +++ b/packages/layout-engine/pm-adapter/src/converters/inline-converters/bookmark-markers.test.ts @@ -64,15 +64,13 @@ describe('bookmarkStartNodeToBlocks (SD-2454)', () => { }); }); - it('suppresses markers for auto-generated bookmarks (names starting with `_`)', () => { - // Matches Word behavior: `_Toc…`, `_Ref…`, `_GoBack` etc. are hidden from - // Show Bookmarks because they are internally generated for headings, - // fields, or navigation — showing them would clutter the document. - for (const name of ['_Toc1234', '_Ref506192326', '_GoBack']) { - const node: PMNode = { type: 'bookmarkStart', attrs: { name, id: '1' } }; - const result = bookmarkStartNodeToBlocks(makeParams(node, { showBookmarks: true })); - expect(result, `should suppress marker for "${name}"`).toBeUndefined(); - } + // Matches Word behavior: `_Toc…`, `_Ref…`, `_GoBack` etc. are hidden from + // Show Bookmarks because they are internally generated for headings, + // fields, or navigation — showing them would clutter the document. + it.each(['_Toc1234', '_Ref506192326', '_GoBack'])('suppresses marker for auto-generated bookmark "%s"', (name) => { + const node: PMNode = { type: 'bookmarkStart', attrs: { name, id: '1' } }; + const result = bookmarkStartNodeToBlocks(makeParams(node, { showBookmarks: true })); + expect(result).toBeUndefined(); }); it('still records bookmark position for cross-reference resolution regardless of showBookmarks', () => { diff --git a/packages/layout-engine/pm-adapter/src/converters/inline-converters/cross-reference.test.ts b/packages/layout-engine/pm-adapter/src/converters/inline-converters/cross-reference.test.ts index 66cf1e30f9..a9dacf0792 100644 --- a/packages/layout-engine/pm-adapter/src/converters/inline-converters/cross-reference.test.ts +++ b/packages/layout-engine/pm-adapter/src/converters/inline-converters/cross-reference.test.ts @@ -85,4 +85,25 @@ describe('crossReferenceNodeToRun (SD-2495)', () => { ); expect(run!.link).toBeUndefined(); }); + + it('forwards node.marks to textNodeToRun so surrounding styles (italic, textStyle) survive', async () => { + // Guards against SD-2537's "preserve surrounding run styling" AC — + // a refactor that dropped node.marks from the synthesized text node + // would silently strip italic/color from every cross-reference. + const { textNodeToRun } = await import('./text-run.js'); + vi.mocked(textNodeToRun).mockClear(); + const marks = [ + { type: 'italic', attrs: {} }, + { type: 'textStyle', attrs: { color: '#ff0000' } }, + ]; + const node: PMNode = { + type: 'crossReference', + attrs: { resolvedText: '15', target: '_Ref1', instruction: 'REF _Ref1 \\h' }, + marks, + }; + crossReferenceNodeToRun(makeParams(node.attrs as Record, { node })); + + const call = vi.mocked(textNodeToRun).mock.calls.at(-1)?.[0]; + expect(call?.node?.marks).toEqual(marks); + }); }); diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.anchorClick.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.anchorClick.test.ts new file mode 100644 index 0000000000..1ee225aa10 --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.anchorClick.test.ts @@ -0,0 +1,150 @@ +/** + * SD-2495 / SD-2537 regression guard for cross-reference click-to-navigate. + * + * The existing behavior before this PR only routed TOC-entry clicks through + * `goToAnchor`. Cross-reference rendered anchors (``) were + * dispatched as generic `superdoc-link-click` custom events — host apps had + * to handle navigation themselves, and most didn't, so clicks silently + * opened nothing. The fix generalized the internal-anchor branch in + * `#handleLinkClick` to cover every `#…` href. + * + * This test pins the new behavior: + * - clicks on `` invoke `goToAnchor('#someBookmark')` + * - the browser's default navigation is prevented + * so a future refactor that narrows the branch back to TOC-only breaks this. + */ +import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest'; + +import { + EditorInputManager, + type EditorInputDependencies, + type EditorInputCallbacks, +} from '../pointer-events/EditorInputManager.js'; + +describe('EditorInputManager — anchor-href click routing (SD-2537)', () => { + let manager: EditorInputManager; + let viewportHost: HTMLElement; + let visibleHost: HTMLElement; + let goToAnchor: Mock; + let mockEditor: { + isEditable: boolean; + state: { doc: { content: { size: number } }; selection: { $anchor: null } }; + view: { dispatch: Mock; dom: HTMLElement; focus: Mock; hasFocus: Mock }; + on: Mock; + off: Mock; + emit: Mock; + }; + + beforeEach(() => { + viewportHost = document.createElement('div'); + viewportHost.className = 'presentation-editor__viewport'; + visibleHost = document.createElement('div'); + visibleHost.className = 'presentation-editor__visible'; + visibleHost.appendChild(viewportHost); + document.body.appendChild(visibleHost); + + mockEditor = { + isEditable: true, + state: { doc: { content: { size: 100 } }, selection: { $anchor: null } }, + view: { dispatch: vi.fn(), dom: document.createElement('div'), focus: vi.fn(), hasFocus: vi.fn(() => false) }, + on: vi.fn(), + off: vi.fn(), + emit: vi.fn(), + }; + + const deps: EditorInputDependencies = { + getActiveEditor: vi.fn(() => mockEditor as unknown as ReturnType), + getEditor: vi.fn(() => mockEditor as unknown as ReturnType), + getLayoutState: vi.fn(() => ({ layout: {} as never, blocks: [], measures: [] })), + getEpochMapper: vi.fn(() => ({ + mapPosFromLayoutToCurrentDetailed: vi.fn(() => ({ ok: true, pos: 12, toEpoch: 1 })), + })) as unknown as EditorInputDependencies['getEpochMapper'], + getViewportHost: vi.fn(() => viewportHost), + getVisibleHost: vi.fn(() => visibleHost), + getLayoutMode: vi.fn(() => 'vertical' as const), + getHeaderFooterSession: vi.fn(() => null), + getPageGeometryHelper: vi.fn(() => null), + getZoom: vi.fn(() => 1), + isViewLocked: vi.fn(() => false), + getDocumentMode: vi.fn(() => 'editing' as const), + getPageElement: vi.fn(() => null), + isSelectionAwareVirtualizationEnabled: vi.fn(() => false), + }; + + goToAnchor = vi.fn(); + const callbacks: EditorInputCallbacks = { + normalizeClientPoint: vi.fn((clientX: number, clientY: number) => ({ x: clientX, y: clientY })), + scheduleSelectionUpdate: vi.fn(), + updateSelectionDebugHud: vi.fn(), + goToAnchor, + }; + + manager = new EditorInputManager(); + manager.setDependencies(deps); + manager.setCallbacks(callbacks); + manager.bind(); + }); + + afterEach(() => { + manager.destroy(); + document.body.innerHTML = ''; + vi.clearAllMocks(); + }); + + const makeAnchor = (href: string): HTMLAnchorElement => { + const a = document.createElement('a'); + a.className = 'superdoc-link'; + a.setAttribute('href', href); + a.textContent = '15'; + viewportHost.appendChild(a); + return a; + }; + + const firePointerDown = (el: HTMLElement) => { + const PointerEventImpl = + (globalThis as unknown as { PointerEvent?: typeof PointerEvent }).PointerEvent ?? globalThis.MouseEvent; + el.dispatchEvent( + new PointerEventImpl('pointerdown', { + bubbles: true, + cancelable: true, + button: 0, + buttons: 1, + clientX: 10, + clientY: 10, + } as PointerEventInit), + ); + }; + + it('routes `#` anchor clicks through goToAnchor', () => { + const a = makeAnchor('#_Ref506192326'); + firePointerDown(a); + expect(goToAnchor).toHaveBeenCalledWith('#_Ref506192326'); + }); + + it('routes TOC-inside `#…` anchor clicks through goToAnchor (backward compat)', () => { + // The pre-PR behavior was TOC-only. Make sure generalizing the branch + // didn't accidentally exclude TOC entries. + const tocWrapper = document.createElement('span'); + tocWrapper.className = 'superdoc-toc-entry'; + const a = document.createElement('a'); + a.className = 'superdoc-link'; + a.setAttribute('href', '#_Toc123'); + tocWrapper.appendChild(a); + viewportHost.appendChild(tocWrapper); + + firePointerDown(a); + expect(goToAnchor).toHaveBeenCalledWith('#_Toc123'); + }); + + it('does not route external hrefs through goToAnchor', () => { + const a = makeAnchor('https://example.com/page'); + firePointerDown(a); + expect(goToAnchor).not.toHaveBeenCalled(); + }); + + it('does not route bare `#` (empty fragment) to goToAnchor', () => { + const a = makeAnchor('#'); + firePointerDown(a); + expect(goToAnchor).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts index 40f9fd8b4e..218c90eef7 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 @@ -1025,6 +1025,21 @@ describe('PresentationEditor', () => { } }, ); + + // SD-2495 anchor-nav fix: #scrollPageIntoView must write to the real + // scroll ancestor, not just #visibleHost. This is enforced by the + // existing scrollToPage tests in this describe block — both mock + // scrollTop on the container (which serves as both visibleHost and the + // effective scroll target in the test harness) and assert that value + // gets written. Those tests would fail if the fix reverted to writing + // only to a zero-effect target. + // + // The "scrollContainer != visibleHost" branch (the exact shape of the + // dev app and real consumers) isn't unit-testable here because happy-dom + // doesn't propagate inline overflow styles through getComputedStyle, + // which is what #findScrollableAncestor uses. Browser-level verification + // lives in the SD-2495 behavior/manual testing; see the click-to- + // navigate agent-browser runs in the PR description. }); describe('setDocumentMode', () => { diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/crossReferenceImporter.integration.test.js b/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/crossReferenceImporter.integration.test.js new file mode 100644 index 0000000000..9e6c070c06 --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/crossReferenceImporter.integration.test.js @@ -0,0 +1,191 @@ +/** + * Integration tests for SD-2495 / IT-949. + * + * Why this file exists (root cause recap): + * The `sd:crossReference` v3 translator was registered in `registeredHandlers` + * but NOT wired into the v2 importer's `defaultNodeListHandler` entity list. + * The passthrough fallback refused to wrap it (because it was "registered"), + * and no entity claimed it, so every REF field in every imported DOCX was + * silently dropped — erasing "Section 15" and every other cross-reference + * from the viewer. + * + * These tests exercise the full v2 body pipeline: preprocessor → dispatcher → + * entity handler → v3 translator → PM node. If any link in that chain breaks + * (most likely: the entity gets removed from the entities list during a + * refactor), the `crossReference` PM node disappears and these tests fail. + * + * The unit tests of the translator alone (`crossReference-translator.test.js`) + * don't catch this class of regression because they bypass the dispatcher. + */ +import { describe, it, expect } from 'vitest'; +import { defaultNodeListHandler } from './docxImporter.js'; +import { preProcessNodesForFldChar } from '../../field-references/index.js'; +import { crossReferenceEntity } from './crossReferenceImporter.js'; + +const createEditorStub = () => ({ + schema: { + nodes: { + run: { isInline: true, spec: { group: 'inline' } }, + crossReference: { isInline: true, spec: { group: 'inline', atom: true } }, + }, + }, +}); + +// Produces the exact XML shape Word emits for a REF field with `\h` — matches +// the Brillio lease fragment that produces the "Section 15" customer bug. +const buildRefField = (target, cachedText) => { + const run = (inner) => ({ + name: 'w:r', + elements: [{ name: 'w:rPr', elements: [{ name: 'w:i' }] }, ...inner], + }); + return [ + run([{ name: 'w:fldChar', attributes: { 'w:fldCharType': 'begin' } }]), + run([ + { + name: 'w:instrText', + attributes: { 'xml:space': 'preserve' }, + elements: [{ type: 'text', text: ` REF ${target} \\w \\h ` }], + }, + ]), + run([]), + run([{ name: 'w:fldChar', attributes: { 'w:fldCharType': 'separate' } }]), + run([{ name: 'w:t', elements: [{ type: 'text', text: cachedText }] }]), + run([{ name: 'w:fldChar', attributes: { 'w:fldCharType': 'end' } }]), + ]; +}; + +describe('SD-2495 v2 importer wiring (IT-949 regression guard)', () => { + it('registers crossReferenceEntity in defaultNodeListHandler — guards the miss that produced IT-949', () => { + // This membership assertion is the cheapest possible regression guard + // against the exact bug root cause: if a future refactor drops the + // entity from the entities list, this fails immediately. + expect(defaultNodeListHandler().handlerEntities).toContain(crossReferenceEntity); + }); + + it('REF field inside a paragraph produces a crossReference PM node with cached text + target', () => { + const paragraph = { + name: 'w:p', + elements: [ + { + name: 'w:r', + elements: [{ name: 'w:t', elements: [{ type: 'text', text: 'If terminated under this Section ' }] }], + }, + ...buildRefField('_Ref506192326', '15'), + { + name: 'w:r', + elements: [{ name: 'w:t', elements: [{ type: 'text', text: ', Landlord...' }] }], + }, + ], + }; + + // Mirror the real body pipeline: preprocess fldChar runs into + // sd:crossReference, then dispatch through the v2 entity list. + const { processedNodes } = preProcessNodesForFldChar([paragraph], {}); + const nodeListHandler = defaultNodeListHandler(); + const pmNodes = nodeListHandler.handler({ + nodes: processedNodes, + docx: {}, + editor: createEditorStub(), + path: [], + }); + + const para = pmNodes.find((n) => n.type === 'paragraph'); + expect(para, 'paragraph should be produced').toBeTruthy(); + + const crossRefs = collectNodesOfType(para, 'crossReference'); + expect(crossRefs).toHaveLength(1); + expect(crossRefs[0].attrs.target).toBe('_Ref506192326'); + expect(crossRefs[0].attrs.resolvedText).toBe('15'); + // Instruction preserves the `\h` switch — the render layer reads this to + // decide whether to attach an internal-link mark (SD-2537 hyperlink vs + // plain-text variant). + expect(crossRefs[0].attrs.instruction).toMatch(/\\h/); + }); + + it('REF with \\h switch records the target so the render layer can navigate on click', () => { + const paragraph = { + name: 'w:p', + elements: [...buildRefField('_Ref123', '7')], + }; + + const { processedNodes } = preProcessNodesForFldChar([paragraph], {}); + const nodeListHandler = defaultNodeListHandler(); + const pmNodes = nodeListHandler.handler({ + nodes: processedNodes, + docx: {}, + editor: createEditorStub(), + path: [], + }); + + const crossRef = collectNodesOfType(pmNodes[0], 'crossReference')[0]; + expect(crossRef).toBeTruthy(); + // `display` is derived from switches so PM-adapter knows which variant. + // `\w \h` → numberFullContext. If this regresses, cross-ref visuals change. + expect(crossRef.attrs.display).toBe('numberFullContext'); + }); + + it('plain text surrounding a REF field still reaches PM unchanged (guards against REF dispatch consuming sibling runs)', () => { + // The `xml:space="preserve"` attribute is what keeps trailing whitespace + // around. Without it, OOXML parsers strip leading/trailing whitespace from + // w:t elements. The real customer document (Brillio lease) preserves this + // attribute on runs adjacent to REF fields so "Section " doesn't collapse + // to "Section" before the number. Mirror that here. + const textRun = (text) => ({ + name: 'w:r', + elements: [{ name: 'w:t', attributes: { 'xml:space': 'preserve' }, elements: [{ type: 'text', text }] }], + }); + const paragraph = { + name: 'w:p', + elements: [textRun('Section '), ...buildRefField('_Ref1', '15'), textRun(', Landlord')], + }; + + const { processedNodes } = preProcessNodesForFldChar([paragraph], {}); + const nodeListHandler = defaultNodeListHandler(); + const [pmPara] = nodeListHandler.handler({ + nodes: processedNodes, + docx: {}, + editor: createEditorStub(), + path: [], + }); + + // Children of the paragraph, in order, excluding the crossReference (it's + // an atom — its cached text contributes to the visual output but lives + // inside the xref node, not beside it). We care here about the SURROUNDING + // text surviving unchanged. + const collectSiblingTextBeforeAndAfterXref = (paraNode) => { + const parts = { before: '', after: '' }; + let sawXref = false; + const visitChildren = (nodes) => { + for (const child of nodes ?? []) { + if (child?.type === 'crossReference') { + sawXref = true; + continue; + } + if (Array.isArray(child?.content)) visitChildren(child.content); + else if (child?.type === 'text' && typeof child.text === 'string') { + if (sawXref) parts.after += child.text; + else parts.before += child.text; + } + } + }; + visitChildren(paraNode.content ?? []); + return parts; + }; + + const { before, after } = collectSiblingTextBeforeAndAfterXref(pmPara); + expect(before).toBe('Section '); + expect(after).toBe(', Landlord'); + }); +}); + +/** Collect all descendants of a given type from a nested PM node tree. */ +function collectNodesOfType(root, type) { + const out = []; + const visit = (node) => { + if (!node) return; + if (node.type === type) out.push(node); + if (Array.isArray(node.content)) node.content.forEach(visit); + }; + visit(root); + return out; +}