Skip to content

fix: track new lines in suggested mode #3598

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

Closed
harbournick wants to merge 1 commit into from
Closed

fix: track new lines in suggested mode #3598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 37 additions & 0 deletions packages/super-editor/src/editors/v1/core/super-converter/v2/importer/markImporter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,37 @@ import { getHexColorFromDocxSystem, isValidHexColor, twipsToInches, twipsToLines
import { translator as wRPrTranslator } from '../../v3/handlers/w/rpr/index.js';
import { encodeMarksFromRPr } from '@converter/styles.js';
import { resolveTrackedChangeImportIds, stampImportTrackingAttrs } from './importTrackingContext.js';
import {
ParagraphSplitSnapshotType,
SuperDocParagraphSplitAnchorAttr,
SuperDocParagraphSplitAttr,
} from '../../v3/handlers/helpers.js';

function getInlineParagraphChange(params) {
return params?.extraParams?.inlineParagraphProperties?.change || null;
}

function isMatchingParagraphSplitChange(change, ids) {
if (!change || typeof change !== 'object') return false;
if (String(change[SuperDocParagraphSplitAttr] ?? '') !== '1') return false;

const changeId = change.id;
if (changeId == null) return false;
return ids.some((id) => id != null && String(id) === String(changeId));
}

function createParagraphSplitSnapshots(change) {
const anchor = change?.[SuperDocParagraphSplitAnchorAttr] === 'source' ? 'source' : 'inserted';
const snapshot = {
type: ParagraphSplitSnapshotType,
attrs: { anchor },
};

return {
before: [snapshot],
after: [{ ...snapshot, attrs: { ...snapshot.attrs } }],
};
}

