diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/del/del-translator.js b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/del/del-translator.js index d2106b2539..4c29fd7888 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/del/del-translator.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/del/del-translator.js @@ -88,9 +88,13 @@ function decode(params) { return /** @type {import('@translator').SCDecoderResult} */ (/** @type {unknown} */ (null)); } - node.marks = marks.filter((m) => m.type !== 'trackDelete'); + // Strip the tracked mark on a copy: `node` belongs to the caller, and the + // header/footer export path passes the converter's persistent import-time + // tree by reference. Mutating it here makes the strip permanent, so the + // second export loses the tracked change entirely (issue #3893). + const strippedNode = { ...node, marks: marks.filter((m) => m.type !== 'trackDelete') }; - const translatedResult = exportSchemaToJson({ ...params, node }); + const translatedResult = exportSchemaToJson({ ...params, node: strippedNode }); if (params.isFinalDoc) { return null; diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/del/del-translator.test.js b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/del/del-translator.test.js index 0be0fe48ef..6895e83f74 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/del/del-translator.test.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/del/del-translator.test.js @@ -235,6 +235,39 @@ describe('w:del translator', () => { expect(result.elements[0].elements[0].name).toBe('w:delText'); }); + it('strips the tracked mark on a copy without mutating the caller node (#3893)', () => { + // The header/footer export path hands decode the converter's persistent + // import-time tree by reference; an in-place strip makes the second + // export lose the tracked change entirely. + const mockTrackedMark = { + type: 'trackDelete', + attrs: { + id: '123', + sourceId: '', + author: 'Test', + authorEmail: 'test@example.com', + date: '2025-10-09T12:00:00Z', + }, + }; + + exportSchemaToJson.mockReturnValue({ elements: [{ name: 'w:t', text: 'deleted text' }] }); + + const node = { + type: 'text', + text: 'deleted text', + marks: [mockTrackedMark, { type: 'bold' }], + }; + + config.decode({ node }); + + expect(node.marks).toEqual([mockTrackedMark, { type: 'bold' }]); + expect(exportSchemaToJson).toHaveBeenCalledWith( + expect.objectContaining({ + node: expect.objectContaining({ marks: [{ type: 'bold' }] }), + }), + ); + }); + it('renames every in a multi-segment run to (newline split)', () => { const mockTrackedMark = { type: 'trackDelete', diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/ins/ins-translator.js b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/ins/ins-translator.js index 741dbc9c7d..bd7ab81eec 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/ins/ins-translator.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/ins/ins-translator.js @@ -87,9 +87,13 @@ function decode(params) { return /** @type {import('@translator').SCDecoderResult} */ (/** @type {unknown} */ (null)); } - node.marks = marks.filter((m) => m.type !== 'trackInsert'); + // Strip the tracked mark on a copy: `node` belongs to the caller, and the + // header/footer export path passes the converter's persistent import-time + // tree by reference. Mutating it here makes the strip permanent, so the + // second export loses the tracked change entirely (issue #3893). + const strippedNode = { ...node, marks: marks.filter((m) => m.type !== 'trackInsert') }; - const translatedTextNode = exportSchemaToJson({ ...params, node }); + const translatedTextNode = exportSchemaToJson({ ...params, node: strippedNode }); if (params.isFinalDoc) { return translatedTextNode; diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/ins/ins-translator.test.js b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/ins/ins-translator.test.js index e1ac8110ad..3d4295ceeb 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/ins/ins-translator.test.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/ins/ins-translator.test.js @@ -253,6 +253,39 @@ describe('w:ins translator', () => { }); }); + it('strips the tracked mark on a copy without mutating the caller node (#3893)', () => { + // The header/footer export path hands decode the converter's persistent + // import-time tree by reference; an in-place strip makes the second + // export lose the tracked change entirely. + const mockTrackedMark = { + type: 'trackInsert', + attrs: { + id: '123', + sourceId: '', + author: 'Test', + authorEmail: 'test@example.com', + date: '2025-10-09T12:00:00Z', + }, + }; + + exportSchemaToJson.mockReturnValue({ elements: [{ name: 'w:t' }] }); + + const node = { + type: 'text', + text: 'added text', + marks: [mockTrackedMark, { type: 'bold' }], + }; + + config.decode({ node }); + + expect(node.marks).toEqual([mockTrackedMark, { type: 'bold' }]); + expect(exportSchemaToJson).toHaveBeenCalledWith( + expect.objectContaining({ + node: expect.objectContaining({ marks: [{ type: 'bold' }] }), + }), + ); + }); + it('writes sourceId to w:id for round-trip fidelity', () => { const mockTrackedMark = { type: 'trackInsert', diff --git a/packages/super-editor/src/editors/v1/tests/data/header-tracked-changes.docx b/packages/super-editor/src/editors/v1/tests/data/header-tracked-changes.docx new file mode 100644 index 0000000000..aca116d575 Binary files /dev/null and b/packages/super-editor/src/editors/v1/tests/data/header-tracked-changes.docx differ diff --git a/packages/super-editor/src/editors/v1/tests/import-export/headerTrackedChangesDoubleExport.test.js b/packages/super-editor/src/editors/v1/tests/import-export/headerTrackedChangesDoubleExport.test.js new file mode 100644 index 0000000000..c345ef7f8c --- /dev/null +++ b/packages/super-editor/src/editors/v1/tests/import-export/headerTrackedChangesDoubleExport.test.js @@ -0,0 +1,62 @@ +import { describe, it, expect } from 'vitest'; +import { loadTestDataForEditorTests, initTestEditor } from '../helpers/helpers.js'; +import DocxZipper from '@core/DocxZipper.js'; +import { parseXmlToJson } from '@converter/v2/docxHelper.js'; + +const countTrackNodes = (node, tracker) => { + if (!node || typeof node !== 'object') return; + if (node.name === 'w:ins') tracker.ins += 1; + if (node.name === 'w:del') tracker.del += 1; + if (Array.isArray(node.elements)) node.elements.forEach((child) => countTrackNodes(child, tracker)); +}; + +const loadExportedHeaderCensus = async (exportedBuffer) => { + const zipper = new DocxZipper(); + const exportedFiles = await zipper.getDocxData(exportedBuffer, true); + const headerXmlEntry = exportedFiles.find((entry) => entry.name === 'word/header1.xml'); + expect(headerXmlEntry).toBeDefined(); + + const headerJson = parseXmlToJson(headerXmlEntry.content); + const tracker = { ins: 0, del: 0 }; + countTrackNodes(headerJson, tracker); + return tracker; +}; + +describe('header tracked changes across repeated exports (#3893)', () => { + // Once a header sub-editor is registered (the UI mounts one as soon as the + // user clicks into the header), #exportProcessHeadersFooters serializes the + // converter's persistent import-time header tree. The tracked-change decoders + // used to strip trackInsert/trackDelete marks off that tree in place, so the + // first export was correct and every later export silently dropped the + // header redline — a counterparty's tracked deletion came back as accepted + // plain text. Saving twice with no intervening edit must produce the same + // tracked changes both times. + it('preserves header w:ins/w:del on the second export once a header sub-editor is registered', async () => { + const fileName = 'header-tracked-changes.docx'; + const { docx, media, mediaFiles, fonts } = await loadTestDataForEditorTests(fileName); + const { editor } = await initTestEditor({ content: docx, media, mediaFiles, fonts, isHeadless: true }); + const { editor: headerSubEditor } = await initTestEditor({ + content: docx, + media, + mediaFiles, + fonts, + isHeadless: true, + }); + + const headerIds = Object.keys(editor.converter.headers); + expect(headerIds.length).toBeGreaterThan(0); + + // Register a sub-editor for every header part, the way + // HeaderFooterEditorManager does when a user clicks into a header; the + // export loop only reads `.editor` off each entry. + headerIds.forEach((id) => { + editor.converter.headerEditors.push({ id, editor: headerSubEditor }); + }); + + const firstExport = await loadExportedHeaderCensus(await editor.exportDocx({ isFinalDoc: false })); + expect(firstExport).toEqual({ ins: 1, del: 1 }); + + const secondExport = await loadExportedHeaderCensus(await editor.exportDocx({ isFinalDoc: false })); + expect(secondExport).toEqual({ ins: 1, del: 1 }); + }); +});