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 80279e34e..925468f79 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); - 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 existingDeleteMarks = node.marks.filter((mark) => mark.type.name === TrackDeleteMarkName); + + if (insertMark && isOwnInsertion(insertMark)) { + 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 36e0e4e2d..f1e35335b 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,11 +136,12 @@ 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) { + // 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; } 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 d442fbbb0..ab96e32b8 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,15 +33,25 @@ describe('trackChangesHelpers replaceStep', () => { plugins: basePlugins, }); - const findTextPos = (docNode, exactText) => { - let found = null; - docNode.descendants((node, pos) => { - if (found) return false; + 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; - if (node.text !== exactText) return; - found = pos; + const hasMark = node.marks.some((mark) => mark.type.name === markName && mark.attrs.id === id); + if (hasMark) { + text += node.text; + } }); - return found; + return text; }; it('types characters in correct order after fully deleting content (SD-1624)', () => { @@ -493,4 +504,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/testUtils.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/testUtils.js new file mode 100644 index 000000000..62d9928ba --- /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 cb365356d..56baee61b 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; @@ -170,6 +171,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 ed074f62e..e2c188a07 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,21 @@ 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); + // 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 { - 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/behavior/fixtures/superdoc.ts b/tests/behavior/fixtures/superdoc.ts index 581ee3763..04150358c 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') { diff --git a/tests/behavior/harness/vite.config.ts b/tests/behavior/harness/vite.config.ts index f3bc953c7..b6e79f174 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 new file mode 100644 index 000000000..82925a0bd --- /dev/null +++ b/tests/behavior/tests/comments/replace-over-multi-paragraph-tracked-changes.spec.ts @@ -0,0 +1,104 @@ +import { test, expect } from '../../fixtures/superdoc.js'; + +test.use({ config: { toolbar: 'full', comments: 'on', trackChanges: true } }); + +test('markDeletion plain delete preserves existing deletion ids', async ({ superdoc }) => { + // 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; + 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 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); +}); 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 000000000..0cff2bd4b --- /dev/null +++ b/tests/visual/tests/behavior/comments-tcs/replace-over-multi-paragraph-tracked-changes.spec.ts @@ -0,0 +1,35 @@ +import { test } from '../../fixtures/superdoc.js'; + +test.use({ config: { comments: 'on', trackChanges: true, hideSelection: false } }); + +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 superdoc.findTextRange('tailword2'); + await superdoc.setTextSelection(tail2.from, tail2.to); + await superdoc.press('Backspace'); + + 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 superdoc.findTextRange('Line two keeps'); + const line3Tail = await superdoc.findTextRange('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'); +}); diff --git a/tests/visual/tests/fixtures/superdoc.ts b/tests/visual/tests/fixtures/superdoc.ts index 49eb61efe..acc7a5c3e 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();