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..21d5b1f0ef --- /dev/null +++ b/packages/layout-engine/pm-adapter/src/converters/inline-converters/bookmark-markers.test.ts @@ -0,0 +1,113 @@ +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', + }); + }); + + // 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', () => { + 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/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..a9dacf0792 --- /dev/null +++ b/packages/layout-engine/pm-adapter/src/converters/inline-converters/cross-reference.test.ts @@ -0,0 +1,109 @@ +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(); + }); + + 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/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/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 2c9d1dab9d..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 } : {}), }); @@ -5839,8 +5859,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; } } 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/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/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/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; +} 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)'); + }); +}); 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" /> +