From 84d8c34044b2d017d02c7d8e12d37338c3d3cb62 Mon Sep 17 00:00:00 2001 From: Clarence Palmer Date: Tue, 17 Feb 2026 22:23:21 -0800 Subject: [PATCH 1/9] fix: consolidate deletions under new replacement --- .../trackChangesHelpers/markDeletion.js | 54 ++++--- .../trackChangesHelpers/replaceStep.js | 14 +- .../trackChangesHelpers/replaceStep.test.js | 146 ++++++++++++++++++ .../trackChangesHelpers.test.js | 132 ++++++++++++++++ .../trackChangesHelpers/trackedTransaction.js | 22 +-- ...er-multi-paragraph-tracked-changes.spec.ts | 60 +++++++ 6 files changed, 389 insertions(+), 39 deletions(-) create mode 100644 tests/visual/tests/behavior/comments-tcs/replace-over-multi-paragraph-tracked-changes.spec.ts diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/markDeletion.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/markDeletion.js index 80279e34e8..939ff96ec4 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/markDeletion.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/markDeletion.js @@ -51,38 +51,54 @@ export const markDeletion = ({ tr, from, to, user, date, id: providedId }) => { }); const deletionMap = new Mapping(); + const shouldReassignExistingDeletions = Boolean(providedId); - // Add deletion mark to block nodes (figures, text blocks) and find already deleted inline nodes (and leave them alone). + // Add deletion mark to inline nodes in range. + // Behavior when replacing over existing tracked changes: + // - Own insertions are removed (collapsed). + // - Existing deletions are reassigned to the new deletion mark ID. + // - Non-deleted inline nodes are marked as deleted. let nodes = []; tr.doc.nodesBetween(from, to, (node, pos) => { if (node.type.name.includes('table')) { return; } + // Skip inline containers (e.g. run), operate on leaf inline nodes only. + if (!node.isInline || !node.isLeaf) { + return; + } + + const mappedFrom = deletionMap.map(Math.max(from, pos)); + const mappedTo = deletionMap.map(Math.min(to, pos + node.nodeSize)); + if (mappedFrom >= mappedTo) { + return; + } + const insertMark = node.marks.find((mark) => mark.type.name === TrackInsertMarkName); + const existingDeleteMarks = node.marks.filter((mark) => mark.type.name === TrackDeleteMarkName); + if (node.isInline && insertMark && isOwnInsertion(insertMark)) { - const removeStep = new ReplaceStep( - deletionMap.map(Math.max(from, pos)), - deletionMap.map(Math.min(to, pos + node.nodeSize)), - Slice.empty, - ); + const removeStep = new ReplaceStep(mappedFrom, mappedTo, Slice.empty); if (!tr.maybeStep(removeStep).failed) { deletionMap.appendMap(removeStep.getMap()); } - } else if (node.isInline && !node.marks.find((mark) => mark.type.name === TrackDeleteMarkName)) { - nodes.push(node); - tr.addMark( - deletionMap.map(Math.max(from, pos)), - deletionMap.map(Math.min(to, pos + node.nodeSize)), - deletionMark, - ); - } else if ( - node.attrs.track && - !node.attrs.track.find((trackAttr) => trackAttr.type === TrackDeleteMarkName) && - !['bulletList', 'orderedList'].includes(node.type.name) - ) { - // Skip for now. + return; + } + + if (existingDeleteMarks.length > 0) { + if (shouldReassignExistingDeletions) { + nodes.push(node); + existingDeleteMarks.forEach((existingDeleteMark) => { + tr.removeMark(mappedFrom, mappedTo, existingDeleteMark); + }); + tr.addMark(mappedFrom, mappedTo, deletionMark); + } + return; } + + nodes.push(node); + tr.addMark(mappedFrom, mappedTo, deletionMark); }); return { deletionMark, deletionMap, nodes }; diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.js index 36e0e4e2df..f1ed8c247a 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.js @@ -54,7 +54,6 @@ export const replaceStep = ({ state, tr, step, newTr, map, user, date, originalS // NOTE: Only adjust position for single-step transactions. Multi-step transactions (like input rules) // have subsequent steps that depend on original positions, and adjusting breaks their mapping. let positionTo = step.to; - let positionAdjusted = false; const isSingleStep = tr.steps.length === 1; if (isSingleStep) { @@ -62,7 +61,6 @@ export const replaceStep = ({ state, tr, step, newTr, map, user, date, originalS const deletionSpan = findMarkPosition(trTemp.doc, probePos, TrackDeleteMarkName); if (deletionSpan && deletionSpan.to > positionTo) { positionTo = deletionSpan.to; - positionAdjusted = true; } } @@ -138,14 +136,10 @@ export const replaceStep = ({ state, tr, step, newTr, map, user, date, originalS if (insertedFrom !== insertedTo) { meta.insertedMark = insertedMark; meta.step = condensedStep; - // Store the actual insertion end position for cursor placement (SD-1624). - // Only needed when position was adjusted to insert after a deletion span. - // For single-step transactions, positionTo is in newTr.doc coordinates after our condensedStep, - // so we just add the insertion length to get the cursor position. - if (positionAdjusted) { - const insertionLength = insertedTo - insertedFrom; - meta.insertedTo = positionTo + insertionLength; - } + // Store insertion end position for deterministic caret placement. + // positionTo is in newTr.doc coordinates after condensedStep is applied. + const insertionLength = insertedTo - insertedFrom; + meta.insertedTo = positionTo + insertionLength; } if (!newTr.selection.eq(tempTr.selection)) { diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.test.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.test.js index d442fbbb0b..05b466791c 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.test.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.test.js @@ -43,6 +43,27 @@ describe('trackChangesHelpers replaceStep', () => { return found; }; + const getParagraphRange = (docNode, index) => { + let range = null; + docNode.forEach((node, offset, childIndex) => { + if (childIndex !== index) return; + range = { from: offset + 1, to: offset + node.nodeSize - 1 }; + }); + return range; + }; + + const getTrackedTextById = (docNode, id, markName) => { + let text = ''; + docNode.descendants((node) => { + if (!node.isText) return; + const hasMark = node.marks.some((mark) => mark.type.name === markName && mark.attrs.id === id); + if (hasMark) { + text += node.text; + } + }); + return text; + }; + it('types characters in correct order after fully deleting content (SD-1624)', () => { // Setup: Create a paragraph with "AB" fully marked as deleted const deletionMark = schema.marks[TrackDeleteMarkName].create({ @@ -493,4 +514,129 @@ describe('trackChangesHelpers replaceStep', () => { true, ); }); + + it('supersedes tracked changes across multiple paragraphs with one replacement ID', () => { + const line1 = 'Line one base'; + const line2 = 'Line two base'; + const tail = 'Tail line'; + + const doc = schema.nodes.doc.create({}, [ + schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text(line1)])), + schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text(line2)])), + schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text(tail)])), + ]); + let state = createState(doc); + + const applyTrackedReplace = ({ from, to, text }) => { + let tr = state.tr.replaceWith(from, to, schema.text(text)); + tr.setSelection(TextSelection.create(tr.doc, from + text.length)); + tr.setMeta('inputType', 'insertText'); + const tracked = trackedTransaction({ tr, state, user }); + state = state.apply(tracked); + }; + + const line1Pos = findTextPos(state.doc, line1); + expect(line1Pos).toBeTypeOf('number'); + applyTrackedReplace({ from: line1Pos, to: line1Pos + line1.length, text: 'Line one change' }); + + const line2Pos = findTextPos(state.doc, line2); + expect(line2Pos).toBeTypeOf('number'); + applyTrackedReplace({ from: line2Pos, to: line2Pos + line2.length, text: 'Line two change' }); + + const para1 = getParagraphRange(state.doc, 0); + const para2 = getParagraphRange(state.doc, 1); + expect(para1).toBeTruthy(); + expect(para2).toBeTruthy(); + + state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, para1.from, para2.to))); + let tr = state.tr.replaceWith(para1.from, para2.to, schema.text('Merged suggestion')); + tr.setSelection(TextSelection.create(tr.doc, tr.selection.from)); + tr.setMeta('inputType', 'insertText'); + + const tracked = trackedTransaction({ tr, state, user }); + const meta = tracked.getMeta(TrackChangesBasePluginKey); + const finalState = state.apply(tracked); + + expect(meta?.insertedMark).toBeDefined(); + expect(meta?.deletionMark).toBeDefined(); + expect(meta.insertedMark.attrs.id).toBe(meta.deletionMark.attrs.id); + + const replacementId = meta.insertedMark.attrs.id; + const insertedText = getTrackedTextById(finalState.doc, replacementId, TrackInsertMarkName); + const deletedText = getTrackedTextById(finalState.doc, replacementId, TrackDeleteMarkName); + expect(insertedText).toContain('Merged suggestion'); + expect(deletedText).toContain(line1); + expect(deletedText).toContain(line2); + expect(deletedText).not.toContain('Line one change'); + expect(deletedText).not.toContain('Line two change'); + + const insertIds = new Set(); + finalState.doc.descendants((node) => { + if (!node.isText) return; + node.marks.forEach((mark) => { + if (mark.type.name === TrackInsertMarkName) { + insertIds.add(mark.attrs.id); + } + }); + }); + expect(insertIds.size).toBe(1); + }); + + it('keeps caret stable after superseding multi-paragraph tracked changes', () => { + const line1 = 'Alpha base'; + const line2 = 'Beta base'; + const tail = 'Tail text'; + + const doc = schema.nodes.doc.create({}, [ + schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text(line1)])), + schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text(line2)])), + schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text(tail)])), + ]); + let state = createState(doc); + + const applyTrackedReplace = ({ from, to, text }) => { + let tr = state.tr.replaceWith(from, to, schema.text(text)); + tr.setSelection(TextSelection.create(tr.doc, from + text.length)); + tr.setMeta('inputType', 'insertText'); + const tracked = trackedTransaction({ tr, state, user }); + state = state.apply(tracked); + }; + + const line1Pos = findTextPos(state.doc, line1); + applyTrackedReplace({ from: line1Pos, to: line1Pos + line1.length, text: 'Alpha change' }); + + const line2Pos = findTextPos(state.doc, line2); + applyTrackedReplace({ from: line2Pos, to: line2Pos + line2.length, text: 'Beta change' }); + + const para1 = getParagraphRange(state.doc, 0); + const para2 = getParagraphRange(state.doc, 1); + state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, para1.from, para2.to))); + let tr = state.tr.replaceWith(para1.from, para2.to, schema.text('Merged')); + tr.setSelection(TextSelection.create(tr.doc, tr.selection.from)); + tr.setMeta('inputType', 'insertText'); + state = state.apply(trackedTransaction({ tr, state, user })); + + ['X', 'Y', 'Z'].forEach((char) => { + const prevSelection = state.selection.from; + let typingTr = state.tr.replaceWith(state.selection.from, state.selection.from, schema.text(char)); + typingTr.setSelection(TextSelection.create(typingTr.doc, typingTr.selection.from)); + typingTr.setMeta('inputType', 'insertText'); + state = state.apply(trackedTransaction({ tr: typingTr, state, user })); + + expect(state.selection.from).toBe(prevSelection + 1); + const tailPos = findTextPos(state.doc, tail); + expect(tailPos).toBeTypeOf('number'); + expect(state.selection.from).toBeLessThanOrEqual(tailPos); + }); + + const insertedText = []; + state.doc.descendants((node) => { + if (!node.isText) return; + if (node.marks.some((mark) => mark.type.name === TrackInsertMarkName)) { + insertedText.push(node.text); + } + }); + + expect(insertedText.join('')).toContain('MergedXYZ'); + }); }); diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js index cb365356df..c12226be76 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js @@ -57,6 +57,16 @@ describe('trackChangesHelpers', () => { plugins: basePlugins, }); + const findTextPos = (docNode, exactText) => { + let found = null; + docNode.descendants((node, pos) => { + if (found) return false; + if (!node.isText || node.text !== exactText) return; + found = pos; + }); + return found; + }; + it('findMarkPosition returns full mark span', () => { const mark = schema.marks[TrackInsertMarkName].create({ id: 'ins-1', @@ -170,6 +180,128 @@ describe('trackChangesHelpers', () => { expect(hasDelete).toBe(true); }); + it('markDeletion reassigns existing deletions and removes own insertions for replacements', () => { + const ownInsertMark = schema.marks[TrackInsertMarkName].create({ + id: 'ins-own', + author: user.name, + authorEmail: user.email, + date, + }); + const oldDeleteMark = schema.marks[TrackDeleteMarkName].create({ + id: 'del-old', + author: 'Other User', + authorEmail: 'other@example.com', + date, + }); + + const run = schema.nodes.run.create({}, [ + schema.text('Keep '), + schema.text('OwnInsert', [ownInsertMark]), + schema.text(' OldDelete', [oldDeleteMark]), + schema.text(' Plain'), + ]); + const doc = schema.nodes.doc.create({}, schema.nodes.paragraph.create({}, run)); + const state = createState(doc); + const tr = state.tr; + + const ownInsertPos = findTextPos(state.doc, 'OwnInsert'); + const plainPos = findTextPos(state.doc, ' Plain'); + expect(ownInsertPos).toBeTypeOf('number'); + expect(plainPos).toBeTypeOf('number'); + + const replacementId = 'replacement-id'; + const result = markDeletion({ + tr, + from: ownInsertPos, + to: plainPos + ' Plain'.length, + user, + date, + id: replacementId, + }); + const finalState = state.apply(tr); + + expect(result.deletionMark.attrs.id).toBe(replacementId); + expect(result.nodes.some((node) => node.text?.includes('OldDelete'))).toBe(true); + expect(result.nodes.some((node) => node.text?.includes('Plain'))).toBe(true); + expect(result.deletionMap.maps.length).toBeGreaterThan(0); + + expect(finalState.doc.textContent).not.toContain('OwnInsert'); + + let reassignedDeletedText = ''; + let hasOldDeletionId = false; + finalState.doc.descendants((node) => { + if (!node.isText) return; + node.marks.forEach((mark) => { + if (mark.type.name !== TrackDeleteMarkName) return; + if (mark.attrs.id === replacementId) { + reassignedDeletedText += node.text; + } + if (mark.attrs.id === 'del-old') { + hasOldDeletionId = true; + } + }); + }); + + expect(reassignedDeletedText).toContain('OldDelete'); + expect(reassignedDeletedText).toContain('Plain'); + expect(hasOldDeletionId).toBe(false); + }); + + it('markDeletion preserves existing deletions on plain delete actions', () => { + const oldDeleteMark = schema.marks[TrackDeleteMarkName].create({ + id: 'del-old', + author: 'Other User', + authorEmail: 'other@example.com', + date, + }); + + const run = schema.nodes.run.create({}, [ + schema.text('Keep '), + schema.text('OldDelete', [oldDeleteMark]), + schema.text(' Plain'), + ]); + const doc = schema.nodes.doc.create({}, schema.nodes.paragraph.create({}, run)); + const state = createState(doc); + const tr = state.tr; + + const oldDeletePos = findTextPos(state.doc, 'OldDelete'); + const plainPos = findTextPos(state.doc, ' Plain'); + expect(oldDeletePos).toBeTypeOf('number'); + expect(plainPos).toBeTypeOf('number'); + + markDeletion({ + tr, + from: oldDeletePos, + to: plainPos + ' Plain'.length, + user, + date, + }); + const finalState = state.apply(tr); + + let hasOldDeleteId = false; + let plainHasDeleteMark = false; + let plainHasOldDeleteId = false; + + finalState.doc.descendants((node) => { + if (!node.isText) return; + const deleteMarks = node.marks.filter((mark) => mark.type.name === TrackDeleteMarkName); + if (!deleteMarks.length) return; + + if (deleteMarks.some((mark) => mark.attrs.id === 'del-old')) { + hasOldDeleteId = true; + } + + if (node.text.includes('Plain')) { + plainHasDeleteMark = true; + plainHasOldDeleteId = deleteMarks.some((mark) => mark.attrs.id === 'del-old'); + } + }); + + expect(hasOldDeleteId).toBe(true); + expect(plainHasDeleteMark).toBe(true); + expect(plainHasOldDeleteId).toBe(false); + }); + it('removes Word-imported insertions without authorEmail when deleted', () => { const insertXml = ` diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackedTransaction.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackedTransaction.js index ed074f62e5..db277fc84e 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackedTransaction.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackedTransaction.js @@ -101,9 +101,6 @@ export const trackedTransaction = ({ tr, state, user }) => { const trackMeta = newTr.getMeta(TrackChangesBasePluginKey); if (tr.selectionSet) { - const deletionMarkSchema = state.schema.marks[TrackDeleteMarkName]; - const deletionMark = findMark(state, deletionMarkSchema, false); - if ( tr.selection instanceof TextSelection && (tr.selection.from < state.selection.from || tr.getMeta('inputType') === 'deleteContentBackward') @@ -111,14 +108,19 @@ export const trackedTransaction = ({ tr, state, user }) => { const caretPos = map.map(tr.selection.from, -1); newTr.setSelection(new TextSelection(newTr.doc.resolve(caretPos))); } else if (trackMeta?.insertedTo !== undefined) { - // SD-1624: When content was inserted after a deletion span, position cursor after the insertion. - // This must be checked before the deletionMark branch to handle fully-deleted content correctly. - newTr.setSelection(new TextSelection(newTr.doc.resolve(trackMeta.insertedTo))); - } else if (tr.selection.from > state.selection.from && deletionMark) { - const caretPos = map.map(deletionMark.to + 1, 1); - newTr.setSelection(new TextSelection(newTr.doc.resolve(caretPos))); + const boundedInsertedTo = Math.max(0, Math.min(trackMeta.insertedTo, newTr.doc.content.size)); + const $insertPos = newTr.doc.resolve(boundedInsertedTo); + newTr.setSelection(TextSelection.near($insertPos, 1)); } else { - newTr.setSelection(tr.selection.map(newTr.doc, map)); + const deletionMarkSchema = state.schema.marks[TrackDeleteMarkName]; + const deletionMark = findMark(state, deletionMarkSchema, false); + + if (tr.selection.from > state.selection.from && deletionMark) { + const caretPos = map.map(deletionMark.to + 1, 1); + newTr.setSelection(new TextSelection(newTr.doc.resolve(caretPos))); + } else { + newTr.setSelection(tr.selection.map(newTr.doc, map)); + } } } else if (state.selection.from - tr.selection.from > 1 && tr.selection.$head.depth > 1) { const caretPos = map.map(tr.selection.from - 2, -1); diff --git a/tests/visual/tests/behavior/comments-tcs/replace-over-multi-paragraph-tracked-changes.spec.ts b/tests/visual/tests/behavior/comments-tcs/replace-over-multi-paragraph-tracked-changes.spec.ts new file mode 100644 index 0000000000..fbe93bd90e --- /dev/null +++ b/tests/visual/tests/behavior/comments-tcs/replace-over-multi-paragraph-tracked-changes.spec.ts @@ -0,0 +1,60 @@ +import { type Page } from '@playwright/test'; +import { test } from '../../fixtures/superdoc.js'; + +test.use({ config: { comments: 'on', trackChanges: true, hideSelection: false } }); + +async function findTextRange(page: Page, text: string) { + return page.evaluate((needle) => { + const editor = (window as any).editor; + let found: { from: number; to: number } | null = null; + + editor.state.doc.descendants((node: any, pos: number) => { + if (found) return false; + if (!node.isText || !node.text) return true; + + const index = node.text.indexOf(needle); + if (index === -1) return true; + + found = { from: pos + index, to: pos + index + needle.length }; + return false; + }); + + if (!found) { + throw new Error(`Text not found: ${needle}`); + } + + return found; + }, text); +} + +test('replace over multi-paragraph tracked changes stays coherent', async ({ superdoc }) => { + await superdoc.type('Line one stays'); + await superdoc.newLine(); + await superdoc.type('Line two keeps tailword2'); + await superdoc.newLine(); + await superdoc.type('Line three keeps tailword3'); + await superdoc.waitForStable(); + await superdoc.screenshot('it-67-step-1-initial-lines'); + + await superdoc.setDocumentMode('suggesting'); + await superdoc.waitForStable(); + + const tail2 = await findTextRange(superdoc.page, 'tailword2'); + await superdoc.setTextSelection(tail2.from, tail2.to); + await superdoc.press('Backspace'); + + const tail3 = await findTextRange(superdoc.page, 'tailword3'); + await superdoc.setTextSelection(tail3.from, tail3.to); + await superdoc.press('Backspace'); + + await superdoc.waitForStable(); + await superdoc.screenshot('it-67-step-2-lines-2-3-last-word-deleted'); + + const line2Start = await findTextRange(superdoc.page, 'Line two keeps'); + const line3Tail = await findTextRange(superdoc.page, 'tailword3'); + await superdoc.setTextSelection(line2Start.from, line3Tail.to); + await superdoc.type('Merged suggestion'); + + await superdoc.waitForStable(); + await superdoc.screenshot('it-67-step-3-replaced-lines-2-3-with-single-change'); +}); From d3c23d13047f97c4e2c0cfe7aee4746d8a026834 Mon Sep 17 00:00:00 2001 From: Clarence Palmer Date: Wed, 18 Feb 2026 21:43:26 -0800 Subject: [PATCH 2/9] fix: be more specific about cursor movement --- .../trackChangesHelpers/replaceStep.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.js index f1ed8c247a..f1e35335bb 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.js @@ -136,10 +136,15 @@ export const replaceStep = ({ state, tr, step, newTr, map, user, date, originalS if (insertedFrom !== insertedTo) { meta.insertedMark = insertedMark; meta.step = condensedStep; - // Store insertion end position for deterministic caret placement. - // positionTo is in newTr.doc coordinates after condensedStep is applied. - const insertionLength = insertedTo - insertedFrom; - meta.insertedTo = positionTo + insertionLength; + // Store insertion end position when (1) we adjusted the insertion position (e.g. past a + // deletion span), or (2) single-step replace of a range — selection mapping is wrong then + // so we need an explicit caret position. Skip for multi-step (e.g. input rules) so their + // intended selection is preserved. + const needInsertedTo = positionTo !== step.to || (isSingleStep && step.from !== step.to); + if (needInsertedTo) { + const insertionLength = insertedTo - insertedFrom; + meta.insertedTo = positionTo + insertionLength; + } } if (!newTr.selection.eq(tempTr.selection)) { From 9e8468840a0b88b932eeeb1f1c8cc5d5fb358f68 Mon Sep 17 00:00:00 2001 From: Clarence Palmer Date: Wed, 18 Feb 2026 22:10:21 -0800 Subject: [PATCH 3/9] chore: comment for near --- .../track-changes/trackChangesHelpers/trackedTransaction.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackedTransaction.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackedTransaction.js index db277fc84e..e2c188a07a 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackedTransaction.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackedTransaction.js @@ -110,6 +110,8 @@ export const trackedTransaction = ({ tr, state, user }) => { } else if (trackMeta?.insertedTo !== undefined) { const boundedInsertedTo = Math.max(0, Math.min(trackMeta.insertedTo, newTr.doc.content.size)); const $insertPos = newTr.doc.resolve(boundedInsertedTo); + // Near is used here because its safer than an exact position + // exact is not guaranteed to be a valid cursor position newTr.setSelection(TextSelection.near($insertPos, 1)); } else { const deletionMarkSchema = state.schema.marks[TrackDeleteMarkName]; From c47308c391d920e4e8569f0a9b4aacce13ddabf8 Mon Sep 17 00:00:00 2001 From: Clarence Palmer Date: Wed, 18 Feb 2026 22:44:07 -0800 Subject: [PATCH 4/9] fix(track-changes): simplify deletion mark condition in markDeletion helper --- .../track-changes/trackChangesHelpers/markDeletion.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/markDeletion.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/markDeletion.js index 939ff96ec4..925468f799 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/markDeletion.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/markDeletion.js @@ -78,7 +78,7 @@ export const markDeletion = ({ tr, from, to, user, date, id: providedId }) => { const insertMark = node.marks.find((mark) => mark.type.name === TrackInsertMarkName); const existingDeleteMarks = node.marks.filter((mark) => mark.type.name === TrackDeleteMarkName); - if (node.isInline && insertMark && isOwnInsertion(insertMark)) { + if (insertMark && isOwnInsertion(insertMark)) { const removeStep = new ReplaceStep(mappedFrom, mappedTo, Slice.empty); if (!tr.maybeStep(removeStep).failed) { deletionMap.appendMap(removeStep.getMap()); From e3b37ea04c9f13a1511e5aac26ed505eb6d6f5e9 Mon Sep 17 00:00:00 2001 From: Clarence Palmer Date: Wed, 18 Feb 2026 23:20:01 -0800 Subject: [PATCH 5/9] refactor(track-changes): extract findTextPos utility to testUtils for reuse in tests --- .../trackChangesHelpers/replaceStep.test.js | 12 +----------- .../trackChangesHelpers/testUtils.js | 19 +++++++++++++++++++ .../trackChangesHelpers.test.js | 11 +---------- 3 files changed, 21 insertions(+), 21 deletions(-) create mode 100644 packages/super-editor/src/extensions/track-changes/trackChangesHelpers/testUtils.js diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.test.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.test.js index 05b466791c..ab96e32b83 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.test.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.test.js @@ -5,6 +5,7 @@ import { trackedTransaction, documentHelpers } from './index.js'; import { TrackInsertMarkName, TrackDeleteMarkName } from '../constants.js'; import { TrackChangesBasePluginKey } from '../plugins/trackChangesBasePlugin.js'; import { initTestEditor } from '@tests/helpers/helpers.js'; +import { findTextPos } from './testUtils.js'; describe('trackChangesHelpers replaceStep', () => { let editor; @@ -32,17 +33,6 @@ describe('trackChangesHelpers replaceStep', () => { plugins: basePlugins, }); - const findTextPos = (docNode, exactText) => { - let found = null; - docNode.descendants((node, pos) => { - if (found) return false; - if (!node.isText) return; - if (node.text !== exactText) return; - found = pos; - }); - return found; - }; - const getParagraphRange = (docNode, index) => { let range = null; docNode.forEach((node, offset, childIndex) => { diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/testUtils.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/testUtils.js new file mode 100644 index 0000000000..62d9928ba2 --- /dev/null +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/testUtils.js @@ -0,0 +1,19 @@ +/** + * Shared test utilities for trackChangesHelpers tests. + */ + +/** + * Find the document position of a text node whose text equals exactText. + * @param {import('prosemirror-model').Node} docNode - Document or node to search + * @param {string} exactText - Exact text to find + * @returns {number | null} Start position of the text node, or null if not found + */ +export function findTextPos(docNode, exactText) { + let found = null; + docNode.descendants((node, pos) => { + if (found) return false; + if (!node.isText || node.text !== exactText) return; + found = pos; + }); + return found; +} diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js index c12226be76..56baee61bd 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js @@ -24,6 +24,7 @@ import { handleTrackChangeNode } from '@converter/v2/importer/trackChangesImport import { defaultNodeListHandler } from '@converter/v2/importer/docxImporter.js'; import { parseXmlToJson } from '@converter/v2/docxHelper.js'; import { initTestEditor } from '@tests/helpers/helpers.js'; +import { findTextPos } from './testUtils.js'; describe('trackChangesHelpers', () => { let editor; @@ -57,16 +58,6 @@ describe('trackChangesHelpers', () => { plugins: basePlugins, }); - const findTextPos = (docNode, exactText) => { - let found = null; - docNode.descendants((node, pos) => { - if (found) return false; - if (!node.isText || node.text !== exactText) return; - found = pos; - }); - return found; - }; - it('findMarkPosition returns full mark span', () => { const mark = schema.marks[TrackInsertMarkName].create({ id: 'ins-1', From 651622ac3e09b0412154307f8bcaac5ec511c75c Mon Sep 17 00:00:00 2001 From: Clarence Palmer Date: Wed, 18 Feb 2026 23:28:19 -0800 Subject: [PATCH 6/9] refactor: move to sd fixture --- ...er-multi-paragraph-tracked-changes.spec.ts | 33 +++---------------- tests/visual/tests/fixtures/superdoc.ts | 26 +++++++++++++++ 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/tests/visual/tests/behavior/comments-tcs/replace-over-multi-paragraph-tracked-changes.spec.ts b/tests/visual/tests/behavior/comments-tcs/replace-over-multi-paragraph-tracked-changes.spec.ts index fbe93bd90e..0cff2bd4bf 100644 --- a/tests/visual/tests/behavior/comments-tcs/replace-over-multi-paragraph-tracked-changes.spec.ts +++ b/tests/visual/tests/behavior/comments-tcs/replace-over-multi-paragraph-tracked-changes.spec.ts @@ -1,32 +1,7 @@ -import { type Page } from '@playwright/test'; import { test } from '../../fixtures/superdoc.js'; test.use({ config: { comments: 'on', trackChanges: true, hideSelection: false } }); -async function findTextRange(page: Page, text: string) { - return page.evaluate((needle) => { - const editor = (window as any).editor; - let found: { from: number; to: number } | null = null; - - editor.state.doc.descendants((node: any, pos: number) => { - if (found) return false; - if (!node.isText || !node.text) return true; - - const index = node.text.indexOf(needle); - if (index === -1) return true; - - found = { from: pos + index, to: pos + index + needle.length }; - return false; - }); - - if (!found) { - throw new Error(`Text not found: ${needle}`); - } - - return found; - }, text); -} - test('replace over multi-paragraph tracked changes stays coherent', async ({ superdoc }) => { await superdoc.type('Line one stays'); await superdoc.newLine(); @@ -39,19 +14,19 @@ test('replace over multi-paragraph tracked changes stays coherent', async ({ sup await superdoc.setDocumentMode('suggesting'); await superdoc.waitForStable(); - const tail2 = await findTextRange(superdoc.page, 'tailword2'); + const tail2 = await superdoc.findTextRange('tailword2'); await superdoc.setTextSelection(tail2.from, tail2.to); await superdoc.press('Backspace'); - const tail3 = await findTextRange(superdoc.page, 'tailword3'); + const tail3 = await superdoc.findTextRange('tailword3'); await superdoc.setTextSelection(tail3.from, tail3.to); await superdoc.press('Backspace'); await superdoc.waitForStable(); await superdoc.screenshot('it-67-step-2-lines-2-3-last-word-deleted'); - const line2Start = await findTextRange(superdoc.page, 'Line two keeps'); - const line3Tail = await findTextRange(superdoc.page, 'tailword3'); + const line2Start = await superdoc.findTextRange('Line two keeps'); + const line3Tail = await superdoc.findTextRange('tailword3'); await superdoc.setTextSelection(line2Start.from, line3Tail.to); await superdoc.type('Merged suggestion'); diff --git a/tests/visual/tests/fixtures/superdoc.ts b/tests/visual/tests/fixtures/superdoc.ts index 49eb61efea..acc7a5c3e8 100644 --- a/tests/visual/tests/fixtures/superdoc.ts +++ b/tests/visual/tests/fixtures/superdoc.ts @@ -77,6 +77,8 @@ export interface SuperDocFixture { setDocumentMode(mode: 'editing' | 'suggesting' | 'viewing'): Promise; /** Set cursor/selection position via ProseMirror positions */ setTextSelection(from: number, to?: number): Promise; + /** Find the first occurrence of a text string in the document and return its ProseMirror position range. */ + findTextRange(text: string): Promise<{ from: number; to: number }>; /** Single click on a line by index */ clickOnLine(lineIndex: number, xOffset?: number): Promise; /** Click on a comment highlight containing the given text */ @@ -198,6 +200,30 @@ export const test = base.extend<{ superdoc: SuperDocFixture } & SuperDocOptions> ); }, + async findTextRange(text: string): Promise<{ from: number; to: number }> { + return page.evaluate((needle) => { + const editor = (window as any).editor; + let found: { from: number; to: number } | null = null; + + editor.state.doc.descendants((node: any, pos: number) => { + if (found) return false; + if (!node.isText || !node.text) return true; + + const index = node.text.indexOf(needle); + if (index === -1) return true; + + found = { from: pos + index, to: pos + index + needle.length }; + return false; + }); + + if (!found) { + throw new Error(`Text not found: ${needle}`); + } + + return found; + }, text); + }, + async clickOnLine(lineIndex: number, xOffset = 10) { const line = page.locator('.superdoc-line').nth(lineIndex); const box = await line.boundingBox(); From dca97792ee1cba707b31a337b674b75ed8db279b Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Fri, 20 Feb 2026 07:49:11 -0800 Subject: [PATCH 7/9] test(behavior): add behavior test for this PR --- ...er-multi-paragraph-tracked-changes.spec.ts | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 tests/behavior/tests/comments/replace-over-multi-paragraph-tracked-changes.spec.ts diff --git a/tests/behavior/tests/comments/replace-over-multi-paragraph-tracked-changes.spec.ts b/tests/behavior/tests/comments/replace-over-multi-paragraph-tracked-changes.spec.ts new file mode 100644 index 0000000000..f9f26fa429 --- /dev/null +++ b/tests/behavior/tests/comments/replace-over-multi-paragraph-tracked-changes.spec.ts @@ -0,0 +1,98 @@ +import { test, expect } from '../../fixtures/superdoc.js'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +test.use({ config: { toolbar: 'full', comments: 'on', trackChanges: true } }); + +test('markDeletion plain delete preserves existing deletion ids', async ({ superdoc }) => { + const __dirname = path.dirname(fileURLToPath(import.meta.url)); + const repoRoot = path.resolve(__dirname, '../../../../'); + const modulePath = `${repoRoot}/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/markDeletion.js`; + const moduleUrl = `/@fs${modulePath}`; + + const result = await superdoc.page.evaluate( + async ({ url }) => { + const { markDeletion } = await import(url); + + const editor = (window as any).editor; + const schema = editor.state.schema; + + const user = { name: 'Track Tester', email: 'track@example.com' }; + const date = new Date().toISOString(); + + const oldDeleteMark = schema.marks.trackDelete.create({ + id: 'del-old', + author: 'Other User', + authorEmail: 'other@example.com', + date, + }); + + const run = schema.nodes.run.create({}, [ + schema.text('Keep '), + schema.text('OldDelete', [oldDeleteMark]), + schema.text(' Plain'), + ]); + const doc = schema.nodes.doc.create({}, schema.nodes.paragraph.create({}, run)); + + editor.view.dispatch(editor.state.tr.replaceWith(0, editor.state.doc.content.size, doc.content)); + + const findTextPos = (needle: string): number => { + let found: number | null = null; + editor.state.doc.descendants((node: any, pos: number) => { + if (found !== null) return false; + if (!node.isText || !node.text) return; + const idx = node.text.indexOf(needle); + if (idx === -1) return; + found = pos + idx; + }); + if (found === null) { + throw new Error(`Text not found: ${needle}`); + } + return found; + }; + + const beforeById: Record = {}; + editor.state.doc.descendants((node: any) => { + if (!node.isText || !node.text) return; + for (const mark of node.marks ?? []) { + if (mark.type?.name !== 'trackDelete') continue; + const id = mark.attrs?.id; + if (!id) continue; + beforeById[id] = (beforeById[id] ?? '') + node.text; + } + }); + + const from = findTextPos('OldDelete'); + const plainPos = findTextPos(' Plain'); + const to = plainPos + ' Plain'.length; + + const tr = editor.state.tr; + markDeletion({ tr, from, to, user, date }); + editor.view.dispatch(tr); + + const afterById: Record = {}; + editor.state.doc.descendants((node: any) => { + if (!node.isText || !node.text) return; + for (const mark of node.marks ?? []) { + if (mark.type?.name !== 'trackDelete') continue; + const id = mark.attrs?.id; + if (!id) continue; + afterById[id] = (afterById[id] ?? '') + node.text; + } + }); + + const beforeOldId = Object.keys(beforeById).find((id) => beforeById[id].includes('OldDelete')) ?? null; + const afterOldId = Object.keys(afterById).find((id) => afterById[id].includes('OldDelete')) ?? null; + const afterPlainId = Object.keys(afterById).find((id) => afterById[id].includes('Plain')) ?? null; + + return { beforeById, afterById, beforeOldId, afterOldId, afterPlainId }; + }, + { url: moduleUrl }, + ); + + expect(result.beforeOldId).not.toBeNull(); + expect(result.afterOldId).not.toBeNull(); + expect(result.afterPlainId).not.toBeNull(); + expect(result.afterOldId).toBe(result.beforeOldId); + expect(result.afterOldId).not.toBe(result.afterPlainId); +}); From ac048f52c2f2f6a2c6730c5dcb79285f9812243e Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Fri, 20 Feb 2026 09:19:32 -0800 Subject: [PATCH 8/9] chore: fix behavior harness ci issues --- tests/behavior/harness/vite.config.ts | 3 + ...er-multi-paragraph-tracked-changes.spec.ts | 186 +++++++++--------- 2 files changed, 99 insertions(+), 90 deletions(-) diff --git a/tests/behavior/harness/vite.config.ts b/tests/behavior/harness/vite.config.ts index f3bc953c72..b6e79f1746 100644 --- a/tests/behavior/harness/vite.config.ts +++ b/tests/behavior/harness/vite.config.ts @@ -6,6 +6,9 @@ export default defineConfig({ strictPort: true, }, optimizeDeps: { + // Do NOT use /@fs dynamic imports in tests — they cause Vite to discover + // and re-optimize deps mid-run, which invalidates browser contexts and + // breaks parallel workers (especially WebKit) in CI. exclude: ['superdoc'], }, }); diff --git a/tests/behavior/tests/comments/replace-over-multi-paragraph-tracked-changes.spec.ts b/tests/behavior/tests/comments/replace-over-multi-paragraph-tracked-changes.spec.ts index f9f26fa429..82925a0bd9 100644 --- a/tests/behavior/tests/comments/replace-over-multi-paragraph-tracked-changes.spec.ts +++ b/tests/behavior/tests/comments/replace-over-multi-paragraph-tracked-changes.spec.ts @@ -1,98 +1,104 @@ import { test, expect } from '../../fixtures/superdoc.js'; -import path from 'node:path'; -import { fileURLToPath } from 'node:url'; test.use({ config: { toolbar: 'full', comments: 'on', trackChanges: true } }); test('markDeletion plain delete preserves existing deletion ids', async ({ superdoc }) => { - const __dirname = path.dirname(fileURLToPath(import.meta.url)); - const repoRoot = path.resolve(__dirname, '../../../../'); - const modulePath = `${repoRoot}/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/markDeletion.js`; - const moduleUrl = `/@fs${modulePath}`; - - const result = await superdoc.page.evaluate( - async ({ url }) => { - const { markDeletion } = await import(url); - - const editor = (window as any).editor; - const schema = editor.state.schema; - - const user = { name: 'Track Tester', email: 'track@example.com' }; - const date = new Date().toISOString(); - - const oldDeleteMark = schema.marks.trackDelete.create({ - id: 'del-old', - author: 'Other User', - authorEmail: 'other@example.com', - date, - }); - - const run = schema.nodes.run.create({}, [ - schema.text('Keep '), - schema.text('OldDelete', [oldDeleteMark]), - schema.text(' Plain'), - ]); - const doc = schema.nodes.doc.create({}, schema.nodes.paragraph.create({}, run)); - - editor.view.dispatch(editor.state.tr.replaceWith(0, editor.state.doc.content.size, doc.content)); - - const findTextPos = (needle: string): number => { - let found: number | null = null; - editor.state.doc.descendants((node: any, pos: number) => { - if (found !== null) return false; - if (!node.isText || !node.text) return; - const idx = node.text.indexOf(needle); - if (idx === -1) return; - found = pos + idx; - }); - if (found === null) { - throw new Error(`Text not found: ${needle}`); - } - return found; - }; - - const beforeById: Record = {}; - editor.state.doc.descendants((node: any) => { + // Seed document with a pre-existing foreign trackDelete mark before enabling suggesting mode. + await superdoc.page.evaluate(() => { + const editor = (window as any).editor; + const schema = editor.state.schema; + const date = new Date().toISOString(); + + const oldDeleteMark = schema.marks.trackDelete.create({ + id: 'del-old', + author: 'Other User', + authorEmail: 'other@example.com', + date, + }); + + const run = schema.nodes.run.create({}, [ + schema.text('Keep '), + schema.text('OldDelete', [oldDeleteMark]), + schema.text(' Plain'), + ]); + const doc = schema.nodes.doc.create({}, schema.nodes.paragraph.create({}, run)); + editor.view.dispatch(editor.state.tr.replaceWith(0, editor.state.doc.content.size, doc.content)); + }); + await superdoc.waitForStable(); + + // Record mark IDs before the delete. + const beforeById = await superdoc.page.evaluate(() => { + const editor = (window as any).editor; + const result: Record = {}; + editor.state.doc.descendants((node: any) => { + if (!node.isText || !node.text) return; + for (const mark of node.marks ?? []) { + if (mark.type?.name !== 'trackDelete') continue; + const id = mark.attrs?.id; + if (!id) continue; + result[id] = (result[id] ?? '') + node.text; + } + }); + return result; + }); + + // Configure user for tracked transactions and switch to suggesting mode. + await superdoc.page.evaluate(() => { + (window as any).editor.setOptions({ + user: { name: 'Track Tester', email: 'track@example.com' }, + }); + }); + await superdoc.setDocumentMode('suggesting'); + await superdoc.waitForStable(); + + // Select the range covering "OldDelete" through " Plain" and delete it. + const { from, to } = await superdoc.page.evaluate(() => { + const editor = (window as any).editor; + const findTextPos = (needle: string): number => { + let found: number | null = null; + editor.state.doc.descendants((node: any, pos: number) => { + if (found !== null) return false; if (!node.isText || !node.text) return; - for (const mark of node.marks ?? []) { - if (mark.type?.name !== 'trackDelete') continue; - const id = mark.attrs?.id; - if (!id) continue; - beforeById[id] = (beforeById[id] ?? '') + node.text; - } + const idx = node.text.indexOf(needle); + if (idx === -1) return; + found = pos + idx; }); - - const from = findTextPos('OldDelete'); - const plainPos = findTextPos(' Plain'); - const to = plainPos + ' Plain'.length; - - const tr = editor.state.tr; - markDeletion({ tr, from, to, user, date }); - editor.view.dispatch(tr); - - const afterById: Record = {}; - editor.state.doc.descendants((node: any) => { - if (!node.isText || !node.text) return; - for (const mark of node.marks ?? []) { - if (mark.type?.name !== 'trackDelete') continue; - const id = mark.attrs?.id; - if (!id) continue; - afterById[id] = (afterById[id] ?? '') + node.text; - } - }); - - const beforeOldId = Object.keys(beforeById).find((id) => beforeById[id].includes('OldDelete')) ?? null; - const afterOldId = Object.keys(afterById).find((id) => afterById[id].includes('OldDelete')) ?? null; - const afterPlainId = Object.keys(afterById).find((id) => afterById[id].includes('Plain')) ?? null; - - return { beforeById, afterById, beforeOldId, afterOldId, afterPlainId }; - }, - { url: moduleUrl }, - ); - - expect(result.beforeOldId).not.toBeNull(); - expect(result.afterOldId).not.toBeNull(); - expect(result.afterPlainId).not.toBeNull(); - expect(result.afterOldId).toBe(result.beforeOldId); - expect(result.afterOldId).not.toBe(result.afterPlainId); + if (found === null) throw new Error(`Text not found: ${needle}`); + return found; + }; + const from = findTextPos('OldDelete'); + const plainPos = findTextPos(' Plain'); + const to = plainPos + ' Plain'.length; + return { from, to }; + }); + + await superdoc.setTextSelection(from, to); + await superdoc.page.keyboard.press('Delete'); + await superdoc.waitForStable(); + + // Read resulting marks. + const afterById = await superdoc.page.evaluate(() => { + const editor = (window as any).editor; + const result: Record = {}; + editor.state.doc.descendants((node: any) => { + if (!node.isText || !node.text) return; + for (const mark of node.marks ?? []) { + if (mark.type?.name !== 'trackDelete') continue; + const id = mark.attrs?.id; + if (!id) continue; + result[id] = (result[id] ?? '') + node.text; + } + }); + return result; + }); + + const beforeOldId = Object.keys(beforeById).find((id) => beforeById[id].includes('OldDelete')) ?? null; + const afterOldId = Object.keys(afterById).find((id) => afterById[id].includes('OldDelete')) ?? null; + const afterPlainId = Object.keys(afterById).find((id) => afterById[id].includes('Plain')) ?? null; + + expect(beforeOldId).not.toBeNull(); + expect(afterOldId).not.toBeNull(); + expect(afterPlainId).not.toBeNull(); + expect(afterOldId).toBe(beforeOldId); + expect(afterOldId).not.toBe(afterPlainId); }); From 18734a2b68c2fae31ad98c891a185bfb134ae400 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Fri, 20 Feb 2026 12:09:55 -0800 Subject: [PATCH 9/9] chore: fix test fixture in ci --- tests/behavior/fixtures/superdoc.ts | 45 +++++++++++++++++------------ 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/tests/behavior/fixtures/superdoc.ts b/tests/behavior/fixtures/superdoc.ts index 581ee37633..04150358cf 100644 --- a/tests/behavior/fixtures/superdoc.ts +++ b/tests/behavior/fixtures/superdoc.ts @@ -556,29 +556,36 @@ function createFixture(page: Page, editor: Locator, modKey: string) { }, async assertCommentHighlightExists(opts?: { text?: string; commentId?: string }) { - const highlights = page.locator('.superdoc-comment-highlight'); - await expect(highlights.first()).toBeAttached(); + const expectedText = opts?.text; + const expectedCommentId = opts?.commentId; + await expect + .poll(() => + page.evaluate( + ({ text, commentId }) => { + const highlights = Array.from(document.querySelectorAll('.superdoc-comment-highlight')); + if (highlights.length === 0) return false; - if (opts?.text) { - await expect(highlights.filter({ hasText: opts.text }).first()).toBeAttached(); - } - if (opts?.commentId) { - const commentId = opts.commentId; - await expect - .poll(() => - page.evaluate( - (id) => - Array.from(document.querySelectorAll('.superdoc-comment-highlight')).some((el) => + if (text) { + const hasTextMatch = highlights.some((el) => (el.textContent ?? '').includes(text)); + if (!hasTextMatch) return false; + } + + if (commentId) { + const hasCommentId = highlights.some((el) => (el.getAttribute('data-comment-ids') ?? '') .split(/[\s,]+/) .filter(Boolean) - .includes(id), - ), - commentId, - ), - ) - .toBe(true); - } + .includes(commentId), + ); + if (!hasCommentId) return false; + } + + return true; + }, + { text: expectedText, commentId: expectedCommentId }, + ), + ) + .toBe(true); }, async assertTrackedChangeExists(type: 'insert' | 'delete' | 'format') {