From a9b898d9d907b4205ce0f8e4e1b083d958e5c600 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Wed, 27 May 2026 09:33:33 -0700 Subject: [PATCH] chore: fix tests --- .../src/contract/operation-definitions.ts | 4 +- .../v1/core/super-converter/exporter.js | 17 +- .../contract-conformance.test.ts | 150 ++++++++++++ .../v1/tests/export/docxExporter.test.js | 39 ++++ ...-final-doc-delinstrtext-regression.test.js | 221 ++++++++++++++++++ 5 files changed, 421 insertions(+), 10 deletions(-) create mode 100644 packages/super-editor/src/editors/v1/tests/import-export/tracked-changes-final-doc-delinstrtext-regression.test.js diff --git a/packages/document-api/src/contract/operation-definitions.ts b/packages/document-api/src/contract/operation-definitions.ts index 5214f96fa8..df14717b87 100644 --- a/packages/document-api/src/contract/operation-definitions.ts +++ b/packages/document-api/src/contract/operation-definitions.ts @@ -972,7 +972,7 @@ export const OPERATION_DEFINITIONS = { idempotency: 'conditional', supportsDryRun: true, supportsTrackedMode: true, - possibleFailureCodes: ['INVALID_TARGET'], + possibleFailureCodes: ['INVALID_TARGET', 'NO_OP'], throws: [...T_NOT_FOUND_CAPABLE, 'INVALID_TARGET', 'INVALID_INPUT', ...T_STORY], }), referenceDocPath: 'format/apply.mdx', @@ -1054,7 +1054,7 @@ export const OPERATION_DEFINITIONS = { idempotency: 'conditional', supportsDryRun: true, supportsTrackedMode: true, - possibleFailureCodes: ['INVALID_TARGET'], + possibleFailureCodes: ['INVALID_TARGET', 'NO_OP'], throws: [...T_NOT_FOUND_CAPABLE, 'INVALID_TARGET', 'INVALID_INPUT', ...T_STORY], }), referenceDocPath: 'format/apply.mdx', diff --git a/packages/super-editor/src/editors/v1/core/super-converter/exporter.js b/packages/super-editor/src/editors/v1/core/super-converter/exporter.js index 3c3aa348c1..de2b377c07 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/exporter.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/exporter.js @@ -696,16 +696,15 @@ export class DocxExporter { * }; * // Returns: ['', 'Textcontent', ''] */ - #generateXml(node) { + #generateXml(node, insideDeletion = false) { if (!node) return null; let { name } = node; const { elements, attributes } = node; - // Normalize w:delInstrText โ†’ w:instrText. During import, w:del wrappers around - // field character runs lose their trackDelete marks (only text content gets marked), - // so on export the w:del wrapper is absent. Per ECMA-376 ยง17.16.13, w:delInstrText - // outside w:del is non-conformant โ€” renaming to w:instrText keeps the field valid. - if (name === 'w:delInstrText') { + // Normalize w:delInstrText โ†’ w:instrText only when the field instruction is + // no longer inside a surviving w:del wrapper. Inside w:del, ECMA-376 expects + // w:delInstrText to remain intact. + if (name === 'w:delInstrText' && !insideDeletion) { name = 'w:instrText'; } @@ -726,8 +725,10 @@ export class DocxExporter { return this.#replaceSpecialCharacters(node.text ?? ''); } + const nextInsideDeletion = insideDeletion || name === 'w:del'; + if (elements) { - if (name === 'w:instrText') { + if (name === 'w:instrText' || name === 'w:delInstrText') { const textContent = (elements || []) .map((child) => (typeof child?.text === 'string' ? child.text : '')) .join(''); @@ -754,7 +755,7 @@ export class DocxExporter { } else { if (elements) { for (let child of elements) { - const newElements = this.#generateXml(child); + const newElements = this.#generateXml(child, nextInsideDeletion); if (!newElements) { continue; } diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/__conformance__/contract-conformance.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/__conformance__/contract-conformance.test.ts index de2f3b874d..8518379881 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/__conformance__/contract-conformance.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/__conformance__/contract-conformance.test.ts @@ -7,6 +7,7 @@ import { MUTATING_OPERATION_IDS, OPERATION_IDS, buildInternalContractSchemas, + createDocumentApi, textReceiptToSDReceipt, type InlineRunPatchKey, type OperationId, @@ -18,6 +19,7 @@ import { } from '../../extensions/track-changes/constants.js'; import { ListHelpers } from '../../core/helpers/list-numbering-helpers.js'; import { createCommentsWrapper } from '../plan-engine/comments-wrappers.js'; +import { assembleDocumentApiAdapters } from '../assemble-adapters.js'; import { createParagraphWrapper, createHeadingWrapper } from '../plan-engine/create-wrappers.js'; import { blocksDeleteWrapper, blocksDeleteRangeWrapper } from '../plan-engine/blocks-wrappers.js'; import { clearContentWrapper } from '../plan-engine/clear-content-wrapper.js'; @@ -521,6 +523,56 @@ type MockParagraphNode = { textContent: string; }; +function resolveMockNodePosition(root: ProseMirrorNode, pos: number) { + const path: Array<{ node: ProseMirrorNode; start: number }> = [{ node: root, start: -1 }]; + + const walk = (node: ProseMirrorNode, contentStart: number): void => { + const children = ((node as unknown as { _children?: ProseMirrorNode[] })._children ?? []) as ProseMirrorNode[]; + let offset = contentStart; + + for (const child of children) { + const childStart = offset; + const childEnd = childStart + child.nodeSize; + if (pos >= childStart && pos < childEnd) { + path.push({ node: child, start: childStart }); + const grandChildren = ((child as unknown as { _children?: ProseMirrorNode[] })._children ?? + []) as ProseMirrorNode[]; + if (grandChildren.length > 0) { + walk(child, childStart + 1); + } + return; + } + offset = childEnd; + } + }; + + walk(root, 0); + + return { + depth: path.length - 1, + node(depth: number) { + return path[depth]?.node ?? root; + }, + before(depth: number) { + return path[depth]?.start ?? 0; + }, + start(depth: number) { + if (depth <= 0) return 0; + return (path[depth]?.start ?? 0) + 1; + }, + end(depth: number) { + const entry = path[depth]; + if (!entry) return 0; + return (path[depth]?.start ?? 0) + 1 + entry.node.content.size; + }, + after(depth: number) { + const entry = path[depth]; + if (!entry) return 0; + return (path[depth]?.start ?? 0) + entry.node.nodeSize; + }, + }; +} + function createNode(typeName: string, children: ProseMirrorNode[] = [], options: NodeOptions = {}): ProseMirrorNode { const attrs = options.attrs ?? {}; const text = options.text ?? ''; @@ -600,10 +652,42 @@ function createNode(typeName: string, children: ProseMirrorNode[] = [], options: } walk(children, 0); }, + nodesBetween(from: number, to: number, callback: (node: ProseMirrorNode, pos: number) => boolean | void) { + function walk(kids: ProseMirrorNode[], startPos: number) { + let offset = startPos; + for (const child of kids) { + const childStart = offset; + const childEnd = childStart + child.nodeSize; + if (childEnd < from) { + offset = childEnd; + continue; + } + if (childStart > to) { + break; + } + const result = callback(child, childStart); + if (result !== false) { + const innerKids = (child as unknown as { _children?: ProseMirrorNode[] })._children; + if (innerKids && innerKids.length > 0) { + walk(innerKids, childStart + 1); + } + } + offset = childEnd; + } + } + walk(children, 0); + }, + resolve(pos: number) { + return resolveMockNodePosition(node as unknown as ProseMirrorNode, pos); + }, }; return node as unknown as ProseMirrorNode; } +function makeDocumentApiForEditor(editor: Editor) { + return createDocumentApi(assembleDocumentApiAdapters(editor)); +} + function makeTextEditor( text = 'Hello', overrides: Partial & { @@ -768,6 +852,7 @@ function makeTextEditor( })), dispatch, ...overrides, + on: vi.fn(), schema: { marks: baseMarks, ...(overrides.schema ?? {}), @@ -4468,6 +4553,53 @@ const mutationVectors: Partial> = { ); }, }, + formatRange: { + throwCase: () => { + const { editor } = makeTextEditor(); + const api = makeDocumentApiForEditor(editor); + return api.formatRange( + { + target: { + kind: 'selection', + start: { kind: 'text', blockId: 'missing', offset: 0 }, + end: { kind: 'text', blockId: 'missing', offset: 1 }, + }, + properties: { bold: true }, + }, + { changeMode: 'direct' }, + ); + }, + failureCase: () => { + const { editor } = makeTextEditor(); + const api = makeDocumentApiForEditor(editor); + return api.formatRange( + { + target: { + kind: 'selection', + start: { kind: 'text', blockId: 'p1', offset: 2 }, + end: { kind: 'text', blockId: 'p1', offset: 2 }, + }, + properties: { bold: true }, + }, + { changeMode: 'direct' }, + ); + }, + applyCase: () => { + const { editor } = makeTextEditor(); + const api = makeDocumentApiForEditor(editor); + return api.formatRange( + { + target: { + kind: 'selection', + start: { kind: 'text', blockId: 'p1', offset: 0 }, + end: { kind: 'text', blockId: 'p1', offset: 5 }, + }, + properties: { bold: true, italic: false }, + }, + { changeMode: 'direct' }, + ); + }, + }, ...formatInlineMutationVectors, ...paragraphMutationVectors, 'create.paragraph': { @@ -8971,6 +9103,24 @@ const dryRunVectors: Partial unknown>> = { expect(tr.addMark).not.toHaveBeenCalled(); return result; }, + formatRange: () => { + const { editor, dispatch, tr } = makeTextEditor(); + const api = makeDocumentApiForEditor(editor); + const result = api.formatRange( + { + target: { + kind: 'selection', + start: { kind: 'text', blockId: 'p1', offset: 0 }, + end: { kind: 'text', blockId: 'p1', offset: 5 }, + }, + properties: { bold: true }, + }, + { changeMode: 'direct', dryRun: true }, + ); + expect(dispatch).not.toHaveBeenCalled(); + expect(tr.addMark).not.toHaveBeenCalled(); + return result; + }, ...formatInlineDryRunVectors, ...paragraphDryRunVectors, 'create.paragraph': () => { diff --git a/packages/super-editor/src/editors/v1/tests/export/docxExporter.test.js b/packages/super-editor/src/editors/v1/tests/export/docxExporter.test.js index 72a0ebecc3..41e466aacb 100644 --- a/packages/super-editor/src/editors/v1/tests/export/docxExporter.test.js +++ b/packages/super-editor/src/editors/v1/tests/export/docxExporter.test.js @@ -502,6 +502,45 @@ describe('DocxExporter', () => { expect(xml).toContain('REF _Ref258418237'); }); + it('preserves w:delInstrText when it remains inside w:del', () => { + const exporter = new DocxExporter(createConverterStub()); + + const data = { + name: 'w:document', + attributes: {}, + elements: [ + { + name: 'w:del', + attributes: { 'w:id': '1' }, + elements: [ + { + name: 'w:r', + attributes: {}, + elements: [ + { + name: 'w:delInstrText', + attributes: { 'xml:space': 'preserve' }, + elements: [ + { + type: 'text', + text: ' PAGE \\\\* MERGEFORMAT ', + }, + ], + }, + ], + }, + ], + }, + ], + }; + + const xml = exporter.schemaToXml(data); + + expect(xml).toContain(' { const exporter = new DocxExporter(createConverterStub()); diff --git a/packages/super-editor/src/editors/v1/tests/import-export/tracked-changes-final-doc-delinstrtext-regression.test.js b/packages/super-editor/src/editors/v1/tests/import-export/tracked-changes-final-doc-delinstrtext-regression.test.js new file mode 100644 index 0000000000..4c76474ed4 --- /dev/null +++ b/packages/super-editor/src/editors/v1/tests/import-export/tracked-changes-final-doc-delinstrtext-regression.test.js @@ -0,0 +1,221 @@ +import path from 'node:path'; +import { describe, expect, it } from 'vitest'; +import JSZip from 'jszip'; +import * as xmljs from 'xml-js'; + +import { Editor } from '@core/Editor.js'; +import { getTestDataAsFileBuffer, initTestEditor } from '../helpers/helpers.js'; + +const FOOTER_RELATIONSHIP_TYPE = 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/footer'; + +const deepClone = (value) => JSON.parse(JSON.stringify(value)); + +const getDirectChild = (node, name) => { + return (node?.elements ?? []).find((child) => child.name === name); +}; + +const collectRelationshipTargets = (relationshipsXml) => { + const parsed = xmljs.xml2js(relationshipsXml, { compact: false }); + const relationshipsRoot = parsed.elements?.find((element) => element.name === 'Relationships'); + return (relationshipsRoot?.elements ?? []) + .filter((element) => element.name === 'Relationship') + .map((element) => ({ + target: element.attributes?.Target ?? '', + targetMode: element.attributes?.TargetMode ?? '', + type: element.attributes?.Type ?? '', + })); +}; + +const collectContentTypeOverrides = (contentTypesXml) => { + const parsed = xmljs.xml2js(contentTypesXml, { compact: false }); + const typesRoot = parsed.elements?.find((element) => element.name === 'Types'); + return (typesRoot?.elements ?? []) + .filter((element) => element.name === 'Override') + .map((element) => ({ + partName: element.attributes?.PartName ?? '', + contentType: element.attributes?.ContentType ?? '', + })); +}; + +const partContainsInstrTextInsideDeletion = (xml) => { + const parsed = xmljs.xml2js(xml, { compact: false }); + let invalid = false; + + const visit = (node, insideDeletion = false) => { + if (!node || typeof node !== 'object' || invalid) return; + + if (node.name === 'w:instrText' && insideDeletion) { + invalid = true; + return; + } + + const nextInsideDeletion = insideDeletion || node.name === 'w:del'; + for (const child of node.elements ?? []) { + visit(child, nextInsideDeletion); + if (invalid) return; + } + }; + + for (const element of parsed.elements ?? []) { + visit(element, false); + if (invalid) break; + } + + return invalid; +}; + +const collectInvalidDelInstrParts = async (zip) => { + const invalidParts = []; + + for (const xmlPath of Object.keys(zip.files).filter((name) => name.endsWith('.xml'))) { + const xml = await zip.file(xmlPath)?.async('string'); + if (!xml) continue; + if (partContainsInstrTextInsideDeletion(xml)) invalidParts.push(xmlPath); + } + + return invalidParts; +}; + +const collectMissingRelationshipTargets = async (zip) => { + const missingTargets = []; + + for (const relPath of Object.keys(zip.files).filter((name) => name.endsWith('.rels'))) { + const relXml = await zip.file(relPath)?.async('string'); + if (!relXml) continue; + + const relDir = path.posix.dirname(relPath); + const sourceDir = relDir.endsWith('_rels') ? path.posix.dirname(relDir) : relDir; + + for (const { target, targetMode } of collectRelationshipTargets(relXml)) { + if ( + !target || + targetMode === 'External' || + target.startsWith('http:') || + target.startsWith('https:') || + target.startsWith('mailto:') || + target.startsWith('#') + ) { + continue; + } + + const resolvedTarget = path.posix.normalize(path.posix.join(sourceDir, target)); + if (!zip.file(resolvedTarget)) missingTargets.push(`${relPath} -> ${target}`); + } + } + + return missingTargets; +}; + +async function buildDeletedFieldInstructionFooterDocx() { + // Start from a public fixture that already has valid footer refs/content types. + const baseBuffer = await getTestDataAsFileBuffer('basic-page-nums.docx'); + const zip = await JSZip.loadAsync(baseBuffer); + const footerXml = await zip.file('word/footer1.xml')?.async('string'); + + if (!footerXml) { + throw new Error('basic-page-nums.docx is missing word/footer1.xml'); + } + + const footerJson = xmljs.xml2js(footerXml, { compact: false }); + const footerRoot = footerJson.elements?.find((element) => element.name === 'w:ftr'); + if (!footerRoot) { + throw new Error('word/footer1.xml is missing the w:ftr root'); + } + + const sdt = getDirectChild(footerRoot, 'w:sdt'); + const sdtContent = getDirectChild(sdt, 'w:sdtContent'); + const footerParagraph = (sdtContent?.elements ?? []).find((element) => element.name === 'w:p'); + if (!footerParagraph?.elements) { + throw new Error('word/footer1.xml is missing the page-number paragraph'); + } + + const instrRunIndex = footerParagraph.elements.findIndex( + (element) => element.name === 'w:r' && getDirectChild(element, 'w:instrText'), + ); + + if (instrRunIndex === -1) { + throw new Error('word/footer1.xml is missing the PAGE field instruction run'); + } + + const instrRun = footerParagraph.elements[instrRunIndex]; + const instrText = getDirectChild(instrRun, 'w:instrText'); + const instrRunProperties = getDirectChild(instrRun, 'w:rPr'); + + footerParagraph.elements.splice(instrRunIndex, 1, { + type: 'element', + name: 'w:del', + attributes: { + 'w:id': '1544', + 'w:author': 'Regression Test', + 'w:date': '2024-01-01T00:00:00Z', + }, + elements: [ + { + type: 'element', + name: 'w:r', + elements: [ + ...(instrRunProperties ? [deepClone(instrRunProperties)] : []), + { + type: 'element', + name: 'w:delInstrText', + attributes: { + 'xml:space': instrText?.attributes?.['xml:space'] ?? 'preserve', + }, + elements: [{ type: 'text', text: ' PAGE \\* MERGEFORMAT ' }], + }, + ], + }, + ], + }); + + zip.file('word/footer1.xml', xmljs.js2xml(footerJson, { compact: false, spaces: 0 })); + return zip.generateAsync({ type: 'nodebuffer' }); +} + +describe('tracked-change normalization deleted field-instruction regression', () => { + it('does not emit w:instrText inside surviving w:del wrappers during final-doc export', async () => { + const source = await buildDeletedFieldInstructionFooterDocx(); + const sourceZip = await JSZip.loadAsync(source); + const sourceFooter = await sourceZip.file('word/footer1.xml')?.async('string'); + + expect(sourceFooter).toContain(' entry.partName === '/word/footer1.xml', + ); + const footerRelationships = collectRelationshipTargets(documentRels).filter( + (entry) => entry.type === FOOTER_RELATIONSHIP_TYPE, + ); + + expect(invalidDelInstrParts).toEqual([]); + expect(missingTargets).toEqual([]); + expect(footerOverrides).toHaveLength(1); + expect(footerRelationships.map((entry) => entry.target)).toContain('footer1.xml'); + } finally { + editor.destroy(); + } + }); +});