diff --git a/packages/super-editor/src/editors/v1/core/super-converter/SuperConverter.js b/packages/super-editor/src/editors/v1/core/super-converter/SuperConverter.js index 4e453eb2de..56a0976ef6 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/SuperConverter.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/SuperConverter.js @@ -34,11 +34,6 @@ import { syncBibliographyPartToPackage, getBibliographyPartExportPaths, } from './citation-sources.js'; -import { - collectReferencedNumIds, - filterOrphanedNumberingDefinitions, -} from './export-helpers/strip-orphaned-numbering.js'; - const FONT_FAMILY_FALLBACKS = Object.freeze({ swiss: 'Arial, sans-serif', roman: 'Times New Roman, serif', @@ -1434,13 +1429,17 @@ class SuperConverter { if (!numberingXml) numberingXml = baseNumbering; const currentNumberingXml = numberingXml.elements[0]; - // D7: Strip orphaned numbering definitions (entries not referenced by any - // paragraph in the exported document parts). - const referencedNumIds = collectReferencedNumIds(this.convertedXml); - + // SD-2911: emit every abstractNum / num the importer captured. The previous + // implementation pruned definitions whose numId wasn't referenced from the + // exported document parts, but that couldn't distinguish "user just deleted a + // list in this session" from "definition was always unused in the source file" + // (Word's tentative numbering). Both arrived here with no referencing paragraph, + // and both were dropped — silently lossy on round-trip. Word tolerates unused + // definitions, so the safe default is to preserve. if (this.numbering?.definitions && this.numbering?.abstracts) { - const { liveAbstracts, liveDefinitions } = filterOrphanedNumberingDefinitions(this.numbering, referencedNumIds); - currentNumberingXml.elements = [...liveAbstracts, ...liveDefinitions]; + const abstracts = Object.values(this.numbering.abstracts); + const definitions = Object.values(this.numbering.definitions); + currentNumberingXml.elements = [...abstracts, ...definitions]; } else { currentNumberingXml.elements = []; } diff --git a/packages/super-editor/src/editors/v1/core/super-converter/export-helpers/strip-orphaned-numbering.js b/packages/super-editor/src/editors/v1/core/super-converter/export-helpers/strip-orphaned-numbering.js deleted file mode 100644 index c6ae4611f7..0000000000 --- a/packages/super-editor/src/editors/v1/core/super-converter/export-helpers/strip-orphaned-numbering.js +++ /dev/null @@ -1,77 +0,0 @@ -/** - * Collect all w:numId values referenced in exported document parts. - * Walks all word/* XML entries except word/numbering.xml itself. - * - * @param {Record} convertedXml - The full set of exported XML-JSON objects - * @returns {Set} Set of numId values referenced in the document - */ -export function collectReferencedNumIds(convertedXml) { - const numIds = new Set(); - - function walkElements(elements) { - if (!Array.isArray(elements)) return; - for (const el of elements) { - if (!el || typeof el !== 'object') continue; - if (el.name === 'w:numId' && el.attributes?.['w:val'] != null) { - numIds.add(Number(el.attributes['w:val'])); - } - if (el.elements) walkElements(el.elements); - } - } - - for (const [path, xml] of Object.entries(convertedXml)) { - if (path.startsWith('word/') && path !== 'word/numbering.xml' && xml?.elements) { - walkElements(xml.elements); - } - } - - return numIds; -} - -/** - * Extract the w:abstractNumId value from a w:num XML-JSON element. - * - * @param {object} numDef - A w:num XML-JSON element from numbering.definitions - * @returns {number | undefined} The abstractNumId, or undefined if not found - */ -function getAbstractNumIdFromDef(numDef) { - const abstractEl = numDef.elements?.find((el) => el.name === 'w:abstractNumId'); - if (abstractEl?.attributes?.['w:val'] != null) { - return Number(abstractEl.attributes['w:val']); - } - return undefined; -} - -/** - * Filter numbering definitions to remove orphaned entries not referenced by - * any paragraph in the exported document. Returns new arrays (does not mutate). - * - * @param {{ abstracts: Record, definitions: Record }} numbering - * The converter's numbering data (abstracts keyed by abstractNumId, definitions keyed by numId) - * @param {Set} referencedNumIds - * The set of numId values actually referenced in the exported document - * @returns {{ liveAbstracts: any[], liveDefinitions: any[] }} - * Filtered arrays ready to be written to word/numbering.xml - */ -export function filterOrphanedNumberingDefinitions(numbering, referencedNumIds) { - // Keep only w:num entries whose numId is still referenced - const liveDefinitions = Object.values(numbering.definitions).filter((def) => - referencedNumIds.has(Number(def.attributes?.['w:numId'])), - ); - - // Derive the set of abstractNumIds referenced by surviving w:num entries - const referencedAbstractIds = new Set(); - for (const def of liveDefinitions) { - const abstractId = getAbstractNumIdFromDef(def); - if (abstractId != null) { - referencedAbstractIds.add(abstractId); - } - } - - // Keep only w:abstractNum entries still referenced by a surviving w:num - const liveAbstracts = Object.values(numbering.abstracts).filter((abs) => - referencedAbstractIds.has(Number(abs.attributes?.['w:abstractNumId'])), - ); - - return { liveAbstracts, liveDefinitions }; -} diff --git a/packages/super-editor/src/editors/v1/core/super-converter/export-helpers/strip-orphaned-numbering.test.js b/packages/super-editor/src/editors/v1/core/super-converter/export-helpers/strip-orphaned-numbering.test.js deleted file mode 100644 index 2d2499ade5..0000000000 --- a/packages/super-editor/src/editors/v1/core/super-converter/export-helpers/strip-orphaned-numbering.test.js +++ /dev/null @@ -1,245 +0,0 @@ -import { describe, it, expect } from 'vitest'; -import { collectReferencedNumIds, filterOrphanedNumberingDefinitions } from './strip-orphaned-numbering.js'; - -// --------------------------------------------------------------------------- -// Helpers for building XML-JSON structures -// --------------------------------------------------------------------------- - -function makeNumIdElement(numId) { - return { - name: 'w:numId', - type: 'element', - attributes: { 'w:val': String(numId) }, - }; -} - -function makeParagraphWithNumId(numId) { - return { - name: 'w:p', - type: 'element', - elements: [ - { - name: 'w:pPr', - type: 'element', - elements: [ - { - name: 'w:numPr', - type: 'element', - elements: [makeNumIdElement(numId), { name: 'w:ilvl', type: 'element', attributes: { 'w:val': '0' } }], - }, - ], - }, - ], - }; -} - -function makeDocumentXml(paragraphs) { - return { - elements: [ - { - name: 'w:document', - type: 'element', - elements: [{ name: 'w:body', type: 'element', elements: paragraphs }], - }, - ], - }; -} - -function makeNumDef(numId, abstractNumId, extraElements = []) { - return { - name: 'w:num', - type: 'element', - attributes: { 'w:numId': String(numId) }, - elements: [ - { - name: 'w:abstractNumId', - type: 'element', - attributes: { 'w:val': String(abstractNumId) }, - }, - ...extraElements, - ], - }; -} - -function makeAbstractDef(abstractNumId) { - return { - name: 'w:abstractNum', - type: 'element', - attributes: { 'w:abstractNumId': String(abstractNumId) }, - elements: [], - }; -} - -// --------------------------------------------------------------------------- -// collectReferencedNumIds -// --------------------------------------------------------------------------- - -describe('collectReferencedNumIds', () => { - it('collects numIds from document body paragraphs', () => { - const convertedXml = { - 'word/document.xml': makeDocumentXml([makeParagraphWithNumId(1), makeParagraphWithNumId(3)]), - }; - const result = collectReferencedNumIds(convertedXml); - expect(result).toEqual(new Set([1, 3])); - }); - - it('collects numIds from headers and footers', () => { - const convertedXml = { - 'word/document.xml': makeDocumentXml([makeParagraphWithNumId(1)]), - 'word/header1.xml': { - elements: [{ name: 'w:hdr', type: 'element', elements: [makeParagraphWithNumId(5)] }], - }, - 'word/footer1.xml': { - elements: [{ name: 'w:ftr', type: 'element', elements: [makeParagraphWithNumId(7)] }], - }, - }; - const result = collectReferencedNumIds(convertedXml); - expect(result).toEqual(new Set([1, 5, 7])); - }); - - it('ignores word/numbering.xml to avoid self-referencing', () => { - const convertedXml = { - 'word/document.xml': makeDocumentXml([makeParagraphWithNumId(1)]), - 'word/numbering.xml': { - elements: [ - { - name: 'w:numbering', - type: 'element', - elements: [makeNumDef(1, 10), makeNumDef(99, 20)], - }, - ], - }, - }; - const result = collectReferencedNumIds(convertedXml); - // Only numId 1 from document body — numId 99 from numbering.xml should NOT appear - expect(result).toEqual(new Set([1])); - }); - - it('ignores non-word paths', () => { - const convertedXml = { - 'word/document.xml': makeDocumentXml([makeParagraphWithNumId(1)]), - 'docProps/custom.xml': { elements: [makeParagraphWithNumId(999)] }, - }; - const result = collectReferencedNumIds(convertedXml); - expect(result).toEqual(new Set([1])); - }); - - it('returns empty set when no paragraphs have numbering', () => { - const convertedXml = { - 'word/document.xml': makeDocumentXml([{ name: 'w:p', type: 'element', elements: [] }]), - }; - const result = collectReferencedNumIds(convertedXml); - expect(result).toEqual(new Set()); - }); - - it('deduplicates repeated numIds', () => { - const convertedXml = { - 'word/document.xml': makeDocumentXml([ - makeParagraphWithNumId(2), - makeParagraphWithNumId(2), - makeParagraphWithNumId(2), - ]), - }; - const result = collectReferencedNumIds(convertedXml); - expect(result).toEqual(new Set([2])); - }); -}); - -// --------------------------------------------------------------------------- -// filterOrphanedNumberingDefinitions -// --------------------------------------------------------------------------- - -describe('filterOrphanedNumberingDefinitions', () => { - it('keeps definitions referenced by document paragraphs', () => { - const numbering = { - abstracts: { 10: makeAbstractDef(10) }, - definitions: { 1: makeNumDef(1, 10) }, - }; - const referencedNumIds = new Set([1]); - - const { liveAbstracts, liveDefinitions } = filterOrphanedNumberingDefinitions(numbering, referencedNumIds); - - expect(liveDefinitions).toHaveLength(1); - expect(liveDefinitions[0].attributes['w:numId']).toBe('1'); - expect(liveAbstracts).toHaveLength(1); - expect(liveAbstracts[0].attributes['w:abstractNumId']).toBe('10'); - }); - - it('strips orphaned w:num not referenced by any paragraph', () => { - const numbering = { - abstracts: { 10: makeAbstractDef(10), 20: makeAbstractDef(20) }, - definitions: { 1: makeNumDef(1, 10), 99: makeNumDef(99, 20) }, - }; - // Only numId 1 is referenced — numId 99 is orphaned - const referencedNumIds = new Set([1]); - - const { liveAbstracts, liveDefinitions } = filterOrphanedNumberingDefinitions(numbering, referencedNumIds); - - expect(liveDefinitions).toHaveLength(1); - expect(liveDefinitions[0].attributes['w:numId']).toBe('1'); - // abstractNum 20 is also orphaned (only referenced by stripped numId 99) - expect(liveAbstracts).toHaveLength(1); - expect(liveAbstracts[0].attributes['w:abstractNumId']).toBe('10'); - }); - - it('keeps abstract shared by multiple w:num when at least one survives', () => { - const numbering = { - abstracts: { 10: makeAbstractDef(10) }, - definitions: { - 1: makeNumDef(1, 10), - 2: makeNumDef(2, 10), // same abstract as numId 1 - 3: makeNumDef(3, 10), // orphaned — not referenced - }, - }; - const referencedNumIds = new Set([1, 2]); - - const { liveAbstracts, liveDefinitions } = filterOrphanedNumberingDefinitions(numbering, referencedNumIds); - - expect(liveDefinitions).toHaveLength(2); - expect(liveAbstracts).toHaveLength(1); - expect(liveAbstracts[0].attributes['w:abstractNumId']).toBe('10'); - }); - - it('strips all definitions when no numIds are referenced', () => { - const numbering = { - abstracts: { 10: makeAbstractDef(10) }, - definitions: { 1: makeNumDef(1, 10) }, - }; - const referencedNumIds = new Set(); - - const { liveAbstracts, liveDefinitions } = filterOrphanedNumberingDefinitions(numbering, referencedNumIds); - - expect(liveDefinitions).toHaveLength(0); - expect(liveAbstracts).toHaveLength(0); - }); - - it('handles empty numbering gracefully', () => { - const numbering = { abstracts: {}, definitions: {} }; - const referencedNumIds = new Set([1]); - - const { liveAbstracts, liveDefinitions } = filterOrphanedNumberingDefinitions(numbering, referencedNumIds); - - expect(liveDefinitions).toHaveLength(0); - expect(liveAbstracts).toHaveLength(0); - }); - - it('preserves w:num entries with lvlOverride elements', () => { - const lvlOverride = { - name: 'w:lvlOverride', - type: 'element', - attributes: { 'w:ilvl': '0' }, - elements: [{ name: 'w:startOverride', type: 'element', attributes: { 'w:val': '5' } }], - }; - const numbering = { - abstracts: { 10: makeAbstractDef(10) }, - definitions: { 1: makeNumDef(1, 10, [lvlOverride]) }, - }; - const referencedNumIds = new Set([1]); - - const { liveDefinitions } = filterOrphanedNumberingDefinitions(numbering, referencedNumIds); - - expect(liveDefinitions).toHaveLength(1); - // Verify lvlOverride is preserved - expect(liveDefinitions[0].elements).toHaveLength(2); // abstractNumId + lvlOverride - }); -}); diff --git a/packages/super-editor/src/editors/v1/tests/data/sd-2911-active-numbering.docx b/packages/super-editor/src/editors/v1/tests/data/sd-2911-active-numbering.docx new file mode 100644 index 0000000000..cfe610c352 Binary files /dev/null and b/packages/super-editor/src/editors/v1/tests/data/sd-2911-active-numbering.docx differ diff --git a/packages/super-editor/src/editors/v1/tests/data/sd-2911-tentative-numbering.docx b/packages/super-editor/src/editors/v1/tests/data/sd-2911-tentative-numbering.docx new file mode 100644 index 0000000000..7b9b214fb1 Binary files /dev/null and b/packages/super-editor/src/editors/v1/tests/data/sd-2911-tentative-numbering.docx differ diff --git a/packages/super-editor/src/editors/v1/tests/import-export/sd-2911-numbering-roundtrip.test.js b/packages/super-editor/src/editors/v1/tests/import-export/sd-2911-numbering-roundtrip.test.js new file mode 100644 index 0000000000..2c0fa2270f --- /dev/null +++ b/packages/super-editor/src/editors/v1/tests/import-export/sd-2911-numbering-roundtrip.test.js @@ -0,0 +1,77 @@ +import { describe, it, expect } from 'vitest'; +import { dirname, join } from 'path'; +import { fileURLToPath } from 'node:url'; +import { promises as fs } from 'fs'; +import { Editor } from '@core/Editor.js'; +import DocxZipper from '@core/DocxZipper.js'; +import { parseXmlToJson } from '@converter/v2/docxHelper.js'; +import { initTestEditor } from '../helpers/helpers.js'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); + +const findNumberingRoot = (json) => { + if (!json?.elements?.length) return null; + if (json.elements[0]?.name === 'w:numbering') return json.elements[0]; + return json.elements.find((el) => el?.name === 'w:numbering') || null; +}; + +const countByName = (numberingRoot, elementName) => + (numberingRoot?.elements || []).filter((el) => el?.name === elementName).length; + +const collectIds = (numberingRoot, elementName, attrName) => + (numberingRoot?.elements || []) + .filter((el) => el?.name === elementName) + .map((el) => String(el.attributes?.[attrName])) + .filter(Boolean) + .sort(); + +async function roundTripNumberingCounts(fileName) { + const docxPath = join(__dirname, '../data', fileName); + const docxBuffer = await fs.readFile(docxPath); + + const originalZipper = new DocxZipper(); + const originalEntries = await originalZipper.getDocxData(docxBuffer, true); + const originalNumberingEntry = originalEntries.find((entry) => entry.name === 'word/numbering.xml'); + expect(originalNumberingEntry, 'fixture must contain word/numbering.xml').toBeDefined(); + const originalRoot = findNumberingRoot(parseXmlToJson(originalNumberingEntry.content)); + + const [docx, media, mediaFiles, fonts] = await Editor.loadXmlData(docxBuffer, true); + const { editor } = await initTestEditor({ content: docx, media, mediaFiles, fonts, isHeadless: true }); + + const exportedBuffer = await editor.exportDocx({ isFinalDoc: false }); + const exportedZipper = new DocxZipper(); + const exportedFiles = await exportedZipper.getDocxData(exportedBuffer, true); + const exportedNumberingEntry = exportedFiles.find((entry) => entry.name === 'word/numbering.xml'); + expect(exportedNumberingEntry, 'export must contain word/numbering.xml').toBeDefined(); + const exportedRoot = findNumberingRoot(parseXmlToJson(exportedNumberingEntry.content)); + + return { originalRoot, exportedRoot }; +} + +describe('SD-2911 — numbering.xml definitions preserved on DOCX round-trip', () => { + it('preserves every abstractNum and num for the active-numbering fixture (numId 1 is used)', async () => { + const { originalRoot, exportedRoot } = await roundTripNumberingCounts('sd-2911-active-numbering.docx'); + + expect(countByName(originalRoot, 'w:abstractNum')).toBe(8); + expect(countByName(originalRoot, 'w:num')).toBe(8); + expect(countByName(exportedRoot, 'w:abstractNum')).toBe(countByName(originalRoot, 'w:abstractNum')); + expect(countByName(exportedRoot, 'w:num')).toBe(countByName(originalRoot, 'w:num')); + }); + + it('preserves every abstractNumId and numId verbatim for the active-numbering fixture', async () => { + const { originalRoot, exportedRoot } = await roundTripNumberingCounts('sd-2911-active-numbering.docx'); + expect(collectIds(exportedRoot, 'w:abstractNum', 'w:abstractNumId')).toEqual( + collectIds(originalRoot, 'w:abstractNum', 'w:abstractNumId'), + ); + expect(collectIds(exportedRoot, 'w:num', 'w:numId')).toEqual(collectIds(originalRoot, 'w:num', 'w:numId')); + }); + + it('preserves tentative numbering even when no numId is referenced in the document body', async () => { + const { originalRoot, exportedRoot } = await roundTripNumberingCounts('sd-2911-tentative-numbering.docx'); + + expect(countByName(originalRoot, 'w:abstractNum')).toBe(2); + expect(countByName(originalRoot, 'w:num')).toBe(1); + expect(countByName(exportedRoot, 'w:abstractNum')).toBe(countByName(originalRoot, 'w:abstractNum')); + expect(countByName(exportedRoot, 'w:num')).toBe(countByName(originalRoot, 'w:num')); + }); +});