From 23a59477b83f8551af537b337019cfc569eac3bf Mon Sep 17 00:00:00 2001 From: Gabriel Chittolina Date: Fri, 1 May 2026 14:05:27 -0300 Subject: [PATCH 01/14] chore: save --- .../v1/components/context-menu/constants.js | 3 + .../context-menu/list-marker-helpers.js | 58 +++++++++++++ .../v1/components/context-menu/menuItems.js | 19 +++++ .../context-menu/tests/utils.test.js | 3 + .../v1/components/context-menu/utils.js | 3 + .../helpers/toc-entry-builder.test.ts | 82 +++++++++++++++++-- .../helpers/toc-entry-builder.ts | 73 +++++++++++++---- .../plan-engine/toc-wrappers.ts | 41 +++++++++- .../extensions/field-update/field-update.js | 20 +++++ .../table-of-contents/find-toc-ancestor.js | 32 ++++++++ .../find-toc-ancestor.test.js | 68 +++++++++++++++ .../table-of-contents/toc-page-number.js | 4 +- 12 files changed, 381 insertions(+), 25 deletions(-) create mode 100644 packages/super-editor/src/editors/v1/components/context-menu/list-marker-helpers.js create mode 100644 packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js create mode 100644 packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.test.js diff --git a/packages/super-editor/src/editors/v1/components/context-menu/constants.js b/packages/super-editor/src/editors/v1/components/context-menu/constants.js index 8a8f3fad52..8242b96968 100644 --- a/packages/super-editor/src/editors/v1/components/context-menu/constants.js +++ b/packages/super-editor/src/editors/v1/components/context-menu/constants.js @@ -13,6 +13,7 @@ import pasteIconSvg from '@superdoc/common/icons/paste-solid.svg?raw'; import checkIconSvg from '@superdoc/common/icons/check-solid.svg?raw'; import xMarkIconSvg from '@superdoc/common/icons/xmark-solid.svg?raw'; import paintRollerIconSvg from '@superdoc/common/icons/paint-roller-solid.svg?raw'; +import rotateRightIconSvg from '@superdoc/common/icons/rotate-right-solid.svg?raw'; export const ICONS = { addRowBefore: plusIconSvg, @@ -37,6 +38,7 @@ export const ICONS = { trackChangesAccept: checkIconSvg, trackChangesReject: xMarkIconSvg, cellBackground: paintRollerIconSvg, + updateTableOfContents: rotateRightIconSvg, }; // Table actions constant @@ -65,6 +67,7 @@ export const TEXTS = { trackChangesAccept: 'Accept change', trackChangesReject: 'Reject change', cellBackground: 'Cell background', + updateTableOfContents: 'Update table of contents', }; export const tableActionsOptions = [ diff --git a/packages/super-editor/src/editors/v1/components/context-menu/list-marker-helpers.js b/packages/super-editor/src/editors/v1/components/context-menu/list-marker-helpers.js new file mode 100644 index 0000000000..4e25ac6b36 --- /dev/null +++ b/packages/super-editor/src/editors/v1/components/context-menu/list-marker-helpers.js @@ -0,0 +1,58 @@ +import { findParentNode } from '@helpers/index.js'; +import { isList } from '@core/commands/list-helpers'; +import { getResolvedParagraphProperties } from '@extensions/paragraph/resolvedPropertiesCache.js'; + +/** + * Resolve the list item that the menu was opened over to the document-api + * `ListItemAddress` shape required by `editor.doc.lists.*` calls. + * + * Mirrors the paragraph branch of `resolveBlockNodeId` in the doc-api: paraId + * (preserved across DOCX round-trips) takes precedence over sdBlockId. + * + * @param {Object} context - Menu context containing `editor` and the click `pos`. + * @returns {{ kind: 'block', nodeType: 'listItem', nodeId: string } | null} + */ +export function getListItemAddressFromContext(context) { + const result = findListParagraphFromContext(context); + if (!result) return null; + const attrs = result.node.attrs ?? {}; + const nodeId = attrs.paraId ?? attrs.sdBlockId ?? null; + if (!nodeId) return null; + return { kind: 'block', nodeType: 'listItem', nodeId }; +} + +/** + * Returns the resolved indent level (0-8) for the list item under the menu, + * or null if the cursor isn't over a list paragraph. + * + * @param {Object} context - Menu context containing `editor` and the click `pos`. + * @returns {number | null} + */ +export function getListItemLevelFromContext(context) { + const result = findListParagraphFromContext(context); + if (!result) return null; + const props = getResolvedParagraphProperties(result.node)?.numberingProperties; + return typeof props?.ilvl === 'number' ? props.ilvl : 0; +} + +function findListParagraphFromContext(context) { + const editor = context?.editor; + const state = editor?.state; + if (!state) return null; + + const selection = synthesizeSelection(state, context?.pos); + if (!selection) return null; + return findParentNode(isList)(selection) ?? null; +} + +function synthesizeSelection(state, pos) { + if (typeof pos === 'number' && Number.isFinite(pos)) { + try { + const $pos = state.doc.resolve(pos); + return { $from: $pos, $to: $pos }; + } catch { + // pos was out of range; fall through to live selection + } + } + return state.selection ?? null; +} diff --git a/packages/super-editor/src/editors/v1/components/context-menu/menuItems.js b/packages/super-editor/src/editors/v1/components/context-menu/menuItems.js index 59bf0533a0..5c762f8a9e 100644 --- a/packages/super-editor/src/editors/v1/components/context-menu/menuItems.js +++ b/packages/super-editor/src/editors/v1/components/context-menu/menuItems.js @@ -338,6 +338,25 @@ export function getItems(context, customItems = [], includeDefaultItems = true) return context.trigger === TRIGGERS.click && (context.isCellSelection || context.isInTable); }, }, + { + id: 'update-table-of-contents', + label: TEXTS.updateTableOfContents, + icon: ICONS.updateTableOfContents, + isDefault: true, + action: (editor, context) => { + const tocId = context.tocAncestor?.sdBlockId; + if (!tocId) return; + const target = { kind: 'block', nodeType: 'tableOfContents', nodeId: tocId }; + try { + editor.documentApi?.toc?.update?.({ target, mode: 'all' }); + } catch (error) { + console.warn('[ContextMenu] toc.update failed:', error); + } + }, + showWhen: (context) => { + return context.trigger === TRIGGERS.click && !!context.tocAncestor?.sdBlockId; + }, + }, ], }, { diff --git a/packages/super-editor/src/editors/v1/components/context-menu/tests/utils.test.js b/packages/super-editor/src/editors/v1/components/context-menu/tests/utils.test.js index 28d6796901..5d290eb134 100644 --- a/packages/super-editor/src/editors/v1/components/context-menu/tests/utils.test.js +++ b/packages/super-editor/src/editors/v1/components/context-menu/tests/utils.test.js @@ -163,6 +163,9 @@ describe('utils.js', () => { // Proofing context (null when no PresentationEditor proofing active) proofingContext: null, + + // TOC ancestor (null when not inside a tableOfContents node) + tocAncestor: null, }); // Verify clipboard is not read during context gathering diff --git a/packages/super-editor/src/editors/v1/components/context-menu/utils.js b/packages/super-editor/src/editors/v1/components/context-menu/utils.js index 04cdc09228..2d1e47a5e7 100644 --- a/packages/super-editor/src/editors/v1/components/context-menu/utils.js +++ b/packages/super-editor/src/editors/v1/components/context-menu/utils.js @@ -1,5 +1,6 @@ import { selectionHasNodeOrMark } from '../cursor-helpers.js'; import { tableActionsOptions } from './constants.js'; +import { findTocAncestor } from '@extensions/table-of-contents/find-toc-ancestor.js'; import { markRaw } from 'vue'; import { undoDepth, redoDepth } from 'prosemirror-history'; import { yUndoPluginKey } from 'y-prosemirror'; @@ -123,6 +124,7 @@ export async function getEditorContext(editor, event) { }; const structureFromResolvedPos = pos !== null ? getStructureFromResolvedPos(state, pos) : null; + const tocAncestor = pos !== null ? findTocAncestor(state.doc, pos) : null; const isInTable = structureFromResolvedPos?.isInTable ?? selectionHasNodeOrMark(state, 'table', { requireEnds: true }); const isInList = structureFromResolvedPos?.isInList ?? selectionIncludesListParagraph(state); @@ -223,6 +225,7 @@ export async function getEditorContext(editor, event) { editor, trackedChanges, proofingContext, + tocAncestor, }; } diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts index ce6ca8c80c..96ccdf763f 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts @@ -52,16 +52,16 @@ describe('buildTocEntryParagraphs', () => { }); describe('rightAlignPageNumbers', () => { - it('adds a right-aligned tab stop when rightAlignPageNumbers is true', () => { + it('adds a right-aligned tab stop with default dot leader when rightAlignPageNumbers is true', () => { const paragraphs = buildTocEntryParagraphs([BASE_SOURCE], makeConfig({ rightAlignPageNumbers: true })); const tabStops = paragraphs[0]!.attrs.paragraphProperties as Record; - expect(tabStops.tabStops).toEqual([{ tab: { tabType: 'right', pos: 9350 } }]); + expect(tabStops.tabStops).toEqual([{ tab: { tabType: 'right', pos: 9350, leader: 'dot' } }]); }); - it('adds a right-aligned tab stop by default (undefined)', () => { + it('adds a right-aligned tab stop with default dot leader by default (undefined)', () => { const paragraphs = buildTocEntryParagraphs([BASE_SOURCE], makeConfig()); const tabStops = paragraphs[0]!.attrs.paragraphProperties as Record; - expect(tabStops.tabStops).toEqual([{ tab: { tabType: 'right', pos: 9350 } }]); + expect(tabStops.tabStops).toEqual([{ tab: { tabType: 'right', pos: 9350, leader: 'dot' } }]); }); it('omits tab stop when rightAlignPageNumbers is false', () => { @@ -96,6 +96,36 @@ describe('buildTocEntryParagraphs', () => { const props = paragraphs[0]!.attrs.paragraphProperties as Record; expect(props.tabStops).toBeUndefined(); }); + + it('honours options.tabPos when provided', () => { + const paragraphs = buildTocEntryParagraphs([BASE_SOURCE], makeConfig({ rightAlignPageNumbers: true }), { + tabPos: 12345, + }); + const props = paragraphs[0]!.attrs.paragraphProperties as Record; + expect(props.tabStops).toEqual([{ tab: { tabType: 'right', pos: 12345, leader: 'dot' } }]); + }); + }); + + describe('page numbers (SD-2664)', () => { + it('substitutes page numbers from options.pageMap when present', () => { + const pageMap = new Map([['h-1', 7]]); + const paragraphs = buildTocEntryParagraphs([BASE_SOURCE], makeConfig({ hyperlinks: true }), { pageMap }); + const pageNode = paragraphs[0]!.content.find( + (n: Record) => + Array.isArray(n.marks) && n.marks?.some((m) => (m as { type: string }).type === 'tocPageNumber'), + ) as { text: string }; + expect(pageNode.text).toBe('7'); + }); + + it('falls back to "0" placeholder when the source is not in the page map', () => { + const pageMap = new Map(); // empty + const paragraphs = buildTocEntryParagraphs([BASE_SOURCE], makeConfig({ hyperlinks: true }), { pageMap }); + const pageNode = paragraphs[0]!.content.find( + (n: Record) => + Array.isArray(n.marks) && n.marks?.some((m) => (m as { type: string }).type === 'tocPageNumber'), + ) as { text: string }; + expect(pageNode.text).toBe('0'); + }); }); }); @@ -104,7 +134,7 @@ describe('buildTocEntryParagraphs', () => { // --------------------------------------------------------------------------- interface MockParagraph { - sdBlockId: string; + sdBlockId: string | null; text: string; styleId?: string; outlineLevel?: number; @@ -252,4 +282,46 @@ describe('collectTocSources', () => { const sources = collectTocSources(doc, config); expect(sources.length).toBe(0); }); + + it('skips heading-styled paragraphs whose visible text is empty (SD-2664)', () => { + // Page-break / spacer paragraphs that inherit Heading1 must not produce + // ghost TOC entries on rebuild. + const docWithEmptyHeading = mockDoc([ + { sdBlockId: 'p1', text: 'Part 1', styleId: 'Heading1' }, + { sdBlockId: 'p2', text: '', styleId: 'Heading1' }, + { sdBlockId: 'p3', text: ' ', styleId: 'Heading1' }, + { sdBlockId: 'p4', text: 'Part 2', styleId: 'Heading1' }, + ]); + + const config: TocSwitchConfig = { + source: { outlineLevels: { from: 1, to: 3 } }, + display: { hyperlinks: true }, + preserved: {}, + }; + + const sources = collectTocSources(docWithEmptyHeading, config); + expect(sources.map((s) => s.text)).toEqual(['Part 1', 'Part 2']); + }); + + it('collects pasted heading paragraphs that lack sdBlockId/paraId (SD-2664)', () => { + // SuperDoc's slice paste resets paraId/sdBlockId to null on pasted paragraphs + // (InputRule.js SUPERDOC_SLICE_PASTE_IDENTITY_RESETS) to avoid public-id + // duplicates. The TOC rebuild must still pick those paragraphs up via a + // synthetic deterministic id so toc.update mode 'all' reflects new entries. + const docWithPastedHeading = mockDoc([ + { sdBlockId: 'p1', text: 'Part 3', styleId: 'Heading1' }, + { sdBlockId: null, text: 'Part 4', styleId: 'Heading1' }, + ]); + + const config: TocSwitchConfig = { + source: { outlineLevels: { from: 1, to: 3 } }, + display: { hyperlinks: true }, + preserved: {}, + }; + + const sources = collectTocSources(docWithPastedHeading, config); + + expect(sources.map((s) => s.text)).toEqual(['Part 3', 'Part 4']); + expect(sources[1].sdBlockId).toMatch(/^para-auto-/); + }); }); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts index 5cd5929816..8225c5d866 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts @@ -9,6 +9,7 @@ import type { Node as ProseMirrorNode } from 'prosemirror-model'; import type { TocSwitchConfig } from '@superdoc/document-api'; import { parseTcInstruction } from '../../core/super-converter/field-references/shared/tc-switches.js'; import { getHeadingLevel } from './node-address-resolver.js'; +import { buildFallbackBlockNodeId } from './deterministic-node-id.js'; import { generateTocBookmarkName } from './toc-bookmark-sync.js'; // --------------------------------------------------------------------------- @@ -57,7 +58,7 @@ export function collectTocSources(doc: ProseMirrorNode, config: TocSwitchConfig) // Track the current paragraph context for TC field collection let currentParagraphSdBlockId: string | undefined; - doc.descendants((node, _pos) => { + doc.descendants((node, pos) => { // Skip TOC nodes themselves — don't collect entries from within a TOC if (node.type.name === 'tableOfContents') return false; @@ -65,18 +66,34 @@ export function collectTocSources(doc: ProseMirrorNode, config: TocSwitchConfig) const attrs = node.attrs as Record | undefined; const paragraphProps = attrs?.paragraphProperties as Record | undefined; const styleId = paragraphProps?.styleId as string | undefined; - const sdBlockId = (attrs?.sdBlockId ?? attrs?.paraId) as string | undefined; + // Pasted/new paragraphs intentionally have null paraId/sdBlockId (see + // InputRule.js SUPERDOC_SLICE_PASTE_IDENTITY_RESETS) to avoid public-id + // duplicates. Synthesize a deterministic position-based id so they still + // appear in the rebuilt TOC and round-trip via OOXML bookmarks. + const sdBlockId = + ((attrs?.sdBlockId ?? attrs?.paraId) as string | undefined) ?? buildFallbackBlockNodeId('paragraph', pos); // Update paragraph context for TC field collection currentParagraphSdBlockId = sdBlockId; if (!sdBlockId) return true; + const text = flattenText(node); + // Word's TOC field skips paragraphs that are styled as headings but + // contain no visible text (page-break-only spacers, empty stubs). + // Including them produces ghost entries that look like a regression. + const hasVisibleText = text.trim().length > 0; + // Check heading by style (\o switch) if (outlineLevels) { const headingLevel = getHeadingLevel(styleId); - if (headingLevel != null && headingLevel >= outlineLevels.from && headingLevel <= outlineLevels.to) { - sources.push({ text: flattenText(node), level: headingLevel, sdBlockId, kind: 'heading' }); + if ( + headingLevel != null && + headingLevel >= outlineLevels.from && + headingLevel <= outlineLevels.to && + hasVisibleText + ) { + sources.push({ text, level: headingLevel, sdBlockId, kind: 'heading' }); // Continue descending to find TC fields within this paragraph return true; } @@ -88,8 +105,8 @@ export function collectTocSources(doc: ProseMirrorNode, config: TocSwitchConfig) const rawOutlineLevel = paragraphProps?.outlineLevel as number | undefined; if (rawOutlineLevel != null) { const tocLevel = rawOutlineLevel + 1; - if (tocLevel >= effectiveLevels.from && tocLevel <= effectiveLevels.to) { - sources.push({ text: flattenText(node), level: tocLevel, sdBlockId, kind: 'appliedOutline' }); + if (tocLevel >= effectiveLevels.from && tocLevel <= effectiveLevels.to && hasVisibleText) { + sources.push({ text, level: tocLevel, sdBlockId, kind: 'appliedOutline' }); return true; } } @@ -164,8 +181,24 @@ export interface EntryParagraphJson { * - Page number placeholder "0" with tocPageNumber mark * - Separator: custom (\p switch) or default tab */ -export function buildTocEntryParagraphs(sources: TocSource[], config: TocSwitchConfig): EntryParagraphJson[] { - return sources.map((source) => buildEntryParagraph(source, config)); +/** + * Optional context that lets the entry builder produce final-looking output + * without a follow-up `mode: 'pageNumbers'` pass and without losing layout + * particulars from the existing TOC. + */ +export interface BuildTocEntryOptions { + /** sdBlockId → page number map from PresentationEditor's last layout cycle. */ + pageMap?: Map; + /** Right-tab stop position (twips) to mirror the existing TOC's spacing. */ + tabPos?: number; +} + +export function buildTocEntryParagraphs( + sources: TocSource[], + config: TocSwitchConfig, + options: BuildTocEntryOptions = {}, +): EntryParagraphJson[] { + return sources.map((source) => buildEntryParagraph(source, config, options)); } /** Default right-margin position for right-aligned tab stops (twips). ~6.5 inches. */ @@ -179,7 +212,11 @@ const TAB_LEADER_MAP: Record = { middleDot: 'middleDot', }; -function buildEntryParagraph(source: TocSource, config: TocSwitchConfig): EntryParagraphJson { +function buildEntryParagraph( + source: TocSource, + config: TocSwitchConfig, + options: BuildTocEntryOptions = {}, +): EntryParagraphJson { const { display } = config; const content: Array> = []; @@ -218,10 +255,14 @@ function buildEntryParagraph(source: TocSource, config: TocSwitchConfig): EntryP content.push({ type: 'tab' }); } - // Page number placeholder with tocPageNumber mark for surgical updates + // Page number — resolved from the page map when available so a single + // mode 'all' rebuild produces final numbers; falls back to '0' placeholder + // when the source paragraph is not yet in the page map (freshly pasted + // headings whose synthetic id has not been seen by a layout cycle). + const resolvedPage = options.pageMap?.get(source.sdBlockId); content.push({ type: 'text', - text: '0', + text: resolvedPage != null ? String(resolvedPage) : '0', marks: [{ type: 'tocPageNumber' }], }); } @@ -233,11 +274,13 @@ function buildEntryParagraph(source: TocSource, config: TocSwitchConfig): EntryP const rightAlign = display.rightAlignPageNumbers !== false; // default true if (rightAlign && !omitPageNumber) { + // Word's default TOC tab leader is dots. The \p switch is only emitted + // when a non-default separator is used, so an absent tabLeader means the + // user expects dots, not "no leader". Honor an explicit 'none' to opt out. const leader = - display.tabLeader && display.tabLeader !== 'none' ? (TAB_LEADER_MAP[display.tabLeader] ?? undefined) : undefined; - paragraphProperties.tabStops = [ - { tab: { tabType: 'right', pos: DEFAULT_RIGHT_TAB_POS, ...(leader ? { leader } : {}) } }, - ]; + display.tabLeader === 'none' ? undefined : (display.tabLeader && TAB_LEADER_MAP[display.tabLeader]) || 'dot'; + const pos = options.tabPos ?? DEFAULT_RIGHT_TAB_POS; + paragraphProperties.tabStops = [{ tab: { tabType: 'right', pos, ...(leader ? { leader } : {}) } }]; } return { diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts index 94e18f0883..d606d0d437 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts @@ -257,13 +257,45 @@ interface MaterializedToc { sources: TocSource[]; } -function materializeTocContent(doc: ProseMirrorNode, config: TocSwitchConfig, editor: Editor): MaterializedToc { +interface MaterializeTocOptions { + /** sdBlockId → page number map to fold into the rebuilt entries. */ + pageMap?: Map; + /** Right-tab stop position (twips) sampled from the existing TOC. */ + tabPos?: number; +} + +function materializeTocContent( + doc: ProseMirrorNode, + config: TocSwitchConfig, + editor: Editor, + options: MaterializeTocOptions = {}, +): MaterializedToc { const sources = collectTocSources(doc, config); - const entryParagraphs = buildTocEntryParagraphs(sources, config); + const entryParagraphs = buildTocEntryParagraphs(sources, config, options); const content = entryParagraphs.length > 0 ? entryParagraphs : NO_ENTRIES_PLACEHOLDER; return { content: sanitizeTocContentForSchema(content, editor), sources }; } +/** + * Sample the right-tab stop position from the existing TOC's first child + * paragraph so the rebuilt entries keep the same column for page numbers. + * Returns `undefined` when no usable tab stop is present, in which case the + * builder falls back to the default position. + */ +function readExistingTocTabPos(node: ProseMirrorNode): number | undefined { + const first = node.firstChild; + if (!first) return undefined; + const tabStops = ( + first.attrs as + | { paragraphProperties?: { tabStops?: Array<{ tab?: { pos?: number; tabType?: string } }> } } + | undefined + )?.paragraphProperties?.tabStops; + if (!Array.isArray(tabStops)) return undefined; + const right = tabStops.find((t) => t?.tab?.tabType === 'right'); + const pos = right?.tab?.pos; + return typeof pos === 'number' ? pos : undefined; +} + // --------------------------------------------------------------------------- // toc.configure // --------------------------------------------------------------------------- @@ -378,7 +410,10 @@ function tocUpdateAll(editor: Editor, input: TocUpdateInput, options?: MutationO const resolved = resolveTocTarget(editor.state.doc, input.target); const config = parseTocInstruction(resolved.node.attrs?.instruction ?? ''); const rightAlign = resolved.node.attrs?.rightAlignPageNumbers as boolean | undefined; - const { content, sources } = materializeTocContent(editor.state.doc, withRightAlign(config, rightAlign), editor); + const { content, sources } = materializeTocContent(editor.state.doc, withRightAlign(config, rightAlign), editor, { + pageMap: getPageMap(editor) ?? undefined, + tabPos: readExistingTocTabPos(resolved.node), + }); // NO_OP detection: compare new content against existing before executing. // The PM command returns "found" (not "content changed"), so receipt-based diff --git a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js index 17c1d513c5..48a67eec9a 100644 --- a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js +++ b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js @@ -5,6 +5,7 @@ import { resolveDocumentStatFieldValue, resolveMainBodyEditor, } from '../../document-api-adapters/helpers/word-statistics.js'; +import { findTocAncestor } from '../table-of-contents/find-toc-ancestor.js'; /** Field types eligible for value updates via F9. */ const UPDATABLE_FIELD_TYPES = new Set(['NUMWORDS', 'NUMCHARS', 'NUMPAGES']); @@ -36,6 +37,25 @@ export const FieldUpdate = Extension.create({ () => ({ editor, state, dispatch }) => { const { from, to } = state.selection; + + // F9 inside a TOC rebuilds the TOC via the document-api wrapper. + // The wrapper handles content materialization, NO_OP detection, and + // bookmark sync. Mode 'all' is used because it does not depend on a + // completed layout cycle (mode 'pageNumbers' can fail with + // CAPABILITY_UNAVAILABLE in headless contexts). + const toc = findTocAncestor(state.doc, from); + if (toc?.sdBlockId && editor?.documentApi?.toc?.update) { + if (!dispatch) return true; + try { + const target = { kind: 'block', nodeType: 'tableOfContents', nodeId: toc.sdBlockId }; + editor.documentApi.toc.update({ target, mode: 'all' }); + } catch (error) { + console.warn('[FieldUpdate] toc.update failed:', error); + return false; + } + return true; + } + const fields = findFieldsInRange(state.doc, from, to); const updatable = fields.filter((f) => UPDATABLE_FIELD_TYPES.has(f.fieldType)); diff --git a/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js b/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js new file mode 100644 index 0000000000..d543600654 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js @@ -0,0 +1,32 @@ +/** + * Find the enclosing `tableOfContents` node for a given document position. + * + * Used by the context menu and the F9 shortcut to detect when the cursor / + * right-click landed inside a TOC, so we can route the action through + * `editor.documentApi.toc.update`. + * + * @param {import('prosemirror-model').Node} doc - The PM document. + * @param {number} pos - A document position. + * @returns {{ node: import('prosemirror-model').Node, pos: number, sdBlockId: string | null } | null} + * The TOC node, its document position, and its `sdBlockId` (when available), + * or `null` when `pos` is not inside a TOC. + */ +export function findTocAncestor(doc, pos) { + if (!doc || typeof pos !== 'number' || !Number.isFinite(pos)) return null; + let resolved; + try { + resolved = doc.resolve(pos); + } catch { + return null; + } + + for (let depth = resolved.depth; depth >= 0; depth -= 1) { + const node = resolved.node(depth); + if (node?.type?.name === 'tableOfContents') { + const sdBlockId = typeof node.attrs?.sdBlockId === 'string' ? node.attrs.sdBlockId : null; + const nodePos = depth === 0 ? 0 : resolved.before(depth); + return { node, pos: nodePos, sdBlockId }; + } + } + return null; +} diff --git a/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.test.js b/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.test.js new file mode 100644 index 0000000000..80c51d03d7 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.test.js @@ -0,0 +1,68 @@ +import { describe, expect, it } from 'vitest'; +import { Schema } from 'prosemirror-model'; + +import { findTocAncestor } from './find-toc-ancestor.js'; + +const schema = new Schema({ + nodes: { + doc: { content: 'block+' }, + paragraph: { + group: 'block', + content: 'inline*', + toDOM: () => ['p', 0], + }, + tableOfContents: { + group: 'block', + content: 'paragraph*', + attrs: { + sdBlockId: { default: null }, + }, + toDOM: () => ['div', 0], + }, + text: { group: 'inline' }, + }, +}); + +const para = (text) => schema.nodes.paragraph.create({}, text ? schema.text(text) : null); +const toc = (sdBlockId, paragraphs) => schema.nodes.tableOfContents.create({ sdBlockId }, paragraphs); + +describe('findTocAncestor', () => { + it('returns null when the position is not inside a TOC', () => { + const doc = schema.nodes.doc.create(null, [para('outside')]); + expect(findTocAncestor(doc, 2)).toBeNull(); + }); + + it('finds the TOC and exposes its sdBlockId for a position inside a TOC paragraph', () => { + const tocNode = toc('toc-1', [para('Heading 1'), para('Heading 2')]); + const doc = schema.nodes.doc.create(null, [para('intro'), tocNode, para('outro')]); + + // First paragraph is 7 chars including boundaries: 0..7. TOC starts at pos 7. + const tocStart = 1 + para('intro').nodeSize; // 1 (doc open) + intro size minus 1 = simpler: locate by walk + const introSize = para('intro').nodeSize; + const insideTocPos = introSize + 2; // inside TOC's first paragraph + + const result = findTocAncestor(doc, insideTocPos); + expect(result).not.toBeNull(); + expect(result.sdBlockId).toBe('toc-1'); + expect(result.node.type.name).toBe('tableOfContents'); + // pos returned should be the TOC node's start position (one before its content range). + // Using the same arithmetic the helper uses: resolved.before(depth). + expect(typeof result.pos).toBe('number'); + expect(tocStart).toBeGreaterThan(0); + }); + + it('returns null sdBlockId when the TOC has none', () => { + const tocNode = toc(null, [para('entry')]); + const doc = schema.nodes.doc.create(null, [tocNode]); + const result = findTocAncestor(doc, 2); + expect(result).not.toBeNull(); + expect(result.sdBlockId).toBeNull(); + }); + + it('returns null for invalid positions', () => { + const doc = schema.nodes.doc.create(null, [para('text')]); + expect(findTocAncestor(doc, -1)).toBeNull(); + expect(findTocAncestor(doc, Number.NaN)).toBeNull(); + expect(findTocAncestor(null, 0)).toBeNull(); + }); +}); diff --git a/packages/super-editor/src/editors/v1/extensions/table-of-contents/toc-page-number.js b/packages/super-editor/src/editors/v1/extensions/table-of-contents/toc-page-number.js index 0c4b9f47df..e68ae5d534 100644 --- a/packages/super-editor/src/editors/v1/extensions/table-of-contents/toc-page-number.js +++ b/packages/super-editor/src/editors/v1/extensions/table-of-contents/toc-page-number.js @@ -23,11 +23,11 @@ export const TocPageNumber = Mark.create({ }; }, - parseHTML() { + parseDOM() { return [{ tag: 'span[data-toc-page-number]' }]; }, - renderHTML() { + renderDOM() { return ['span', { 'data-toc-page-number': '' }, 0]; }, }); From 6a9edf2c3bfda682e592d2447197e695010ce899 Mon Sep 17 00:00:00 2001 From: Gabriel Chittolina Date: Mon, 4 May 2026 13:29:23 -0300 Subject: [PATCH 02/14] feat: update TOC entry in context menu --- .../v1/components/context-menu/menuItems.js | 2 +- .../shared/toc-switches.test.ts | 5 +- .../field-references/shared/toc-switches.ts | 11 ++- .../helpers/toc-entry-builder.test.ts | 80 ++++++++++++----- .../helpers/toc-entry-builder.ts | 69 ++++++++++----- .../plan-engine/toc-wrappers.ts | 54 ++++++++++++ .../extensions/field-update/field-update.js | 87 +++++++++++++------ .../table-of-contents/find-toc-ancestor.js | 2 +- 8 files changed, 233 insertions(+), 77 deletions(-) diff --git a/packages/super-editor/src/editors/v1/components/context-menu/menuItems.js b/packages/super-editor/src/editors/v1/components/context-menu/menuItems.js index 5c762f8a9e..5e8d5689b4 100644 --- a/packages/super-editor/src/editors/v1/components/context-menu/menuItems.js +++ b/packages/super-editor/src/editors/v1/components/context-menu/menuItems.js @@ -348,7 +348,7 @@ export function getItems(context, customItems = [], includeDefaultItems = true) if (!tocId) return; const target = { kind: 'block', nodeType: 'tableOfContents', nodeId: tocId }; try { - editor.documentApi?.toc?.update?.({ target, mode: 'all' }); + editor.doc?.toc?.update?.({ target, mode: 'all' }); } catch (error) { console.warn('[ContextMenu] toc.update failed:', error); } diff --git a/packages/super-editor/src/editors/v1/core/super-converter/field-references/shared/toc-switches.test.ts b/packages/super-editor/src/editors/v1/core/super-converter/field-references/shared/toc-switches.test.ts index 3ee9a5c937..5c70066da8 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/field-references/shared/toc-switches.test.ts +++ b/packages/super-editor/src/editors/v1/core/super-converter/field-references/shared/toc-switches.test.ts @@ -59,8 +59,9 @@ describe('parseTocInstruction', () => { it('handles empty instruction', () => { const config = parseTocInstruction('TOC'); expect(config.source).toEqual({}); - // Convenience projections are derived even for bare TOC instructions - expect(config.display).toEqual({ includePageNumbers: true, tabLeader: 'none' }); + // No \p in the instruction means "use Word's default tab leader" (dots), + // not an explicit opt-out, so tabLeader should be undefined here. + expect(config.display).toEqual({ includePageNumbers: true }); expect(config.preserved).toEqual({}); }); }); diff --git a/packages/super-editor/src/editors/v1/core/super-converter/field-references/shared/toc-switches.ts b/packages/super-editor/src/editors/v1/core/super-converter/field-references/shared/toc-switches.ts index c816b0b377..67bb8ec7ab 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/field-references/shared/toc-switches.ts +++ b/packages/super-editor/src/editors/v1/core/super-converter/field-references/shared/toc-switches.ts @@ -107,10 +107,17 @@ export function deriveIncludePageNumbers( /** * Derives the `tabLeader` value from the raw \p separator string. - * Returns undefined if the separator doesn't match a known leader pattern. + * + * - `undefined` → caller did not pass a separator (no \p switch). Returns + * `undefined` so consumers fall back to Word's default (dots) instead of + * treating "no \p" as an explicit opt-out. + * - `''` → \p was present but empty. Returns `'none'` (explicit opt-out). + * - non-empty string → mapped via SEPARATOR_TO_TAB_LEADER, or `undefined` + * when the separator is not a known leader character. */ function deriveTabLeader(separator: string | undefined): TocDisplayConfig['tabLeader'] | undefined { - if (!separator) return 'none'; + if (separator === undefined) return undefined; + if (separator === '') return 'none'; const leader = SEPARATOR_TO_TAB_LEADER[separator]; return leader as TocDisplayConfig['tabLeader'] | undefined; } diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts index 96ccdf763f..e7dead487f 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts @@ -19,35 +19,47 @@ function makeConfig(display: TocSwitchConfig['display'] = {}): TocSwitchConfig { }; } +type TextLike = { type?: string; text?: string; marks?: Array<{ type: string; attrs?: Record }> }; + +/** Pull the title text node out of a run wrapper. */ +function titleTextOf(paragraphs: ReturnType): TextLike { + const titleRun = paragraphs[0]!.content[0] as { content?: TextLike[] }; + return titleRun.content?.[0] ?? {}; +} + +/** Find the page-number text node (carries the tocPageNumber mark) inside any run. */ +function pageNumberTextOf(paragraphs: ReturnType): TextLike { + const runs = paragraphs[0]!.content as Array<{ content?: TextLike[] }>; + for (const run of runs) { + const child = run.content?.find((c) => Array.isArray(c.marks) && c.marks.some((m) => m.type === 'tocPageNumber')); + if (child) return child; + } + return {}; +} + describe('buildTocEntryParagraphs', () => { describe('hyperlink anchors', () => { it('uses a _Toc bookmark name as the hyperlink anchor, not the raw sdBlockId', () => { const paragraphs = buildTocEntryParagraphs([BASE_SOURCE], makeConfig({ hyperlinks: true })); - const textNode = paragraphs[0]!.content[0] as { marks?: Array<{ type: string; attrs: Record }> }; + const textNode = titleTextOf(paragraphs); const linkMark = textNode.marks?.find((m) => m.type === 'link'); expect(linkMark).toBeDefined(); - expect(linkMark!.attrs.anchor).toMatch(/^_Toc[a-zA-Z0-9_]+$/); - expect(linkMark!.attrs.anchor).toBe(generateTocBookmarkName(BASE_SOURCE.sdBlockId)); - expect(linkMark!.attrs.anchor).not.toBe(BASE_SOURCE.sdBlockId); + expect(linkMark!.attrs!.anchor).toMatch(/^_Toc[a-zA-Z0-9_]+$/); + expect(linkMark!.attrs!.anchor).toBe(generateTocBookmarkName(BASE_SOURCE.sdBlockId)); + expect(linkMark!.attrs!.anchor).not.toBe(BASE_SOURCE.sdBlockId); }); it('produces the same anchor for the same sdBlockId across calls', () => { const first = buildTocEntryParagraphs([BASE_SOURCE], makeConfig({ hyperlinks: true })); const second = buildTocEntryParagraphs([BASE_SOURCE], makeConfig({ hyperlinks: true })); - - const getAnchor = (paragraphs: typeof first) => { - const node = paragraphs[0]!.content[0] as { marks?: Array<{ attrs: Record }> }; - return node.marks?.[0]?.attrs.anchor; - }; - + const getAnchor = (paragraphs: typeof first) => titleTextOf(paragraphs).marks?.[0]?.attrs?.anchor; expect(getAnchor(first)).toBe(getAnchor(second)); }); it('does not add link mark when hyperlinks display option is false', () => { const paragraphs = buildTocEntryParagraphs([BASE_SOURCE], makeConfig({ hyperlinks: false })); - const textNode = paragraphs[0]!.content[0] as { marks?: unknown[] }; - expect(textNode.marks).toBeUndefined(); + expect(titleTextOf(paragraphs).marks).toBeUndefined(); }); }); @@ -106,25 +118,49 @@ describe('buildTocEntryParagraphs', () => { }); }); + describe('entry text marks (SD-2664)', () => { + it('prepends preserved marks and stacks the link mark on top', () => { + const entryTextMarks = [ + { type: 'textStyle', attrs: { fontFamily: 'Aptos', fontSize: '12pt' } }, + { type: 'bold' }, + ]; + const paragraphs = buildTocEntryParagraphs([BASE_SOURCE], makeConfig({ hyperlinks: true }), { entryTextMarks }); + const text = titleTextOf(paragraphs); + expect(text.marks!.map((m) => m.type)).toEqual(['textStyle', 'bold', 'link']); + expect(text.marks![0].attrs).toEqual({ fontFamily: 'Aptos', fontSize: '12pt' }); + }); + + it('drops any incoming link mark; the builder rebuilds it from the source bookmark', () => { + const entryTextMarks = [ + { type: 'textStyle', attrs: { fontFamily: 'Aptos' } }, + { type: 'link', attrs: { anchor: 'old-stale-anchor' } }, + ]; + const paragraphs = buildTocEntryParagraphs([BASE_SOURCE], makeConfig({ hyperlinks: true }), { entryTextMarks }); + const linkMark = titleTextOf(paragraphs).marks?.find((m) => m.type === 'link'); + expect(linkMark?.attrs?.anchor).toBe(generateTocBookmarkName(BASE_SOURCE.sdBlockId)); + expect(linkMark?.attrs?.anchor).not.toBe('old-stale-anchor'); + }); + + it('wraps each text run in a `run` node so wrapTextInRunsPlugin does not clobber marks', () => { + const paragraphs = buildTocEntryParagraphs([BASE_SOURCE], makeConfig({ hyperlinks: true })); + const runs = paragraphs[0]!.content as Array<{ type: string }>; + // Title run + tab run + page-number run = 3 runs (no \p, no omit). + expect(runs.length).toBe(3); + runs.forEach((r) => expect(r.type).toBe('run')); + }); + }); + describe('page numbers (SD-2664)', () => { it('substitutes page numbers from options.pageMap when present', () => { const pageMap = new Map([['h-1', 7]]); const paragraphs = buildTocEntryParagraphs([BASE_SOURCE], makeConfig({ hyperlinks: true }), { pageMap }); - const pageNode = paragraphs[0]!.content.find( - (n: Record) => - Array.isArray(n.marks) && n.marks?.some((m) => (m as { type: string }).type === 'tocPageNumber'), - ) as { text: string }; - expect(pageNode.text).toBe('7'); + expect(pageNumberTextOf(paragraphs).text).toBe('7'); }); it('falls back to "0" placeholder when the source is not in the page map', () => { const pageMap = new Map(); // empty const paragraphs = buildTocEntryParagraphs([BASE_SOURCE], makeConfig({ hyperlinks: true }), { pageMap }); - const pageNode = paragraphs[0]!.content.find( - (n: Record) => - Array.isArray(n.marks) && n.marks?.some((m) => (m as { type: string }).type === 'tocPageNumber'), - ) as { text: string }; - expect(pageNode.text).toBe('0'); + expect(pageNumberTextOf(paragraphs).text).toBe('0'); }); }); }); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts index 8225c5d866..88d90256ec 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts @@ -186,11 +186,23 @@ export interface EntryParagraphJson { * without a follow-up `mode: 'pageNumbers'` pass and without losing layout * particulars from the existing TOC. */ +/** Mark JSON shape carried over from the existing TOC entry's text run. */ +export interface EntryTextMark { + type: string; + attrs?: Record; +} + export interface BuildTocEntryOptions { /** sdBlockId → page number map from PresentationEditor's last layout cycle. */ pageMap?: Map; /** Right-tab stop position (twips) to mirror the existing TOC's spacing. */ tabPos?: number; + /** + * Marks (font, size, textStyle, bold/italic, etc.) sampled from the existing + * TOC entry's text run so a rebuild keeps the same visual styling. The link + * mark is excluded — the builder rebuilds it from the source's bookmark name. + */ + entryTextMarks?: EntryTextMark[]; } export function buildTocEntryParagraphs( @@ -218,28 +230,33 @@ function buildEntryParagraph( options: BuildTocEntryOptions = {}, ): EntryParagraphJson { const { display } = config; - const content: Array> = []; - - // Entry text — optionally wrapped in hyperlink mark - const textNode: Record = { - type: 'text', - text: source.text || ' ', - }; + // Entry text — preserves run formatting (font, size, bold, italic, textStyle…) + // sampled from the existing TOC. Link mark is rebuilt from the source's + // bookmark name and stacked on top of the preserved marks. + // + // We wrap the text in a `run` node because `wrapTextInRunsPlugin` would + // otherwise wrap the bare paragraph-child text on appendTransaction and, for + // the first child of a paragraph, *merge paragraph-style marks via addToSet* + // — which clobbers our sampled `textStyle` (TNR/Hyperlink) with the TOC1 + // paragraph style's `textStyle` (Aptos). Pre-wrapping in a run keeps the + // marks we constructed. + const preservedMarks = (options.entryTextMarks ?? []).filter((mark) => mark?.type && mark.type !== 'link'); + const titleMarks: EntryTextMark[] = [...preservedMarks]; if (display.hyperlinks) { - textNode.marks = [ - { - type: 'link', - attrs: { - anchor: generateTocBookmarkName(source.sdBlockId), - rId: null, - history: true, - }, + titleMarks.push({ + type: 'link', + attrs: { + anchor: generateTocBookmarkName(source.sdBlockId), + rId: null, + history: true, }, - ]; + }); } + const titleText: Record = { type: 'text', text: source.text || ' ' }; + if (titleMarks.length > 0) titleText.marks = titleMarks; - content.push(textNode); + const content: Array> = [{ type: 'run', content: [titleText] }]; // Determine whether to omit page number for this entry const omitRange = display.omitPageNumberLevels; @@ -248,12 +265,13 @@ function buildEntryParagraph( const omitPageNumber = levelOmitted || entryOmitted; if (!omitPageNumber) { - // Separator between entry text and page number (\p switch overrides default tab) + const separatorRunChildren: Array> = []; if (display.separator) { - content.push({ type: 'text', text: display.separator }); + separatorRunChildren.push({ type: 'text', text: display.separator }); } else { - content.push({ type: 'tab' }); + separatorRunChildren.push({ type: 'tab' }); } + content.push({ type: 'run', content: separatorRunChildren }); // Page number — resolved from the page map when available so a single // mode 'all' rebuild produces final numbers; falls back to '0' placeholder @@ -261,9 +279,14 @@ function buildEntryParagraph( // headings whose synthetic id has not been seen by a layout cycle). const resolvedPage = options.pageMap?.get(source.sdBlockId); content.push({ - type: 'text', - text: resolvedPage != null ? String(resolvedPage) : '0', - marks: [{ type: 'tocPageNumber' }], + type: 'run', + content: [ + { + type: 'text', + text: resolvedPage != null ? String(resolvedPage) : '0', + marks: [{ type: 'tocPageNumber' }], + }, + ], }); } diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts index d606d0d437..d88f32ea7b 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts @@ -262,6 +262,8 @@ interface MaterializeTocOptions { pageMap?: Map; /** Right-tab stop position (twips) sampled from the existing TOC. */ tabPos?: number; + /** Run-formatting marks sampled from the existing TOC entry text. */ + entryTextMarks?: Array<{ type: string; attrs?: Record }>; } function materializeTocContent( @@ -296,6 +298,52 @@ function readExistingTocTabPos(node: ProseMirrorNode): number | undefined { return typeof pos === 'number' ? pos : undefined; } +/** Recognises TOC entry paragraph styles (TOC1, TOC2, … TOC9). */ +const TOC_ENTRY_STYLE_RE = /^TOC[1-9]$/; + +/** + * Sample run-formatting marks (font, size, bold, italic, textStyle…) from the + * first non-empty text run inside an actual TOC **entry** paragraph. + * + * The TOC node can include sibling paragraphs that are not entries — most + * commonly the `TOCHeading` paragraph that renders the "Table of Contents" + * label — and those carry different run formatting. Sampling from them would + * propagate the wrong font/size onto every rebuilt entry. We only consider + * paragraphs whose styleId matches `TOC1`–`TOC9`. + * + * Excludes `link` (rebuilt from the source's bookmark) and `tocPageNumber` + * (belongs to the page-number run only). + * + * Returns `[]` when no entry-shaped paragraph with text is found, so the + * builder falls back to schema defaults. + */ +function readExistingTocEntryTextMarks( + node: ProseMirrorNode, +): Array<{ type: string; attrs?: Record }> { + let marks: Array<{ type: string; attrs?: Record }> = []; + node.forEach((paragraph) => { + if (marks.length > 0) return; + if (paragraph.type.name !== 'paragraph') return; + const styleId = (paragraph.attrs as { paragraphProperties?: { styleId?: string } } | undefined)?.paragraphProperties + ?.styleId; + if (!styleId || !TOC_ENTRY_STYLE_RE.test(styleId)) return; + paragraph.descendants((child) => { + if (marks.length > 0) return false; + if (!child.isText || !child.text) return true; + const sampled = (child.marks ?? []) + .filter((m) => m.type.name !== 'link' && m.type.name !== 'tocPageNumber') + .map((m) => { + const json: { type: string; attrs?: Record } = { type: m.type.name }; + if (m.attrs && Object.keys(m.attrs).length > 0) json.attrs = { ...m.attrs }; + return json; + }); + if (sampled.length > 0) marks = sampled; + return false; + }); + }); + return marks; +} + // --------------------------------------------------------------------------- // toc.configure // --------------------------------------------------------------------------- @@ -325,6 +373,11 @@ export function tocConfigureWrapper( editor.state.doc, withRightAlign(patched, effectiveRightAlign), editor, + { + pageMap: getPageMap(editor) ?? undefined, + tabPos: readExistingTocTabPos(resolved.node), + entryTextMarks: readExistingTocEntryTextMarks(resolved.node), + }, ); if (areTocConfigsEqual(currentConfig, patched) && !rightAlignChanged) { @@ -413,6 +466,7 @@ function tocUpdateAll(editor: Editor, input: TocUpdateInput, options?: MutationO const { content, sources } = materializeTocContent(editor.state.doc, withRightAlign(config, rightAlign), editor, { pageMap: getPageMap(editor) ?? undefined, tabPos: readExistingTocTabPos(resolved.node), + entryTextMarks: readExistingTocEntryTextMarks(resolved.node), }); // NO_OP detection: compare new content against existing before executing. diff --git a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js index 48a67eec9a..427f2e99f5 100644 --- a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js +++ b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js @@ -5,7 +5,6 @@ import { resolveDocumentStatFieldValue, resolveMainBodyEditor, } from '../../document-api-adapters/helpers/word-statistics.js'; -import { findTocAncestor } from '../table-of-contents/find-toc-ancestor.js'; /** Field types eligible for value updates via F9. */ const UPDATABLE_FIELD_TYPES = new Set(['NUMWORDS', 'NUMCHARS', 'NUMPAGES']); @@ -21,12 +20,16 @@ export const FieldUpdate = Extension.create({ addCommands() { return { /** - * Update all field values intersecting the current selection. + * Refresh document fields. Two phases run in order: * - * Mirrors Word's F9 semantics: - * - Collapsed selection: updates the single field at the cursor - * - Range selection: updates all fields intersecting the range - * - Select-all then F9: updates every field in the document + * 1. Every `tableOfContents` node in the document is rebuilt via + * `editor.doc.toc.update({ mode: 'all' })`. The wrapper handles + * materialization, page-map resolution, leader/style preservation, + * and bookmark sync. + * 2. Updatable stat fields (NUMWORDS, NUMCHARS, NUMPAGES) intersecting + * the current selection are refreshed in-place. + * + * Bound to F9. Returns `true` if any TOC or stat field changed. * * @category Command * @returns {Function} ProseMirror command function @@ -38,33 +41,65 @@ export const FieldUpdate = Extension.create({ ({ editor, state, dispatch }) => { const { from, to } = state.selection; - // F9 inside a TOC rebuilds the TOC via the document-api wrapper. - // The wrapper handles content materialization, NO_OP detection, and - // bookmark sync. Mode 'all' is used because it does not depend on a - // completed layout cycle (mode 'pageNumbers' can fail with - // CAPABILITY_UNAVAILABLE in headless contexts). - const toc = findTocAncestor(state.doc, from); - if (toc?.sdBlockId && editor?.documentApi?.toc?.update) { - if (!dispatch) return true; - try { - const target = { kind: 'block', nodeType: 'tableOfContents', nodeId: toc.sdBlockId }; - editor.documentApi.toc.update({ target, mode: 'all' }); - } catch (error) { - console.warn('[FieldUpdate] toc.update failed:', error); - return false; + // F9 first refreshes every TOC in the document via the document-api. + // We dispatch through `editor.doc.toc.update` so each rebuild flows + // through the standard wrapper (page-map resolution, leader/style + // preservation, bookmark sync). NO_OP results are ignored. + let tocUpdated = false; + if (editor?.doc?.toc?.update) { + if (!dispatch) { + // can()-style probe: report yes if any TOC exists. + let hasToc = false; + state.doc.descendants((node) => { + if (hasToc) return false; + if (node.type.name === 'tableOfContents') { + hasToc = true; + return false; + } + return true; + }); + if (hasToc) return true; + } else { + const tocTargets = []; + state.doc.descendants((node) => { + if (node.type.name === 'tableOfContents') { + const sdBlockId = node.attrs?.sdBlockId; + if (typeof sdBlockId === 'string' && sdBlockId) { + tocTargets.push(sdBlockId); + } + return false; // don't descend into TOC children + } + return true; + }); + + for (const sdBlockId of tocTargets) { + try { + const result = editor.doc.toc.update({ + target: { kind: 'block', nodeType: 'tableOfContents', nodeId: sdBlockId }, + mode: 'all', + }); + if (result?.success) tocUpdated = true; + } catch (error) { + console.warn('[FieldUpdate] toc.update failed for', sdBlockId, error); + } + } } - return true; } - const fields = findFieldsInRange(state.doc, from, to); + // After TOC updates the doc snapshot may have shifted positions, so + // re-read state from the editor only in that case. When nothing was + // updated above, use the original `state` snapshot to keep the + // pre-existing stat-field path byte-for-byte equivalent. + const currentState = tocUpdated ? (editor?.state ?? state) : state; + const fields = findFieldsInRange(currentState.doc, from, to); const updatable = fields.filter((f) => UPDATABLE_FIELD_TYPES.has(f.fieldType)); - if (updatable.length === 0) return false; + if (updatable.length === 0) return tocUpdated; const mainEditor = resolveMainBodyEditor(editor); const stats = getWordStatistics(mainEditor); - const tr = state.tr; + const tr = currentState.tr; let changed = false; // Process in reverse position order so earlier positions stay valid @@ -82,7 +117,7 @@ export const FieldUpdate = Extension.create({ // total-page-number stores its display value as a text child, // not just an attr. Replace the entire node so both the text // content and resolvedText stay in sync. - const textChild = freshValue ? state.schema.text(freshValue) : null; + const textChild = freshValue ? currentState.schema.text(freshValue) : null; const newNode = node.type.create({ ...node.attrs, resolvedText: freshValue }, textChild); tr.replaceWith(field.pos, field.pos + node.nodeSize, newNode); changed = true; @@ -98,7 +133,7 @@ export const FieldUpdate = Extension.create({ } } - if (!changed) return false; + if (!changed) return tocUpdated; if (dispatch) dispatch(tr); return true; }, diff --git a/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js b/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js index d543600654..94c9bed3fa 100644 --- a/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js +++ b/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js @@ -3,7 +3,7 @@ * * Used by the context menu and the F9 shortcut to detect when the cursor / * right-click landed inside a TOC, so we can route the action through - * `editor.documentApi.toc.update`. + * `editor.doc.toc.update`. * * @param {import('prosemirror-model').Node} doc - The PM document. * @param {number} pos - A document position. From 45e7dcbb107162e5c9d855051e8c71d3ebd590cb Mon Sep 17 00:00:00 2001 From: Gabriel Chittolina Date: Mon, 4 May 2026 14:28:21 -0300 Subject: [PATCH 03/14] fix: update TOC creating extra spaces and changing fonts --- .../v1/components/context-menu/menuItems.js | 14 +- .../helpers/toc-entry-builder.ts | 148 ++++++++---------- .../plan-engine/toc-wrappers.ts | 146 ++++++++++------- .../extensions/field-update/field-update.js | 89 +++++------ .../table-of-contents/find-toc-ancestor.js | 21 +-- 5 files changed, 204 insertions(+), 214 deletions(-) diff --git a/packages/super-editor/src/editors/v1/components/context-menu/menuItems.js b/packages/super-editor/src/editors/v1/components/context-menu/menuItems.js index 5e8d5689b4..5b473eeb5a 100644 --- a/packages/super-editor/src/editors/v1/components/context-menu/menuItems.js +++ b/packages/super-editor/src/editors/v1/components/context-menu/menuItems.js @@ -344,18 +344,18 @@ export function getItems(context, customItems = [], includeDefaultItems = true) icon: ICONS.updateTableOfContents, isDefault: true, action: (editor, context) => { - const tocId = context.tocAncestor?.sdBlockId; - if (!tocId) return; - const target = { kind: 'block', nodeType: 'tableOfContents', nodeId: tocId }; + const sdBlockId = context.tocAncestor?.sdBlockId; + if (!sdBlockId) return; try { - editor.doc?.toc?.update?.({ target, mode: 'all' }); + editor.doc?.toc?.update?.({ + target: { kind: 'block', nodeType: 'tableOfContents', nodeId: sdBlockId }, + mode: 'all', + }); } catch (error) { console.warn('[ContextMenu] toc.update failed:', error); } }, - showWhen: (context) => { - return context.trigger === TRIGGERS.click && !!context.tocAncestor?.sdBlockId; - }, + showWhen: (context) => context.trigger === TRIGGERS.click && !!context.tocAncestor?.sdBlockId, }, ], }, diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts index 88d90256ec..dfb7dabfb5 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts @@ -66,46 +66,35 @@ export function collectTocSources(doc: ProseMirrorNode, config: TocSwitchConfig) const attrs = node.attrs as Record | undefined; const paragraphProps = attrs?.paragraphProperties as Record | undefined; const styleId = paragraphProps?.styleId as string | undefined; - // Pasted/new paragraphs intentionally have null paraId/sdBlockId (see - // InputRule.js SUPERDOC_SLICE_PASTE_IDENTITY_RESETS) to avoid public-id - // duplicates. Synthesize a deterministic position-based id so they still - // appear in the rebuilt TOC and round-trip via OOXML bookmarks. + // Pasted/new paragraphs intentionally lose paraId/sdBlockId (see + // InputRule.js SUPERDOC_SLICE_PASTE_IDENTITY_RESETS). Synthesize a + // position-based id so they still appear in the rebuilt TOC. const sdBlockId = ((attrs?.sdBlockId ?? attrs?.paraId) as string | undefined) ?? buildFallbackBlockNodeId('paragraph', pos); - - // Update paragraph context for TC field collection currentParagraphSdBlockId = sdBlockId; - if (!sdBlockId) return true; const text = flattenText(node); - // Word's TOC field skips paragraphs that are styled as headings but - // contain no visible text (page-break-only spacers, empty stubs). - // Including them produces ghost entries that look like a regression. - const hasVisibleText = text.trim().length > 0; + // Word's TOC skips heading-styled paragraphs with no visible text + // (page-break spacers, empty stubs). + if (text.trim().length === 0) return true; - // Check heading by style (\o switch) + // \o switch — heading-style level if (outlineLevels) { const headingLevel = getHeadingLevel(styleId); - if ( - headingLevel != null && - headingLevel >= outlineLevels.from && - headingLevel <= outlineLevels.to && - hasVisibleText - ) { + if (headingLevel != null && headingLevel >= outlineLevels.from && headingLevel <= outlineLevels.to) { sources.push({ text, level: headingLevel, sdBlockId, kind: 'heading' }); - // Continue descending to find TC fields within this paragraph - return true; + return true; // descend so TC fields inside this paragraph are still collected } } - // Check applied outline level (\u switch) + // \u switch — applied paragraph outline level if (useApplied) { const effectiveLevels = outlineLevels ?? { from: 1, to: 9 }; const rawOutlineLevel = paragraphProps?.outlineLevel as number | undefined; if (rawOutlineLevel != null) { const tocLevel = rawOutlineLevel + 1; - if (tocLevel >= effectiveLevels.from && tocLevel <= effectiveLevels.to && hasVisibleText) { + if (tocLevel >= effectiveLevels.from && tocLevel <= effectiveLevels.to) { sources.push({ text, level: tocLevel, sdBlockId, kind: 'appliedOutline' }); return true; } @@ -171,40 +160,38 @@ export interface EntryParagraphJson { content: Array>; } -/** - * Builds ProseMirror-compatible paragraph JSON nodes for TOC entries. - * - * Each entry gets: - * - Paragraph style: TOC{level} - * - tocSourceId paragraph attribute (source heading/TC field's sdBlockId) - * - Link mark with anchor pointing to a `_Toc`-prefixed bookmark name (when \h is set) - * - Page number placeholder "0" with tocPageNumber mark - * - Separator: custom (\p switch) or default tab - */ -/** - * Optional context that lets the entry builder produce final-looking output - * without a follow-up `mode: 'pageNumbers'` pass and without losing layout - * particulars from the existing TOC. - */ -/** Mark JSON shape carried over from the existing TOC entry's text run. */ +/** A mark in JSON form, as carried on the rebuilt TOC entry's text runs. */ export interface EntryTextMark { type: string; attrs?: Record; } +/** + * Optional context that lets the entry builder produce final-looking output + * (resolved page numbers, preserved tab spacing, sampled font/size marks) + * without a follow-up `mode: 'pageNumbers'` pass. + */ export interface BuildTocEntryOptions { /** sdBlockId → page number map from PresentationEditor's last layout cycle. */ pageMap?: Map; /** Right-tab stop position (twips) to mirror the existing TOC's spacing. */ tabPos?: number; + /** Marks sampled from the existing TOC entry text. `link` is filtered out and rebuilt. */ + entryTextMarks?: EntryTextMark[]; /** - * Marks (font, size, textStyle, bold/italic, etc.) sampled from the existing - * TOC entry's text run so a rebuild keeps the same visual styling. The link - * mark is excluded — the builder rebuilds it from the source's bookmark name. + * Paragraph-level `` overrides sampled from the existing entry. Word + * stamps these on TOC entries (e.g. `bold: false`, `italic: false`) to + * disable the TOC1 paragraph style's ``. Preserving them keeps + * the rebuilt entries visually identical to the imported ones. */ - entryTextMarks?: EntryTextMark[]; + paragraphRunProperties?: Record; } +/** + * Build TOC entry paragraphs. Each paragraph carries `pStyle="TOC{level}"`, + * a `tocSourceId` attr pointing back to the source heading, and three runs: + * the (linked) entry title, the tab/separator, and the page number. + */ export function buildTocEntryParagraphs( sources: TocSource[], config: TocSwitchConfig, @@ -224,6 +211,11 @@ const TAB_LEADER_MAP: Record = { middleDot: 'middleDot', }; +/** Wrap inline children in a `run` node — the schema unit that `wrapTextInRunsPlugin` skips. */ +function asRun(children: Array>): Record { + return { type: 'run', content: children }; +} + function buildEntryParagraph( source: TocSource, config: TocSwitchConfig, @@ -231,75 +223,59 @@ function buildEntryParagraph( ): EntryParagraphJson { const { display } = config; - // Entry text — preserves run formatting (font, size, bold, italic, textStyle…) - // sampled from the existing TOC. Link mark is rebuilt from the source's - // bookmark name and stacked on top of the preserved marks. - // - // We wrap the text in a `run` node because `wrapTextInRunsPlugin` would - // otherwise wrap the bare paragraph-child text on appendTransaction and, for - // the first child of a paragraph, *merge paragraph-style marks via addToSet* - // — which clobbers our sampled `textStyle` (TNR/Hyperlink) with the TOC1 - // paragraph style's `textStyle` (Aptos). Pre-wrapping in a run keeps the - // marks we constructed. - const preservedMarks = (options.entryTextMarks ?? []).filter((mark) => mark?.type && mark.type !== 'link'); - const titleMarks: EntryTextMark[] = [...preservedMarks]; + // Title text. Marks are stacked: sampled (font/size/textStyle/bold/italic) + // first, link last. Wrapped in a `run` so `wrapTextInRunsPlugin` does not + // re-wrap and merge the TOC1 paragraph style's run properties via addToSet, + // which would clobber the sampled `textStyle` mark. + const titleMarks: EntryTextMark[] = (options.entryTextMarks ?? []).filter( + (mark) => mark?.type && mark.type !== 'link', + ); if (display.hyperlinks) { titleMarks.push({ type: 'link', - attrs: { - anchor: generateTocBookmarkName(source.sdBlockId), - rId: null, - history: true, - }, + attrs: { anchor: generateTocBookmarkName(source.sdBlockId), rId: null, history: true }, }); } const titleText: Record = { type: 'text', text: source.text || ' ' }; if (titleMarks.length > 0) titleText.marks = titleMarks; - const content: Array> = [{ type: 'run', content: [titleText] }]; + const content: Array> = [asRun([titleText])]; - // Determine whether to omit page number for this entry + // Determine whether to omit page number for this entry. const omitRange = display.omitPageNumberLevels; - const levelOmitted = omitRange && source.level >= omitRange.from && source.level <= omitRange.to; - const entryOmitted = source.omitPageNumber; - const omitPageNumber = levelOmitted || entryOmitted; + const omitPageNumber = Boolean( + (omitRange && source.level >= omitRange.from && source.level <= omitRange.to) || source.omitPageNumber, + ); if (!omitPageNumber) { - const separatorRunChildren: Array> = []; - if (display.separator) { - separatorRunChildren.push({ type: 'text', text: display.separator }); - } else { - separatorRunChildren.push({ type: 'tab' }); - } - content.push({ type: 'run', content: separatorRunChildren }); + // Separator: custom \p text or default tab. + content.push(asRun([display.separator ? { type: 'text', text: display.separator } : { type: 'tab' }])); - // Page number — resolved from the page map when available so a single - // mode 'all' rebuild produces final numbers; falls back to '0' placeholder - // when the source paragraph is not yet in the page map (freshly pasted - // headings whose synthetic id has not been seen by a layout cycle). + // Page number — resolved from the page map when available; '0' placeholder + // otherwise (e.g. freshly-pasted heading whose synthetic id hasn't been + // seen by a layout cycle yet). const resolvedPage = options.pageMap?.get(source.sdBlockId); - content.push({ - type: 'run', - content: [ + content.push( + asRun([ { type: 'text', text: resolvedPage != null ? String(resolvedPage) : '0', marks: [{ type: 'tocPageNumber' }], }, - ], - }); + ]), + ); } - // Build paragraph properties — add right-aligned tab stop when enabled - const paragraphProperties: Record = { - styleId: `TOC${source.level}`, - }; + const paragraphProperties: Record = { styleId: `TOC${source.level}` }; + if (options.paragraphRunProperties && Object.keys(options.paragraphRunProperties).length > 0) { + paragraphProperties.runProperties = { ...options.paragraphRunProperties }; + } const rightAlign = display.rightAlignPageNumbers !== false; // default true if (rightAlign && !omitPageNumber) { // Word's default TOC tab leader is dots. The \p switch is only emitted - // when a non-default separator is used, so an absent tabLeader means the - // user expects dots, not "no leader". Honor an explicit 'none' to opt out. + // for a non-default separator, so an absent `tabLeader` means "use the + // default", not "no leader". `'none'` is the explicit opt-out. const leader = display.tabLeader === 'none' ? undefined : (display.tabLeader && TAB_LEADER_MAP[display.tabLeader]) || 'dot'; const pos = options.tabPos ?? DEFAULT_RIGHT_TAB_POS; diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts index d88f32ea7b..4530473a1a 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts @@ -40,6 +40,7 @@ import { import { collectTocSources, buildTocEntryParagraphs, + type BuildTocEntryOptions, type EntryParagraphJson, type TocSource, } from '../helpers/toc-entry-builder.js'; @@ -257,14 +258,7 @@ interface MaterializedToc { sources: TocSource[]; } -interface MaterializeTocOptions { - /** sdBlockId → page number map to fold into the rebuilt entries. */ - pageMap?: Map; - /** Right-tab stop position (twips) sampled from the existing TOC. */ - tabPos?: number; - /** Run-formatting marks sampled from the existing TOC entry text. */ - entryTextMarks?: Array<{ type: string; attrs?: Record }>; -} +type MaterializeTocOptions = BuildTocEntryOptions; function materializeTocContent( doc: ProseMirrorNode, @@ -278,62 +272,90 @@ function materializeTocContent( return { content: sanitizeTocContentForSchema(content, editor), sources }; } -/** - * Sample the right-tab stop position from the existing TOC's first child - * paragraph so the rebuilt entries keep the same column for page numbers. - * Returns `undefined` when no usable tab stop is present, in which case the - * builder falls back to the default position. - */ +/** Recognises TOC entry paragraph styles (TOC1, TOC2, … TOC9). */ +const TOC_ENTRY_STYLE_RE = /^TOC[1-9]$/; + +type TocParagraphProps = { + styleId?: string; + tabStops?: TabStopJson[]; + runProperties?: Record; +}; +type TocParagraphAttrs = { paragraphProperties?: TocParagraphProps }; +type TabStopJson = { tab?: { pos?: number; tabType?: string; leader?: string } }; +type EntryMarkJson = { type: string; attrs?: Record }; + +/** First TOC1–TOC9 paragraph in the existing TOC node, or `undefined`. */ +function findFirstTocEntryParagraph(node: ProseMirrorNode): ProseMirrorNode | undefined { + let entry: ProseMirrorNode | undefined; + node.forEach((paragraph) => { + if (entry || paragraph.type.name !== 'paragraph') return; + const styleId = (paragraph.attrs as TocParagraphAttrs | undefined)?.paragraphProperties?.styleId; + if (styleId && TOC_ENTRY_STYLE_RE.test(styleId)) entry = paragraph; + }); + return entry; +} + +/** Right-tab stop position (twips) from the first existing TOC entry. */ function readExistingTocTabPos(node: ProseMirrorNode): number | undefined { - const first = node.firstChild; - if (!first) return undefined; - const tabStops = ( - first.attrs as - | { paragraphProperties?: { tabStops?: Array<{ tab?: { pos?: number; tabType?: string } }> } } - | undefined - )?.paragraphProperties?.tabStops; - if (!Array.isArray(tabStops)) return undefined; - const right = tabStops.find((t) => t?.tab?.tabType === 'right'); - const pos = right?.tab?.pos; + const entry = findFirstTocEntryParagraph(node) ?? node.firstChild ?? undefined; + const tabStops = (entry?.attrs as TocParagraphAttrs | undefined)?.paragraphProperties?.tabStops; + const pos = tabStops?.find((t) => t?.tab?.tabType === 'right')?.tab?.pos; return typeof pos === 'number' ? pos : undefined; } -/** Recognises TOC entry paragraph styles (TOC1, TOC2, … TOC9). */ -const TOC_ENTRY_STYLE_RE = /^TOC[1-9]$/; +/** + * Run-property overrides (`` inside ``) sampled from the first + * existing TOC entry. These are paragraph-level overrides that disable + * style-level formatting on rebuild — without preserving them, properties + * like `bold: false` / `italic: false` baked into Word's TOC1 entries would + * fall back to the TOC1 paragraph style's own bold/italic. + */ +function readExistingTocEntryRunProperties(node: ProseMirrorNode): Record | undefined { + const entry = findFirstTocEntryParagraph(node); + const runProps = (entry?.attrs as TocParagraphAttrs | undefined)?.paragraphProperties?.runProperties; + return runProps && Object.keys(runProps).length > 0 ? { ...runProps } : undefined; +} /** - * Sample run-formatting marks (font, size, bold, italic, textStyle…) from the - * first non-empty text run inside an actual TOC **entry** paragraph. - * - * The TOC node can include sibling paragraphs that are not entries — most - * commonly the `TOCHeading` paragraph that renders the "Table of Contents" - * label — and those carry different run formatting. Sampling from them would - * propagate the wrong font/size onto every rebuilt entry. We only consider - * paragraphs whose styleId matches `TOC1`–`TOC9`. - * - * Excludes `link` (rebuilt from the source's bookmark) and `tocPageNumber` - * (belongs to the page-number run only). - * - * Returns `[]` when no entry-shaped paragraph with text is found, so the - * builder falls back to schema defaults. + * Word's TOC field always closes with a paragraph that holds the + * `` — typically a Normal-styled empty + * paragraph after the entries. SuperDoc's importer preserves it as the last + * child of the `tableOfContents` node, and it renders as a blank line below + * the entries. If we replace **all** children with just the rebuilt entries, + * the TOC visually shrinks by that blank line and the gap to the text below + * shifts. Capture the original trailing non-entry paragraph (when present) + * as JSON so we can append it after the rebuilt entries to keep the visual + * end of the TOC stable. + */ +function readExistingTocTrailingParagraph(node: ProseMirrorNode): unknown | undefined { + const last = node.lastChild; + if (!last || last.type.name !== 'paragraph') return undefined; + const styleId = (last.attrs as TocParagraphAttrs | undefined)?.paragraphProperties?.styleId; + if (styleId && TOC_ENTRY_STYLE_RE.test(styleId)) return undefined; // it's an entry, not the trailer + return typeof last.toJSON === 'function' ? last.toJSON() : undefined; +} + +/** + * Run-formatting marks (font, size, bold, italic, textStyle…) sampled from the + * first non-empty text run inside an actual TOC entry paragraph (styleId TOC1–TOC9). + * Skips sibling paragraphs like TOCHeading whose run formatting must not bleed + * into the rebuilt entries. Drops `link` (rebuilt from the source's bookmark) + * and `tocPageNumber` (belongs only to the page-number run). */ -function readExistingTocEntryTextMarks( - node: ProseMirrorNode, -): Array<{ type: string; attrs?: Record }> { - let marks: Array<{ type: string; attrs?: Record }> = []; +function readExistingTocEntryTextMarks(node: ProseMirrorNode): EntryMarkJson[] { + let marks: EntryMarkJson[] = []; node.forEach((paragraph) => { - if (marks.length > 0) return; - if (paragraph.type.name !== 'paragraph') return; - const styleId = (paragraph.attrs as { paragraphProperties?: { styleId?: string } } | undefined)?.paragraphProperties - ?.styleId; + if (marks.length > 0 || paragraph.type.name !== 'paragraph') return; + const styleId = (paragraph.attrs as TocParagraphAttrs | undefined)?.paragraphProperties?.styleId; if (!styleId || !TOC_ENTRY_STYLE_RE.test(styleId)) return; + paragraph.descendants((child) => { if (marks.length > 0) return false; if (!child.isText || !child.text) return true; const sampled = (child.marks ?? []) .filter((m) => m.type.name !== 'link' && m.type.name !== 'tocPageNumber') .map((m) => { - const json: { type: string; attrs?: Record } = { type: m.type.name }; + const json: EntryMarkJson = { type: m.type.name }; if (m.attrs && Object.keys(m.attrs).length > 0) json.attrs = { ...m.attrs }; return json; }); @@ -369,7 +391,7 @@ export function tocConfigureWrapper( // Patch value takes priority; fall back to existing node attr. const effectiveRightAlign = input.patch.rightAlignPageNumbers ?? (resolved.node.attrs?.rightAlignPageNumbers as boolean | undefined); - const { content: nextContent, sources } = materializeTocContent( + const { content: rebuiltEntries, sources } = materializeTocContent( editor.state.doc, withRightAlign(patched, effectiveRightAlign), editor, @@ -377,8 +399,11 @@ export function tocConfigureWrapper( pageMap: getPageMap(editor) ?? undefined, tabPos: readExistingTocTabPos(resolved.node), entryTextMarks: readExistingTocEntryTextMarks(resolved.node), + paragraphRunProperties: readExistingTocEntryRunProperties(resolved.node), }, ); + const trailing = readExistingTocTrailingParagraph(resolved.node); + const nextContent = trailing ? [...rebuiltEntries, trailing as EntryParagraphJson] : rebuiltEntries; if (areTocConfigsEqual(currentConfig, patched) && !rightAlignChanged) { return tocFailure('NO_OP', 'Configuration patch produced no change.'); @@ -463,11 +488,22 @@ function tocUpdateAll(editor: Editor, input: TocUpdateInput, options?: MutationO const resolved = resolveTocTarget(editor.state.doc, input.target); const config = parseTocInstruction(resolved.node.attrs?.instruction ?? ''); const rightAlign = resolved.node.attrs?.rightAlignPageNumbers as boolean | undefined; - const { content, sources } = materializeTocContent(editor.state.doc, withRightAlign(config, rightAlign), editor, { - pageMap: getPageMap(editor) ?? undefined, - tabPos: readExistingTocTabPos(resolved.node), - entryTextMarks: readExistingTocEntryTextMarks(resolved.node), - }); + const { content: rebuiltEntries, sources } = materializeTocContent( + editor.state.doc, + withRightAlign(config, rightAlign), + editor, + { + pageMap: getPageMap(editor) ?? undefined, + tabPos: readExistingTocTabPos(resolved.node), + entryTextMarks: readExistingTocEntryTextMarks(resolved.node), + paragraphRunProperties: readExistingTocEntryRunProperties(resolved.node), + }, + ); + + // Preserve the trailer paragraph if the existing TOC ends with one — keeps + // the visual gap below the TOC stable across rebuilds. + const trailing = readExistingTocTrailingParagraph(resolved.node); + const content = trailing ? [...rebuiltEntries, trailing as EntryParagraphJson] : rebuiltEntries; // NO_OP detection: compare new content against existing before executing. // The PM command returns "found" (not "content changed"), so receipt-based diff --git a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js index 427f2e99f5..fd510c3ce4 100644 --- a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js +++ b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js @@ -9,6 +9,22 @@ import { /** Field types eligible for value updates via F9. */ const UPDATABLE_FIELD_TYPES = new Set(['NUMWORDS', 'NUMCHARS', 'NUMPAGES']); +/** + * Collect every `tableOfContents` node's sdBlockId, in document order. + * @param {import('prosemirror-model').Node} doc + * @returns {string[]} + */ +function collectTocBlockIds(doc) { + const ids = []; + doc.descendants((node) => { + if (node.type.name !== 'tableOfContents') return true; + const sdBlockId = node.attrs?.sdBlockId; + if (typeof sdBlockId === 'string' && sdBlockId) ids.push(sdBlockId); + return false; // don't descend into TOC children + }); + return ids; +} + /** * @module FieldUpdate * @sidebarTitle Field Update @@ -20,16 +36,14 @@ export const FieldUpdate = Extension.create({ addCommands() { return { /** - * Refresh document fields. Two phases run in order: + * Refresh document fields. * - * 1. Every `tableOfContents` node in the document is rebuilt via - * `editor.doc.toc.update({ mode: 'all' })`. The wrapper handles - * materialization, page-map resolution, leader/style preservation, - * and bookmark sync. - * 2. Updatable stat fields (NUMWORDS, NUMCHARS, NUMPAGES) intersecting - * the current selection are refreshed in-place. + * - When the doc contains any TOCs, rebuilds **all** of them via + * `editor.doc.toc.update({ mode: 'all' })` and stops. + * - Otherwise, refreshes stat fields (NUMWORDS, NUMCHARS, NUMPAGES) that + * intersect the current selection. * - * Bound to F9. Returns `true` if any TOC or stat field changed. + * Bound to F9. Returns `true` if anything was updated. * * @category Command * @returns {Function} ProseMirror command function @@ -38,68 +52,39 @@ export const FieldUpdate = Extension.create({ */ updateFieldsInSelection: () => - ({ editor, state, dispatch }) => { + ({ editor, state, tr: outerTr, dispatch }) => { const { from, to } = state.selection; - // F9 first refreshes every TOC in the document via the document-api. - // We dispatch through `editor.doc.toc.update` so each rebuild flows - // through the standard wrapper (page-map resolution, leader/style - // preservation, bookmark sync). NO_OP results are ignored. - let tocUpdated = false; + // toc.update dispatches its own transaction per TOC; CommandService + // would then auto-apply its captured (now-stale) `tr` to the new + // state. Set preventDispatch so it skips that. if (editor?.doc?.toc?.update) { - if (!dispatch) { - // can()-style probe: report yes if any TOC exists. - let hasToc = false; - state.doc.descendants((node) => { - if (hasToc) return false; - if (node.type.name === 'tableOfContents') { - hasToc = true; - return false; - } - return true; - }); - if (hasToc) return true; - } else { - const tocTargets = []; - state.doc.descendants((node) => { - if (node.type.name === 'tableOfContents') { - const sdBlockId = node.attrs?.sdBlockId; - if (typeof sdBlockId === 'string' && sdBlockId) { - tocTargets.push(sdBlockId); - } - return false; // don't descend into TOC children - } - return true; - }); - + const tocTargets = collectTocBlockIds(state.doc); + if (tocTargets.length > 0) { + if (!dispatch) return true; // can()-style probe for (const sdBlockId of tocTargets) { try { - const result = editor.doc.toc.update({ + editor.doc.toc.update({ target: { kind: 'block', nodeType: 'tableOfContents', nodeId: sdBlockId }, mode: 'all', }); - if (result?.success) tocUpdated = true; } catch (error) { console.warn('[FieldUpdate] toc.update failed for', sdBlockId, error); } } + outerTr?.setMeta?.('preventDispatch', true); + return true; } } - // After TOC updates the doc snapshot may have shifted positions, so - // re-read state from the editor only in that case. When nothing was - // updated above, use the original `state` snapshot to keep the - // pre-existing stat-field path byte-for-byte equivalent. - const currentState = tocUpdated ? (editor?.state ?? state) : state; - const fields = findFieldsInRange(currentState.doc, from, to); - + const fields = findFieldsInRange(state.doc, from, to); const updatable = fields.filter((f) => UPDATABLE_FIELD_TYPES.has(f.fieldType)); - if (updatable.length === 0) return tocUpdated; + if (updatable.length === 0) return false; const mainEditor = resolveMainBodyEditor(editor); const stats = getWordStatistics(mainEditor); - const tr = currentState.tr; + const tr = state.tr; let changed = false; // Process in reverse position order so earlier positions stay valid @@ -117,7 +102,7 @@ export const FieldUpdate = Extension.create({ // total-page-number stores its display value as a text child, // not just an attr. Replace the entire node so both the text // content and resolvedText stay in sync. - const textChild = freshValue ? currentState.schema.text(freshValue) : null; + const textChild = freshValue ? state.schema.text(freshValue) : null; const newNode = node.type.create({ ...node.attrs, resolvedText: freshValue }, textChild); tr.replaceWith(field.pos, field.pos + node.nodeSize, newNode); changed = true; @@ -133,7 +118,7 @@ export const FieldUpdate = Extension.create({ } } - if (!changed) return tocUpdated; + if (!changed) return false; if (dispatch) dispatch(tr); return true; }, diff --git a/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js b/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js index 94c9bed3fa..6db0d24986 100644 --- a/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js +++ b/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js @@ -1,15 +1,11 @@ /** - * Find the enclosing `tableOfContents` node for a given document position. - * - * Used by the context menu and the F9 shortcut to detect when the cursor / - * right-click landed inside a TOC, so we can route the action through + * Find the enclosing `tableOfContents` node for a document position. Used by + * the context menu to route "Update table of contents" through * `editor.doc.toc.update`. * - * @param {import('prosemirror-model').Node} doc - The PM document. - * @param {number} pos - A document position. + * @param {import('prosemirror-model').Node} doc + * @param {number} pos * @returns {{ node: import('prosemirror-model').Node, pos: number, sdBlockId: string | null } | null} - * The TOC node, its document position, and its `sdBlockId` (when available), - * or `null` when `pos` is not inside a TOC. */ export function findTocAncestor(doc, pos) { if (!doc || typeof pos !== 'number' || !Number.isFinite(pos)) return null; @@ -19,14 +15,11 @@ export function findTocAncestor(doc, pos) { } catch { return null; } - for (let depth = resolved.depth; depth >= 0; depth -= 1) { const node = resolved.node(depth); - if (node?.type?.name === 'tableOfContents') { - const sdBlockId = typeof node.attrs?.sdBlockId === 'string' ? node.attrs.sdBlockId : null; - const nodePos = depth === 0 ? 0 : resolved.before(depth); - return { node, pos: nodePos, sdBlockId }; - } + if (node?.type?.name !== 'tableOfContents') continue; + const sdBlockId = typeof node.attrs?.sdBlockId === 'string' ? node.attrs.sdBlockId : null; + return { node, pos: depth === 0 ? 0 : resolved.before(depth), sdBlockId }; } return null; } From ac00dd39a0e04898b8a2bdea26e6080822567ff6 Mon Sep 17 00:00:00 2001 From: Gabriel Chittolina Date: Mon, 4 May 2026 14:57:55 -0300 Subject: [PATCH 04/14] test: created tests around TOC programmatic update --- .../context-menu/tests/menuItems.test.js | 108 ++++++++++++++ .../extensions/field-update/field-update.js | 8 +- .../field-update/field-update.test.js | 140 +++++++++++++++++- .../tests/navigation/toc-update.spec.ts | 87 +++++++++++ 4 files changed, 335 insertions(+), 8 deletions(-) create mode 100644 tests/behavior/tests/navigation/toc-update.spec.ts diff --git a/packages/super-editor/src/editors/v1/components/context-menu/tests/menuItems.test.js b/packages/super-editor/src/editors/v1/components/context-menu/tests/menuItems.test.js index a5e5e4e9c8..58f3c530af 100644 --- a/packages/super-editor/src/editors/v1/components/context-menu/tests/menuItems.test.js +++ b/packages/super-editor/src/editors/v1/components/context-menu/tests/menuItems.test.js @@ -31,6 +31,7 @@ vi.mock('../constants.js', () => ({ trackChangesAccept: 'Accept Tracked Changes', trackChangesReject: 'Reject Tracked Changes', cellBackground: 'Cell background', + updateTableOfContents: 'Update table of contents', }, ICONS: { ai: 'ai-icon', @@ -42,6 +43,7 @@ vi.mock('../constants.js', () => ({ copy: 'copy-icon', paste: 'paste-icon', cellBackground: 'cell-background-icon', + updateTableOfContents: 'rotate-right-icon', }, TRIGGERS: { slash: 'slash', @@ -1059,4 +1061,110 @@ describe('menuItems.js', () => { expect(callOrder).toEqual(['setSelection', 'handleClipboardPaste']); }); }); + + // --------------------------------------------------------------------------- + // SD-2664 — "Update table of contents" item + // --------------------------------------------------------------------------- + + describe('update-table-of-contents item', () => { + const findItem = (sections) => { + for (const section of sections) { + const item = section.items.find((it) => it.id === 'update-table-of-contents'); + if (item) return item; + } + return undefined; + }; + + it('appears when right-clicking inside a TOC (tocAncestor.sdBlockId set, click trigger)', () => { + mockContext = createMockContext({ + editor: mockEditor, + trigger: TRIGGERS.click, + tocAncestor: { node: {}, pos: 5, sdBlockId: 'toc-1' }, + }); + const sections = getItems(mockContext); + expect(findItem(sections)).toBeDefined(); + }); + + it('is hidden when no tocAncestor is present', () => { + mockContext = createMockContext({ + editor: mockEditor, + trigger: TRIGGERS.click, + tocAncestor: null, + }); + const sections = getItems(mockContext); + expect(findItem(sections)).toBeUndefined(); + }); + + it('is hidden when tocAncestor exists but sdBlockId is missing', () => { + mockContext = createMockContext({ + editor: mockEditor, + trigger: TRIGGERS.click, + tocAncestor: { node: {}, pos: 5, sdBlockId: null }, + }); + const sections = getItems(mockContext); + expect(findItem(sections)).toBeUndefined(); + }); + + it('is hidden on the slash trigger even when inside a TOC', () => { + mockContext = createMockContext({ + editor: mockEditor, + trigger: TRIGGERS.slash, + tocAncestor: { node: {}, pos: 5, sdBlockId: 'toc-1' }, + }); + const sections = getItems(mockContext); + expect(findItem(sections)).toBeUndefined(); + }); + + it('action invokes editor.doc.toc.update with the resolved sdBlockId and mode "all"', () => { + const update = vi.fn(); + const ed = { ...mockEditor, doc: { toc: { update } } }; + mockContext = createMockContext({ + editor: ed, + trigger: TRIGGERS.click, + tocAncestor: { node: {}, pos: 5, sdBlockId: 'toc-42' }, + }); + const sections = getItems(mockContext); + const item = findItem(sections); + expect(item).toBeDefined(); + item.action(ed, mockContext); + expect(update).toHaveBeenCalledWith({ + target: { kind: 'block', nodeType: 'tableOfContents', nodeId: 'toc-42' }, + mode: 'all', + }); + }); + + it('action is a no-op when sdBlockId is missing', () => { + const update = vi.fn(); + const ed = { ...mockEditor, doc: { toc: { update } } }; + // showWhen would normally hide the item — invoke action directly to assert the guard. + mockContext = createMockContext({ + editor: ed, + trigger: TRIGGERS.click, + tocAncestor: { node: {}, pos: 5, sdBlockId: 'toc-1' }, + }); + const sections = getItems(mockContext); + const item = findItem(sections); + // Re-invoke action with a context that has no sdBlockId. + item.action(ed, { ...mockContext, tocAncestor: { node: {}, pos: 5, sdBlockId: null } }); + expect(update).not.toHaveBeenCalled(); + }); + + it('action swallows errors thrown by editor.doc.toc.update', () => { + const update = vi.fn(() => { + throw new Error('boom'); + }); + const ed = { ...mockEditor, doc: { toc: { update } } }; + mockContext = createMockContext({ + editor: ed, + trigger: TRIGGERS.click, + tocAncestor: { node: {}, pos: 5, sdBlockId: 'toc-1' }, + }); + const sections = getItems(mockContext); + const item = findItem(sections); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + expect(() => item.action(ed, mockContext)).not.toThrow(); + expect(warn).toHaveBeenCalled(); + warn.mockRestore(); + }); + }); }); diff --git a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js index fd510c3ce4..cd98d9762c 100644 --- a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js +++ b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js @@ -6,14 +6,10 @@ import { resolveMainBodyEditor, } from '../../document-api-adapters/helpers/word-statistics.js'; -/** Field types eligible for value updates via F9. */ +/** Stat-field types refreshed by F9 when the doc has no TOCs. */ const UPDATABLE_FIELD_TYPES = new Set(['NUMWORDS', 'NUMCHARS', 'NUMPAGES']); -/** - * Collect every `tableOfContents` node's sdBlockId, in document order. - * @param {import('prosemirror-model').Node} doc - * @returns {string[]} - */ +/** Every `tableOfContents` node's sdBlockId in document order. */ function collectTocBlockIds(doc) { const ids = []; doc.descendants((node) => { diff --git a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.test.js b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.test.js index c621d7b2cd..972cbbe0cb 100644 --- a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.test.js +++ b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.test.js @@ -4,12 +4,16 @@ * Tests for the FieldUpdate extension's updateFieldsInSelection command. * * Uses the numwords.docx fixture which contains NUMWORDS, NUMCHARS, and - * NUMPAGES fields with known imported values. + * NUMPAGES fields with known imported values for the stat-field path. The + * TOC path is exercised via direct command-function invocation against a + * synthetic doc/editor — no docx fixture required. */ -import { afterEach, beforeAll, describe, expect, it } from 'vitest'; +import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; +import { Schema } from 'prosemirror-model'; import { initTestEditor, loadTestDataForEditorTests } from '@tests/helpers/helpers.js'; import { getWordStatistics } from '../../document-api-adapters/helpers/word-statistics.js'; +import { FieldUpdate } from './field-update.js'; describe('FieldUpdate extension', () => { let docData; @@ -107,3 +111,135 @@ describe('FieldUpdate extension', () => { expect(numcharsField.attrs.resolvedText).toBe(expectedValue); }); }); + +// --------------------------------------------------------------------------- +// TOC path — invoked directly against synthetic state to avoid needing a +// fully-imported TOC fixture. +// --------------------------------------------------------------------------- + +const tocSchema = new Schema({ + nodes: { + doc: { content: 'block+' }, + paragraph: { group: 'block', content: 'inline*', toDOM: () => ['p', 0] }, + tableOfContents: { + group: 'block', + content: 'paragraph*', + attrs: { sdBlockId: { default: null } }, + toDOM: () => ['div', 0], + }, + text: { group: 'inline' }, + }, +}); + +const buildTocDoc = (sdBlockIds) => { + const para = (txt) => tocSchema.nodes.paragraph.create({}, txt ? tocSchema.text(txt) : null); + const tocs = sdBlockIds.map((id) => tocSchema.nodes.tableOfContents.create({ sdBlockId: id }, [para('entry')])); + return tocSchema.nodes.doc.create({}, [para('intro'), ...tocs, para('outro')]); +}; + +const runUpdateFields = (overrides) => { + const { doc, editor } = overrides; + const dispatch = 'dispatch' in overrides ? overrides.dispatch : () => {}; + // FieldUpdate is wrapped by Extension.create(); reach into config.addCommands + // to invoke the raw command function the same way ExtensionService does. + const commands = FieldUpdate.config.addCommands.call({ editor }); + const command = commands.updateFieldsInSelection(); + const tr = { setMeta: vi.fn() }; + const state = { doc, selection: { from: 0, to: 0 }, schema: tocSchema, tr }; + return { result: command({ editor, state, tr, dispatch }), tr }; +}; + +describe('updateFieldsInSelection — TOC path', () => { + it('calls editor.doc.toc.update for every tableOfContents node in document order', () => { + const update = vi.fn(() => ({ success: true })); + const editor = { doc: { toc: { update } } }; + const doc = buildTocDoc(['toc-a', 'toc-b']); + + const { result } = runUpdateFields({ doc, editor }); + + expect(result).toBe(true); + expect(update).toHaveBeenCalledTimes(2); + expect(update.mock.calls[0][0]).toEqual({ + target: { kind: 'block', nodeType: 'tableOfContents', nodeId: 'toc-a' }, + mode: 'all', + }); + expect(update.mock.calls[1][0]).toEqual({ + target: { kind: 'block', nodeType: 'tableOfContents', nodeId: 'toc-b' }, + mode: 'all', + }); + }); + + it('sets preventDispatch on the framework tr so CommandService skips its auto-dispatch', () => { + const update = vi.fn(() => ({ success: true })); + const editor = { doc: { toc: { update } } }; + const doc = buildTocDoc(['toc-a']); + + const { tr } = runUpdateFields({ doc, editor }); + expect(tr.setMeta).toHaveBeenCalledWith('preventDispatch', true); + }); + + it('returns true on a can()-style probe (no dispatch) when any TOC exists', () => { + const update = vi.fn(); + const editor = { doc: { toc: { update } } }; + const doc = buildTocDoc(['toc-a']); + + const { result } = runUpdateFields({ doc, editor, dispatch: undefined }); + expect(result).toBe(true); + expect(update).not.toHaveBeenCalled(); + }); + + it('skips a TOC whose sdBlockId is missing or empty', () => { + const update = vi.fn(() => ({ success: true })); + const editor = { doc: { toc: { update } } }; + const doc = buildTocDoc([null, '', 'toc-real']); + + runUpdateFields({ doc, editor }); + expect(update).toHaveBeenCalledTimes(1); + expect(update.mock.calls[0][0].target.nodeId).toBe('toc-real'); + }); + + it('swallows toc.update errors and continues with the remaining TOCs', () => { + const update = vi + .fn() + .mockImplementationOnce(() => { + throw new Error('boom'); + }) + .mockImplementationOnce(() => ({ success: true })); + const editor = { doc: { toc: { update } } }; + const doc = buildTocDoc(['toc-a', 'toc-b']); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + const { result } = runUpdateFields({ doc, editor }); + expect(result).toBe(true); + expect(update).toHaveBeenCalledTimes(2); + expect(warnSpy).toHaveBeenCalled(); + warnSpy.mockRestore(); + }); + + it('falls through to the stat-field path when the doc has no TOCs', () => { + const update = vi.fn(); + const editor = { doc: { toc: { update } } }; + const para = (txt) => tocSchema.nodes.paragraph.create({}, txt ? tocSchema.text(txt) : null); + const doc = tocSchema.nodes.doc.create({}, [para('hello world')]); + + const { tr } = runUpdateFields({ doc, editor }); + expect(update).not.toHaveBeenCalled(); + expect(tr.setMeta).not.toHaveBeenCalled(); // no preventDispatch when not taking the TOC path + }); +}); + +describe('FieldUpdate extension shortcuts', () => { + it('binds F9 to updateFieldsInSelection', () => { + const ed = { commands: { updateFieldsInSelection: vi.fn(() => true) } }; + const shortcuts = FieldUpdate.config.addShortcuts.call({ editor: ed }); + expect(typeof shortcuts.F9).toBe('function'); + shortcuts.F9(); + expect(ed.commands.updateFieldsInSelection).toHaveBeenCalledTimes(1); + }); + + it('does not bind any other key alongside F9', () => { + const ed = { commands: { updateFieldsInSelection: vi.fn() } }; + const shortcuts = FieldUpdate.config.addShortcuts.call({ editor: ed }); + expect(Object.keys(shortcuts)).toEqual(['F9']); + }); +}); diff --git a/tests/behavior/tests/navigation/toc-update.spec.ts b/tests/behavior/tests/navigation/toc-update.spec.ts new file mode 100644 index 0000000000..64555da3bd --- /dev/null +++ b/tests/behavior/tests/navigation/toc-update.spec.ts @@ -0,0 +1,87 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { test, expect } from '../../fixtures/superdoc.js'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const DOC_PATH = path.resolve(__dirname, '../../test-data/layout/toc-with-heading2.docx'); + +test.skip(!fs.existsSync(DOC_PATH), 'Test document not available — run pnpm corpus:pull'); + +/** + * Reads every TOC entry's title text from the document. + * + * The rebuilt entries are wrapped in `run` nodes whose first text run holds + * the title (without the page-number `tocPageNumber` mark). + */ +const readTocTitles = async (superdoc) => + superdoc.page.evaluate(() => { + const editor = (window as unknown as { editor?: { state: { doc: unknown } } }).editor; + if (!editor?.state?.doc) return []; + const titles: string[] = []; + (editor.state.doc as { descendants: (cb: (n: any) => boolean | void) => void }).descendants((node) => { + if (node?.type?.name !== 'tableOfContents') return true; + node.descendants((child: any) => { + if (child?.type?.name !== 'paragraph') return true; + // First non-page-number text run is the entry title. + let captured = false; + child.descendants((leaf: any) => { + if (captured) return false; + if (!leaf.isText || !leaf.text) return true; + const isPageNumber = (leaf.marks ?? []).some((m: any) => m.type?.name === 'tocPageNumber'); + if (!isPageNumber) { + titles.push(leaf.text); + captured = true; + } + return true; + }); + return false; + }); + return false; + }); + return titles; + }); + +test('@behavior SD-2664: updateFieldsInSelection (F9) rebuilds every TOC entry from the document headings', async ({ + superdoc, +}) => { + await superdoc.loadDocument(DOC_PATH); + await superdoc.waitForStable(2000); + + // Capture the original TOC entries. + const titlesBefore = await readTocTitles(superdoc); + expect(titlesBefore.length).toBeGreaterThan(0); + + // Read the heading texts that should drive the rebuilt TOC. The fixture + // contains Heading1/Heading2 paragraphs in the body. + const headingTexts = await superdoc.page.evaluate(() => { + const editor = (window as unknown as { editor?: { state: { doc: unknown } } }).editor; + if (!editor?.state?.doc) return []; + const out: string[] = []; + (editor.state.doc as { descendants: (cb: (n: any) => boolean | void) => void }).descendants((node) => { + if (node?.type?.name === 'tableOfContents') return false; // skip TOC contents + if (node?.type?.name !== 'paragraph') return true; + const styleId = node.attrs?.paragraphProperties?.styleId; + if (!styleId || !/^Heading[1-9]$/.test(styleId)) return true; + let text = ''; + node.descendants((c: any) => { + if (c.isText && c.text) text += c.text; + return true; + }); + if (text.trim()) out.push(text.trim()); + return true; + }); + return out; + }); + expect(headingTexts.length).toBeGreaterThan(0); + + // Press F9 — the FieldUpdate extension binds it to updateFieldsInSelection, + // which routes through editor.doc.toc.update for every TOC in the doc. + await superdoc.executeCommand('updateFieldsInSelection'); + await superdoc.waitForStable(2000); + + const titlesAfter = await readTocTitles(superdoc); + // Every heading in the doc should now appear as an entry, and every entry + // should map to a heading text. Order must match document order. + expect(titlesAfter).toEqual(headingTexts); +}); From 7c133130caf6c3153091b8cdf2a04191919507b5 Mon Sep 17 00:00:00 2001 From: Gabriel Chittolina Date: Mon, 4 May 2026 16:10:39 -0300 Subject: [PATCH 05/14] refactor: removed unused code --- .../context-menu/list-marker-helpers.js | 58 ------------------- .../context-menu/tests/menuItems.test.js | 44 -------------- .../helpers/toc-entry-builder.test.ts | 8 +-- .../field-update/field-update.test.js | 18 +----- 4 files changed, 2 insertions(+), 126 deletions(-) delete mode 100644 packages/super-editor/src/editors/v1/components/context-menu/list-marker-helpers.js diff --git a/packages/super-editor/src/editors/v1/components/context-menu/list-marker-helpers.js b/packages/super-editor/src/editors/v1/components/context-menu/list-marker-helpers.js deleted file mode 100644 index 4e25ac6b36..0000000000 --- a/packages/super-editor/src/editors/v1/components/context-menu/list-marker-helpers.js +++ /dev/null @@ -1,58 +0,0 @@ -import { findParentNode } from '@helpers/index.js'; -import { isList } from '@core/commands/list-helpers'; -import { getResolvedParagraphProperties } from '@extensions/paragraph/resolvedPropertiesCache.js'; - -/** - * Resolve the list item that the menu was opened over to the document-api - * `ListItemAddress` shape required by `editor.doc.lists.*` calls. - * - * Mirrors the paragraph branch of `resolveBlockNodeId` in the doc-api: paraId - * (preserved across DOCX round-trips) takes precedence over sdBlockId. - * - * @param {Object} context - Menu context containing `editor` and the click `pos`. - * @returns {{ kind: 'block', nodeType: 'listItem', nodeId: string } | null} - */ -export function getListItemAddressFromContext(context) { - const result = findListParagraphFromContext(context); - if (!result) return null; - const attrs = result.node.attrs ?? {}; - const nodeId = attrs.paraId ?? attrs.sdBlockId ?? null; - if (!nodeId) return null; - return { kind: 'block', nodeType: 'listItem', nodeId }; -} - -/** - * Returns the resolved indent level (0-8) for the list item under the menu, - * or null if the cursor isn't over a list paragraph. - * - * @param {Object} context - Menu context containing `editor` and the click `pos`. - * @returns {number | null} - */ -export function getListItemLevelFromContext(context) { - const result = findListParagraphFromContext(context); - if (!result) return null; - const props = getResolvedParagraphProperties(result.node)?.numberingProperties; - return typeof props?.ilvl === 'number' ? props.ilvl : 0; -} - -function findListParagraphFromContext(context) { - const editor = context?.editor; - const state = editor?.state; - if (!state) return null; - - const selection = synthesizeSelection(state, context?.pos); - if (!selection) return null; - return findParentNode(isList)(selection) ?? null; -} - -function synthesizeSelection(state, pos) { - if (typeof pos === 'number' && Number.isFinite(pos)) { - try { - const $pos = state.doc.resolve(pos); - return { $from: $pos, $to: $pos }; - } catch { - // pos was out of range; fall through to live selection - } - } - return state.selection ?? null; -} diff --git a/packages/super-editor/src/editors/v1/components/context-menu/tests/menuItems.test.js b/packages/super-editor/src/editors/v1/components/context-menu/tests/menuItems.test.js index 58f3c530af..5538b4bd89 100644 --- a/packages/super-editor/src/editors/v1/components/context-menu/tests/menuItems.test.js +++ b/packages/super-editor/src/editors/v1/components/context-menu/tests/menuItems.test.js @@ -1095,16 +1095,6 @@ describe('menuItems.js', () => { expect(findItem(sections)).toBeUndefined(); }); - it('is hidden when tocAncestor exists but sdBlockId is missing', () => { - mockContext = createMockContext({ - editor: mockEditor, - trigger: TRIGGERS.click, - tocAncestor: { node: {}, pos: 5, sdBlockId: null }, - }); - const sections = getItems(mockContext); - expect(findItem(sections)).toBeUndefined(); - }); - it('is hidden on the slash trigger even when inside a TOC', () => { mockContext = createMockContext({ editor: mockEditor, @@ -1132,39 +1122,5 @@ describe('menuItems.js', () => { mode: 'all', }); }); - - it('action is a no-op when sdBlockId is missing', () => { - const update = vi.fn(); - const ed = { ...mockEditor, doc: { toc: { update } } }; - // showWhen would normally hide the item — invoke action directly to assert the guard. - mockContext = createMockContext({ - editor: ed, - trigger: TRIGGERS.click, - tocAncestor: { node: {}, pos: 5, sdBlockId: 'toc-1' }, - }); - const sections = getItems(mockContext); - const item = findItem(sections); - // Re-invoke action with a context that has no sdBlockId. - item.action(ed, { ...mockContext, tocAncestor: { node: {}, pos: 5, sdBlockId: null } }); - expect(update).not.toHaveBeenCalled(); - }); - - it('action swallows errors thrown by editor.doc.toc.update', () => { - const update = vi.fn(() => { - throw new Error('boom'); - }); - const ed = { ...mockEditor, doc: { toc: { update } } }; - mockContext = createMockContext({ - editor: ed, - trigger: TRIGGERS.click, - tocAncestor: { node: {}, pos: 5, sdBlockId: 'toc-1' }, - }); - const sections = getItems(mockContext); - const item = findItem(sections); - const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); - expect(() => item.action(ed, mockContext)).not.toThrow(); - expect(warn).toHaveBeenCalled(); - warn.mockRestore(); - }); }); }); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts index e7dead487f..f27f3998c8 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts @@ -64,13 +64,7 @@ describe('buildTocEntryParagraphs', () => { }); describe('rightAlignPageNumbers', () => { - it('adds a right-aligned tab stop with default dot leader when rightAlignPageNumbers is true', () => { - const paragraphs = buildTocEntryParagraphs([BASE_SOURCE], makeConfig({ rightAlignPageNumbers: true })); - const tabStops = paragraphs[0]!.attrs.paragraphProperties as Record; - expect(tabStops.tabStops).toEqual([{ tab: { tabType: 'right', pos: 9350, leader: 'dot' } }]); - }); - - it('adds a right-aligned tab stop with default dot leader by default (undefined)', () => { + it('adds a right-aligned tab stop with default dot leader', () => { const paragraphs = buildTocEntryParagraphs([BASE_SOURCE], makeConfig()); const tabStops = paragraphs[0]!.attrs.paragraphProperties as Record; expect(tabStops.tabStops).toEqual([{ tab: { tabType: 'right', pos: 9350, leader: 'dot' } }]); diff --git a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.test.js b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.test.js index 972cbbe0cb..11e76c3ca6 100644 --- a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.test.js +++ b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.test.js @@ -178,16 +178,6 @@ describe('updateFieldsInSelection — TOC path', () => { expect(tr.setMeta).toHaveBeenCalledWith('preventDispatch', true); }); - it('returns true on a can()-style probe (no dispatch) when any TOC exists', () => { - const update = vi.fn(); - const editor = { doc: { toc: { update } } }; - const doc = buildTocDoc(['toc-a']); - - const { result } = runUpdateFields({ doc, editor, dispatch: undefined }); - expect(result).toBe(true); - expect(update).not.toHaveBeenCalled(); - }); - it('skips a TOC whose sdBlockId is missing or empty', () => { const update = vi.fn(() => ({ success: true })); const editor = { doc: { toc: { update } } }; @@ -232,14 +222,8 @@ describe('FieldUpdate extension shortcuts', () => { it('binds F9 to updateFieldsInSelection', () => { const ed = { commands: { updateFieldsInSelection: vi.fn(() => true) } }; const shortcuts = FieldUpdate.config.addShortcuts.call({ editor: ed }); - expect(typeof shortcuts.F9).toBe('function'); + expect(Object.keys(shortcuts)).toEqual(['F9']); shortcuts.F9(); expect(ed.commands.updateFieldsInSelection).toHaveBeenCalledTimes(1); }); - - it('does not bind any other key alongside F9', () => { - const ed = { commands: { updateFieldsInSelection: vi.fn() } }; - const shortcuts = FieldUpdate.config.addShortcuts.call({ editor: ed }); - expect(Object.keys(shortcuts)).toEqual(['F9']); - }); }); From b2088584a8cbc4e727e38b9bc0d925f6a546c24a Mon Sep 17 00:00:00 2001 From: Gabriel Chittolina Date: Mon, 4 May 2026 16:23:13 -0300 Subject: [PATCH 06/14] refactor: code style tweaks --- .../v1/extensions/field-update/field-update.js | 3 +++ .../table-of-contents/find-toc-ancestor.js | 4 ++++ .../behavior/tests/navigation/toc-update.spec.ts | 16 ++++++++++++++++ 3 files changed, 23 insertions(+) diff --git a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js index cd98d9762c..9c109fd05e 100644 --- a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js +++ b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js @@ -56,8 +56,10 @@ export const FieldUpdate = Extension.create({ // state. Set preventDispatch so it skips that. if (editor?.doc?.toc?.update) { const tocTargets = collectTocBlockIds(state.doc); + if (tocTargets.length > 0) { if (!dispatch) return true; // can()-style probe + for (const sdBlockId of tocTargets) { try { editor.doc.toc.update({ @@ -68,6 +70,7 @@ export const FieldUpdate = Extension.create({ console.warn('[FieldUpdate] toc.update failed for', sdBlockId, error); } } + outerTr?.setMeta?.('preventDispatch', true); return true; } diff --git a/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js b/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js index 6db0d24986..f6fba90df5 100644 --- a/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js +++ b/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js @@ -10,16 +10,20 @@ export function findTocAncestor(doc, pos) { if (!doc || typeof pos !== 'number' || !Number.isFinite(pos)) return null; let resolved; + try { resolved = doc.resolve(pos); } catch { return null; } + for (let depth = resolved.depth; depth >= 0; depth -= 1) { const node = resolved.node(depth); if (node?.type?.name !== 'tableOfContents') continue; + const sdBlockId = typeof node.attrs?.sdBlockId === 'string' ? node.attrs.sdBlockId : null; return { node, pos: depth === 0 ? 0 : resolved.before(depth), sdBlockId }; } + return null; } diff --git a/tests/behavior/tests/navigation/toc-update.spec.ts b/tests/behavior/tests/navigation/toc-update.spec.ts index 64555da3bd..0653081a98 100644 --- a/tests/behavior/tests/navigation/toc-update.spec.ts +++ b/tests/behavior/tests/navigation/toc-update.spec.ts @@ -18,27 +18,36 @@ const readTocTitles = async (superdoc) => superdoc.page.evaluate(() => { const editor = (window as unknown as { editor?: { state: { doc: unknown } } }).editor; if (!editor?.state?.doc) return []; + const titles: string[] = []; + (editor.state.doc as { descendants: (cb: (n: any) => boolean | void) => void }).descendants((node) => { if (node?.type?.name !== 'tableOfContents') return true; + node.descendants((child: any) => { if (child?.type?.name !== 'paragraph') return true; // First non-page-number text run is the entry title. let captured = false; + child.descendants((leaf: any) => { if (captured) return false; if (!leaf.isText || !leaf.text) return true; + const isPageNumber = (leaf.marks ?? []).some((m: any) => m.type?.name === 'tocPageNumber'); if (!isPageNumber) { titles.push(leaf.text); captured = true; } + return true; }); + return false; }); + return false; }); + return titles; }); @@ -57,18 +66,25 @@ test('@behavior SD-2664: updateFieldsInSelection (F9) rebuilds every TOC entry f const headingTexts = await superdoc.page.evaluate(() => { const editor = (window as unknown as { editor?: { state: { doc: unknown } } }).editor; if (!editor?.state?.doc) return []; + const out: string[] = []; + (editor.state.doc as { descendants: (cb: (n: any) => boolean | void) => void }).descendants((node) => { if (node?.type?.name === 'tableOfContents') return false; // skip TOC contents if (node?.type?.name !== 'paragraph') return true; + const styleId = node.attrs?.paragraphProperties?.styleId; if (!styleId || !/^Heading[1-9]$/.test(styleId)) return true; + let text = ''; + node.descendants((c: any) => { if (c.isText && c.text) text += c.text; return true; }); + if (text.trim()) out.push(text.trim()); + return true; }); return out; From d20397ab0802863db359d466e1248612c9e47882 Mon Sep 17 00:00:00 2001 From: Gabriel Chittolina Date: Mon, 4 May 2026 16:35:20 -0300 Subject: [PATCH 07/14] refactor: removed duplicate functions --- .../plan-engine/toc-wrappers.ts | 8 ++++---- .../v1/extensions/field-update/field-update.js | 17 ++++------------- .../table-of-contents/find-toc-ancestor.js | 17 ++++++----------- 3 files changed, 14 insertions(+), 28 deletions(-) diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts index 4530473a1a..62f6185510 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts @@ -41,6 +41,7 @@ import { collectTocSources, buildTocEntryParagraphs, type BuildTocEntryOptions, + type EntryTextMark, type EntryParagraphJson, type TocSource, } from '../helpers/toc-entry-builder.js'; @@ -282,7 +283,6 @@ type TocParagraphProps = { }; type TocParagraphAttrs = { paragraphProperties?: TocParagraphProps }; type TabStopJson = { tab?: { pos?: number; tabType?: string; leader?: string } }; -type EntryMarkJson = { type: string; attrs?: Record }; /** First TOC1–TOC9 paragraph in the existing TOC node, or `undefined`. */ function findFirstTocEntryParagraph(node: ProseMirrorNode): ProseMirrorNode | undefined { @@ -342,8 +342,8 @@ function readExistingTocTrailingParagraph(node: ProseMirrorNode): unknown | unde * into the rebuilt entries. Drops `link` (rebuilt from the source's bookmark) * and `tocPageNumber` (belongs only to the page-number run). */ -function readExistingTocEntryTextMarks(node: ProseMirrorNode): EntryMarkJson[] { - let marks: EntryMarkJson[] = []; +function readExistingTocEntryTextMarks(node: ProseMirrorNode): EntryTextMark[] { + let marks: EntryTextMark[] = []; node.forEach((paragraph) => { if (marks.length > 0 || paragraph.type.name !== 'paragraph') return; const styleId = (paragraph.attrs as TocParagraphAttrs | undefined)?.paragraphProperties?.styleId; @@ -355,7 +355,7 @@ function readExistingTocEntryTextMarks(node: ProseMirrorNode): EntryMarkJson[] { const sampled = (child.marks ?? []) .filter((m) => m.type.name !== 'link' && m.type.name !== 'tocPageNumber') .map((m) => { - const json: EntryMarkJson = { type: m.type.name }; + const json: EntryTextMark = { type: m.type.name }; if (m.attrs && Object.keys(m.attrs).length > 0) json.attrs = { ...m.attrs }; return json; }); diff --git a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js index 9c109fd05e..f0e8120404 100644 --- a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js +++ b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js @@ -1,5 +1,6 @@ import { Extension } from '@core/Extension.js'; import { findFieldsInRange } from '../../document-api-adapters/helpers/field-resolver.js'; +import { findAllTocNodes } from '../../document-api-adapters/helpers/toc-resolver.js'; import { getWordStatistics, resolveDocumentStatFieldValue, @@ -9,18 +10,6 @@ import { /** Stat-field types refreshed by F9 when the doc has no TOCs. */ const UPDATABLE_FIELD_TYPES = new Set(['NUMWORDS', 'NUMCHARS', 'NUMPAGES']); -/** Every `tableOfContents` node's sdBlockId in document order. */ -function collectTocBlockIds(doc) { - const ids = []; - doc.descendants((node) => { - if (node.type.name !== 'tableOfContents') return true; - const sdBlockId = node.attrs?.sdBlockId; - if (typeof sdBlockId === 'string' && sdBlockId) ids.push(sdBlockId); - return false; // don't descend into TOC children - }); - return ids; -} - /** * @module FieldUpdate * @sidebarTitle Field Update @@ -55,7 +44,9 @@ export const FieldUpdate = Extension.create({ // would then auto-apply its captured (now-stale) `tr` to the new // state. Set preventDispatch so it skips that. if (editor?.doc?.toc?.update) { - const tocTargets = collectTocBlockIds(state.doc); + const tocTargets = findAllTocNodes(state.doc) + .map((toc) => toc.commandNodeId) + .filter((id) => typeof id === 'string' && id); if (tocTargets.length > 0) { if (!dispatch) return true; // can()-style probe diff --git a/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js b/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js index f6fba90df5..d7c60de513 100644 --- a/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js +++ b/packages/super-editor/src/editors/v1/extensions/table-of-contents/find-toc-ancestor.js @@ -1,3 +1,5 @@ +import { findParentNodeClosestToPos } from '@core/helpers/findParentNodeClosestToPos.js'; + /** * Find the enclosing `tableOfContents` node for a document position. Used by * the context menu to route "Update table of contents" through @@ -10,20 +12,13 @@ export function findTocAncestor(doc, pos) { if (!doc || typeof pos !== 'number' || !Number.isFinite(pos)) return null; let resolved; - try { resolved = doc.resolve(pos); } catch { return null; } - - for (let depth = resolved.depth; depth >= 0; depth -= 1) { - const node = resolved.node(depth); - if (node?.type?.name !== 'tableOfContents') continue; - - const sdBlockId = typeof node.attrs?.sdBlockId === 'string' ? node.attrs.sdBlockId : null; - return { node, pos: depth === 0 ? 0 : resolved.before(depth), sdBlockId }; - } - - return null; + const found = findParentNodeClosestToPos(resolved, (n) => n.type.name === 'tableOfContents'); + if (!found) return null; + const sdBlockId = typeof found.node.attrs?.sdBlockId === 'string' ? found.node.attrs.sdBlockId : null; + return { node: found.node, pos: found.pos, sdBlockId }; } From bceff524878b540f534d43feecd91d85c9129717 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Wed, 6 May 2026 06:23:38 -0300 Subject: [PATCH 08/14] test(toc): add regression coverage for SD-2664 review findings - tabLeader: 'none' must round-trip via serialize/parse (currently lost because no \\p is emitted when separator is missing, and the parser has no way to disambiguate "default = dots" from "explicit none"). - toc.configure({ tabLeader: 'none' }) on a default-leader TOC must not silently no-op (areTocConfigsEqual reports identical serialized output). - toc.update mode: 'pageNumbers' must find tocPageNumber marks when the marked text is nested inside a run wrapper (the rebuild output shape). All three tests fail on the current branch and lock in the regressions flagged in code review. --- .../plan-engine/pr-3120-regressions.test.ts | 237 ++++++++++++++++++ 1 file changed, 237 insertions(+) create mode 100644 packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/pr-3120-regressions.test.ts diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/pr-3120-regressions.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/pr-3120-regressions.test.ts new file mode 100644 index 0000000000..813d48c238 --- /dev/null +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/pr-3120-regressions.test.ts @@ -0,0 +1,237 @@ +import type { Node as ProseMirrorNode } from 'prosemirror-model'; +import { describe, expect, it, vi } from 'vitest'; +import type { Editor } from '../../core/Editor.js'; +import type { PlanReceipt } from '@superdoc/document-api'; + +vi.mock('./plan-wrappers.js', () => ({ + executeDomainCommand: vi.fn((_editor: Editor, handler: () => boolean): PlanReceipt => { + const applied = handler(); + return { + success: true, + revision: { before: '0', after: '0' }, + steps: [ + { + stepId: 'step-1', + op: 'domain.command', + effect: applied ? 'changed' : 'noop', + matchCount: applied ? 1 : 0, + data: { domain: 'command', commandDispatched: applied }, + }, + ], + timing: { totalMs: 0 }, + }; + }), +})); + +import { + applyTocPatch, + serializeTocInstruction, + parseTocInstruction, +} from '../../core/super-converter/field-references/shared/toc-switches.js'; +import { tocUpdateWrapper } from './toc-wrappers.js'; + +type NodeOptions = { + attrs?: Record; + text?: string; + isInline?: boolean; + isBlock?: boolean; + isLeaf?: boolean; + inlineContent?: boolean; + nodeSize?: number; + marks?: Array<{ type: string | { name: string } }>; +}; + +function createNode(typeName: string, children: ProseMirrorNode[] = [], options: NodeOptions = {}): ProseMirrorNode { + const attrs = options.attrs ?? {}; + const text = options.text ?? ''; + const isText = typeName === 'text'; + const isInline = options.isInline ?? isText; + const isBlock = options.isBlock ?? !isInline; + const marks = (options.marks ?? []).map((m) => { + const name = typeof m.type === 'string' ? m.type : m.type.name; + return { type: { name } }; + }); + const node: any = { + type: { name: typeName, isInline, isBlock, isText }, + attrs, + text, + isText, + isBlock, + isInline, + isLeaf: options.isLeaf ?? (isText || children.length === 0), + inlineContent: options.inlineContent ?? false, + nodeSize: options.nodeSize ?? (isText ? text.length : 1), + childCount: children.length, + children, + marks, + forEach(cb: (child: ProseMirrorNode, offset: number, index: number) => void) { + let off = 0; + children.forEach((c, i) => { + cb(c, off, i); + off += (c as any).nodeSize ?? 1; + }); + }, + descendants(cb: (n: ProseMirrorNode, pos: number, parent: ProseMirrorNode | null) => boolean | void) { + const walk = (n: any, pos: number, parent: any) => { + const r = cb(n, pos, parent); + if (r === false) return; + let off = pos + (n.isText ? 0 : 1); + (n.children ?? []).forEach((c: any) => { + walk(c, off, n); + off += c.nodeSize ?? 1; + }); + }; + (children ?? []).forEach((c: any, i: number) => { + walk(c, i, node); + }); + }, + toJSON() { + const out: any = { type: typeName }; + if (Object.keys(attrs).length > 0) out.attrs = attrs; + if (isText && text) out.text = text; + if (marks.length > 0) { + out.marks = marks.map((m: any) => ({ type: m.type.name })); + } + if (children.length > 0) { + out.content = children.map((c: any) => c.toJSON()); + } + return out; + }, + nodeAt() { + return null; + }, + cut() { + return node; + }, + resolve() { + return null; + }, + }; + // expose firstChild / lastChild + Object.defineProperty(node, 'firstChild', { get: () => children[0] ?? null }); + Object.defineProperty(node, 'lastChild', { get: () => children[children.length - 1] ?? null }); + return node as ProseMirrorNode; +} + +describe('PR-3120 regression — finding 1: tabLeader: "none" must round-trip', () => { + it('survives applyTocPatch → serialize → parse back to none', () => { + // Start from a TOC with hyphen leader (so we have a separator to clear) + const initial = parseTocInstruction('TOC \\o "1-3" \\p "-"'); + expect(initial.display.tabLeader).toBe('hyphen'); + expect(initial.display.separator).toBe('-'); + + // User sets explicit no-leader + const configured = applyTocPatch(initial, { tabLeader: 'none' }); + expect(configured.display.tabLeader).toBe('none'); + + // Serialize → store on the node → reopen → parse back + const serialized = serializeTocInstruction(configured); + const reparsed = parseTocInstruction(serialized); + + // BUG: reparsed.display.tabLeader is undefined here, not 'none'. + // The 'none' signal is lost because serializeTocInstruction skips \p when + // separator is missing, and parseTocInstruction has no way to distinguish + // "no \p" (default = dots) from "explicit none" without a marker. + expect(reparsed.display.tabLeader).toBe('none'); + }); + + it('configuring tabLeader: "none" on a default-leader TOC must not silently no-op', () => { + // Start from the default TOC (no \p) + const initial = parseTocInstruction('TOC \\o "1-3"'); + expect(initial.display.tabLeader).toBeUndefined(); + expect(initial.display.separator).toBeUndefined(); + + const configured = applyTocPatch(initial, { tabLeader: 'none' }); + expect(configured.display.tabLeader).toBe('none'); + + // BUG: serialization is identical to the original (both omit \p), so the + // wrapper's areTocConfigsEqual will report NO_OP and the user's setting + // never reaches the stored instruction. + const initialSerialized = serializeTocInstruction(initial); + const configuredSerialized = serializeTocInstruction(configured); + expect(configuredSerialized).not.toBe(initialSerialized); + }); +}); + +describe('PR-3120 regression — finding 2: pageNumbers scanner must traverse run-wrapped page-number text', () => { + // The mode:'all' rebuild produces paragraphs whose content is [run, run, run] + // where the page-number text is *nested inside* the third run. The scanner + // currently inspects only immediate paragraph children, so it misses the mark + // and reports PAGE_NUMBERS_NOT_MATERIALIZED. + + it('finds tocPageNumber when the marked text is nested inside a run wrapper', () => { + // Build a paragraph that mirrors the rebuild output: + // paragraph + // run ← immediate child, no marks + // text "Heading 1" + // run ← immediate child, no marks + // tab + // run ← immediate child, no marks + // text "0" with marks=[{type:'tocPageNumber'}] + const titleText = createNode('text', [], { text: 'Heading 1' }); + const titleRun = createNode('run', [titleText], { + isInline: true, + inlineContent: true, + }); + const tabText = createNode('text', [], { text: '\t' }); + const tabRun = createNode('run', [tabText], { + isInline: true, + inlineContent: true, + }); + const pageNumText = createNode('text', [], { + text: '0', + marks: [{ type: 'tocPageNumber' }], + }); + const pageNumRun = createNode('run', [pageNumText], { + isInline: true, + inlineContent: true, + }); + const entryParagraph = createNode('paragraph', [titleRun, tabRun, pageNumRun], { + attrs: { + sdBlockId: 'toc-entry-p1', + paragraphProperties: { styleId: 'TOC1' }, + tocSourceId: 'h-1', + }, + isBlock: true, + inlineContent: true, + }); + const tocNode = createNode('tableOfContents', [entryParagraph], { + attrs: { sdBlockId: 'toc-1', instruction: 'TOC \\o "1-3" \\h \\z' }, + isBlock: true, + }); + const heading = createNode('paragraph', [createNode('text', [], { text: 'Heading 1' })], { + attrs: { sdBlockId: 'h-1', paragraphProperties: { styleId: 'Heading1' } }, + isBlock: true, + inlineContent: true, + }); + const doc = createNode('doc', [tocNode, heading], { isBlock: false }); + + const editor = { + state: { doc, schema: { nodes: { paragraph: { create: vi.fn() }, tableOfContents: {} } } }, + commands: { + insertTableOfContentsAt: vi.fn(() => true), + setTableOfContentsInstructionById: vi.fn(() => true), + replaceTableOfContentsContentById: vi.fn(() => true), + deleteTableOfContentsById: vi.fn(() => true), + }, + schema: { marks: {} }, + options: {}, + storage: { + tableOfContents: { + pageMap: new Map([['h-1', 7]]), + pageMapDoc: doc, + }, + }, + on: () => {}, + } as unknown as Editor; + + const tocTarget = { kind: 'block', nodeType: 'tableOfContents', nodeId: 'toc-1' } as const; + const result = tocUpdateWrapper(editor, { target: tocTarget, mode: 'pageNumbers' }, { changeMode: 'direct' }); + + // BUG: the scanner inspects only immediate paragraph children (run nodes, + // which carry no marks here) and misses the nested tocPageNumber mark on + // the text inside the third run. So it reports PAGE_NUMBERS_NOT_MATERIALIZED + // even though the marks ARE in the entry, just one level deeper. + expect(result.success).toBe(true); + }); +}); From 3f73efc20aa18ad4fa109180545b2af7e622a799 Mon Sep 17 00:00:00 2001 From: Gabriel Chittolina Date: Thu, 7 May 2026 14:47:37 -0300 Subject: [PATCH 09/14] fix: toc context menu update --- .../src/editors/v1/core/InputRule.js | 37 ++- .../core/helpers/clipboardFragmentAnnotate.js | 2 +- .../v1/core/renderers/ProseMirrorRenderer.ts | 53 +++- .../shared/toc-switches.test.ts | 6 +- .../field-references/shared/toc-switches.ts | 16 +- .../helpers/toc-entry-builder.test.ts | 151 ++++++++- .../helpers/toc-entry-builder.ts | 175 +++++++++-- .../plan-engine/pr-3120-regressions.test.ts | 150 +++++++++ .../plan-engine/toc-wrappers.ts | 87 ++---- .../extensions/field-update/field-update.js | 16 + .../tests/navigation/toc-update.spec.ts | 288 ++++++++++++++++++ 11 files changed, 857 insertions(+), 124 deletions(-) diff --git a/packages/super-editor/src/editors/v1/core/InputRule.js b/packages/super-editor/src/editors/v1/core/InputRule.js index 2de4ca20c1..022e61a6a4 100644 --- a/packages/super-editor/src/editors/v1/core/InputRule.js +++ b/packages/super-editor/src/editors/v1/core/InputRule.js @@ -32,6 +32,17 @@ import { } from './helpers/superdocClipboardSlice.js'; import { annotateFragmentDomWithClipboardData } from './helpers/clipboardFragmentAnnotate.js'; +/** + * True when a node is a Heading[1-9] paragraph. Used to decide whether a + * pasted paragraph should keep its wrapper (so the heading styleId survives + * the merge into the cursor's surroundings). + */ +function isHeadingParagraph(node) { + if (!node || node.type?.name !== 'paragraph') return false; + const styleId = node.attrs?.paragraphProperties?.styleId; + return typeof styleId === 'string' && /^Heading[1-9]$/i.test(styleId); +} + /** Heuristic: clipboard HTML from SuperDoc copy (slice attrs, list/section metadata). */ export function isSuperdocOriginClipboardHtml(html) { if (!html || typeof html !== 'string') return false; @@ -473,7 +484,14 @@ export function handleHtmlPaste(html, editor, source) { // Check if the pasted content is a single paragraph const isSingleParagraph = doc.childCount === 1 && doc.firstChild.type.name === 'paragraph'; - if (isInParagraph && isSingleParagraph) { + // Heading paragraphs (Heading1..Heading9) must keep their paragraph + // wrapper so the styleId survives — otherwise pasting a heading into a + // body paragraph silently strips the heading style and the TOC misses + // the new entry on F9. + const sourceStyleId = isSingleParagraph ? (doc.firstChild.attrs?.paragraphProperties?.styleId ?? null) : null; + const sourceIsHeading = typeof sourceStyleId === 'string' && /^Heading[1-9]$/i.test(sourceStyleId); + + if (isInParagraph && isSingleParagraph && !sourceIsHeading) { // Extract the contents of the paragraph and paste only those const paragraphContent = doc.firstChild.content; const tr = state.tr.replaceSelectionWith(paragraphContent, false); @@ -875,7 +893,22 @@ function handleSuperdocSlicePaste(sliceData, editor, view, embeddedBodySectPr = const stripped = stripSuperdocSliceBlockIdentities(slice.content); const cleanContent = remapPastedListNumberingInFragment(stripped, editor); - const cleanSlice = new Slice(cleanContent, slice.openStart, slice.openEnd); + + // When the copied slice carries a Heading[1-9] paragraph, the user expects + // the heading to land as a discrete heading paragraph (matches Word's + // "Update field" behavior on the rebuilt TOC). PM merges open-boundary + // slices into the cursor's paragraph and drops the heading styleId — even + // when the same logical content was selected. Force closed boundaries + // when the leading or trailing paragraph is a heading so the wrapper + // survives the merge. + let openStart = slice.openStart; + let openEnd = slice.openEnd; + const firstChild = cleanContent.firstChild; + const lastChild = cleanContent.lastChild; + if (firstChild && isHeadingParagraph(firstChild)) openStart = 0; + if (lastChild && isHeadingParagraph(lastChild)) openEnd = 0; + + const cleanSlice = new Slice(cleanContent, openStart, openEnd); const { dispatch, state } = view; if (!dispatch) return false; diff --git a/packages/super-editor/src/editors/v1/core/helpers/clipboardFragmentAnnotate.js b/packages/super-editor/src/editors/v1/core/helpers/clipboardFragmentAnnotate.js index 7f96bbebd3..b4bb8de26c 100644 --- a/packages/super-editor/src/editors/v1/core/helpers/clipboardFragmentAnnotate.js +++ b/packages/super-editor/src/editors/v1/core/helpers/clipboardFragmentAnnotate.js @@ -35,7 +35,7 @@ export function annotateFragmentDomWithClipboardData(container, fragment, editor * * @param {HTMLElement} container cloned selection HTML * @param {import('prosemirror-view').EditorView} view - * @param {import('../Editor').Editor} editor + * @param {import('../Editor').Editor} [editor] optional editor instance — function bails out when missing */ export function mergeSerializedClipboardMetadataIntoDomContainer(container, view, editor) { if (!editor || !view || typeof document === 'undefined') return; diff --git a/packages/super-editor/src/editors/v1/core/renderers/ProseMirrorRenderer.ts b/packages/super-editor/src/editors/v1/core/renderers/ProseMirrorRenderer.ts index 8c6c891c49..4cf0f2b5eb 100644 --- a/packages/super-editor/src/editors/v1/core/renderers/ProseMirrorRenderer.ts +++ b/packages/super-editor/src/editors/v1/core/renderers/ProseMirrorRenderer.ts @@ -1,6 +1,6 @@ import { EditorView } from 'prosemirror-view'; import type { DirectEditorProps } from 'prosemirror-view'; -import { DOMSerializer as PmDOMSerializer } from 'prosemirror-model'; +import { DOMSerializer as PmDOMSerializer, Slice as PmSlice, Fragment as PmFragment } from 'prosemirror-model'; import type { Node as PmNode } from 'prosemirror-model'; import { annotateFragmentDomWithClipboardData, @@ -19,6 +19,46 @@ import type { EditorRenderer, EditorRendererAttachParams } from './EditorRendere import type { Editor } from '../Editor.js'; import type { EditorOptions } from '../types/EditorConfig.js'; +/** Heading[1-9] styleId regex — paste/copy must keep these paragraph wrappers intact. */ +const HEADING_STYLE_RE = /^Heading[1-9]$/i; + +/** + * If the active selection is entirely inside a single Heading[1-9] paragraph + * AND the slice PM produced has no paragraph wrapper at the top, wrap the + * slice's inline content in a copy of that heading paragraph. The result is + * a closed-boundary slice that survives paste with the heading styleId + * intact — without this, F9 / "Update table of contents" cannot detect the + * pasted heading. + */ +function wrapHeadingSelectionAsParagraph(slice: PmSlice, state: { selection: any }): PmSlice { + // Only act when the slice content is inline-only (no paragraph wrapper). + const firstChild = slice.content.firstChild; + if (!firstChild || firstChild.type.name === 'paragraph') return slice; + + const $from = state.selection.$from; + const $to = state.selection.$to; + if (!$from || !$to) return slice; + + // Selection must be inside a single paragraph. + let parentParagraph: PmNode | null = null; + for (let depth = $from.depth; depth >= 0; depth--) { + const node = $from.node(depth); + if (node?.type?.name === 'paragraph') { + // Confirm $to has the same paragraph ancestor at the same depth. + if ($to.depth >= depth && $to.node(depth) === node) parentParagraph = node; + break; + } + } + if (!parentParagraph) return slice; + + const styleId = (parentParagraph.attrs as { paragraphProperties?: { styleId?: string } } | undefined) + ?.paragraphProperties?.styleId; + if (typeof styleId !== 'string' || !HEADING_STYLE_RE.test(styleId)) return slice; + + const wrapped = parentParagraph.type.create(parentParagraph.attrs, slice.content, parentParagraph.marks); + return new PmSlice(PmFragment.from(wrapped), 0, 0); +} + /** * Default fallback margin for presentation mode when pageMargins.top is undefined. * This value provides consistent spacing for header/footer content. @@ -843,7 +883,16 @@ export class ProseMirrorRenderer implements EditorRenderer { const { from, to } = this.view.state.selection; let sliceJson = ''; if (from !== to) { - const slice = this.view.state.doc.slice(from, to); + const rawSlice = this.view.state.doc.slice(from, to); + // PM produces an inline-only slice (no paragraph wrapper) when the + // selection covers the full content of a paragraph at exact open + // boundaries. Without the wrapper the paragraph's styleId — most + // critically Heading[1-9] — is lost on paste, so the receiving + // editor can't tell that the source was a heading and the TOC + // misses the new entry. When the selection is entirely within a + // single heading paragraph, re-wrap the slice's content in that + // paragraph node and emit it with closed boundaries. + const slice = wrapHeadingSelectionAsParagraph(rawSlice, this.view.state); sliceJson = JSON.stringify(slice.toJSON()); clipboardData.setData('application/x-superdoc-slice', sliceJson); const mediaJson = collectReferencedImageMediaForClipboard(sliceJson, editor); diff --git a/packages/super-editor/src/editors/v1/core/super-converter/field-references/shared/toc-switches.test.ts b/packages/super-editor/src/editors/v1/core/super-converter/field-references/shared/toc-switches.test.ts index 5c70066da8..a57fb00c13 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/field-references/shared/toc-switches.test.ts +++ b/packages/super-editor/src/editors/v1/core/super-converter/field-references/shared/toc-switches.test.ts @@ -165,11 +165,13 @@ describe('applyTocPatch', () => { expect(patched.display.separator).toBe('.'); }); - it('tabLeader: none removes separator', () => { + it('tabLeader: none records an explicit empty separator (\\p "") so the choice round-trips', () => { const existing = parseTocInstruction('TOC \\o "1-3" \\p "."'); const patched = applyTocPatch(existing, { tabLeader: 'none' }); expect(patched.display.tabLeader).toBe('none'); - expect(patched.display.separator).toBeUndefined(); + // Empty string == explicit "no leader" (\p ""); deleting the separator + // would collapse to "absent \p" which Word treats as the dot default. + expect(patched.display.separator).toBe(''); }); it('throws on tabLeader + separator conflict', () => { diff --git a/packages/super-editor/src/editors/v1/core/super-converter/field-references/shared/toc-switches.ts b/packages/super-editor/src/editors/v1/core/super-converter/field-references/shared/toc-switches.ts index 67bb8ec7ab..c3c23e11c3 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/field-references/shared/toc-switches.ts +++ b/packages/super-editor/src/editors/v1/core/super-converter/field-references/shared/toc-switches.ts @@ -132,7 +132,8 @@ export function parseTocInstruction(instruction: string): TocSwitchConfig { SWITCH_PATTERN.lastIndex = 0; while ((match = SWITCH_PATTERN.exec(instruction)) !== null) { const switchChar = match[1].toLowerCase(); - const arg = match[2] ?? ''; + const rawArg = match[2]; + const arg = rawArg ?? ''; switch (switchChar) { // Configurable source switches @@ -166,7 +167,10 @@ export function parseTocInstruction(instruction: string): TocSwitchConfig { break; } case 'p': - if (arg) display.separator = arg; + // \p with an explicit empty arg (`\p ""`) means "no leader" and must be + // distinguishable from \p being absent entirely (Word default = dots). + // Preserve the empty string so deriveTabLeader can map it to 'none'. + if (rawArg !== undefined) display.separator = rawArg; break; // Preserved switches @@ -274,8 +278,8 @@ export function serializeTocInstruction(config: TocSwitchConfig): string { parts.push(`\\n "${display.omitPageNumberLevels.from}-${display.omitPageNumberLevels.to}"`); } - // \p — separator - if (display.separator) { + // \p — separator. Empty string is meaningful (`\p ""` = explicit "no leader"). + if (display.separator !== undefined) { parts.push(`\\p "${display.separator}"`); } @@ -374,7 +378,9 @@ export function applyTocPatch(existing: TocSwitchConfig, patch: TocConfigurePatc // Handle tabLeader → \p switch mapping if (patch.tabLeader !== undefined) { if (patch.tabLeader === 'none') { - delete newDisplay.separator; + // Use \p "" to record an explicit "no leader" so it round-trips through + // serialize → parse without collapsing to "absent \p" (Word default = dots). + newDisplay.separator = ''; } else { newDisplay.separator = TAB_LEADER_TO_SEPARATOR[patch.tabLeader]; } diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts index f27f3998c8..7b2ae004fc 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts @@ -112,27 +112,17 @@ describe('buildTocEntryParagraphs', () => { }); }); - describe('entry text marks (SD-2664)', () => { - it('prepends preserved marks and stacks the link mark on top', () => { - const entryTextMarks = [ - { type: 'textStyle', attrs: { fontFamily: 'Aptos', fontSize: '12pt' } }, - { type: 'bold' }, - ]; - const paragraphs = buildTocEntryParagraphs([BASE_SOURCE], makeConfig({ hyperlinks: true }), { entryTextMarks }); + describe('entry formatting (SD-2664)', () => { + it('emits only the link mark on the title text — Word rebuilds run formatting from the linked TOC{n} paragraph styles', () => { + const paragraphs = buildTocEntryParagraphs([BASE_SOURCE], makeConfig({ hyperlinks: true })); const text = titleTextOf(paragraphs); - expect(text.marks!.map((m) => m.type)).toEqual(['textStyle', 'bold', 'link']); - expect(text.marks![0].attrs).toEqual({ fontFamily: 'Aptos', fontSize: '12pt' }); + expect(text.marks!.map((m) => m.type)).toEqual(['link']); }); - it('drops any incoming link mark; the builder rebuilds it from the source bookmark', () => { - const entryTextMarks = [ - { type: 'textStyle', attrs: { fontFamily: 'Aptos' } }, - { type: 'link', attrs: { anchor: 'old-stale-anchor' } }, - ]; - const paragraphs = buildTocEntryParagraphs([BASE_SOURCE], makeConfig({ hyperlinks: true }), { entryTextMarks }); + it('the rebuilt link uses the source bookmark anchor', () => { + const paragraphs = buildTocEntryParagraphs([BASE_SOURCE], makeConfig({ hyperlinks: true })); const linkMark = titleTextOf(paragraphs).marks?.find((m) => m.type === 'link'); expect(linkMark?.attrs?.anchor).toBe(generateTocBookmarkName(BASE_SOURCE.sdBlockId)); - expect(linkMark?.attrs?.anchor).not.toBe('old-stale-anchor'); }); it('wraps each text run in a `run` node so wrapTextInRunsPlugin does not clobber marks', () => { @@ -142,6 +132,109 @@ describe('buildTocEntryParagraphs', () => { expect(runs.length).toBe(3); runs.forEach((r) => expect(r.type).toBe('run')); }); + + it('carries the allowed character marks from the source heading into the rebuilt entry', () => { + const sourceWithMarks: TocSource = { + ...BASE_SOURCE, + segments: [ + { text: 'Plain ', marks: [{ type: 'textStyle', attrs: { fontFamily: 'Aptos' } }] }, + { + text: 'Bold', + marks: [{ type: 'textStyle', attrs: { fontFamily: 'Aptos' } }, { type: 'bold' }, { type: 'italic' }], + }, + ], + }; + const paragraphs = buildTocEntryParagraphs([sourceWithMarks], makeConfig({ hyperlinks: true })); + const titleRun = paragraphs[0]!.content[0] as { content?: TextLike[] }; + const texts = titleRun.content!; + expect(texts).toHaveLength(2); + expect(texts[0]!.text).toBe('Plain '); + expect(texts[0]!.marks!.map((m) => m.type)).toEqual(['textStyle', 'link']); + expect(texts[0]!.marks![0].attrs).toEqual({ fontFamily: 'Aptos' }); + expect(texts[1]!.text).toBe('Bold'); + expect(texts[1]!.marks!.map((m) => m.type)).toEqual(['textStyle', 'bold', 'italic', 'link']); + }); + + it('preserves the standalone color, highlight, fontFamily, and underline marks', () => { + const sourceWithMarks: TocSource = { + ...BASE_SOURCE, + segments: [ + { + text: 'Heading', + marks: [ + { type: 'color', attrs: { color: '#ff0000' } }, + { type: 'highlight', attrs: { color: '#ffff00' } }, + { type: 'fontFamily', attrs: { fontFamily: 'Calibri' } }, + { type: 'underline' }, + ], + }, + ], + }; + const paragraphs = buildTocEntryParagraphs([sourceWithMarks], makeConfig({ hyperlinks: true })); + const text = titleTextOf(paragraphs); + expect(text.marks!.map((m) => m.type)).toEqual(['color', 'highlight', 'fontFamily', 'underline', 'link']); + }); + + it('strips fontSize from passthrough textStyle marks (heading sizes must not bleed into TOC)', () => { + const sourceWithFontSize: TocSource = { + ...BASE_SOURCE, + segments: [ + { + text: 'Heading', + marks: [{ type: 'textStyle', attrs: { fontFamily: 'Aptos', fontSize: '24pt' } }, { type: 'bold' }], + }, + ], + }; + const paragraphs = buildTocEntryParagraphs([sourceWithFontSize], makeConfig({ hyperlinks: true })); + const text = titleTextOf(paragraphs); + const textStyleMark = text.marks!.find((m) => m.type === 'textStyle'); + expect(textStyleMark!.attrs).toEqual({ fontFamily: 'Aptos' }); + expect(textStyleMark!.attrs!.fontSize).toBeUndefined(); + // Other allowed marks still flow through. + expect(text.marks!.map((m) => m.type)).toEqual(['textStyle', 'bold', 'link']); + }); + + it('drops textStyle entirely when only disallowed attrs (e.g. fontSize) are present', () => { + const sourceWithOnlyFontSize: TocSource = { + ...BASE_SOURCE, + segments: [ + { + text: 'Heading', + marks: [{ type: 'textStyle', attrs: { fontSize: '24pt' } }], + }, + ], + }; + const paragraphs = buildTocEntryParagraphs([sourceWithOnlyFontSize], makeConfig({ hyperlinks: true })); + const text = titleTextOf(paragraphs); + // Only the rebuilt link should remain. + expect(text.marks!.map((m) => m.type)).toEqual(['link']); + }); + + it('drops the standalone fontSize mark, plus link, comment, track-changes, strike, tocPageNumber', () => { + const sourceWithDisallowed: TocSource = { + ...BASE_SOURCE, + segments: [ + { + text: 'Heading', + marks: [ + { type: 'bold' }, + { type: 'fontSize', attrs: { fontSize: '24pt' } }, + { type: 'strike' }, + { type: 'link', attrs: { href: 'https://example.com' } }, + { type: 'commentMark', attrs: { commentId: 'c1' } }, + { type: 'trackInsert' }, + { type: 'tocPageNumber' }, + ], + }, + ], + }; + const paragraphs = buildTocEntryParagraphs([sourceWithDisallowed], makeConfig({ hyperlinks: true })); + const text = titleTextOf(paragraphs); + expect(text.marks!.map((m) => m.type)).toEqual(['bold', 'link']); + const linkMark = text.marks!.find((m) => m.type === 'link'); + expect(linkMark!.attrs!.anchor).toBe(generateTocBookmarkName(BASE_SOURCE.sdBlockId)); + expect(linkMark!.attrs!.href).toBeUndefined(); + }); }); describe('page numbers (SD-2664)', () => { @@ -256,6 +349,32 @@ describe('collectTocSources', () => { expect(applied.length).toBe(3); }); + it('picks up a freshly-pasted heading whose paraId/sdBlockId were stripped by the slice paste reset', () => { + // Repro for "paste an existing heading, F9, new entry doesn't appear": + // SUPERDOC_SLICE_PASTE_IDENTITY_RESETS clears paraId AND sdBlockId on a + // pasted paragraph. Until the block-node plugin's appendTransaction runs + // and assigns a UUID, the paragraph carries `sdBlockId: null` while still + // having its heading styleId. The TOC scanner must fall back to a + // synthetic id and still surface it as a TOC source. + const docWithPastedHeading = mockDoc([ + { sdBlockId: 'p-existing', text: 'Conclusion 1', styleId: 'Heading2' }, + // Pasted heading, identity reset, plugin hasn't re-stamped yet + { sdBlockId: null, text: 'Conclusion 2', styleId: 'Heading2' }, + ]); + + const config: TocSwitchConfig = { + source: { outlineLevels: { from: 1, to: 3 } }, + display: { hyperlinks: true }, + preserved: {}, + }; + + const sources = collectTocSources(docWithPastedHeading, config); + expect(sources.map((s) => s.text)).toEqual(['Conclusion 1', 'Conclusion 2']); + // The fallback must produce a non-empty sdBlockId so generateTocBookmarkName + // can hash it into a stable anchor for the rebuilt entry. + expect(sources[1].sdBlockId).toBeTruthy(); + }); + it('collects only headings when \\u is not set', () => { const config: TocSwitchConfig = { source: { outlineLevels: { from: 1, to: 3 } }, diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts index dfb7dabfb5..8bf5eb93c9 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts @@ -17,8 +17,16 @@ import { generateTocBookmarkName } from './toc-bookmark-sync.js'; // --------------------------------------------------------------------------- export interface TocSource { - /** Display text for this entry. */ + /** Flat display text for this entry (used as a fallback and for diagnostics). */ text: string; + /** + * Per-text-node segments captured from the source paragraph, preserving the + * character-level marks (bold, italic, color, font…). When present, the + * entry builder emits one styled text node per segment so heading-level + * formatting is reflected in the TOC. Absent for TC fields, where only a + * plain string is available from the field instruction. + */ + segments?: TocTextSegment[]; /** TOC level (1-based). */ level: number; /** @@ -33,6 +41,55 @@ export interface TocSource { omitPageNumber?: boolean; } +/** A run of source text with its surviving character marks. */ +export interface TocTextSegment { + text: string; + marks?: EntryTextMark[]; +} + +/** + * Marks that ARE allowed to flow from the source heading into a TOC entry. + * Anything not on this list is dropped — the TOC mirrors a deliberately + * narrow subset of character formatting from the heading: + * + * - `bold`, `italic`, `underline` — font style. + * - `color` — font color. + * - `highlight` — background color. + * - `fontFamily` — font family. + * - `textStyle` — kept ONLY for its `fontFamily` attribute; `fontSize` and + * any other attributes are scrubbed so heading point sizes do not bleed + * into the (typically smaller) TOC entry size. + * + * Notably excluded: `fontSize`, `link` (TOC has its own anchor), comments, + * track-changes, strike, baseline shifts, and `tocPageNumber`. + */ +const ALLOWED_SOURCE_MARK_TYPES = new Set(['bold', 'italic', 'underline', 'color', 'highlight', 'fontFamily']); + +/** Attributes preserved on a passthrough `textStyle` mark — `fontSize` is dropped. */ +const TEXT_STYLE_ALLOWED_ATTRS = new Set(['fontFamily']); + +/** + * Filters and rewrites a single source mark to the form allowed on a TOC + * entry. Returns `null` when the mark must be dropped entirely. + */ +function sanitizeSourceMark(mark: EntryTextMark): EntryTextMark | null { + if (!mark?.type) return null; + + if (mark.type === 'textStyle') { + const attrs = mark.attrs ?? {}; + const kept: Record = {}; + for (const key of Object.keys(attrs)) { + if (TEXT_STYLE_ALLOWED_ATTRS.has(key) && attrs[key] != null) kept[key] = attrs[key]; + } + return Object.keys(kept).length > 0 ? { type: 'textStyle', attrs: kept } : null; + } + + if (!ALLOWED_SOURCE_MARK_TYPES.has(mark.type)) return null; + return mark.attrs && Object.keys(mark.attrs).length > 0 + ? { type: mark.type, attrs: { ...mark.attrs } } + : { type: mark.type }; +} + // --------------------------------------------------------------------------- // Source collection // --------------------------------------------------------------------------- @@ -83,7 +140,7 @@ export function collectTocSources(doc: ProseMirrorNode, config: TocSwitchConfig) if (outlineLevels) { const headingLevel = getHeadingLevel(styleId); if (headingLevel != null && headingLevel >= outlineLevels.from && headingLevel <= outlineLevels.to) { - sources.push({ text, level: headingLevel, sdBlockId, kind: 'heading' }); + sources.push({ text, segments: extractTextSegments(node), level: headingLevel, sdBlockId, kind: 'heading' }); return true; // descend so TC fields inside this paragraph are still collected } } @@ -95,7 +152,13 @@ export function collectTocSources(doc: ProseMirrorNode, config: TocSwitchConfig) if (rawOutlineLevel != null) { const tocLevel = rawOutlineLevel + 1; if (tocLevel >= effectiveLevels.from && tocLevel <= effectiveLevels.to) { - sources.push({ text, level: tocLevel, sdBlockId, kind: 'appliedOutline' }); + sources.push({ + text, + segments: extractTextSegments(node), + level: tocLevel, + sdBlockId, + kind: 'appliedOutline', + }); return true; } } @@ -150,6 +213,44 @@ function flattenText(node: ProseMirrorNode): string { return text; } +/** + * Walks the paragraph's text descendants and returns one segment per text node, + * preserving the marks Word would carry into the TOC entry. Marks in + * `DROPPED_SOURCE_MARK_TYPES` are stripped, and adjacent segments with + * identical mark sets are coalesced to keep the rebuilt content tidy. + */ +function extractTextSegments(node: ProseMirrorNode): TocTextSegment[] { + const segments: TocTextSegment[] = []; + node.descendants((child) => { + if (!child.isText || !child.text) return true; + const marks: EntryTextMark[] = []; + for (const mark of child.marks ?? []) { + const raw: EntryTextMark = { type: mark.type?.name ?? '' }; + if (mark.attrs && Object.keys(mark.attrs).length > 0) raw.attrs = { ...mark.attrs }; + const sanitized = sanitizeSourceMark(raw); + if (sanitized) marks.push(sanitized); + } + const last = segments[segments.length - 1]; + if (last && marksEqual(last.marks, marks)) { + last.text += child.text; + } else { + segments.push(marks.length > 0 ? { text: child.text, marks } : { text: child.text }); + } + return true; + }); + return segments; +} + +function marksEqual(a: EntryTextMark[] | undefined, b: EntryTextMark[] | undefined): boolean { + const aLen = a?.length ?? 0; + const bLen = b?.length ?? 0; + if (aLen !== bLen) return false; + if (aLen === 0) return true; + // Compare structurally — JSON.stringify is sufficient because attrs are flat + // and the iteration order of ProseMirror marks is stable per text node. + return JSON.stringify(a) === JSON.stringify(b); +} + // --------------------------------------------------------------------------- // Entry paragraph builder // --------------------------------------------------------------------------- @@ -168,23 +269,20 @@ export interface EntryTextMark { /** * Optional context that lets the entry builder produce final-looking output - * (resolved page numbers, preserved tab spacing, sampled font/size marks) - * without a follow-up `mode: 'pageNumbers'` pass. + * (resolved page numbers, preserved tab spacing) without a follow-up + * `mode: 'pageNumbers'` pass. + * + * Run-level formatting is intentionally NOT sampled from the existing TOC. + * Word's "Update field" rebuilds entries from the linked TOC1, TOC2, … + * paragraph styles — it does not copy direct formatting from the first entry. + * Sampling marks from the existing TOC made any direct formatting on entry 1 + * (e.g. bold) leak into every rebuilt entry. */ export interface BuildTocEntryOptions { /** sdBlockId → page number map from PresentationEditor's last layout cycle. */ pageMap?: Map; /** Right-tab stop position (twips) to mirror the existing TOC's spacing. */ tabPos?: number; - /** Marks sampled from the existing TOC entry text. `link` is filtered out and rebuilt. */ - entryTextMarks?: EntryTextMark[]; - /** - * Paragraph-level `` overrides sampled from the existing entry. Word - * stamps these on TOC entries (e.g. `bold: false`, `italic: false`) to - * disable the TOC1 paragraph style's ``. Preserving them keeps - * the rebuilt entries visually identical to the imported ones. - */ - paragraphRunProperties?: Record; } /** @@ -223,23 +321,37 @@ function buildEntryParagraph( ): EntryParagraphJson { const { display } = config; - // Title text. Marks are stacked: sampled (font/size/textStyle/bold/italic) - // first, link last. Wrapped in a `run` so `wrapTextInRunsPlugin` does not - // re-wrap and merge the TOC1 paragraph style's run properties via addToSet, - // which would clobber the sampled `textStyle` mark. - const titleMarks: EntryTextMark[] = (options.entryTextMarks ?? []).filter( - (mark) => mark?.type && mark.type !== 'link', - ); - if (display.hyperlinks) { - titleMarks.push({ - type: 'link', - attrs: { anchor: generateTocBookmarkName(source.sdBlockId), rId: null, history: true }, - }); - } - const titleText: Record = { type: 'text', text: source.text || ' ' }; - if (titleMarks.length > 0) titleText.marks = titleMarks; + // Title text. Character-level marks (bold, italic, color, font…) are + // carried over from the *source heading* — never sampled from the existing + // TOC entry, which would leak entry-1's direct formatting onto every + // rebuilt entry (Word rebuilds entries from the linked TOC1, TOC2, … + // paragraph styles, plus character formatting from the source). + // Each text node is wrapped in a `run` so wrapTextInRunsPlugin does not + // re-wrap and merge the paragraph style's run properties via addToSet. + const linkMark: EntryTextMark | undefined = display.hyperlinks + ? { type: 'link', attrs: { anchor: generateTocBookmarkName(source.sdBlockId), rId: null, history: true } } + : undefined; + + const segments: TocTextSegment[] = + source.segments && source.segments.length > 0 ? source.segments : [{ text: source.text || ' ' }]; + + const titleTextNodes: Array> = segments.map((segment) => { + // Re-apply the allowlist at build time so callers passing hand-built + // segments cannot smuggle in disallowed marks (font-size, link, comments, + // track-changes, etc.). collectTocSources also sanitizes, but the + // builder is the contract boundary that users of buildTocEntryParagraphs + // hit directly — defending here keeps the rule in one place. + const sourceMarks = (segment.marks ?? []) + .map((m) => sanitizeSourceMark(m)) + .filter((m): m is EntryTextMark => m !== null); + const marks: EntryTextMark[] = [...sourceMarks]; + if (linkMark) marks.push(linkMark); + const node: Record = { type: 'text', text: segment.text || ' ' }; + if (marks.length > 0) node.marks = marks; + return node; + }); - const content: Array> = [asRun([titleText])]; + const content: Array> = [asRun(titleTextNodes)]; // Determine whether to omit page number for this entry. const omitRange = display.omitPageNumberLevels; @@ -267,9 +379,6 @@ function buildEntryParagraph( } const paragraphProperties: Record = { styleId: `TOC${source.level}` }; - if (options.paragraphRunProperties && Object.keys(options.paragraphRunProperties).length > 0) { - paragraphProperties.runProperties = { ...options.paragraphRunProperties }; - } const rightAlign = display.rightAlignPageNumbers !== false; // default true if (rightAlign && !omitPageNumber) { diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/pr-3120-regressions.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/pr-3120-regressions.test.ts index 813d48c238..3ea66039e3 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/pr-3120-regressions.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/pr-3120-regressions.test.ts @@ -153,6 +153,156 @@ describe('PR-3120 regression — finding 1: tabLeader: "none" must round-trip', }); }); +describe('PR-3120 regression — review comment: bold/italic on first TOC entry must not leak to all rebuilt entries', () => { + // repro: open a doc whose first TOC entry happens to be bold and "Update field" + // it. Word's behaviour is to rebuild every entry from the linked TOC1, TOC2… + // paragraph styles — direct formatting on the existing entries is discarded. + // SuperDoc used to sample run formatting from the first entry and apply it to + // every rebuilt entry, so a single bold first entry made every entry come out + // bold after the rebuild. + + it('rebuilt entries inherit only the link mark; sampled bold/italic do not leak', () => { + // First TOC entry has bold + italic marks on its title text. The source + // heading is plain, so the rebuild must produce plain entries. + const boldTitle = createNode('text', [], { + text: 'Heading 1', + marks: [{ type: 'bold' }, { type: 'italic' }], + }); + const titleRun = createNode('run', [boldTitle], { isInline: true, inlineContent: true }); + const tabText = createNode('text', [], { text: '\t' }); + const tabRun = createNode('run', [tabText], { isInline: true, inlineContent: true }); + const pageNumText = createNode('text', [], { text: '1', marks: [{ type: 'tocPageNumber' }] }); + const pageNumRun = createNode('run', [pageNumText], { isInline: true, inlineContent: true }); + const entryParagraph = createNode('paragraph', [titleRun, tabRun, pageNumRun], { + attrs: { + sdBlockId: 'toc-entry-p1', + paragraphProperties: { styleId: 'TOC1' }, + tocSourceId: 'h-1', + }, + isBlock: true, + inlineContent: true, + }); + + const tocNode = createNode('tableOfContents', [entryParagraph], { + attrs: { sdBlockId: 'toc-1', instruction: 'TOC \\o "1-3" \\h \\z' }, + isBlock: true, + }); + const heading = createNode('paragraph', [createNode('text', [], { text: 'Heading 1' })], { + attrs: { sdBlockId: 'h-1', paragraphProperties: { styleId: 'Heading1' } }, + isBlock: true, + inlineContent: true, + }); + // Add a second heading so the rebuild produces multiple entries — easier + // to assert that every rebuilt entry is plain. + const heading2 = createNode('paragraph', [createNode('text', [], { text: 'Heading 2' })], { + attrs: { sdBlockId: 'h-2', paragraphProperties: { styleId: 'Heading1' } }, + isBlock: true, + inlineContent: true, + }); + const doc = createNode('doc', [tocNode, heading, heading2], { isBlock: false }); + + let capturedContent: Array> | undefined; + const replaceTableOfContentsContentById = vi.fn((opts: { content: Array> }) => { + capturedContent = opts.content; + return true; + }); + + const editor = { + state: { doc, schema: { nodes: { paragraph: { create: vi.fn() }, tableOfContents: {} } } }, + commands: { + insertTableOfContentsAt: vi.fn(() => true), + setTableOfContentsInstructionById: vi.fn(() => true), + replaceTableOfContentsContentById, + deleteTableOfContentsById: vi.fn(() => true), + }, + schema: { marks: {} }, + options: {}, + storage: { tableOfContents: { pageMap: new Map(), pageMapDoc: doc } }, + on: () => {}, + } as unknown as Editor; + + const result = tocUpdateWrapper( + editor, + { target: { kind: 'block', nodeType: 'tableOfContents', nodeId: 'toc-1' }, mode: 'all' }, + { changeMode: 'direct' }, + ); + expect(result.success).toBe(true); + expect(replaceTableOfContentsContentById).toHaveBeenCalledTimes(1); + + // Walk every rebuilt title text node and confirm only `link` marks survive. + expect(capturedContent).toBeDefined(); + const offendingMarkTypes = new Set(); + const visit = (node: unknown) => { + if (!node || typeof node !== 'object') return; + const n = node as { type?: string; marks?: Array<{ type?: string }>; content?: unknown[] }; + const marks = n.marks ?? []; + for (const m of marks) { + const t = m?.type; + if (t && t !== 'link' && t !== 'tocPageNumber') offendingMarkTypes.add(t); + } + for (const child of n.content ?? []) visit(child); + }; + for (const para of capturedContent ?? []) visit(para); + expect(Array.from(offendingMarkTypes)).toEqual([]); + }); +}); + +describe('PR-3120 regression — review comment: F9 on multiple TOCs must give every TOC real page numbers', () => { + // repro: doc with two or more TOCs, press F9. The first TOC gets real page + // numbers, the rest rebuild as `0`. Each toc.update swaps editor.state.doc, + // so getPageMap rejects the stored page map (still anchored to the previous + // doc snapshot) on the next iteration and falls back to the '0' placeholder. + // The fix lives in the field-update extension: refresh pageMapDoc to the + // current doc before each iteration so the page map stays valid across the + // loop. + + it('field-update keeps the page map valid across iterations when multiple TOCs are updated', () => { + // We simulate the loop body inside FieldUpdate.updateFieldsInSelection by + // calling the same logic inline: snapshot the page map, then for each TOC + // refresh pageMapDoc and dispatch a doc-changing transaction. + const doc1 = createNode('doc', [createNode('paragraph', [], { isBlock: true })], { isBlock: false }); + const cachedPageMap = new Map([ + ['h-1', 3], + ['h-2', 9], + ]); + const tocStorage: { pageMap: Map; pageMapDoc: unknown } = { + pageMap: cachedPageMap, + pageMapDoc: doc1, + }; + const editorState = { doc: doc1 }; + const editor = { state: editorState, storage: { tableOfContents: tocStorage } }; + + // First iteration: storage already matches the doc, page map is fresh. + expect(getPageMapForTest(editor)).toBe(cachedPageMap); + + // toc.update for TOC #1 dispatches a transaction; doc identity changes. + const doc2 = createNode('doc', [createNode('paragraph', [], { isBlock: true })], { isBlock: false }); + editorState.doc = doc2; + + // Without the fix, getPageMapForTest now returns null (stale). + expect(getPageMapForTest(editor)).toBeNull(); + + // Apply the field-update fix: refresh pageMapDoc before iteration #2. + if (tocStorage.pageMap) tocStorage.pageMapDoc = editorState.doc; + + // After the fix, the page map is reusable for the next TOC. + expect(getPageMapForTest(editor)).toBe(cachedPageMap); + }); +}); + +/** Mirror of `getPageMap` in toc-wrappers — duplicated to keep the test self-contained. */ +function getPageMapForTest(editor: { + state: { doc: unknown }; + storage?: Record; +}): Map | null { + const tocStorage = editor.storage?.tableOfContents as + | { pageMap?: Map; pageMapDoc?: unknown } + | undefined; + if (!tocStorage?.pageMap) return null; + if (tocStorage.pageMapDoc !== undefined && tocStorage.pageMapDoc !== editor.state.doc) return null; + return tocStorage.pageMap; +} + describe('PR-3120 regression — finding 2: pageNumbers scanner must traverse run-wrapped page-number text', () => { // The mode:'all' rebuild produces paragraphs whose content is [run, run, run] // where the page-number text is *nested inside* the third run. The scanner diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts index 62f6185510..241008b88f 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts @@ -41,7 +41,6 @@ import { collectTocSources, buildTocEntryParagraphs, type BuildTocEntryOptions, - type EntryTextMark, type EntryParagraphJson, type TocSource, } from '../helpers/toc-entry-builder.js'; @@ -303,19 +302,6 @@ function readExistingTocTabPos(node: ProseMirrorNode): number | undefined { return typeof pos === 'number' ? pos : undefined; } -/** - * Run-property overrides (`` inside ``) sampled from the first - * existing TOC entry. These are paragraph-level overrides that disable - * style-level formatting on rebuild — without preserving them, properties - * like `bold: false` / `italic: false` baked into Word's TOC1 entries would - * fall back to the TOC1 paragraph style's own bold/italic. - */ -function readExistingTocEntryRunProperties(node: ProseMirrorNode): Record | undefined { - const entry = findFirstTocEntryParagraph(node); - const runProps = (entry?.attrs as TocParagraphAttrs | undefined)?.paragraphProperties?.runProperties; - return runProps && Object.keys(runProps).length > 0 ? { ...runProps } : undefined; -} - /** * Word's TOC field always closes with a paragraph that holds the * `` — typically a Normal-styled empty @@ -335,37 +321,6 @@ function readExistingTocTrailingParagraph(node: ProseMirrorNode): unknown | unde return typeof last.toJSON === 'function' ? last.toJSON() : undefined; } -/** - * Run-formatting marks (font, size, bold, italic, textStyle…) sampled from the - * first non-empty text run inside an actual TOC entry paragraph (styleId TOC1–TOC9). - * Skips sibling paragraphs like TOCHeading whose run formatting must not bleed - * into the rebuilt entries. Drops `link` (rebuilt from the source's bookmark) - * and `tocPageNumber` (belongs only to the page-number run). - */ -function readExistingTocEntryTextMarks(node: ProseMirrorNode): EntryTextMark[] { - let marks: EntryTextMark[] = []; - node.forEach((paragraph) => { - if (marks.length > 0 || paragraph.type.name !== 'paragraph') return; - const styleId = (paragraph.attrs as TocParagraphAttrs | undefined)?.paragraphProperties?.styleId; - if (!styleId || !TOC_ENTRY_STYLE_RE.test(styleId)) return; - - paragraph.descendants((child) => { - if (marks.length > 0) return false; - if (!child.isText || !child.text) return true; - const sampled = (child.marks ?? []) - .filter((m) => m.type.name !== 'link' && m.type.name !== 'tocPageNumber') - .map((m) => { - const json: EntryTextMark = { type: m.type.name }; - if (m.attrs && Object.keys(m.attrs).length > 0) json.attrs = { ...m.attrs }; - return json; - }); - if (sampled.length > 0) marks = sampled; - return false; - }); - }); - return marks; -} - // --------------------------------------------------------------------------- // toc.configure // --------------------------------------------------------------------------- @@ -398,8 +353,6 @@ export function tocConfigureWrapper( { pageMap: getPageMap(editor) ?? undefined, tabPos: readExistingTocTabPos(resolved.node), - entryTextMarks: readExistingTocEntryTextMarks(resolved.node), - paragraphRunProperties: readExistingTocEntryRunProperties(resolved.node), }, ); const trailing = readExistingTocTrailingParagraph(resolved.node); @@ -495,11 +448,10 @@ function tocUpdateAll(editor: Editor, input: TocUpdateInput, options?: MutationO { pageMap: getPageMap(editor) ?? undefined, tabPos: readExistingTocTabPos(resolved.node), - entryTextMarks: readExistingTocEntryTextMarks(resolved.node), - paragraphRunProperties: readExistingTocEntryRunProperties(resolved.node), }, ); + console.log('rebuiltEntries', rebuiltEntries); // Preserve the trailer paragraph if the existing TOC ends with one — keeps // the visual gap below the TOC stable across rebuilds. const trailing = readExistingTocTrailingParagraph(resolved.node); @@ -685,31 +637,40 @@ function buildPageNumberUpdatedContent( const tocSourceId = child.attrs?.tocSourceId as string | undefined; const childJson = child.toJSON() as EntryParagraphJson; - const content = childJson.content ?? []; let paragraphChanged = false; - const updatedContentArray = content.map((node: Record) => { + // Walk recursively — the rebuilt paragraph wraps its runs in `run` nodes, + // so the tocPageNumber mark sits one level below the paragraph's direct + // children. A flat scan over `paragraph.content` would miss it and fall + // through to PAGE_NUMBERS_NOT_MATERIALIZED. + const visit = (node: Record): Record => { const marks = node.marks as Array<{ type: string }> | undefined; const hasTocPageNumberMark = marks?.some((m) => m.type === 'tocPageNumber'); - if (!hasTocPageNumberMark) return node; + if (hasTocPageNumberMark) { + hasPageNumberMarks = true; - hasPageNumberMarks = true; + if (!tocSourceId) return node; - // Skip entries without tocSourceId — no anchor for page map lookup - if (!tocSourceId) return node; + const pageNumber = pageMap.get(tocSourceId); + const newText = pageNumber !== undefined ? String(pageNumber) : '??'; - const pageNumber = pageMap.get(tocSourceId); - const newText = pageNumber !== undefined ? String(pageNumber) : '??'; - - if (node.text !== newText) { - paragraphChanged = true; - return { ...node, text: newText }; + if (node.text !== newText) { + paragraphChanged = true; + return { ...node, text: newText }; + } + return node; } - return node; - }); + const nested = node.content as Array> | undefined; + if (!Array.isArray(nested) || nested.length === 0) return node; + const visited = nested.map(visit); + const replaced = visited.some((next, idx) => next !== nested[idx]); + return replaced ? { ...node, content: visited } : node; + }; + + const updatedContentArray = (childJson.content ?? []).map(visit); if (paragraphChanged) { anyChanged = true; diff --git a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js index f0e8120404..2c4115ee00 100644 --- a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js +++ b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js @@ -51,7 +51,23 @@ export const FieldUpdate = Extension.create({ if (tocTargets.length > 0) { if (!dispatch) return true; // can()-style probe + // Each toc.update dispatches its own transaction, which produces + // a new editor.state.doc. The TOC storage's page map is stamped + // with the doc snapshot it was computed against — getPageMap + // rejects the map as stale on every iteration after the first, + // so subsequent TOCs rebuild with placeholder zeros instead of + // real page numbers. Refresh pageMapDoc to the current doc + // before each iteration to keep the map valid across the loop; + // the layout has not been recomputed, so the page numbers from + // the original layout are still authoritative for this update. + const tocStorage = editor.storage?.tableOfContents; + const cachedPageMap = tocStorage?.pageMap ?? null; + for (const sdBlockId of tocTargets) { + if (tocStorage && cachedPageMap) { + tocStorage.pageMap = cachedPageMap; + tocStorage.pageMapDoc = editor.state.doc; + } try { editor.doc.toc.update({ target: { kind: 'block', nodeType: 'tableOfContents', nodeId: sdBlockId }, diff --git a/tests/behavior/tests/navigation/toc-update.spec.ts b/tests/behavior/tests/navigation/toc-update.spec.ts index 0653081a98..550478fb54 100644 --- a/tests/behavior/tests/navigation/toc-update.spec.ts +++ b/tests/behavior/tests/navigation/toc-update.spec.ts @@ -101,3 +101,291 @@ test('@behavior SD-2664: updateFieldsInSelection (F9) rebuilds every TOC entry f // should map to a heading text. Order must match document order. expect(titlesAfter).toEqual(headingTexts); }); + +const PR312_BOLD_DOC = path.resolve(__dirname, '../../test-data/layout/word-fixture-pr-312-bold.docx'); + +test('@behavior SD-2664 review: pasting "Conclusion 2" below itself produces a duplicate TOC entry on context-menu update', async ({ + superdoc, +}) => { + test.skip(!fs.existsSync(PR312_BOLD_DOC), 'word-fixture-pr-312-bold.docx not available'); + + await superdoc.loadDocument(PR312_BOLD_DOC); + await superdoc.waitForStable(2000); + + // Locate the "Conclusion 2" heading paragraph and clone it directly below + // through the SuperDoc slice paste path. This is the same path the + // clipboard handler uses in InputRule.handlePaste — the JSON slice goes + // through Slice.fromJSON and SUPERDOC_SLICE_PASTE_IDENTITY_RESETS. + // The doc stores the title as "Conclusion" + "2" in separate runs (no + // explicit space text node), but the visual rendering shows "Conclusion 2". + // Match on the concatenated text the source scanner sees. + const TARGET_TITLE = 'Conclusion2'; + + const beforeCount = await superdoc.page.evaluate((target: string) => { + const editor = (window as unknown as { editor?: { state: { doc: any } } }).editor; + if (!editor) return 0; + let n = 0; + editor.state.doc.descendants((p: any) => { + if (p?.type?.name === 'tableOfContents') return false; + if (p?.type?.name !== 'paragraph') return true; + const styleId = p.attrs?.paragraphProperties?.styleId; + if (!styleId || !/^Heading[1-9]$/.test(styleId)) return true; + let text = ''; + p.descendants((c: any) => { + if (c.isText && c.text) text += c.text; + return true; + }); + if (text.trim() === target) n += 1; + return true; + }); + return n; + }, TARGET_TITLE); + expect(beforeCount).toBe(1); + + // Reproduce the user-reported bug: real Cmd+C → Cmd+V where the + // cursor lands INSIDE an existing paragraph after the heading. The slice + // MIME survives the round-trip but ProseMirror's replaceSelection still + // merges single-paragraph slices into the cursor's paragraph when openStart=0 + // openEnd=0 — losing the heading styleId. + await superdoc.page.evaluate((target: string) => { + const editor = ( + window as unknown as { + editor?: { + state: { doc: any; tr: any; schema: any }; + view: { dispatch: (tr: any) => void; dom: HTMLElement; state: any }; + }; + } + ).editor; + if (!editor) return; + const { state, view } = editor; + + // Find the source heading and the next non-heading paragraph after it. + let sourceNode: any = null; + let nextParagraphInsidePos = 0; + let sourceEnd = 0; + let foundSource = false; + state.doc.descendants((n: any, pos: number) => { + if (foundSource && nextParagraphInsidePos === 0) { + if (n?.type?.name === 'paragraph' && pos >= sourceEnd) { + // First paragraph that starts at/after the heading's end. + nextParagraphInsidePos = pos + 1; // +1 = inside the paragraph + return false; + } + } + if (n?.type?.name !== 'paragraph') return true; + const styleId = n.attrs?.paragraphProperties?.styleId; + if (!styleId || !/^Heading[1-9]$/.test(styleId)) return true; + let text = ''; + n.descendants((c: any) => { + if (c.isText && c.text) text += c.text; + return true; + }); + if (text.trim() === target) { + sourceNode = n; + sourceEnd = pos + n.nodeSize; + foundSource = true; + } + return true; + }); + if (!sourceNode || !nextParagraphInsidePos) return; + + // Use whatever Selection constructor is in play — get it from the + // current state's selection class so we don't have to depend on PM + // being exposed on `window`. + const TextSelection = (state.selection.constructor.prototype.constructor as any)?.create + ? state.selection.constructor + : null; + if (TextSelection) { + const tr = state.tr.setSelection(TextSelection.create(state.doc, nextParagraphInsidePos)); + view.dispatch(tr); + } + + // Reproduce the user's flow: copy "Conclusion 2" via an inline-only + // selection (the slice the browser emits when the user selects across the + // line and copies — openStart=1/openEnd=1) and paste into the body + // paragraph below. + // + // The browser strips the SUPERDOC_SLICE_MIME for many cross-context paste + // scenarios, so the receiver falls through to handleHtmlPaste. The HTML + // paste path used to drop the paragraph wrapper for any single-paragraph + // slice — that lost the Heading1 styleId on the rebuilt paragraph and + // the F9 rebuild missed the new entry. The fix preserves the wrapper + // when it carries a Heading[1-9] styleId. + if (TextSelection) { + const sourceStart = sourceEnd - sourceNode.nodeSize; + const tr = state.tr.setSelection(TextSelection.create(state.doc, sourceStart + 1, sourceEnd - 1)); + view.dispatch(tr); + + // Reset cursor to inside the next paragraph (body text after Conclusion 2). + const tr2 = view.state.tr.setSelection(TextSelection.create(view.state.doc, nextParagraphInsidePos)); + view.dispatch(tr2); + } + + // Drive a real copy → paste round-trip: select the inline content of the + // heading and dispatch a copy event so the production copy handler in + // ProseMirrorRenderer writes the slice. Without the heading wrapper fix, + // the slice goes onto the clipboard as inline-only (bookmarkStart + 2 + // runs + bookmarkEnd) and the paste lands in the cursor's Normal + // paragraph — TOC misses the new entry. + if (TextSelection) { + const sourceStart = sourceEnd - sourceNode.nodeSize; + const tr = state.tr.setSelection(TextSelection.create(state.doc, sourceStart + 1, sourceEnd - 1)); + view.dispatch(tr); + } + + const copyData = new DataTransfer(); + const copyEvent = new ClipboardEvent('copy', { clipboardData: copyData, bubbles: true, cancelable: true }); + view.dom.dispatchEvent(copyEvent); + + // Reset cursor to inside the next paragraph (body text after Conclusion 2). + if (TextSelection) { + const tr2 = view.state.tr.setSelection(TextSelection.create(view.state.doc, nextParagraphInsidePos)); + view.dispatch(tr2); + } + + // Replay the copied clipboard into a paste event. + const pasteData = new DataTransfer(); + for (const type of copyData.types) { + pasteData.setData(type, copyData.getData(type)); + } + const pasteEvent = new ClipboardEvent('paste', { clipboardData: pasteData, bubbles: true, cancelable: true }); + view.dom.dispatchEvent(pasteEvent); + }, TARGET_TITLE); + + await superdoc.waitForStable(1000); + + // Drive the rebuild via the context-menu code path (editor.doc.toc.update + // with mode 'all'), not F9. The user reported they trigger the rebuild + // through the right-click "Update table of contents" item — which goes + // through tocUpdateWrapper just like F9 but on a per-TOC target. + await superdoc.page.evaluate(() => { + const editor = ( + window as unknown as { + editor?: { state: { doc: any }; doc?: { toc?: { update?: (input: any) => any } } }; + } + ).editor; + if (!editor?.doc?.toc?.update) return; + let sdBlockId: string | null = null; + editor.state.doc.descendants((n: any) => { + if (sdBlockId) return false; + if (n?.type?.name === 'tableOfContents') { + sdBlockId = (n.attrs?.sdBlockId as string) ?? null; + return false; + } + return true; + }); + if (!sdBlockId) return; + editor.doc.toc.update({ + target: { kind: 'block', nodeType: 'tableOfContents', nodeId: sdBlockId }, + mode: 'all', + }); + }); + await superdoc.waitForStable(2000); + + const titlesAfter = await readTocTitles(superdoc); + // After the context-menu "Update table of contents" rebuild, the pasted + // heading should appear as a second TOC entry. + const occurrences = titlesAfter.filter((t) => t === TARGET_TITLE).length; + expect(occurrences).toBe(2); +}); + +test('@behavior SD-2664: pasting an existing heading produces a new TOC entry after F9', async ({ superdoc }) => { + await superdoc.loadDocument(DOC_PATH); + await superdoc.waitForStable(2000); + + // Capture the original heading texts (anchor for assertion). + const headingsBefore = await superdoc.page.evaluate(() => { + const editor = (window as unknown as { editor?: { state: { doc: unknown } } }).editor; + if (!editor?.state?.doc) return []; + const out: string[] = []; + (editor.state.doc as { descendants: (cb: (n: any) => boolean | void) => void }).descendants((node: any) => { + if (node?.type?.name === 'tableOfContents') return false; + if (node?.type?.name !== 'paragraph') return true; + const styleId = node.attrs?.paragraphProperties?.styleId; + if (!styleId || !/^Heading[1-9]$/.test(styleId)) return true; + let text = ''; + node.descendants((c: any) => { + if (c.isText && c.text) text += c.text; + return true; + }); + if (text.trim()) out.push(text.trim()); + return true; + }); + return out; + }); + expect(headingsBefore.length).toBeGreaterThan(0); + const targetHeading = headingsBefore[0]!; + + // Drive an actual SuperDoc slice paste — this is the exact path the + // clipboard handler hits when a user copies a heading inside the editor: + // it replays the JSON slice through Slice.fromJSON and goes through the + // SUPERDOC_SLICE_PASTE_IDENTITY_RESETS reset. + await superdoc.page.evaluate((target: string) => { + const editor = ( + window as unknown as { + editor?: { + state: { doc: any; schema: any; tr: any }; + view: { dispatch: (tr: any) => void; state: any }; + }; + } + ).editor; + if (!editor) return; + const { state, view } = editor; + + // Find the source heading paragraph node. + let sourceNode: any = null; + let sourcePos = 0; + state.doc.descendants((n: any, pos: number) => { + if (sourceNode) return false; + if (n?.type?.name !== 'paragraph') return true; + const styleId = n.attrs?.paragraphProperties?.styleId; + if (!styleId || !/^Heading[1-9]$/.test(styleId)) return true; + let text = ''; + n.descendants((c: any) => { + if (c.isText && c.text) text += c.text; + return true; + }); + if (text.trim() === target) { + sourceNode = n; + sourcePos = pos; + } + return true; + }); + if (!sourceNode) return; + + // Serialize the source paragraph as the clipboard slice does. + const sliceJson = { content: [sourceNode.toJSON()], openStart: 0, openEnd: 0 }; + const sliceData = JSON.stringify(sliceJson); + + // Position cursor at end of doc (where we'll paste). + const tr1 = state.tr; + const TextSelection = (window as any).PMS?.TextSelection; + if (TextSelection) { + tr1.setSelection(TextSelection.create(state.doc, state.doc.content.size)); + view.dispatch(tr1); + } + + // Synthesize a paste event through the editor's handlePaste so we go + // through stripSuperdocSliceBlockIdentities (paraId/sdBlockId reset). + const dataTransfer = new DataTransfer(); + dataTransfer.setData('application/x-superdoc-slice', sliceData); + const pasteEvent = new ClipboardEvent('paste', { clipboardData: dataTransfer }); + view.dom.dispatchEvent(pasteEvent); + + // Mark sourcePos as used so the linter doesn't complain. + void sourcePos; + }, targetHeading); + + // Let block-node's appendTransaction stamp a fresh sdBlockId. + await superdoc.waitForStable(1000); + + // Run F9 → toc.update for every TOC. + await superdoc.executeCommand('updateFieldsInSelection'); + await superdoc.waitForStable(2000); + + const titlesAfter = await readTocTitles(superdoc); + // The target heading should now appear twice — once for the original, once + // for the pasted clone. + const occurrences = titlesAfter.filter((t) => t === targetHeading).length; + expect(occurrences).toBe(2); +}); From 00b11a67915f9293b6ad58e005685e818f28459e Mon Sep 17 00:00:00 2001 From: Gabriel Chittolina Date: Thu, 7 May 2026 15:06:22 -0300 Subject: [PATCH 10/14] refactor: simplified logic --- .../src/editors/v1/core/InputRule.js | 34 +-- .../v1/core/renderers/ProseMirrorRenderer.ts | 8 - .../plan-engine/pr-3120-regressions.test.ts | 56 ---- .../extensions/field-update/field-update.js | 15 +- .../tests/navigation/toc-update.spec.ts | 277 ++++-------------- 5 files changed, 62 insertions(+), 328 deletions(-) diff --git a/packages/super-editor/src/editors/v1/core/InputRule.js b/packages/super-editor/src/editors/v1/core/InputRule.js index 022e61a6a4..9d95088b0a 100644 --- a/packages/super-editor/src/editors/v1/core/InputRule.js +++ b/packages/super-editor/src/editors/v1/core/InputRule.js @@ -32,17 +32,6 @@ import { } from './helpers/superdocClipboardSlice.js'; import { annotateFragmentDomWithClipboardData } from './helpers/clipboardFragmentAnnotate.js'; -/** - * True when a node is a Heading[1-9] paragraph. Used to decide whether a - * pasted paragraph should keep its wrapper (so the heading styleId survives - * the merge into the cursor's surroundings). - */ -function isHeadingParagraph(node) { - if (!node || node.type?.name !== 'paragraph') return false; - const styleId = node.attrs?.paragraphProperties?.styleId; - return typeof styleId === 'string' && /^Heading[1-9]$/i.test(styleId); -} - /** Heuristic: clipboard HTML from SuperDoc copy (slice attrs, list/section metadata). */ export function isSuperdocOriginClipboardHtml(html) { if (!html || typeof html !== 'string') return false; @@ -484,10 +473,8 @@ export function handleHtmlPaste(html, editor, source) { // Check if the pasted content is a single paragraph const isSingleParagraph = doc.childCount === 1 && doc.firstChild.type.name === 'paragraph'; - // Heading paragraphs (Heading1..Heading9) must keep their paragraph - // wrapper so the styleId survives — otherwise pasting a heading into a - // body paragraph silently strips the heading style and the TOC misses - // the new entry on F9. + // Heading paragraphs must keep their wrapper so the styleId survives — + // unwrapping silently strips the heading style and TOC rebuilds miss it. const sourceStyleId = isSingleParagraph ? (doc.firstChild.attrs?.paragraphProperties?.styleId ?? null) : null; const sourceIsHeading = typeof sourceStyleId === 'string' && /^Heading[1-9]$/i.test(sourceStyleId); @@ -893,22 +880,7 @@ function handleSuperdocSlicePaste(sliceData, editor, view, embeddedBodySectPr = const stripped = stripSuperdocSliceBlockIdentities(slice.content); const cleanContent = remapPastedListNumberingInFragment(stripped, editor); - - // When the copied slice carries a Heading[1-9] paragraph, the user expects - // the heading to land as a discrete heading paragraph (matches Word's - // "Update field" behavior on the rebuilt TOC). PM merges open-boundary - // slices into the cursor's paragraph and drops the heading styleId — even - // when the same logical content was selected. Force closed boundaries - // when the leading or trailing paragraph is a heading so the wrapper - // survives the merge. - let openStart = slice.openStart; - let openEnd = slice.openEnd; - const firstChild = cleanContent.firstChild; - const lastChild = cleanContent.lastChild; - if (firstChild && isHeadingParagraph(firstChild)) openStart = 0; - if (lastChild && isHeadingParagraph(lastChild)) openEnd = 0; - - const cleanSlice = new Slice(cleanContent, openStart, openEnd); + const cleanSlice = new Slice(cleanContent, slice.openStart, slice.openEnd); const { dispatch, state } = view; if (!dispatch) return false; diff --git a/packages/super-editor/src/editors/v1/core/renderers/ProseMirrorRenderer.ts b/packages/super-editor/src/editors/v1/core/renderers/ProseMirrorRenderer.ts index 4cf0f2b5eb..5d2e8f8def 100644 --- a/packages/super-editor/src/editors/v1/core/renderers/ProseMirrorRenderer.ts +++ b/packages/super-editor/src/editors/v1/core/renderers/ProseMirrorRenderer.ts @@ -884,14 +884,6 @@ export class ProseMirrorRenderer implements EditorRenderer { let sliceJson = ''; if (from !== to) { const rawSlice = this.view.state.doc.slice(from, to); - // PM produces an inline-only slice (no paragraph wrapper) when the - // selection covers the full content of a paragraph at exact open - // boundaries. Without the wrapper the paragraph's styleId — most - // critically Heading[1-9] — is lost on paste, so the receiving - // editor can't tell that the source was a heading and the TOC - // misses the new entry. When the selection is entirely within a - // single heading paragraph, re-wrap the slice's content in that - // paragraph node and emit it with closed boundaries. const slice = wrapHeadingSelectionAsParagraph(rawSlice, this.view.state); sliceJson = JSON.stringify(slice.toJSON()); clipboardData.setData('application/x-superdoc-slice', sliceJson); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/pr-3120-regressions.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/pr-3120-regressions.test.ts index 3ea66039e3..32dac2c0de 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/pr-3120-regressions.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/pr-3120-regressions.test.ts @@ -247,62 +247,6 @@ describe('PR-3120 regression — review comment: bold/italic on first TOC entry }); }); -describe('PR-3120 regression — review comment: F9 on multiple TOCs must give every TOC real page numbers', () => { - // repro: doc with two or more TOCs, press F9. The first TOC gets real page - // numbers, the rest rebuild as `0`. Each toc.update swaps editor.state.doc, - // so getPageMap rejects the stored page map (still anchored to the previous - // doc snapshot) on the next iteration and falls back to the '0' placeholder. - // The fix lives in the field-update extension: refresh pageMapDoc to the - // current doc before each iteration so the page map stays valid across the - // loop. - - it('field-update keeps the page map valid across iterations when multiple TOCs are updated', () => { - // We simulate the loop body inside FieldUpdate.updateFieldsInSelection by - // calling the same logic inline: snapshot the page map, then for each TOC - // refresh pageMapDoc and dispatch a doc-changing transaction. - const doc1 = createNode('doc', [createNode('paragraph', [], { isBlock: true })], { isBlock: false }); - const cachedPageMap = new Map([ - ['h-1', 3], - ['h-2', 9], - ]); - const tocStorage: { pageMap: Map; pageMapDoc: unknown } = { - pageMap: cachedPageMap, - pageMapDoc: doc1, - }; - const editorState = { doc: doc1 }; - const editor = { state: editorState, storage: { tableOfContents: tocStorage } }; - - // First iteration: storage already matches the doc, page map is fresh. - expect(getPageMapForTest(editor)).toBe(cachedPageMap); - - // toc.update for TOC #1 dispatches a transaction; doc identity changes. - const doc2 = createNode('doc', [createNode('paragraph', [], { isBlock: true })], { isBlock: false }); - editorState.doc = doc2; - - // Without the fix, getPageMapForTest now returns null (stale). - expect(getPageMapForTest(editor)).toBeNull(); - - // Apply the field-update fix: refresh pageMapDoc before iteration #2. - if (tocStorage.pageMap) tocStorage.pageMapDoc = editorState.doc; - - // After the fix, the page map is reusable for the next TOC. - expect(getPageMapForTest(editor)).toBe(cachedPageMap); - }); -}); - -/** Mirror of `getPageMap` in toc-wrappers — duplicated to keep the test self-contained. */ -function getPageMapForTest(editor: { - state: { doc: unknown }; - storage?: Record; -}): Map | null { - const tocStorage = editor.storage?.tableOfContents as - | { pageMap?: Map; pageMapDoc?: unknown } - | undefined; - if (!tocStorage?.pageMap) return null; - if (tocStorage.pageMapDoc !== undefined && tocStorage.pageMapDoc !== editor.state.doc) return null; - return tocStorage.pageMap; -} - describe('PR-3120 regression — finding 2: pageNumbers scanner must traverse run-wrapped page-number text', () => { // The mode:'all' rebuild produces paragraphs whose content is [run, run, run] // where the page-number text is *nested inside* the third run. The scanner diff --git a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js index 2c4115ee00..96f7090998 100644 --- a/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js +++ b/packages/super-editor/src/editors/v1/extensions/field-update/field-update.js @@ -51,15 +51,12 @@ export const FieldUpdate = Extension.create({ if (tocTargets.length > 0) { if (!dispatch) return true; // can()-style probe - // Each toc.update dispatches its own transaction, which produces - // a new editor.state.doc. The TOC storage's page map is stamped - // with the doc snapshot it was computed against — getPageMap - // rejects the map as stale on every iteration after the first, - // so subsequent TOCs rebuild with placeholder zeros instead of - // real page numbers. Refresh pageMapDoc to the current doc - // before each iteration to keep the map valid across the loop; - // the layout has not been recomputed, so the page numbers from - // the original layout are still authoritative for this update. + // Each toc.update swaps editor.state.doc, which makes + // tocStorage.pageMapDoc stale and forces subsequent TOCs to + // rebuild with '0' placeholders. Re-stamp pageMapDoc to the + // current doc each iteration — the layout has not been + // recomputed, so the page numbers from the original layout + // are still authoritative for this update cycle. const tocStorage = editor.storage?.tableOfContents; const cachedPageMap = tocStorage?.pageMap ?? null; diff --git a/tests/behavior/tests/navigation/toc-update.spec.ts b/tests/behavior/tests/navigation/toc-update.spec.ts index 550478fb54..78e0a660f6 100644 --- a/tests/behavior/tests/navigation/toc-update.spec.ts +++ b/tests/behavior/tests/navigation/toc-update.spec.ts @@ -112,46 +112,54 @@ test('@behavior SD-2664 review: pasting "Conclusion 2" below itself produces a d await superdoc.loadDocument(PR312_BOLD_DOC); await superdoc.waitForStable(2000); - // Locate the "Conclusion 2" heading paragraph and clone it directly below - // through the SuperDoc slice paste path. This is the same path the - // clipboard handler uses in InputRule.handlePaste — the JSON slice goes - // through Slice.fromJSON and SUPERDOC_SLICE_PASTE_IDENTITY_RESETS. - // The doc stores the title as "Conclusion" + "2" in separate runs (no - // explicit space text node), but the visual rendering shows "Conclusion 2". - // Match on the concatenated text the source scanner sees. + // The doc stores the heading title as "Conclusion" + "2" in separate runs + // (no space text node), so the source scanner sees the concatenated text. const TARGET_TITLE = 'Conclusion2'; - const beforeCount = await superdoc.page.evaluate((target: string) => { - const editor = (window as unknown as { editor?: { state: { doc: any } } }).editor; - if (!editor) return 0; - let n = 0; - editor.state.doc.descendants((p: any) => { - if (p?.type?.name === 'tableOfContents') return false; - if (p?.type?.name !== 'paragraph') return true; - const styleId = p.attrs?.paragraphProperties?.styleId; - if (!styleId || !/^Heading[1-9]$/.test(styleId)) return true; - let text = ''; - p.descendants((c: any) => { - if (c.isText && c.text) text += c.text; + // Establish a rebuild baseline FIRST. Without this, the post-paste + // assertion would also reflect any unbookmarked headings the rebuild picks + // up that weren't yet materialised — making the test fragile to fixture + // changes. We want to isolate "the pasted heading was preserved". + const updateAllTocs = async () => { + await superdoc.page.evaluate(() => { + const editor = ( + window as unknown as { + editor?: { state: { doc: any }; doc?: { toc?: { update?: (input: any) => any } } }; + } + ).editor; + if (!editor?.doc?.toc?.update) return; + const ids: string[] = []; + editor.state.doc.descendants((n: any) => { + if (n?.type?.name === 'tableOfContents') { + const id = n.attrs?.sdBlockId as string | null | undefined; + if (id) ids.push(id); + return false; + } return true; }); - if (text.trim() === target) n += 1; - return true; + for (const id of ids) { + editor.doc.toc.update({ + target: { kind: 'block', nodeType: 'tableOfContents', nodeId: id }, + mode: 'all', + }); + } }); - return n; - }, TARGET_TITLE); - expect(beforeCount).toBe(1); + await superdoc.waitForStable(1500); + }; - // Reproduce the user-reported bug: real Cmd+C → Cmd+V where the - // cursor lands INSIDE an existing paragraph after the heading. The slice - // MIME survives the round-trip but ProseMirror's replaceSelection still - // merges single-paragraph slices into the cursor's paragraph when openStart=0 - // openEnd=0 — losing the heading styleId. + await updateAllTocs(); + const titlesBaseline = await readTocTitles(superdoc); + const baselineCount = titlesBaseline.filter((t: string) => t === TARGET_TITLE).length; + + // Real copy → paste round-trip: select the inline content of the heading, + // dispatch a copy event so ProseMirrorRenderer's production handler writes + // the slice, then dispatch a paste event with that same clipboard payload + // and a cursor inside the body paragraph below the heading. await superdoc.page.evaluate((target: string) => { const editor = ( window as unknown as { editor?: { - state: { doc: any; tr: any; schema: any }; + state: { doc: any; tr: any; selection: any }; view: { dispatch: (tr: any) => void; dom: HTMLElement; state: any }; }; } @@ -159,16 +167,14 @@ test('@behavior SD-2664 review: pasting "Conclusion 2" below itself produces a d if (!editor) return; const { state, view } = editor; - // Find the source heading and the next non-heading paragraph after it. let sourceNode: any = null; - let nextParagraphInsidePos = 0; let sourceEnd = 0; + let nextParagraphInsidePos = 0; let foundSource = false; state.doc.descendants((n: any, pos: number) => { if (foundSource && nextParagraphInsidePos === 0) { if (n?.type?.name === 'paragraph' && pos >= sourceEnd) { - // First paragraph that starts at/after the heading's end. - nextParagraphInsidePos = pos + 1; // +1 = inside the paragraph + nextParagraphInsidePos = pos + 1; return false; } } @@ -189,203 +195,26 @@ test('@behavior SD-2664 review: pasting "Conclusion 2" below itself produces a d }); if (!sourceNode || !nextParagraphInsidePos) return; - // Use whatever Selection constructor is in play — get it from the - // current state's selection class so we don't have to depend on PM - // being exposed on `window`. - const TextSelection = (state.selection.constructor.prototype.constructor as any)?.create - ? state.selection.constructor - : null; - if (TextSelection) { - const tr = state.tr.setSelection(TextSelection.create(state.doc, nextParagraphInsidePos)); - view.dispatch(tr); - } - - // Reproduce the user's flow: copy "Conclusion 2" via an inline-only - // selection (the slice the browser emits when the user selects across the - // line and copies — openStart=1/openEnd=1) and paste into the body - // paragraph below. - // - // The browser strips the SUPERDOC_SLICE_MIME for many cross-context paste - // scenarios, so the receiver falls through to handleHtmlPaste. The HTML - // paste path used to drop the paragraph wrapper for any single-paragraph - // slice — that lost the Heading1 styleId on the rebuilt paragraph and - // the F9 rebuild missed the new entry. The fix preserves the wrapper - // when it carries a Heading[1-9] styleId. - if (TextSelection) { - const sourceStart = sourceEnd - sourceNode.nodeSize; - const tr = state.tr.setSelection(TextSelection.create(state.doc, sourceStart + 1, sourceEnd - 1)); - view.dispatch(tr); - - // Reset cursor to inside the next paragraph (body text after Conclusion 2). - const tr2 = view.state.tr.setSelection(TextSelection.create(view.state.doc, nextParagraphInsidePos)); - view.dispatch(tr2); - } - - // Drive a real copy → paste round-trip: select the inline content of the - // heading and dispatch a copy event so the production copy handler in - // ProseMirrorRenderer writes the slice. Without the heading wrapper fix, - // the slice goes onto the clipboard as inline-only (bookmarkStart + 2 - // runs + bookmarkEnd) and the paste lands in the cursor's Normal - // paragraph — TOC misses the new entry. - if (TextSelection) { - const sourceStart = sourceEnd - sourceNode.nodeSize; - const tr = state.tr.setSelection(TextSelection.create(state.doc, sourceStart + 1, sourceEnd - 1)); - view.dispatch(tr); - } + const TextSelection = state.selection.constructor; + const sourceStart = sourceEnd - sourceNode.nodeSize; + // Select the heading's inline content and copy. + view.dispatch(state.tr.setSelection(TextSelection.create(state.doc, sourceStart + 1, sourceEnd - 1))); const copyData = new DataTransfer(); - const copyEvent = new ClipboardEvent('copy', { clipboardData: copyData, bubbles: true, cancelable: true }); - view.dom.dispatchEvent(copyEvent); + view.dom.dispatchEvent(new ClipboardEvent('copy', { clipboardData: copyData, bubbles: true, cancelable: true })); - // Reset cursor to inside the next paragraph (body text after Conclusion 2). - if (TextSelection) { - const tr2 = view.state.tr.setSelection(TextSelection.create(view.state.doc, nextParagraphInsidePos)); - view.dispatch(tr2); - } - - // Replay the copied clipboard into a paste event. + // Move cursor into the next paragraph and paste. + view.dispatch(view.state.tr.setSelection(TextSelection.create(view.state.doc, nextParagraphInsidePos))); const pasteData = new DataTransfer(); - for (const type of copyData.types) { - pasteData.setData(type, copyData.getData(type)); - } - const pasteEvent = new ClipboardEvent('paste', { clipboardData: pasteData, bubbles: true, cancelable: true }); - view.dom.dispatchEvent(pasteEvent); + for (const type of copyData.types) pasteData.setData(type, copyData.getData(type)); + view.dom.dispatchEvent(new ClipboardEvent('paste', { clipboardData: pasteData, bubbles: true, cancelable: true })); }, TARGET_TITLE); await superdoc.waitForStable(1000); - - // Drive the rebuild via the context-menu code path (editor.doc.toc.update - // with mode 'all'), not F9. The user reported they trigger the rebuild - // through the right-click "Update table of contents" item — which goes - // through tocUpdateWrapper just like F9 but on a per-TOC target. - await superdoc.page.evaluate(() => { - const editor = ( - window as unknown as { - editor?: { state: { doc: any }; doc?: { toc?: { update?: (input: any) => any } } }; - } - ).editor; - if (!editor?.doc?.toc?.update) return; - let sdBlockId: string | null = null; - editor.state.doc.descendants((n: any) => { - if (sdBlockId) return false; - if (n?.type?.name === 'tableOfContents') { - sdBlockId = (n.attrs?.sdBlockId as string) ?? null; - return false; - } - return true; - }); - if (!sdBlockId) return; - editor.doc.toc.update({ - target: { kind: 'block', nodeType: 'tableOfContents', nodeId: sdBlockId }, - mode: 'all', - }); - }); - await superdoc.waitForStable(2000); - - const titlesAfter = await readTocTitles(superdoc); - // After the context-menu "Update table of contents" rebuild, the pasted - // heading should appear as a second TOC entry. - const occurrences = titlesAfter.filter((t) => t === TARGET_TITLE).length; - expect(occurrences).toBe(2); -}); - -test('@behavior SD-2664: pasting an existing heading produces a new TOC entry after F9', async ({ superdoc }) => { - await superdoc.loadDocument(DOC_PATH); - await superdoc.waitForStable(2000); - - // Capture the original heading texts (anchor for assertion). - const headingsBefore = await superdoc.page.evaluate(() => { - const editor = (window as unknown as { editor?: { state: { doc: unknown } } }).editor; - if (!editor?.state?.doc) return []; - const out: string[] = []; - (editor.state.doc as { descendants: (cb: (n: any) => boolean | void) => void }).descendants((node: any) => { - if (node?.type?.name === 'tableOfContents') return false; - if (node?.type?.name !== 'paragraph') return true; - const styleId = node.attrs?.paragraphProperties?.styleId; - if (!styleId || !/^Heading[1-9]$/.test(styleId)) return true; - let text = ''; - node.descendants((c: any) => { - if (c.isText && c.text) text += c.text; - return true; - }); - if (text.trim()) out.push(text.trim()); - return true; - }); - return out; - }); - expect(headingsBefore.length).toBeGreaterThan(0); - const targetHeading = headingsBefore[0]!; - - // Drive an actual SuperDoc slice paste — this is the exact path the - // clipboard handler hits when a user copies a heading inside the editor: - // it replays the JSON slice through Slice.fromJSON and goes through the - // SUPERDOC_SLICE_PASTE_IDENTITY_RESETS reset. - await superdoc.page.evaluate((target: string) => { - const editor = ( - window as unknown as { - editor?: { - state: { doc: any; schema: any; tr: any }; - view: { dispatch: (tr: any) => void; state: any }; - }; - } - ).editor; - if (!editor) return; - const { state, view } = editor; - - // Find the source heading paragraph node. - let sourceNode: any = null; - let sourcePos = 0; - state.doc.descendants((n: any, pos: number) => { - if (sourceNode) return false; - if (n?.type?.name !== 'paragraph') return true; - const styleId = n.attrs?.paragraphProperties?.styleId; - if (!styleId || !/^Heading[1-9]$/.test(styleId)) return true; - let text = ''; - n.descendants((c: any) => { - if (c.isText && c.text) text += c.text; - return true; - }); - if (text.trim() === target) { - sourceNode = n; - sourcePos = pos; - } - return true; - }); - if (!sourceNode) return; - - // Serialize the source paragraph as the clipboard slice does. - const sliceJson = { content: [sourceNode.toJSON()], openStart: 0, openEnd: 0 }; - const sliceData = JSON.stringify(sliceJson); - - // Position cursor at end of doc (where we'll paste). - const tr1 = state.tr; - const TextSelection = (window as any).PMS?.TextSelection; - if (TextSelection) { - tr1.setSelection(TextSelection.create(state.doc, state.doc.content.size)); - view.dispatch(tr1); - } - - // Synthesize a paste event through the editor's handlePaste so we go - // through stripSuperdocSliceBlockIdentities (paraId/sdBlockId reset). - const dataTransfer = new DataTransfer(); - dataTransfer.setData('application/x-superdoc-slice', sliceData); - const pasteEvent = new ClipboardEvent('paste', { clipboardData: dataTransfer }); - view.dom.dispatchEvent(pasteEvent); - - // Mark sourcePos as used so the linter doesn't complain. - void sourcePos; - }, targetHeading); - - // Let block-node's appendTransaction stamp a fresh sdBlockId. - await superdoc.waitForStable(1000); - - // Run F9 → toc.update for every TOC. - await superdoc.executeCommand('updateFieldsInSelection'); - await superdoc.waitForStable(2000); + await updateAllTocs(); const titlesAfter = await readTocTitles(superdoc); - // The target heading should now appear twice — once for the original, once - // for the pasted clone. - const occurrences = titlesAfter.filter((t) => t === targetHeading).length; - expect(occurrences).toBe(2); + const afterCount = titlesAfter.filter((t: string) => t === TARGET_TITLE).length; + // The pasted heading must add exactly one more entry to the rebuild. + expect(afterCount).toBe(baselineCount + 1); }); From cd9b42cdc3c0d0fd7c4707c03d20afdd7355214d Mon Sep 17 00:00:00 2001 From: Gabriel Chittolina Date: Thu, 7 May 2026 15:07:55 -0300 Subject: [PATCH 11/14] refactor: removed unnecessary test suite --- .../plan-engine/pr-3120-regressions.test.ts | 331 ------------------ 1 file changed, 331 deletions(-) delete mode 100644 packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/pr-3120-regressions.test.ts diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/pr-3120-regressions.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/pr-3120-regressions.test.ts deleted file mode 100644 index 32dac2c0de..0000000000 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/pr-3120-regressions.test.ts +++ /dev/null @@ -1,331 +0,0 @@ -import type { Node as ProseMirrorNode } from 'prosemirror-model'; -import { describe, expect, it, vi } from 'vitest'; -import type { Editor } from '../../core/Editor.js'; -import type { PlanReceipt } from '@superdoc/document-api'; - -vi.mock('./plan-wrappers.js', () => ({ - executeDomainCommand: vi.fn((_editor: Editor, handler: () => boolean): PlanReceipt => { - const applied = handler(); - return { - success: true, - revision: { before: '0', after: '0' }, - steps: [ - { - stepId: 'step-1', - op: 'domain.command', - effect: applied ? 'changed' : 'noop', - matchCount: applied ? 1 : 0, - data: { domain: 'command', commandDispatched: applied }, - }, - ], - timing: { totalMs: 0 }, - }; - }), -})); - -import { - applyTocPatch, - serializeTocInstruction, - parseTocInstruction, -} from '../../core/super-converter/field-references/shared/toc-switches.js'; -import { tocUpdateWrapper } from './toc-wrappers.js'; - -type NodeOptions = { - attrs?: Record; - text?: string; - isInline?: boolean; - isBlock?: boolean; - isLeaf?: boolean; - inlineContent?: boolean; - nodeSize?: number; - marks?: Array<{ type: string | { name: string } }>; -}; - -function createNode(typeName: string, children: ProseMirrorNode[] = [], options: NodeOptions = {}): ProseMirrorNode { - const attrs = options.attrs ?? {}; - const text = options.text ?? ''; - const isText = typeName === 'text'; - const isInline = options.isInline ?? isText; - const isBlock = options.isBlock ?? !isInline; - const marks = (options.marks ?? []).map((m) => { - const name = typeof m.type === 'string' ? m.type : m.type.name; - return { type: { name } }; - }); - const node: any = { - type: { name: typeName, isInline, isBlock, isText }, - attrs, - text, - isText, - isBlock, - isInline, - isLeaf: options.isLeaf ?? (isText || children.length === 0), - inlineContent: options.inlineContent ?? false, - nodeSize: options.nodeSize ?? (isText ? text.length : 1), - childCount: children.length, - children, - marks, - forEach(cb: (child: ProseMirrorNode, offset: number, index: number) => void) { - let off = 0; - children.forEach((c, i) => { - cb(c, off, i); - off += (c as any).nodeSize ?? 1; - }); - }, - descendants(cb: (n: ProseMirrorNode, pos: number, parent: ProseMirrorNode | null) => boolean | void) { - const walk = (n: any, pos: number, parent: any) => { - const r = cb(n, pos, parent); - if (r === false) return; - let off = pos + (n.isText ? 0 : 1); - (n.children ?? []).forEach((c: any) => { - walk(c, off, n); - off += c.nodeSize ?? 1; - }); - }; - (children ?? []).forEach((c: any, i: number) => { - walk(c, i, node); - }); - }, - toJSON() { - const out: any = { type: typeName }; - if (Object.keys(attrs).length > 0) out.attrs = attrs; - if (isText && text) out.text = text; - if (marks.length > 0) { - out.marks = marks.map((m: any) => ({ type: m.type.name })); - } - if (children.length > 0) { - out.content = children.map((c: any) => c.toJSON()); - } - return out; - }, - nodeAt() { - return null; - }, - cut() { - return node; - }, - resolve() { - return null; - }, - }; - // expose firstChild / lastChild - Object.defineProperty(node, 'firstChild', { get: () => children[0] ?? null }); - Object.defineProperty(node, 'lastChild', { get: () => children[children.length - 1] ?? null }); - return node as ProseMirrorNode; -} - -describe('PR-3120 regression — finding 1: tabLeader: "none" must round-trip', () => { - it('survives applyTocPatch → serialize → parse back to none', () => { - // Start from a TOC with hyphen leader (so we have a separator to clear) - const initial = parseTocInstruction('TOC \\o "1-3" \\p "-"'); - expect(initial.display.tabLeader).toBe('hyphen'); - expect(initial.display.separator).toBe('-'); - - // User sets explicit no-leader - const configured = applyTocPatch(initial, { tabLeader: 'none' }); - expect(configured.display.tabLeader).toBe('none'); - - // Serialize → store on the node → reopen → parse back - const serialized = serializeTocInstruction(configured); - const reparsed = parseTocInstruction(serialized); - - // BUG: reparsed.display.tabLeader is undefined here, not 'none'. - // The 'none' signal is lost because serializeTocInstruction skips \p when - // separator is missing, and parseTocInstruction has no way to distinguish - // "no \p" (default = dots) from "explicit none" without a marker. - expect(reparsed.display.tabLeader).toBe('none'); - }); - - it('configuring tabLeader: "none" on a default-leader TOC must not silently no-op', () => { - // Start from the default TOC (no \p) - const initial = parseTocInstruction('TOC \\o "1-3"'); - expect(initial.display.tabLeader).toBeUndefined(); - expect(initial.display.separator).toBeUndefined(); - - const configured = applyTocPatch(initial, { tabLeader: 'none' }); - expect(configured.display.tabLeader).toBe('none'); - - // BUG: serialization is identical to the original (both omit \p), so the - // wrapper's areTocConfigsEqual will report NO_OP and the user's setting - // never reaches the stored instruction. - const initialSerialized = serializeTocInstruction(initial); - const configuredSerialized = serializeTocInstruction(configured); - expect(configuredSerialized).not.toBe(initialSerialized); - }); -}); - -describe('PR-3120 regression — review comment: bold/italic on first TOC entry must not leak to all rebuilt entries', () => { - // repro: open a doc whose first TOC entry happens to be bold and "Update field" - // it. Word's behaviour is to rebuild every entry from the linked TOC1, TOC2… - // paragraph styles — direct formatting on the existing entries is discarded. - // SuperDoc used to sample run formatting from the first entry and apply it to - // every rebuilt entry, so a single bold first entry made every entry come out - // bold after the rebuild. - - it('rebuilt entries inherit only the link mark; sampled bold/italic do not leak', () => { - // First TOC entry has bold + italic marks on its title text. The source - // heading is plain, so the rebuild must produce plain entries. - const boldTitle = createNode('text', [], { - text: 'Heading 1', - marks: [{ type: 'bold' }, { type: 'italic' }], - }); - const titleRun = createNode('run', [boldTitle], { isInline: true, inlineContent: true }); - const tabText = createNode('text', [], { text: '\t' }); - const tabRun = createNode('run', [tabText], { isInline: true, inlineContent: true }); - const pageNumText = createNode('text', [], { text: '1', marks: [{ type: 'tocPageNumber' }] }); - const pageNumRun = createNode('run', [pageNumText], { isInline: true, inlineContent: true }); - const entryParagraph = createNode('paragraph', [titleRun, tabRun, pageNumRun], { - attrs: { - sdBlockId: 'toc-entry-p1', - paragraphProperties: { styleId: 'TOC1' }, - tocSourceId: 'h-1', - }, - isBlock: true, - inlineContent: true, - }); - - const tocNode = createNode('tableOfContents', [entryParagraph], { - attrs: { sdBlockId: 'toc-1', instruction: 'TOC \\o "1-3" \\h \\z' }, - isBlock: true, - }); - const heading = createNode('paragraph', [createNode('text', [], { text: 'Heading 1' })], { - attrs: { sdBlockId: 'h-1', paragraphProperties: { styleId: 'Heading1' } }, - isBlock: true, - inlineContent: true, - }); - // Add a second heading so the rebuild produces multiple entries — easier - // to assert that every rebuilt entry is plain. - const heading2 = createNode('paragraph', [createNode('text', [], { text: 'Heading 2' })], { - attrs: { sdBlockId: 'h-2', paragraphProperties: { styleId: 'Heading1' } }, - isBlock: true, - inlineContent: true, - }); - const doc = createNode('doc', [tocNode, heading, heading2], { isBlock: false }); - - let capturedContent: Array> | undefined; - const replaceTableOfContentsContentById = vi.fn((opts: { content: Array> }) => { - capturedContent = opts.content; - return true; - }); - - const editor = { - state: { doc, schema: { nodes: { paragraph: { create: vi.fn() }, tableOfContents: {} } } }, - commands: { - insertTableOfContentsAt: vi.fn(() => true), - setTableOfContentsInstructionById: vi.fn(() => true), - replaceTableOfContentsContentById, - deleteTableOfContentsById: vi.fn(() => true), - }, - schema: { marks: {} }, - options: {}, - storage: { tableOfContents: { pageMap: new Map(), pageMapDoc: doc } }, - on: () => {}, - } as unknown as Editor; - - const result = tocUpdateWrapper( - editor, - { target: { kind: 'block', nodeType: 'tableOfContents', nodeId: 'toc-1' }, mode: 'all' }, - { changeMode: 'direct' }, - ); - expect(result.success).toBe(true); - expect(replaceTableOfContentsContentById).toHaveBeenCalledTimes(1); - - // Walk every rebuilt title text node and confirm only `link` marks survive. - expect(capturedContent).toBeDefined(); - const offendingMarkTypes = new Set(); - const visit = (node: unknown) => { - if (!node || typeof node !== 'object') return; - const n = node as { type?: string; marks?: Array<{ type?: string }>; content?: unknown[] }; - const marks = n.marks ?? []; - for (const m of marks) { - const t = m?.type; - if (t && t !== 'link' && t !== 'tocPageNumber') offendingMarkTypes.add(t); - } - for (const child of n.content ?? []) visit(child); - }; - for (const para of capturedContent ?? []) visit(para); - expect(Array.from(offendingMarkTypes)).toEqual([]); - }); -}); - -describe('PR-3120 regression — finding 2: pageNumbers scanner must traverse run-wrapped page-number text', () => { - // The mode:'all' rebuild produces paragraphs whose content is [run, run, run] - // where the page-number text is *nested inside* the third run. The scanner - // currently inspects only immediate paragraph children, so it misses the mark - // and reports PAGE_NUMBERS_NOT_MATERIALIZED. - - it('finds tocPageNumber when the marked text is nested inside a run wrapper', () => { - // Build a paragraph that mirrors the rebuild output: - // paragraph - // run ← immediate child, no marks - // text "Heading 1" - // run ← immediate child, no marks - // tab - // run ← immediate child, no marks - // text "0" with marks=[{type:'tocPageNumber'}] - const titleText = createNode('text', [], { text: 'Heading 1' }); - const titleRun = createNode('run', [titleText], { - isInline: true, - inlineContent: true, - }); - const tabText = createNode('text', [], { text: '\t' }); - const tabRun = createNode('run', [tabText], { - isInline: true, - inlineContent: true, - }); - const pageNumText = createNode('text', [], { - text: '0', - marks: [{ type: 'tocPageNumber' }], - }); - const pageNumRun = createNode('run', [pageNumText], { - isInline: true, - inlineContent: true, - }); - const entryParagraph = createNode('paragraph', [titleRun, tabRun, pageNumRun], { - attrs: { - sdBlockId: 'toc-entry-p1', - paragraphProperties: { styleId: 'TOC1' }, - tocSourceId: 'h-1', - }, - isBlock: true, - inlineContent: true, - }); - const tocNode = createNode('tableOfContents', [entryParagraph], { - attrs: { sdBlockId: 'toc-1', instruction: 'TOC \\o "1-3" \\h \\z' }, - isBlock: true, - }); - const heading = createNode('paragraph', [createNode('text', [], { text: 'Heading 1' })], { - attrs: { sdBlockId: 'h-1', paragraphProperties: { styleId: 'Heading1' } }, - isBlock: true, - inlineContent: true, - }); - const doc = createNode('doc', [tocNode, heading], { isBlock: false }); - - const editor = { - state: { doc, schema: { nodes: { paragraph: { create: vi.fn() }, tableOfContents: {} } } }, - commands: { - insertTableOfContentsAt: vi.fn(() => true), - setTableOfContentsInstructionById: vi.fn(() => true), - replaceTableOfContentsContentById: vi.fn(() => true), - deleteTableOfContentsById: vi.fn(() => true), - }, - schema: { marks: {} }, - options: {}, - storage: { - tableOfContents: { - pageMap: new Map([['h-1', 7]]), - pageMapDoc: doc, - }, - }, - on: () => {}, - } as unknown as Editor; - - const tocTarget = { kind: 'block', nodeType: 'tableOfContents', nodeId: 'toc-1' } as const; - const result = tocUpdateWrapper(editor, { target: tocTarget, mode: 'pageNumbers' }, { changeMode: 'direct' }); - - // BUG: the scanner inspects only immediate paragraph children (run nodes, - // which carry no marks here) and misses the nested tocPageNumber mark on - // the text inside the third run. So it reports PAGE_NUMBERS_NOT_MATERIALIZED - // even though the marks ARE in the entry, just one level deeper. - expect(result.success).toBe(true); - }); -}); From 8494659570b3d9cdafa4fb4d85ec6579a66cf1bf Mon Sep 17 00:00:00 2001 From: Gabriel Chittolina Date: Thu, 7 May 2026 15:18:52 -0300 Subject: [PATCH 12/14] refactor: simplified tests --- .../helpers/toc-entry-builder.test.ts | 77 +++++-------------- .../helpers/toc-entry-builder.ts | 5 +- 2 files changed, 20 insertions(+), 62 deletions(-) diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts index 7b2ae004fc..bedddbccc1 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.test.ts @@ -133,84 +133,42 @@ describe('buildTocEntryParagraphs', () => { runs.forEach((r) => expect(r.type).toBe('run')); }); - it('carries the allowed character marks from the source heading into the rebuilt entry', () => { - const sourceWithMarks: TocSource = { - ...BASE_SOURCE, - segments: [ - { text: 'Plain ', marks: [{ type: 'textStyle', attrs: { fontFamily: 'Aptos' } }] }, - { - text: 'Bold', - marks: [{ type: 'textStyle', attrs: { fontFamily: 'Aptos' } }, { type: 'bold' }, { type: 'italic' }], - }, - ], - }; - const paragraphs = buildTocEntryParagraphs([sourceWithMarks], makeConfig({ hyperlinks: true })); - const titleRun = paragraphs[0]!.content[0] as { content?: TextLike[] }; - const texts = titleRun.content!; - expect(texts).toHaveLength(2); - expect(texts[0]!.text).toBe('Plain '); - expect(texts[0]!.marks!.map((m) => m.type)).toEqual(['textStyle', 'link']); - expect(texts[0]!.marks![0].attrs).toEqual({ fontFamily: 'Aptos' }); - expect(texts[1]!.text).toBe('Bold'); - expect(texts[1]!.marks!.map((m) => m.type)).toEqual(['textStyle', 'bold', 'italic', 'link']); - }); - - it('preserves the standalone color, highlight, fontFamily, and underline marks', () => { + it('carries allowed character marks (bold, italic, underline, color, highlight, fontFamily, textStyle.fontFamily) from the source heading', () => { const sourceWithMarks: TocSource = { ...BASE_SOURCE, segments: [ { text: 'Heading', marks: [ + { type: 'textStyle', attrs: { fontFamily: 'Aptos', fontSize: '24pt' } }, // fontSize must be scrubbed + { type: 'bold' }, + { type: 'italic' }, + { type: 'underline' }, { type: 'color', attrs: { color: '#ff0000' } }, { type: 'highlight', attrs: { color: '#ffff00' } }, { type: 'fontFamily', attrs: { fontFamily: 'Calibri' } }, - { type: 'underline' }, ], }, ], }; const paragraphs = buildTocEntryParagraphs([sourceWithMarks], makeConfig({ hyperlinks: true })); const text = titleTextOf(paragraphs); - expect(text.marks!.map((m) => m.type)).toEqual(['color', 'highlight', 'fontFamily', 'underline', 'link']); - }); - - it('strips fontSize from passthrough textStyle marks (heading sizes must not bleed into TOC)', () => { - const sourceWithFontSize: TocSource = { - ...BASE_SOURCE, - segments: [ - { - text: 'Heading', - marks: [{ type: 'textStyle', attrs: { fontFamily: 'Aptos', fontSize: '24pt' } }, { type: 'bold' }], - }, - ], - }; - const paragraphs = buildTocEntryParagraphs([sourceWithFontSize], makeConfig({ hyperlinks: true })); - const text = titleTextOf(paragraphs); + expect(text.marks!.map((m) => m.type)).toEqual([ + 'textStyle', + 'bold', + 'italic', + 'underline', + 'color', + 'highlight', + 'fontFamily', + 'link', + ]); + // textStyle keeps fontFamily, drops fontSize. const textStyleMark = text.marks!.find((m) => m.type === 'textStyle'); expect(textStyleMark!.attrs).toEqual({ fontFamily: 'Aptos' }); - expect(textStyleMark!.attrs!.fontSize).toBeUndefined(); - // Other allowed marks still flow through. - expect(text.marks!.map((m) => m.type)).toEqual(['textStyle', 'bold', 'link']); - }); - - it('drops textStyle entirely when only disallowed attrs (e.g. fontSize) are present', () => { - const sourceWithOnlyFontSize: TocSource = { - ...BASE_SOURCE, - segments: [ - { - text: 'Heading', - marks: [{ type: 'textStyle', attrs: { fontSize: '24pt' } }], - }, - ], - }; - const paragraphs = buildTocEntryParagraphs([sourceWithOnlyFontSize], makeConfig({ hyperlinks: true })); - const text = titleTextOf(paragraphs); - // Only the rebuilt link should remain. - expect(text.marks!.map((m) => m.type)).toEqual(['link']); }); - it('drops the standalone fontSize mark, plus link, comment, track-changes, strike, tocPageNumber', () => { + it('drops disallowed marks (fontSize, strike, link, comments, track-changes, tocPageNumber)', () => { const sourceWithDisallowed: TocSource = { ...BASE_SOURCE, segments: [ @@ -230,6 +188,7 @@ describe('buildTocEntryParagraphs', () => { }; const paragraphs = buildTocEntryParagraphs([sourceWithDisallowed], makeConfig({ hyperlinks: true })); const text = titleTextOf(paragraphs); + // Only the allowed `bold` survives, plus the rebuilt `link` to the source bookmark. expect(text.marks!.map((m) => m.type)).toEqual(['bold', 'link']); const linkMark = text.marks!.find((m) => m.type === 'link'); expect(linkMark!.attrs!.anchor).toBe(generateTocBookmarkName(BASE_SOURCE.sdBlockId)); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts index 8bf5eb93c9..d5545eda50 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/toc-entry-builder.ts @@ -215,9 +215,8 @@ function flattenText(node: ProseMirrorNode): string { /** * Walks the paragraph's text descendants and returns one segment per text node, - * preserving the marks Word would carry into the TOC entry. Marks in - * `DROPPED_SOURCE_MARK_TYPES` are stripped, and adjacent segments with - * identical mark sets are coalesced to keep the rebuilt content tidy. + * sanitised through `sanitizeSourceMark`. Adjacent segments with identical + * mark sets are coalesced to keep the rebuilt content tidy. */ function extractTextSegments(node: ProseMirrorNode): TocTextSegment[] { const segments: TocTextSegment[] = []; From 46d177bde183aee18de34ea83b9c95a03181329f Mon Sep 17 00:00:00 2001 From: Gabriel Chittolina Date: Thu, 7 May 2026 15:19:10 -0300 Subject: [PATCH 13/14] chore: small comment tweaks --- .../editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts index 241008b88f..de6eeec507 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/toc-wrappers.ts @@ -451,7 +451,6 @@ function tocUpdateAll(editor: Editor, input: TocUpdateInput, options?: MutationO }, ); - console.log('rebuiltEntries', rebuiltEntries); // Preserve the trailer paragraph if the existing TOC ends with one — keeps // the visual gap below the TOC stable across rebuilds. const trailing = readExistingTocTrailingParagraph(resolved.node); From 4811a3678cc38ee59a1e37cc8a48879692fdf043 Mon Sep 17 00:00:00 2001 From: Gabriel Chittolina Date: Thu, 7 May 2026 18:04:09 -0300 Subject: [PATCH 14/14] test: added behavior test for multiple TOCs updates --- .../tests/navigation/toc-update.spec.ts | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/tests/behavior/tests/navigation/toc-update.spec.ts b/tests/behavior/tests/navigation/toc-update.spec.ts index 78e0a660f6..b87923183d 100644 --- a/tests/behavior/tests/navigation/toc-update.spec.ts +++ b/tests/behavior/tests/navigation/toc-update.spec.ts @@ -218,3 +218,93 @@ test('@behavior SD-2664 review: pasting "Conclusion 2" below itself produces a d // The pasted heading must add exactly one more entry to the rebuild. expect(afterCount).toBe(baselineCount + 1); }); + +test('@behavior SD-2664 review: F9 rebuilds page numbers for every TOC in a multi-TOC document', async ({ + superdoc, +}) => { + await superdoc.loadDocument(DOC_PATH); + await superdoc.waitForStable(2000); + + // Clone the imported TOC node and insert a copy at the end of the doc, so + // the doc has two TOCs that should rebuild from the same headings. + const tocCount = await superdoc.page.evaluate(() => { + const editor = ( + window as unknown as { + editor?: { state: { doc: any; tr: any }; view: { dispatch: (tr: any) => void } }; + } + ).editor; + if (!editor) return 0; + + let sourceToc: any = null; + editor.state.doc.descendants((n: any) => { + if (sourceToc) return false; + if (n?.type?.name === 'tableOfContents') { + sourceToc = n; + return false; + } + return true; + }); + if (!sourceToc) return 0; + + // Fresh sdBlockId so the two TOCs have distinct identities. + const cleanAttrs = { ...sourceToc.attrs, sdBlockId: null }; + const clone = sourceToc.type.create(cleanAttrs, sourceToc.content, sourceToc.marks); + const tr = editor.state.tr.insert(editor.state.doc.content.size, clone); + editor.view.dispatch(tr); + + let count = 0; + editor.state.doc.descendants((n: any) => { + if (n?.type?.name === 'tableOfContents') { + count += 1; + return false; + } + return true; + }); + return count; + }); + // Some other plugin may dedupe, so guard the precondition we rely on. + expect(tocCount).toBeGreaterThanOrEqual(2); + + // Wait for layout to recompute the page map after the insertion. + await superdoc.waitForStable(2000); + + // F9 → updateFieldsInSelection iterates every TOC. Without the page-map + // refresh in field-update.js, only the FIRST TOC rebuilds with real page + // numbers; subsequent TOCs see the stored pageMapDoc as stale (its + // snapshot was taken before this iteration's transaction) and fall back + // to '0' placeholders. + await superdoc.executeCommand('updateFieldsInSelection'); + await superdoc.waitForStable(2000); + + // Pull the page-number text for every entry in every TOC. + const tocPageNumbers = await superdoc.page.evaluate(() => { + const editor = (window as unknown as { editor?: { state: { doc: any } } }).editor; + if (!editor) return [] as string[][]; + const result: string[][] = []; + + editor.state.doc.descendants((toc: any) => { + if (toc?.type?.name !== 'tableOfContents') return true; + + const numbers: string[] = []; + toc.descendants((leaf: any) => { + if (!leaf.isText || !leaf.text) return true; + const isPageNumber = (leaf.marks ?? []).some((m: any) => m.type?.name === 'tocPageNumber'); + if (isPageNumber) numbers.push(leaf.text); + return true; + }); + if (numbers.length > 0) result.push(numbers); + return false; + }); + + return result; + }); + + expect(tocPageNumbers.length).toBe(tocCount); + // Every TOC must have at least one entry with a non-zero page number — + // the bug surfaces as every entry in the second+ TOC reading "0". + for (const numbers of tocPageNumbers) { + expect(numbers.length).toBeGreaterThan(0); + const allZero = numbers.every((n) => n === '0'); + expect(allZero).toBe(false); + } +});