Skip to content

SD-1460 - fix: don't drop track format on run nodes #1850

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chittolinag wants to merge 4 commits into
base: main
Choose a base branch
from
Open

SD-1460 - fix: don't drop track format on run nodes #1850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1395048
fix: don't drop track format on run nodes
chittolina Jan 27, 2026
4b9e486
refactor: rename fn
chittolina Jan 27, 2026
95e27f6
refactor: created separate fn to add rPrChange
chittolina Jan 27, 2026
da827bd
fix: tests
chittolina Jan 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions packages/super-editor/src/core/super-converter/v3/handlers/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,17 @@ import { processOutputMarks } from '@converter/exporter.js';
* @param {Array} marks SD node marks.
* @returns {Object|undefined} Properties element for trackFormat change or undefined.
*/
export const createTrackStyleMark = (marks) => {
export const decodeTrackFormatMark = (marks) => {
Copy link
Contributor Author

@chittolinag chittolinag Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed this - the current name seems counter-intuitive, it's not creating a mark, but instead decoding it back to OOXML.

Thoughts?

const trackStyleMark = marks.find((mark) => mark.type === 'trackFormat');
if (trackStyleMark) {
const beforeMarks = Array.isArray(trackStyleMark.attrs.before) ? trackStyleMark.attrs.before : [];
const beforeElements = beforeMarks
.flatMap((mark) => processOutputMarks([mark]) || [])
.filter((element) => element && typeof element === 'object');
const rPrElement = {
name: 'w:rPr',
elements: beforeElements,
};
return {
type: 'element',
name: 'w:rPrChange',
Expand All @@ -16,7 +24,7 @@ export const createTrackStyleMark = (marks) => {
'w:authorEmail': trackStyleMark.attrs.authorEmail,
'w:date': trackStyleMark.attrs.date,
},
elements: trackStyleMark.attrs.before.map((mark) => processOutputMarks([mark])).filter((r) => r !== undefined),
elements: [rPrElement],
};
}
return undefined;
Expand Down
4 changes: 2 additions & 2 deletions packages/super-editor/src/core/super-converter/v3/handlers/w/del/del-translator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
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';
import { decodeTrackFormatMark } from '@converter/v3/handlers/helpers.js';

/** @type {import('@translator').XmlNodeName} */
const XML_NODE_NAME = 'w:del';
Expand Down Expand Up @@ -73,7 +73,7 @@ function decode(params) {
const trackingMarks = ['trackInsert', 'trackFormat', 'trackDelete'];
const marks = node.marks;
const trackedMark = marks.find((m) => m.type === 'trackDelete');
const trackStyleMark = createTrackStyleMark(marks);
const trackStyleMark = decodeTrackFormatMark(marks);
node.marks = marks.filter((m) => !trackingMarks.includes(m.type));
if (trackStyleMark) {
node.marks.push(trackStyleMark);
Expand Down
10 changes: 5 additions & 5 deletions packages/super-editor/src/core/super-converter/v3/handlers/w/del/del-translator.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ 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';
import { decodeTrackFormatMark } 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(),
decodeTrackFormatMark: vi.fn(),
}));

describe('w:del translator', () => {
Expand Down Expand Up @@ -98,7 +98,7 @@ describe('w:del translator', () => {
const mockTranslatedNode = { elements: [mockTextNode] };

exportSchemaToJson.mockReturnValue(mockTranslatedNode);
createTrackStyleMark.mockReturnValue(null);
decodeTrackFormatMark.mockReturnValue(null);

const node = {
type: 'text',
Expand Down Expand Up @@ -132,12 +132,12 @@ describe('w:del translator', () => {
};

const mockTrackStyleMark = { type: 'trackStyle', attrs: {} };
createTrackStyleMark.mockReturnValue(mockTrackStyleMark);
decodeTrackFormatMark.mockReturnValue(mockTrackStyleMark);
exportSchemaToJson.mockReturnValue({ elements: [{ name: 'w:t' }] });

const result = config.decode({ node });

expect(createTrackStyleMark).toHaveBeenCalled();
expect(decodeTrackFormatMark).toHaveBeenCalled();
expect(result).toBeTruthy();
expect(result.elements[0].elements[0].name).toBe('w:delText');
});
Expand Down
4 changes: 2 additions & 2 deletions packages/super-editor/src/core/super-converter/v3/handlers/w/ins/ins-translator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
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';
import { decodeTrackFormatMark } from '@converter/v3/handlers/helpers.js';

/** @type {import('@translator').XmlNodeName} */
const XML_NODE_NAME = 'w:ins';
Expand Down Expand Up @@ -72,7 +72,7 @@ function decode(params) {
const trackingMarks = ['trackInsert', 'trackFormat', 'trackDelete'];
const marks = node.marks;
const trackedMark = marks.find((m) => m.type === 'trackInsert');
const trackStyleMark = createTrackStyleMark(marks);
const trackStyleMark = decodeTrackFormatMark(marks);
node.marks = marks.filter((m) => !trackingMarks.includes(m.type));
if (trackStyleMark) {
node.marks.push(trackStyleMark);
Expand Down
10 changes: 5 additions & 5 deletions packages/super-editor/src/core/super-converter/v3/handlers/w/ins/ins-translator.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ 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';
import { decodeTrackFormatMark } 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(),
decodeTrackFormatMark: vi.fn(),
}));

describe('w:del translator', () => {
Expand Down Expand Up @@ -98,7 +98,7 @@ describe('w:del translator', () => {
const mockTranslatedNode = { elements: [mockTextNode] };

exportSchemaToJson.mockReturnValue(mockTranslatedNode);
createTrackStyleMark.mockReturnValue(null);
decodeTrackFormatMark.mockReturnValue(null);

const node = {
type: 'text',
Expand Down Expand Up @@ -131,12 +131,12 @@ describe('w:del translator', () => {
};

const mockTrackStyleMark = { type: 'trackStyle', attrs: {} };
createTrackStyleMark.mockReturnValue(mockTrackStyleMark);
decodeTrackFormatMark.mockReturnValue(mockTrackStyleMark);
exportSchemaToJson.mockReturnValue({ elements: [{ name: 'w:t' }] });

const result = config.decode({ node });

expect(createTrackStyleMark).toHaveBeenCalled();
expect(decodeTrackFormatMark).toHaveBeenCalled();
expect(result).toBeTruthy();
expect(result.elements[0].elements[0].name).toBe('w:t');
});
Expand Down
2 changes: 1 addition & 1 deletion packages/super-editor/src/core/super-converter/v3/handlers/w/numPr/numPr-translator.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { translator } from './numPr-translator.js';

vi.mock('@converter/exporter', () => ({
exportSchemaToJson: vi.fn(),
createTrackStyleMark: vi.fn(),
decodeTrackFormatMark: vi.fn(),
}));

describe('w:numPr translator', () => {
Expand Down
13 changes: 8 additions & 5 deletions ...ges/super-editor/src/core/super-converter/v3/handlers/w/r/helpers/track-change-helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { TrackDeleteMarkName, TrackInsertMarkName } from '@extensions/track-changes/constants.js';
import { TrackDeleteMarkName, TrackInsertMarkName, TrackFormatMarkName } from '@extensions/track-changes/constants.js';

const cloneMark = (mark) => {
if (!mark) return mark;
Expand All @@ -24,8 +24,9 @@ const cloneRuns = (runs = []) => runs.map((run) => cloneNode(run));

export const prepareRunTrackingContext = (node = {}) => {
const marks = Array.isArray(node.marks) ? node.marks : [];
const trackingMarks = marks.filter(
(mark) => mark?.type === TrackInsertMarkName || mark?.type === TrackDeleteMarkName,

const trackingMarks = marks.filter((mark) =>
[TrackInsertMarkName, TrackDeleteMarkName, TrackFormatMarkName].includes(mark?.type),
Copy link
Contributor

@caio-pizzol caio-pizzol Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trackFormat is now collected into trackingMarksByType here, but the preservedMarks filter on line ~42 still only removes trackInsert and trackDelete — so trackFormat ends up in both places and gets pushed onto children too. nothing downstream processes it today, so no duplicate output — but it's inconsistent with the function's intent. worth aligning the preservedMarks filter too?

const preservedMarks = marks
  .filter((mark) => ![TrackInsertMarkName, TrackDeleteMarkName, TrackFormatMarkName].includes(mark?.type))
  .map((mark) => cloneMark(mark));

Copy link
Contributor

@caio-pizzol caio-pizzol Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tupizz 👀

);

if (!trackingMarks.length) {
Expand Down Expand Up @@ -88,7 +89,8 @@ export const ensureTrackedWrapper = (runs, trackingMarksByType = new Map()) => {

if (!trackingMarksByType.size) return runs;

if (trackingMarksByType.has(TrackInsertMarkName)) {
const isTrackInsertMark = trackingMarksByType.has(TrackInsertMarkName);
if (isTrackInsertMark) {
const mark = trackingMarksByType.get(TrackInsertMarkName);
const clonedRuns = cloneRuns(runs);
const wrapper = {
Expand All @@ -107,7 +109,8 @@ export const ensureTrackedWrapper = (runs, trackingMarksByType = new Map()) => {
return [wrapper];
}

if (trackingMarksByType.has(TrackDeleteMarkName)) {
const isTrackDeleteMark = trackingMarksByType.has(TrackDeleteMarkName);
if (isTrackDeleteMark) {
const mark = trackingMarksByType.get(TrackDeleteMarkName);
const clonedRuns = cloneRuns(runs);
clonedRuns.forEach(renameTextElementsForDeletion);
Expand Down
29 changes: 28 additions & 1 deletion packages/super-editor/src/core/super-converter/v3/handlers/w/r/r-translator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ import { translator as wHyperlinkTranslator } from '../hyperlink/hyperlink-trans
import { translator as wRPrTranslator } from '../rpr';
import validXmlAttributes from './attributes/index.js';
import { handleStyleChangeMarksV2 } from '../../../../v2/importer/markImporter.js';
import { encodeMarksFromRPr, resolveRunProperties } from '../../../../styles.js';
import { encodeMarksFromRPr, resolveRunProperties } from '@converter/styles.js';
import { TrackFormatMarkName } from '@extensions/track-changes/constants.js';
import { decodeTrackFormatMark } from '@converter/v3/handlers/helpers.js';

/** @type {import('@translator').XmlNodeName} */
const XML_NODE_NAME = 'w:r';

Expand Down Expand Up @@ -138,6 +141,25 @@ const encode = (params, encodedAttrs = {}) => {
return splitRuns;
};

const addRPrChange = (runPropertiesElement, trackingMarksByType) => {
let rPrElement = cloneXmlNode(runPropertiesElement);
const trackFormatMark = trackingMarksByType.get(TrackFormatMarkName);

if (trackFormatMark) {
const rPrChangeElement = decodeTrackFormatMark([trackFormatMark]);
if (rPrChangeElement) {
if (!rPrElement) {
rPrElement = { name: 'w:rPr', elements: [] };
}
if (!Array.isArray(rPrElement.elements)) {
rPrElement.elements = [];
}
rPrElement.elements.push(rPrChangeElement);
}
}
return rPrElement;
};

const decode = (params, decodedAttrs = {}) => {
const { node } = params || {};
if (!node) return undefined;
Expand Down Expand Up @@ -176,6 +198,11 @@ const decode = (params, decodedAttrs = {}) => {
node: { attrs: { runProperties: runProperties } },
});

const trackFormatMark = trackingMarksByType.get(TrackFormatMarkName);
if (trackFormatMark) {
runPropertiesElement = addRPrChange(runPropertiesElement, trackingMarksByType);
}

const runPropsTemplate = runPropertiesElement ? cloneXmlNode(runPropertiesElement) : null;
const applyBaseRunProps = (runNode) => applyRunPropertiesTemplate(runNode, runPropsTemplate);
const replaceRunProps = (runNode) => {
Expand Down
46 changes: 46 additions & 0 deletions packages/super-editor/src/core/super-converter/v3/handlers/w/r/r-translator.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,4 +222,50 @@ describe('w:r r-translator (node)', () => {
}),
);
});

it('adds w:rPrChange to run properties when a track format mark is present', () => {
const trackFormatMark = {
type: 'trackFormat',
attrs: {
id: 'fmt-1',
author: 'Alice',
authorEmail: 'alice@example.com',
date: '2024-09-04T09:29:00Z',
before: [{ type: 'bold', attrs: { value: true } }],
},
};

const node = {
type: 'run',
marks: [trackFormatMark],
content: [
{
type: 'text',
text: 'changed text',
marks: [{ type: 'bold', attrs: { value: true } }],
},
],
};

const result = translator.decode({
node,
editor: { extensionService: { extensions: [] } },
});

expect(result.name).toBe('w:r');
const rPr = result.elements?.find((el) => el.name === 'w:rPr');
expect(rPr).toBeDefined();
const change = rPr.elements?.find((el) => el.name === 'w:rPrChange');
expect(change).toBeDefined();
expect(change.attributes).toMatchObject({
'w:id': 'fmt-1',
'w:author': 'Alice',
'w:authorEmail': 'alice@example.com',
'w:date': '2024-09-04T09:29:00Z',
});
const beforeRPr = change.elements?.find((el) => el.name === 'w:rPr');
expect(beforeRPr).toBeDefined();
const boldElement = beforeRPr.elements?.find((el) => el.name === 'w:b');
expect(boldElement).toBeDefined();
});
});
Loading