/**
*
Expand Down Expand Up @@ -135,6 +166,12 @@ export function handleStyleChangeMarksV2(rPrChange, currentMarks, params) {
submarks = encodeMarksFromRPr(runProperties, params?.docx);
}

const paragraphChange = getInlineParagraphChange(params);
if (isMatchingParagraphSplitChange(paragraphChange, [attributes['w:id'], sourceId, logicalId])) {
const snapshots = createParagraphSplitSnapshots(paragraphChange);
return [{ type: TrackFormatMarkName, attrs: { ...mappedAttributes, ...snapshots } }];
}

return [{ type: TrackFormatMarkName, attrs: { ...mappedAttributes, before: submarks, after: [...currentMarks] } }];
}

Expand Down
32 changes: 32 additions & 0 deletions packages/super-editor/src/editors/v1/core/super-converter/v2/importer/markImporter.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,38 @@ describe('handleStyleChangeMarksV2', () => {
expect(result[0].attrs.after).toEqual([]);
});

it('restores paragraphSplit snapshots from matching SuperDoc paragraph metadata', () => {
const currentMarks = [{ type: 'bold', attrs: { value: true } }];
const rPrChange = {
name: 'w:rPrChange',
attributes: {
'w:id': '7',
'w:date': '2026-06-01T17:00:00Z',
'w:author': 'Reviewer',
},
elements: [{ name: 'w:rPr', elements: [] }],
};

const result = handleStyleChangeMarksV2(rPrChange, currentMarks, {
docx: {},
extraParams: {
inlineParagraphProperties: {
change: {
id: '7',
superdocParagraphSplit: '1',
superdocParagraphSplitAnchor: 'source',
paragraphProperties: {},
},
},
},
});

expect(result).toHaveLength(1);
expect(result[0].type).toBe(TrackFormatMarkName);
expect(result[0].attrs.before).toEqual([{ type: 'paragraphSplit', attrs: { anchor: 'source' } }]);
expect(result[0].attrs.after).toEqual([{ type: 'paragraphSplit', attrs: { anchor: 'source' } }]);
});

it('remaps format change ids and stamps overlapParentId through import tracking context', () => {
const context = createImportTrackingContext({});
context.pushParent({
Expand Down
68 changes: 63 additions & 5 deletions packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,27 @@
import { processOutputMarks } from '@converter/exporter.js';
import { TrackFormatMarkName } from '@extensions/track-changes/constants.js';

export const ParagraphSplitSnapshotType = 'paragraphSplit';
export const SuperDocRevisionNamespace = 'https://superdoc.dev/ooxml/revisions/2026';
export const SuperDocRevisionNamespaceAttr = 'superdocXmlns';
export const SuperDocParagraphSplitAttr = 'superdocParagraphSplit';
export const SuperDocParagraphSplitAnchorAttr = 'superdocParagraphSplitAnchor';

const getMarkType = (mark) => mark?.type?.name ?? mark?.type ?? null;
const getSnapshotType = (snapshot) => snapshot?.type?.name ?? snapshot?.type ?? null;

const toRunPropertyElements = (marks = []) =>
processOutputMarks(marks).filter((element) => element && typeof element === 'object' && element.name);

const getTrackFormatChangeWordId = (trackFormatMark, options = {}) => {
const allocator = options?.wordIdAllocator || null;
const partPath = options?.partPath || 'word/document.xml';
const sourceId = trackFormatMark.attrs?.sourceId;
const logicalId = trackFormatMark.attrs?.id;

return allocator ? allocator.allocate({ partPath, sourceId, logicalId }) : sourceId || logicalId;
};

/**
* Return the first trackFormat mark from a mark list.
*
Expand All @@ -15,6 +31,52 @@ const toRunPropertyElements = (marks = []) =>
export const findTrackFormatMark = (marks = []) =>
marks.find((mark) => getMarkType(mark) === TrackFormatMarkName) ?? null;

export const findSnapshotByType = (snapshots = [], type) =>
Array.isArray(snapshots) ? (snapshots.find((snapshot) => getSnapshotType(snapshot) === type) ?? null) : null;

export const findParagraphSplitSnapshot = (trackFormatMark) => {
if (!trackFormatMark) return null;
return (
findSnapshotByType(trackFormatMark.attrs?.before, ParagraphSplitSnapshotType) ||
findSnapshotByType(trackFormatMark.attrs?.after, ParagraphSplitSnapshotType)
);
};

export const isParagraphSplitTrackFormatMark = (mark) =>
getMarkType(mark) === TrackFormatMarkName && Boolean(findParagraphSplitSnapshot(mark));

export const createParagraphSplitPropertiesChange = (trackFormatMark, options = {}) => {
const paragraphSplit = findParagraphSplitSnapshot(trackFormatMark);
if (!paragraphSplit) return undefined;

const anchor = paragraphSplit.attrs?.anchor === 'source' ? 'source' : 'inserted';

return {
id: getTrackFormatChangeWordId(trackFormatMark, options),
author: trackFormatMark.attrs?.author,
date: trackFormatMark.attrs?.date,
[SuperDocRevisionNamespaceAttr]: SuperDocRevisionNamespace,
[SuperDocParagraphSplitAttr]: '1',
[SuperDocParagraphSplitAnchorAttr]: anchor,
paragraphProperties: {},
};
};

export const createParagraphSplitInsertionElement = (trackFormatMark, options = {}) => {
const paragraphSplit = findParagraphSplitSnapshot(trackFormatMark);
if (!paragraphSplit) return undefined;

return {
type: 'element',
name: 'w:ins',
attributes: {
'w:id': getTrackFormatChangeWordId(trackFormatMark, options),
'w:author': trackFormatMark.attrs?.author,
'w:date': trackFormatMark.attrs?.date,
},
};
};

/**
* Build a valid OOXML <w:rPrChange> node from a trackFormat mark.
*
Expand All @@ -38,11 +100,7 @@ export const createRunPropertiesChangeElement = (trackFormatMark, options = {})
// Phase 005 — if an allocator was passed in, mint a Word-native decimal
// `w:id`. Legacy callers (no `options.wordIdAllocator`) keep the prior
// `sourceId || id` behavior so the exported byte stream is unchanged.
const allocator = options?.wordIdAllocator || null;
const partPath = options?.partPath || 'word/document.xml';
const sourceId = trackFormatMark.attrs?.sourceId;
const logicalId = trackFormatMark.attrs?.id;
const wordId = allocator ? allocator.allocate({ partPath, sourceId, logicalId }) : sourceId || logicalId;
const wordId = getTrackFormatChangeWordId(trackFormatMark, options);

return {
type: 'element',
Expand Down
86 changes: 86 additions & 0 deletions .../editors/v1/core/super-converter/v3/handlers/w/p/helpers/generate-paragraph-properties.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,70 @@
import { carbonCopy } from '@core/utilities/carbonCopy.js';
import { translator as wPPrNodeTranslator } from '../../pPr/pPr-translator.js';
import {
createParagraphSplitInsertionElement,
createParagraphSplitPropertiesChange,
isParagraphSplitTrackFormatMark,
} from '../../../helpers.js';

function resolveExportPartPath(params = {}) {
if (typeof params.currentPartPath === 'string' && params.currentPartPath.length > 0) return params.currentPartPath;
if (typeof params.filename === 'string' && params.filename.length > 0) {
return params.filename.startsWith('word/') ? params.filename : `word/${params.filename}`;
}
return 'word/document.xml';
}

function findParagraphSplitTrackFormatMark(node) {
if (!node || typeof node !== 'object') return null;

const marks = Array.isArray(node.marks) ? node.marks : [];
const directMark = marks.find((mark) => isParagraphSplitTrackFormatMark(mark));
if (directMark) return directMark;

if (typeof node.descendants === 'function') {
let found = null;
node.descendants((child) => {
const childMarks = Array.isArray(child?.marks) ? child.marks : [];
found = childMarks.find((mark) => isParagraphSplitTrackFormatMark(mark)) || null;
return !found;
Comment on lines +28 to +29
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot Jun 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

P1: findParagraphSplitTrackFormatMark can lose an already-found split mark because the descendants callback overwrites found on later siblings, causing paragraph split tracking metadata to be skipped.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/p/helpers/generate-paragraph-properties.js, line 28:

<comment>`findParagraphSplitTrackFormatMark` can lose an already-found split mark because the descendants callback overwrites `found` on later siblings, causing paragraph split tracking metadata to be skipped.</comment>

<file context>
@@ -1,5 +1,70 @@
+    let found = null;
+    node.descendants((child) => {
+      const childMarks = Array.isArray(child?.marks) ? child.marks : [];
+      found = childMarks.find((mark) => isParagraphSplitTrackFormatMark(mark)) || null;
+      return !found;
+    });
</file context>
Suggested change
found = childMarks.find((mark) => isParagraphSplitTrackFormatMark(mark)) || null;
return !found;
if (!found) {
found = childMarks.find((mark) => isParagraphSplitTrackFormatMark(mark)) || null;
}
return !found;
Fix with Cubic

});
if (found) return found;
}

const content = Array.isArray(node.content) ? node.content : [];
for (const child of content) {
const childMark = findParagraphSplitTrackFormatMark(child);
if (childMark) return childMark;
}

return null;
}

function ensureParagraphPropertiesNode(pPr) {
return (
pPr || {
type: 'element',
name: 'w:pPr',
elements: [],
}
);
}

function prependParagraphSplitInsertion(pPr, insertionElement) {
if (!pPr || !insertionElement) return pPr;
if (!Array.isArray(pPr.elements)) pPr.elements = [];
const existingRunProperties = pPr.elements.find((element) => element?.name === 'w:rPr');
const runProperties = existingRunProperties || {
type: 'element',
name: 'w:rPr',
elements: [],
};
if (!Array.isArray(runProperties.elements)) runProperties.elements = [];
const hasParagraphInsertion = runProperties.elements.some((element) => element?.name === 'w:ins');
if (!hasParagraphInsertion) runProperties.elements.unshift(insertionElement);
if (!existingRunProperties) pPr.elements.unshift(runProperties);
return pPr;
}

/**
* Generate the w:pPr props for a paragraph node
Expand Down Expand Up @@ -33,7 +98,28 @@ export function generateParagraphProperties(params) {
}
}

const paragraphSplitTrackFormatMark = findParagraphSplitTrackFormatMark(node);
const paragraphSplitWordIdOptions = paragraphSplitTrackFormatMark
? {
wordIdAllocator: params?.converter?.wordIdAllocator || null,
partPath: resolveExportPartPath(params),
}
: null;
if (!params?.isFinalDoc && paragraphSplitTrackFormatMark && !paragraphProperties.change) {
const change = createParagraphSplitPropertiesChange(paragraphSplitTrackFormatMark, paragraphSplitWordIdOptions);
if (change) paragraphProperties.change = change;
}

let pPr = wPPrNodeTranslator.decode({ node: { ...node, attrs: { paragraphProperties } } });
if (!params?.isFinalDoc && paragraphSplitTrackFormatMark) {
const insertionElement = createParagraphSplitInsertionElement(
paragraphSplitTrackFormatMark,
paragraphSplitWordIdOptions,
);
if (insertionElement) {
pPr = prependParagraphSplitInsertion(ensureParagraphPropertiesNode(pPr), insertionElement);
}
}
const sectPr = node.attrs?.paragraphProperties?.sectPr;
if (sectPr) {
if (!pPr) {
Expand Down
Loading
Loading