From 1e2dd84353b4c84ff02438f572705d8e492036f4 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Sat, 14 Mar 2026 17:24:50 -0700 Subject: [PATCH 1/2] fix: tighten collaboration sync handling and cover paragraph-mark tracked formatting --- .../style-engine/src/ooxml/index.test.ts | 5 ++ packages/super-editor/src/core/Editor.ts | 4 +- .../extensions/collaboration/collaboration.js | 5 +- .../tests/import/trackChangesImporter.test.js | 78 +++++++++++++++++++ 4 files changed, 88 insertions(+), 4 deletions(-) diff --git a/packages/layout-engine/style-engine/src/ooxml/index.test.ts b/packages/layout-engine/style-engine/src/ooxml/index.test.ts index b83b0fe5f3..a1c0c16281 100644 --- a/packages/layout-engine/style-engine/src/ooxml/index.test.ts +++ b/packages/layout-engine/style-engine/src/ooxml/index.test.ts @@ -251,6 +251,11 @@ describe('ooxml - resolveRunProperties', () => { expect(result.italic).toBe(true); expect(result.color).toEqual({ val: 'CCCCCC' }); }); + it('does not treat paragraph mark run properties as inherited text styling', () => { + const params = buildParams({ translatedLinkedStyles: null }); + const result = resolveRunProperties(params, { italic: true }, { runProperties: { bold: true } }); + expect(result).toEqual({ italic: true }); + }); }); describe('ooxml - resolveParagraphProperties', () => { diff --git a/packages/super-editor/src/core/Editor.ts b/packages/super-editor/src/core/Editor.ts index 81aa11a234..31897ca222 100644 --- a/packages/super-editor/src/core/Editor.ts +++ b/packages/super-editor/src/core/Editor.ts @@ -3270,8 +3270,6 @@ export class Editor extends EventEmitter { const provider = this.options.collaborationProvider; const doReplaceFileSync = () => { - const mediaFiles = this.options.mediaFiles ?? {}; - // 1. Insert new PM doc into Y fragment (must happen first) this.#insertNewFileData(); @@ -3279,6 +3277,7 @@ export class Editor extends EventEmitter { seedPartsFromEditor(this, ydoc, { replaceExisting: true }); // 3. Replace media map (prune stale + upsert new) + const mediaFiles = this.options.mediaFiles ?? {}; const mediaMap = ydoc.getMap('media'); for (const key of mediaMap.keys()) { if (!(key in mediaFiles)) mediaMap.delete(key); @@ -3319,6 +3318,7 @@ export class Editor extends EventEmitter { }, SYNC_TIMEOUT_MS); } }); + doReplaceFileSync(); } else { this.#insertNewFileData(); } diff --git a/packages/super-editor/src/extensions/collaboration/collaboration.js b/packages/super-editor/src/extensions/collaboration/collaboration.js index a2453f21f5..a5cecc95ee 100644 --- a/packages/super-editor/src/extensions/collaboration/collaboration.js +++ b/packages/super-editor/src/extensions/collaboration/collaboration.js @@ -199,7 +199,7 @@ export const Collaboration = Extension.create({ }; const provider = editor.options.collaborationProvider; - if (!provider) { + if (!provider || isCollaborationProviderSynced(provider)) { doBootstrap(); } else { cleanupState.partSyncPendingCleanup = onCollaborationProviderSynced(provider, doBootstrap); @@ -301,7 +301,8 @@ const initSyncListener = (ydoc, editor, extension) => { const provider = editor.options.collaborationProvider; if (!provider) return; - const emit = () => { + const emit = (synced) => { + if (synced === false) return; extension.options.isReady = true; editor.emit('collaborationReady', { editor, ydoc }); }; diff --git a/packages/super-editor/src/tests/import/trackChangesImporter.test.js b/packages/super-editor/src/tests/import/trackChangesImporter.test.js index 0035c22bc5..f04cb0c119 100644 --- a/packages/super-editor/src/tests/import/trackChangesImporter.test.js +++ b/packages/super-editor/src/tests/import/trackChangesImporter.test.js @@ -181,6 +181,84 @@ describe('TrackChangesImporter', () => { }); }); +describe('paragraph-level rPrChange on the paragraph mark', () => { + const paragraphXml = ` + + + + + + + + + + + + + + inherited + + + + + + + + + + styles + + `; + + it('preserves paragraph mark run properties without applying them to inherited text runs', () => { + const nodes = parseXmlToJson(paragraphXml).elements; + const handler = defaultNodeListHandler(); + const result = handler.handler({ nodes, docx: {} }); + const paragraph = result[0]; + expect(paragraph.type).toBe('paragraph'); + expect(paragraph.attrs.paragraphProperties.runProperties).toMatchObject({ bold: true, italic: true }); + + const inheritedRun = paragraph.content[0]; + expect(inheritedRun.type).toBe('run'); + const textNode = inheritedRun.content[0]; + expect(textNode.text).toBe('inherited '); + const boldMark = textNode.marks.find((m) => m.type === 'bold'); + const italicMark = textNode.marks.find((m) => m.type === 'italic'); + expect(boldMark).toBeUndefined(); + expect(italicMark).toBeUndefined(); + }); + + it('does not propagate paragraph-level rPrChange onto inherited text runs', () => { + const nodes = parseXmlToJson(paragraphXml).elements; + const handler = defaultNodeListHandler(); + const result = handler.handler({ nodes, docx: {} }); + const paragraph = result[0]; + const inheritedRun = paragraph.content[0]; + const trackMark = + inheritedRun.marks?.find((m) => m.type === TrackFormatMarkName) || + inheritedRun.content?.[0]?.marks?.find((m) => m.type === TrackFormatMarkName); + expect(trackMark).toBeUndefined(); + }); + + it('keeps the explicit run styling and run-level trackFormat on the final word', () => { + const nodes = parseXmlToJson(paragraphXml).elements; + const handler = defaultNodeListHandler(); + const result = handler.handler({ nodes, docx: {} }); + const paragraph = result[0]; + const explicitRun = paragraph.content[1]; + expect(explicitRun.type).toBe('run'); + expect(explicitRun.content[0].text).toBe('styles'); + + const explicitTextMarks = explicitRun.content[0].marks || []; + expect(explicitTextMarks.find((m) => m.type === 'bold')).toBeDefined(); + expect(explicitTextMarks.find((m) => m.type === 'italic')).toBeDefined(); + + const runTrackMarks = (explicitRun.marks || []).filter((m) => m.type === TrackFormatMarkName); + expect(runTrackMarks).toHaveLength(1); + expect(runTrackMarks[0].attrs.id).toBe('10'); + }); +}); + describe('trackChanges live xml test', () => { const inserXml = ` From 7d93ea5332488868d01e4d0ad659cd49ec7724d9 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Sun, 15 Mar 2026 13:17:31 -0700 Subject: [PATCH 2/2] fix: preserve tracked format changes through DOCX export roundtrip --- .../super-converter/v3/handlers/helpers.js | 98 +++++++++--- .../v3/handlers/w/del/del-translator.js | 15 +- .../v3/handlers/w/del/del-translator.test.js | 44 ++++-- .../v3/handlers/w/ins/ins-translator.js | 15 +- .../v3/handlers/w/ins/ins-translator.test.js | 46 ++++-- .../v3/handlers/w/r/r-translator.js | 50 ++++++ .../v3/handlers/w/r/r-translator.test.js | 50 ++++++ .../w/t/helpers/translate-text-node.js | 24 ++- .../w/t/helpers/translate-text-node.test.js | 74 +++++++++ .../trackChangesRoundtrip.test.js | 149 ++++++++++++++++++ ...cked-style-change-export-roundtrip.spec.ts | 68 ++++++++ 11 files changed, 564 insertions(+), 69 deletions(-) create mode 100644 packages/super-editor/src/core/super-converter/v3/handlers/w/t/helpers/translate-text-node.test.js create mode 100644 tests/behavior/tests/comments/sd-1460-tracked-style-change-export-roundtrip.spec.ts diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/helpers.js b/packages/super-editor/src/core/super-converter/v3/handlers/helpers.js index 76d3115be0..698cd3fa39 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/helpers.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/helpers.js @@ -1,23 +1,85 @@ import { processOutputMarks } from '@converter/exporter.js'; +import { TrackFormatMarkName } from '@extensions/track-changes/constants.js'; + +const getMarkType = (mark) => mark?.type?.name ?? mark?.type ?? null; + +const toRunPropertyElements = (marks = []) => + processOutputMarks(marks).filter((element) => element && typeof element === 'object' && element.name); + /** - * Creates export element for trackFormat mark - * @param {Array} marks SD node marks. - * @returns {Object|undefined} Properties element for trackFormat change or undefined. + * Return the first trackFormat mark from a mark list. + * + * @param {Array} marks + * @returns {Object|null} */ -export const createTrackStyleMark = (marks) => { - const trackStyleMark = marks.find((mark) => mark.type === 'trackFormat'); - if (trackStyleMark) { - return { - type: 'element', - name: 'w:rPrChange', - attributes: { - 'w:id': trackStyleMark.attrs.id, - 'w:author': trackStyleMark.attrs.author, - 'w:authorEmail': trackStyleMark.attrs.authorEmail, - 'w:date': trackStyleMark.attrs.date, - }, - elements: trackStyleMark.attrs.before.map((mark) => processOutputMarks([mark])).filter((r) => r !== undefined), - }; +export const findTrackFormatMark = (marks = []) => + marks.find((mark) => getMarkType(mark) === TrackFormatMarkName) ?? null; + +/** + * Build a valid OOXML node from a trackFormat mark. + * + * OOXML stores the "before" state under a nested inside . + * The visible "after" state is already represented by the owning run's current + * run properties, so only the "before" marks need to be serialized here. + * + * @param {Object|null|undefined} trackFormatMark + * @returns {Object|undefined} + */ +export const createRunPropertiesChangeElement = (trackFormatMark) => { + if (!trackFormatMark) return undefined; + + const beforeMarks = Array.isArray(trackFormatMark.attrs?.before) ? trackFormatMark.attrs.before : []; + const previousRunProperties = { + type: 'element', + name: 'w:rPr', + elements: toRunPropertyElements(beforeMarks), + }; + + return { + type: 'element', + name: 'w:rPrChange', + attributes: { + 'w:id': trackFormatMark.attrs?.id, + 'w:author': trackFormatMark.attrs?.author, + 'w:authorEmail': trackFormatMark.attrs?.authorEmail, + 'w:date': trackFormatMark.attrs?.date, + }, + elements: [previousRunProperties], + }; +}; + +/** + * Append a track-format change node to an OOXML element if one is not already present. + * + * @param {Object|null|undefined} runPropertiesNode + * @param {Array} marks + * @returns {Object|null|undefined} + */ +export const appendTrackFormatChangeToRunProperties = (runPropertiesNode, marks = []) => { + if (!runPropertiesNode) return runPropertiesNode; + + const trackFormatMark = findTrackFormatMark(marks); + if (!trackFormatMark) return runPropertiesNode; + + if (!Array.isArray(runPropertiesNode.elements)) { + runPropertiesNode.elements = []; + } + + const hasExistingChange = runPropertiesNode.elements.some((element) => element?.name === 'w:rPrChange'); + if (hasExistingChange) return runPropertiesNode; + + const changeElement = createRunPropertiesChangeElement(trackFormatMark); + if (changeElement) { + runPropertiesNode.elements.push(changeElement); } - return undefined; + + return runPropertiesNode; }; + +/** + * Backward-compatible alias kept while older tests and callers migrate. + * + * @param {Array} marks + * @returns {Object|undefined} + */ +export const createTrackStyleMark = (marks) => createRunPropertiesChangeElement(findTrackFormatMark(marks)); diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/del/del-translator.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/del/del-translator.js index 11460b4ad8..5aacf46818 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/del/del-translator.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/del/del-translator.js @@ -2,7 +2,6 @@ import { NodeTranslator } from '@translator'; import { createAttributeHandler } from '@converter/v3/handlers/utils.js'; import { exportSchemaToJson } from '@converter/exporter.js'; -import { createTrackStyleMark } from '@converter/v3/handlers/helpers.js'; /** @type {import('@translator').XmlNodeName} */ const XML_NODE_NAME = 'w:del'; @@ -70,15 +69,15 @@ function decode(params) { return null; } - const trackingMarks = ['trackInsert', 'trackFormat', 'trackDelete']; - const marks = node.marks; + const trackingMarks = ['trackInsert', 'trackDelete']; + const marks = Array.isArray(node.marks) ? node.marks : []; const trackedMark = marks.find((m) => m.type === 'trackDelete'); - const trackStyleMark = createTrackStyleMark(marks); - node.marks = marks.filter((m) => !trackingMarks.includes(m.type)); - if (trackStyleMark) { - node.marks.push(trackStyleMark); + if (!trackedMark) { + return null; } + node.marks = marks.filter((m) => !trackingMarks.includes(m.type)); + const translatedTextNode = exportSchemaToJson({ ...params, node }); const textNode = translatedTextNode.elements.find((n) => n.name === 'w:t'); textNode.name = 'w:delText'; @@ -106,7 +105,7 @@ export const config = { }; /** - * The NodeTranslator instance for the w:b element. + * The NodeTranslator instance for the w:del element. * @type {import('@translator').NodeTranslator} */ export const translator = NodeTranslator.from(config); diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/del/del-translator.test.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/del/del-translator.test.js index 028b4eafd0..76744afa62 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/del/del-translator.test.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/del/del-translator.test.js @@ -2,17 +2,12 @@ import { describe, expect, it, vi } from 'vitest'; import { config, translator } from './del-translator.js'; import { NodeTranslator } from '@translator'; import { exportSchemaToJson } from '@converter/exporter.js'; -import { createTrackStyleMark } from '@converter/v3/handlers/helpers.js'; // Mock external modules vi.mock('@converter/exporter.js', () => ({ exportSchemaToJson: vi.fn(), })); -vi.mock('@converter/v3/handlers/helpers.js', () => ({ - createTrackStyleMark: vi.fn(), -})); - describe('w:del translator', () => { beforeEach(() => { vi.clearAllMocks(); @@ -98,7 +93,6 @@ describe('w:del translator', () => { const mockTranslatedNode = { elements: [mockTextNode] }; exportSchemaToJson.mockReturnValue(mockTranslatedNode); - createTrackStyleMark.mockReturnValue(null); const node = { type: 'text', @@ -125,21 +119,43 @@ describe('w:del translator', () => { expect(config.decode({ node: {} })).toBeNull(); }); - it('preserves trackStyleMark if created', () => { + it('returns null when the node is missing a trackDelete mark', () => { const node = { type: 'text', - marks: [{ type: 'trackDelete', attrs: {} }], + marks: [{ type: 'italic', attrs: { value: true } }], + }; + + expect(config.decode({ node })).toBeNull(); + expect(exportSchemaToJson).not.toHaveBeenCalled(); + }); + + it('keeps trackFormat marks for downstream text export', () => { + const trackFormatMark = { + type: 'trackFormat', + attrs: { + id: 'format-1', + author: 'Missy Fox', + date: '2026-01-07T20:24:39Z', + before: [], + after: [{ type: 'italic', attrs: { value: true } }], + }, + }; + const node = { + type: 'text', + marks: [{ type: 'trackDelete', attrs: {} }, { type: 'italic', attrs: { value: true } }, trackFormatMark], }; - const mockTrackStyleMark = { type: 'trackStyle', attrs: {} }; - createTrackStyleMark.mockReturnValue(mockTrackStyleMark); exportSchemaToJson.mockReturnValue({ elements: [{ name: 'w:t' }] }); - const result = config.decode({ node }); + config.decode({ node }); - expect(createTrackStyleMark).toHaveBeenCalled(); - expect(result).toBeTruthy(); - expect(result.elements[0].elements[0].name).toBe('w:delText'); + expect(exportSchemaToJson).toHaveBeenCalledWith( + expect.objectContaining({ + node: expect.objectContaining({ + marks: [{ type: 'italic', attrs: { value: true } }, trackFormatMark], + }), + }), + ); }); }); }); diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/ins/ins-translator.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/ins/ins-translator.js index 8805bb2825..64490ff2c7 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/ins/ins-translator.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/ins/ins-translator.js @@ -2,7 +2,6 @@ import { NodeTranslator } from '@translator'; import { createAttributeHandler } from '@converter/v3/handlers/utils.js'; import { exportSchemaToJson } from '@converter/exporter.js'; -import { createTrackStyleMark } from '@converter/v3/handlers/helpers.js'; /** @type {import('@translator').XmlNodeName} */ const XML_NODE_NAME = 'w:ins'; @@ -69,15 +68,15 @@ function decode(params) { return null; } - const trackingMarks = ['trackInsert', 'trackFormat', 'trackDelete']; - const marks = node.marks; + const trackingMarks = ['trackInsert', 'trackDelete']; + const marks = Array.isArray(node.marks) ? node.marks : []; const trackedMark = marks.find((m) => m.type === 'trackInsert'); - const trackStyleMark = createTrackStyleMark(marks); - node.marks = marks.filter((m) => !trackingMarks.includes(m.type)); - if (trackStyleMark) { - node.marks.push(trackStyleMark); + if (!trackedMark) { + return null; } + node.marks = marks.filter((m) => !trackingMarks.includes(m.type)); + const translatedTextNode = exportSchemaToJson({ ...params, node }); return { @@ -103,7 +102,7 @@ export const config = { }; /** - * The NodeTranslator instance for the w:b element. + * The NodeTranslator instance for the w:ins element. * @type {import('@translator').NodeTranslator} */ export const translator = NodeTranslator.from(config); diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/ins/ins-translator.test.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/ins/ins-translator.test.js index b433da367a..071be55e27 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/ins/ins-translator.test.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/ins/ins-translator.test.js @@ -2,18 +2,13 @@ import { describe, expect, it, vi } from 'vitest'; import { config, translator } from './ins-translator.js'; import { NodeTranslator } from '@translator'; import { exportSchemaToJson } from '@converter/exporter.js'; -import { createTrackStyleMark } from '@converter/v3/handlers/helpers.js'; // Mock external modules vi.mock('@converter/exporter.js', () => ({ exportSchemaToJson: vi.fn(), })); -vi.mock('@converter/v3/handlers/helpers.js', () => ({ - createTrackStyleMark: vi.fn(), -})); - -describe('w:del translator', () => { +describe('w:ins translator', () => { beforeEach(() => { vi.clearAllMocks(); }); @@ -98,7 +93,6 @@ describe('w:del translator', () => { const mockTranslatedNode = { elements: [mockTextNode] }; exportSchemaToJson.mockReturnValue(mockTranslatedNode); - createTrackStyleMark.mockReturnValue(null); const node = { type: 'text', @@ -124,21 +118,43 @@ describe('w:del translator', () => { expect(config.decode({ node: {} })).toBeNull(); }); - it('preserves trackStyleMark if created', () => { + it('returns null when the node is missing a trackInsert mark', () => { + const node = { + type: 'text', + marks: [{ type: 'bold', attrs: { value: true } }], + }; + + expect(config.decode({ node })).toBeNull(); + expect(exportSchemaToJson).not.toHaveBeenCalled(); + }); + + it('keeps trackFormat marks for downstream text export', () => { + const trackFormatMark = { + type: 'trackFormat', + attrs: { + id: 'format-1', + author: 'Missy Fox', + date: '2026-01-07T20:24:39Z', + before: [], + after: [{ type: 'bold', attrs: { value: true } }], + }, + }; const node = { type: 'text', - marks: [{ type: 'trackInsert', attrs: {} }], + marks: [{ type: 'trackInsert', attrs: {} }, { type: 'bold', attrs: { value: true } }, trackFormatMark], }; - const mockTrackStyleMark = { type: 'trackStyle', attrs: {} }; - createTrackStyleMark.mockReturnValue(mockTrackStyleMark); exportSchemaToJson.mockReturnValue({ elements: [{ name: 'w:t' }] }); - const result = config.decode({ node }); + config.decode({ node }); - expect(createTrackStyleMark).toHaveBeenCalled(); - expect(result).toBeTruthy(); - expect(result.elements[0].elements[0].name).toBe('w:t'); + expect(exportSchemaToJson).toHaveBeenCalledWith( + expect.objectContaining({ + node: expect.objectContaining({ + marks: [{ type: 'bold', attrs: { value: true } }, trackFormatMark], + }), + }), + ); }); }); }); diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/r/r-translator.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/r/r-translator.js index 7aab415d80..7817160fec 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/r/r-translator.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/r/r-translator.js @@ -3,6 +3,7 @@ import { NodeTranslator } from '@translator'; import { translateChildNodes } from '../../../../v2/exporter/helpers/index.js'; import { cloneMark, cloneXmlNode, applyRunPropertiesTemplate, resolveFontFamily } from './helpers/helpers.js'; import { ensureTrackedWrapper, prepareRunTrackingContext } from './helpers/track-change-helpers.js'; +import { appendTrackFormatChangeToRunProperties, findTrackFormatMark } from '../../helpers.js'; import { translator as wHyperlinkTranslator } from '../hyperlink/hyperlink-translator.js'; import { translator as wRPrTranslator } from '../rpr'; import validXmlAttributes from './attributes/index.js'; @@ -30,6 +31,45 @@ const hasXmlNodeNamed = (node, targetName) => { return node.elements.some((child) => hasXmlNodeNamed(child, targetName)); }; +const getRunPropertiesNode = (runNode) => { + if (!runNode) return null; + if (!Array.isArray(runNode.elements)) runNode.elements = []; + + let runPropertiesNode = runNode.elements.find((element) => element?.name === 'w:rPr'); + if (!runPropertiesNode) { + runPropertiesNode = { type: 'element', name: 'w:rPr', elements: [] }; + runNode.elements.unshift(runPropertiesNode); + } + + if (!Array.isArray(runPropertiesNode.elements)) { + runPropertiesNode.elements = []; + } + + return runPropertiesNode; +}; + +const collectRunPropertyChanges = (runNode) => { + const runPropertiesNode = runNode?.elements?.find((element) => element?.name === 'w:rPr'); + if (!Array.isArray(runPropertiesNode?.elements)) return []; + + return runPropertiesNode.elements + .filter((element) => element?.name === 'w:rPrChange') + .map((element) => cloneXmlNode(element)); +}; + +const restoreRunPropertyChanges = (runNode, runPropertyChanges = []) => { + if (!runPropertyChanges.length) return; + + const runPropertiesNode = getRunPropertiesNode(runNode); + const existingNames = new Set(runPropertiesNode.elements.map((element) => element?.name)); + + runPropertyChanges.forEach((changeElement) => { + if (!changeElement?.name || existingNames.has(changeElement.name)) return; + runPropertiesNode.elements.push(changeElement); + existingNames.add(changeElement.name); + }); +}; + const ensureReferenceRunFormatting = (runNode, referenceXmlName) => { const styleId = REFERENCE_RUN_STYLE_BY_XML_NAME[referenceXmlName]; if (!styleId) return; @@ -213,6 +253,7 @@ const decode = (params, decodedAttrs = {}) => { // Separate out tracking marks const { runNode: runNodeForExport, trackingMarksByType } = prepareRunTrackingContext(node); + const runTrackFormatMark = findTrackFormatMark(runNodeForExport.marks); const runAttrs = runNodeForExport.attrs || {}; const runProperties = runAttrs.runProperties || {}; @@ -238,6 +279,8 @@ const decode = (params, decodedAttrs = {}) => { const runPropsTemplate = runPropertiesElement ? cloneXmlNode(runPropertiesElement) : null; const applyBaseRunProps = (runNode) => applyRunPropertiesTemplate(runNode, runPropsTemplate); const replaceRunProps = (runNode) => { + const existingRunPropertyChanges = collectRunPropertyChanges(runNode); + // Remove existing rPr if any if (Array.isArray(runNode.elements)) { runNode.elements = runNode.elements.filter((el) => el?.name !== 'w:rPr'); @@ -247,6 +290,13 @@ const decode = (params, decodedAttrs = {}) => { if (runPropsTemplate) { runNode.elements.unshift(cloneXmlNode(runPropsTemplate)); } + + restoreRunPropertyChanges(runNode, existingRunPropertyChanges); + + if (!existingRunPropertyChanges.length && runTrackFormatMark) { + const runPropertiesNode = getRunPropertiesNode(runNode); + appendTrackFormatChangeToRunProperties(runPropertiesNode, [runTrackFormatMark]); + } }; const runs = []; diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/r/r-translator.test.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/r/r-translator.test.js index 5ce715ddcb..426ad59c87 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/r/r-translator.test.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/r/r-translator.test.js @@ -343,4 +343,54 @@ describe('w:r r-translator (node)', () => { const vertAlign = runProperties?.elements?.find((el) => el?.name === 'w:vertAlign'); expect(vertAlign?.attributes?.['w:val']).toBe('superscript'); }); + + it('preserves child w:rPrChange nodes when replacing base run properties during export', () => { + const result = translator.decode({ + node: { + type: 'run', + attrs: { + runProperties: { + bold: true, + }, + }, + content: [ + { + type: 'text', + text: 'styles', + marks: [ + { type: 'bold', attrs: { value: true } }, + { + type: 'trackFormat', + attrs: { + id: 'format-1', + author: 'Missy Fox', + authorEmail: '', + date: '2026-01-07T20:24:39Z', + before: [], + after: [{ type: 'bold', attrs: { value: true } }], + }, + }, + ], + }, + ], + }, + editor: { extensionService: { extensions: [] } }, + }); + + expect(result?.name).toBe('w:r'); + + const runProperties = result?.elements?.find((element) => element?.name === 'w:rPr'); + expect(runProperties).toBeDefined(); + expect(runProperties.elements.find((element) => element?.name === 'w:b')).toBeDefined(); + + const runPropertiesChange = runProperties.elements.find((element) => element?.name === 'w:rPrChange'); + expect(runPropertiesChange).toEqual( + expect.objectContaining({ + attributes: expect.objectContaining({ + 'w:id': 'format-1', + 'w:author': 'Missy Fox', + }), + }), + ); + }); }); diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/t/helpers/translate-text-node.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/t/helpers/translate-text-node.js index fec8092687..234512c4e5 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/t/helpers/translate-text-node.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/t/helpers/translate-text-node.js @@ -8,17 +8,30 @@ */ import { translator as wRPrNodeTranslator } from '../../rpr/rpr-translator.js'; import { combineRunProperties, decodeRPrFromMarks } from '@converter/styles.js'; +import { appendTrackFormatChangeToRunProperties, findTrackFormatMark } from '@converter/v3/handlers/helpers.js'; export function getTextNodeForExport(text, marks, params) { + const normalizedMarks = Array.isArray(marks) ? marks : []; const hasLeadingOrTrailingSpace = /^\s|\s$/.test(text); const space = hasLeadingOrTrailingSpace ? 'preserve' : null; const nodeAttrs = space ? { 'xml:space': space } : null; const textNodes = []; - const textRunProperties = decodeRPrFromMarks(marks || []); + const textRunProperties = decodeRPrFromMarks(normalizedMarks); const parentRunProperties = params.extraParams?.runProperties || {}; const combinedRunProperties = combineRunProperties([parentRunProperties, textRunProperties]); - const rPrNode = wRPrNodeTranslator.decode({ node: { attrs: { runProperties: combinedRunProperties } } }); + const trackFormatMark = findTrackFormatMark(normalizedMarks); + let rPrNode = wRPrNodeTranslator.decode({ node: { attrs: { runProperties: combinedRunProperties } } }); + + if (!rPrNode && trackFormatMark) { + rPrNode = { + type: 'element', + name: 'w:rPr', + elements: [], + }; + } + + appendTrackFormatChangeToRunProperties(rPrNode, normalizedMarks); textNodes.push({ name: 'w:t', @@ -28,11 +41,10 @@ export function getTextNodeForExport(text, marks, params) { // For custom mark export, we need to add a bookmark start and end tag // And store attributes in the bookmark name - if (params) { - const { editor } = params; - const customMarks = editor.extensionService.extensions.filter((e) => e.isExternal === true); + if (params?.editor?.extensionService?.extensions) { + const customMarks = params.editor.extensionService.extensions.filter((extension) => extension.isExternal === true); - marks.forEach((mark) => { + normalizedMarks.forEach((mark) => { const isCustomMark = customMarks.some((customMark) => { const customMarkName = customMark.name; return mark.type === customMarkName; diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/t/helpers/translate-text-node.test.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/t/helpers/translate-text-node.test.js new file mode 100644 index 0000000000..b86af2ec02 --- /dev/null +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/t/helpers/translate-text-node.test.js @@ -0,0 +1,74 @@ +import { describe, expect, it } from 'vitest'; +import { getTextNodeForExport } from './translate-text-node.js'; + +const buildParams = (runProperties = {}) => ({ + extraParams: { runProperties }, + editor: { extensionService: { extensions: [] } }, +}); + +describe('getTextNodeForExport', () => { + it('adds a nested w:rPrChange for trackFormat marks', () => { + const trackFormatMark = { + type: 'trackFormat', + attrs: { + id: 'format-1', + author: 'Missy Fox', + authorEmail: '', + date: '2026-01-07T20:24:39Z', + before: [], + after: [{ type: 'bold', attrs: { value: true } }], + }, + }; + + const result = getTextNodeForExport( + 'styles', + [{ type: 'bold', attrs: { value: true } }, trackFormatMark], + buildParams(), + ); + + const runProperties = result.elements.find((element) => element.name === 'w:rPr'); + expect(runProperties).toBeDefined(); + + const runPropertiesChange = runProperties.elements.find((element) => element.name === 'w:rPrChange'); + expect(runPropertiesChange).toEqual( + expect.objectContaining({ + name: 'w:rPrChange', + attributes: expect.objectContaining({ + 'w:id': 'format-1', + 'w:author': 'Missy Fox', + 'w:date': '2026-01-07T20:24:39Z', + }), + }), + ); + + const previousRunProperties = runPropertiesChange.elements.find((element) => element.name === 'w:rPr'); + expect(previousRunProperties).toBeDefined(); + expect(previousRunProperties.elements).toEqual([]); + }); + + it('creates an rPr node for pure trackFormat changes even without visible formatting marks', () => { + const trackFormatMark = { + type: 'trackFormat', + attrs: { + id: 'format-2', + author: 'Missy Fox', + authorEmail: '', + date: '2026-01-07T20:24:39Z', + before: [{ type: 'italic', attrs: { value: true } }], + after: [], + }, + }; + + const result = getTextNodeForExport('plain', [trackFormatMark], buildParams()); + const runProperties = result.elements.find((element) => element.name === 'w:rPr'); + const runPropertiesChange = runProperties.elements.find((element) => element.name === 'w:rPrChange'); + const previousRunProperties = runPropertiesChange.elements.find((element) => element.name === 'w:rPr'); + + expect(runProperties).toBeDefined(); + expect(previousRunProperties.elements).toEqual([ + expect.objectContaining({ + name: 'w:i', + }), + ]); + }); +}); diff --git a/packages/super-editor/src/tests/import-export/trackChangesRoundtrip.test.js b/packages/super-editor/src/tests/import-export/trackChangesRoundtrip.test.js index 6178f68b4a..152833a8f5 100644 --- a/packages/super-editor/src/tests/import-export/trackChangesRoundtrip.test.js +++ b/packages/super-editor/src/tests/import-export/trackChangesRoundtrip.test.js @@ -49,6 +49,44 @@ const collectTrackMarkIds = (doc) => { }; }; +const collectTrackFormatMarkIds = (doc) => { + const formatIds = new Set(); + + doc.descendants((node) => { + node.marks?.forEach((mark) => { + if (mark.type.name === 'trackFormat') { + formatIds.add(String(mark.attrs.id)); + } + }); + }); + + return [...formatIds]; +}; + +const collectTextsWithTrackFormatId = (doc, targetId) => { + const matchingTexts = []; + + doc.descendants((node) => { + if (!node.isText) return; + + const hasTargetMark = node.marks?.some( + (mark) => mark.type.name === 'trackFormat' && String(mark.attrs.id) === String(targetId), + ); + + if (hasTargetMark && node.text) { + matchingTexts.push(node.text); + } + }); + + return matchingTexts; +}; + +const replaceEditorDocumentContent = (editor, docJson) => { + const replacementDoc = editor.schema.nodeFromJSON(docJson); + const transaction = editor.state.tr.replaceWith(0, editor.state.doc.content.size, replacementDoc.content); + editor.dispatch(transaction); +}; + const collectTrackIdsFromXml = (rootNode) => { const ids = { insert: new Set(), delete: new Set() }; const visit = (node) => { @@ -70,6 +108,22 @@ const collectTrackIdsFromXml = (rootNode) => { }; }; +const collectRunPropertyChangeIdsFromXml = (rootNode) => { + const ids = new Set(); + + const visit = (node) => { + if (!node || typeof node !== 'object') return; + if (node.name === 'w:rPrChange') { + const id = node.attributes?.['w:id']; + if (id !== undefined) ids.add(String(id)); + } + if (Array.isArray(node.elements)) node.elements.forEach(visit); + }; + + visit(rootNode); + return [...ids]; +}; + const loadExportedDocumentBody = async (exportedBuffer) => { const zipper = new DocxZipper(); const exportedFiles = await zipper.getDocxData(exportedBuffer, true); @@ -197,3 +251,98 @@ describe('msword tracked changes import/export round trip', () => { } }); }); + +describe('tracked format import/export round trip', () => { + const createTrackedFormatDoc = () => { + const trackFormatMark = { + type: 'trackFormat', + attrs: { + id: 'format-1', + author: 'Missy Fox', + authorEmail: '', + date: '2026-01-07T20:24:39Z', + before: [], + after: [ + { type: 'bold', attrs: { value: true } }, + { type: 'italic', attrs: { value: true } }, + ], + }, + }; + + return { + type: 'doc', + content: [ + { + type: 'paragraph', + content: [ + { + type: 'run', + content: [ + { + type: 'text', + text: 'Here is some text with updated ', + }, + ], + }, + { + type: 'run', + content: [ + { + type: 'text', + text: 'styles', + marks: [ + { type: 'bold', attrs: { value: true } }, + { type: 'italic', attrs: { value: true } }, + trackFormatMark, + ], + }, + ], + }, + ], + }, + ], + }; + }; + + it('exports and reimports trackFormat marks as w:rPrChange revisions', async () => { + const { docx, media, mediaFiles, fonts } = await loadTestDataForEditorTests('blank-doc.docx'); + const { editor } = await initTestEditor({ + content: docx, + media, + mediaFiles, + fonts, + isHeadless: true, + }); + + try { + replaceEditorDocumentContent(editor, createTrackedFormatDoc()); + expect(collectTrackFormatMarkIds(editor.state.doc)).toEqual(['format-1']); + expect(collectTextsWithTrackFormatId(editor.state.doc, 'format-1')).toEqual(['styles']); + + const exportedBuffer = await editor.exportDocx({ isFinalDoc: false }); + const exportedBody = await loadExportedDocumentBody(exportedBuffer); + expect(collectRunPropertyChangeIdsFromXml(exportedBody)).toEqual(['format-1']); + + const [reimportedDocx, reimportedMedia, reimportedMediaFiles, reimportedFonts] = await Editor.loadXmlData( + exportedBuffer, + true, + ); + const { editor: reimportedEditor } = await initTestEditor({ + content: reimportedDocx, + media: reimportedMedia, + mediaFiles: reimportedMediaFiles, + fonts: reimportedFonts, + isHeadless: true, + }); + + try { + expect(collectTrackFormatMarkIds(reimportedEditor.state.doc)).toEqual(['format-1']); + expect(collectTextsWithTrackFormatId(reimportedEditor.state.doc, 'format-1')).toEqual(['styles']); + } finally { + reimportedEditor.destroy(); + } + } finally { + editor.destroy(); + } + }); +}); diff --git a/tests/behavior/tests/comments/sd-1460-tracked-style-change-export-roundtrip.spec.ts b/tests/behavior/tests/comments/sd-1460-tracked-style-change-export-roundtrip.spec.ts new file mode 100644 index 0000000000..1d4fde36c3 --- /dev/null +++ b/tests/behavior/tests/comments/sd-1460-tracked-style-change-export-roundtrip.spec.ts @@ -0,0 +1,68 @@ +import fs from 'node:fs'; +import { writeFile } from 'node:fs/promises'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { test, expect, type SuperDocFixture } from '../../fixtures/superdoc.js'; +import { assertDocumentApiReady, listTrackChanges } from '../../helpers/document-api.js'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const DOC_PATH = path.resolve(__dirname, '../../../../test-corpus/comments-tcs/GD Tracked style change.docx'); + +test.skip(!fs.existsSync(DOC_PATH), 'Tracked style change fixture is not available.'); + +test.use({ config: { toolbar: 'full', comments: 'panel', trackChanges: true } }); + +async function loadDocumentAndWait(superdoc: SuperDocFixture, filePath: string): Promise { + await superdoc.loadDocument(filePath); + await superdoc.waitForStable(); + await assertDocumentApiReady(superdoc.page); +} + +async function assertTrackedStyleChangeState(superdoc: SuperDocFixture): Promise { + await superdoc.assertTextContains('Here is some text with updated styles'); + await superdoc.assertTrackedChangeExists('format'); + await superdoc.assertTextHasMarks('styles', ['bold', 'italic']); + await superdoc.assertTextLacksMarks('Here is some text with updated', ['bold', 'italic']); + + await expect.poll(async () => (await listTrackChanges(superdoc.page)).total).toBe(1); + await expect.poll(async () => (await listTrackChanges(superdoc.page, { type: 'format' })).total).toBe(1); + await expect.poll(async () => (await listTrackChanges(superdoc.page, { type: 'insert' })).total).toBe(0); + await expect.poll(async () => (await listTrackChanges(superdoc.page, { type: 'delete' })).total).toBe(0); + + const formatChanges = await listTrackChanges(superdoc.page, { type: 'format' }); + expect(formatChanges.changes).toHaveLength(1); + expect(formatChanges.changes[0]?.excerpt).toBe('styles'); +} + +async function exportCurrentDocument(superdoc: SuperDocFixture, outputPath: string): Promise { + const exportedBytes = await superdoc.page.evaluate(async () => { + const exported = await (window as any).editor.exportDocx({ isFinalDoc: false }); + + if (exported instanceof Blob) { + return Array.from(new Uint8Array(await exported.arrayBuffer())); + } + + if (exported instanceof ArrayBuffer) { + return Array.from(new Uint8Array(exported)); + } + + if (ArrayBuffer.isView(exported)) { + return Array.from(new Uint8Array(exported.buffer, exported.byteOffset, exported.byteLength)); + } + + throw new Error(`Unexpected exportDocx() result: ${Object.prototype.toString.call(exported)}`); + }); + + await writeFile(outputPath, Buffer.from(exportedBytes)); +} + +test('SD-1460 tracked format change survives DOCX export and re-import', async ({ superdoc }, testInfo) => { + await loadDocumentAndWait(superdoc, DOC_PATH); + await assertTrackedStyleChangeState(superdoc); + + const exportedPath = testInfo.outputPath('sd-1460-tracked-style-change-roundtrip.docx'); + await exportCurrentDocument(superdoc, exportedPath); + + await loadDocumentAndWait(superdoc, exportedPath); + await assertTrackedStyleChangeState(superdoc); +});