diff --git a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js index 83bcf65319..72c70cd49e 100644 --- a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js +++ b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js @@ -988,13 +988,13 @@ const normalizeFormatAttrsForCommentText = (attrs = {}, nodes) => { }; }; -const getTrackedChangeText = ({ nodes, mark, trackedChangeType, isDeletionInsertion }) => { +const getTrackedChangeText = ({ nodes, mark, trackedChangeType, isReplacement }) => { let trackedChangeText = ''; let deletionText = ''; let trackedChangeDisplayType = null; // Extract deletion text first - if (trackedChangeType === TrackDeleteMarkName || isDeletionInsertion) { + if (trackedChangeType === TrackDeleteMarkName || isReplacement) { deletionText = nodes.reduce((acc, node) => { const hasDeleteMark = node.marks.find((nodeMark) => nodeMark.type.name === TrackDeleteMarkName); if (!hasDeleteMark) return acc; @@ -1004,7 +1004,7 @@ const getTrackedChangeText = ({ nodes, mark, trackedChangeType, isDeletionInsert }, ''); } - if (trackedChangeType === TrackInsertMarkName || isDeletionInsertion) { + if (trackedChangeType === TrackInsertMarkName || isReplacement) { trackedChangeText = nodes.reduce((acc, node) => { const hasInsertMark = node.marks.find((nodeMark) => nodeMark.type.name === TrackInsertMarkName); if (!hasInsertMark) return acc; @@ -1064,17 +1064,14 @@ const createOrUpdateTrackedChangeComment = ({ const { author, authorEmail, authorImage, date, importedAuthor } = attrs; const id = attrs.id; - // Check metadata first - this should be set correctly by groupChanges() in createCommentForTrackChanges - // for both newly created and imported tracked changes - let isDeletionInsertion = !!(marks.insertedMark && marks.deletionMark); + let isReplacement = !!(marks.insertedMark && marks.deletionMark); - // Fallback: If metadata doesn't indicate replacement (e.g., edge cases during import), - // check the document state directly to detect replacements by finding both marks with same ID - // This ensures robustness even if groupChanges() misses a replacement or metadata isn't set - if (!isDeletionInsertion) { + // Fallback: check the document for both mark types under the same ID + // (covers edge cases where transaction meta only carries one mark) + if (!isReplacement) { const hasInsertMark = trackedChangesWithId.some(({ mark }) => mark.type.name === TrackInsertMarkName); const hasDeleteMark = trackedChangesWithId.some(({ mark }) => mark.type.name === TrackDeleteMarkName); - isDeletionInsertion = hasInsertMark && hasDeleteMark; + isReplacement = hasInsertMark && hasDeleteMark; } // Collect nodes from the tracked changes found @@ -1097,10 +1094,8 @@ const createOrUpdateTrackedChangeComment = ({ }); }); - // For replacements, we need both insertion nodes and deletion nodes - // When isDeletionInsertion is true, nodesWithMark should contain both types let nodesToUse; - if (isDeletionInsertion) { + if (isReplacement) { // For replacements, prefer nodes found in the document to avoid duplicating text // when step.slice/deletionNodes include overlapping content. const hasInsertNode = nodesWithMark.some((node) => @@ -1128,7 +1123,7 @@ const createOrUpdateTrackedChangeComment = ({ nodes: nodesToUse, mark: trackedMark, trackedChangeType, - isDeletionInsertion, + isReplacement, deletionNodes, }); @@ -1141,10 +1136,10 @@ const createOrUpdateTrackedChangeComment = ({ type: 'trackedChange', documentId, changeId: id, - trackedChangeType: isDeletionInsertion ? 'both' : trackedChangeType, + trackedChangeType: isReplacement ? 'both' : trackedChangeType, trackedChangeText, trackedChangeDisplayType, - deletedText: marks.deletionMark ? deletionText : null, + deletedText: isReplacement || marks.deletionMark ? deletionText : null, author, authorEmail, ...(authorImage && { authorImage }), diff --git a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.test.js b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.test.js index bd17e305e7..79b36f7d0e 100644 --- a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.test.js +++ b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.test.js @@ -1022,7 +1022,7 @@ describe('internal helper functions', () => { nodes: insertionNodes, mark: insertMark, trackedChangeType: TrackInsertMarkName, - isDeletionInsertion: false, + isReplacement: false, }); expect(insertionResult.trackedChangeText).toBe('Added'); expect(insertionResult.deletionText).toBe(''); @@ -1031,7 +1031,7 @@ describe('internal helper functions', () => { nodes: deletionNodes, mark: deleteMark, trackedChangeType: TrackDeleteMarkName, - isDeletionInsertion: false, + isReplacement: false, }); expect(deletionResult.deletionText).toBe('Removed'); @@ -1039,7 +1039,7 @@ describe('internal helper functions', () => { nodes: [schema.text('Format', [formatMark])], mark: formatMark, trackedChangeType: TrackFormatMarkName, - isDeletionInsertion: false, + isReplacement: false, }); expect(formatResult.trackedChangeText).toBe('italic, removed bold'); expect(formatResult.trackedChangeDisplayType).toBeNull(); @@ -1053,7 +1053,7 @@ describe('internal helper functions', () => { nodes: [schema.text('Format', [deltaFormatMark])], mark: deltaFormatMark, trackedChangeType: TrackFormatMarkName, - isDeletionInsertion: false, + isReplacement: false, }); expect(deltaFormatResult.trackedChangeText).toContain('bold'); expect(deltaFormatResult.trackedChangeText).not.toContain('undefined'); @@ -1070,7 +1070,7 @@ describe('internal helper functions', () => { nodes: [schema.text('website', [hyperlinkFormatMark, schema.marks.link.create({ href: 'https://example.com' })])], mark: hyperlinkFormatMark, trackedChangeType: TrackFormatMarkName, - isDeletionInsertion: false, + isReplacement: false, }); expect(hyperlinkFormatResult).toMatchObject({ trackedChangeText: 'https://example.com', @@ -1081,7 +1081,7 @@ describe('internal helper functions', () => { nodes: [...insertionNodes, ...deletionNodes], mark: insertMark, trackedChangeType: TrackInsertMarkName, - isDeletionInsertion: true, + isReplacement: true, }); expect(combinedResult.deletionText).toBe('Removed'); }); @@ -1124,6 +1124,49 @@ describe('internal helper functions', () => { expect(payload?.deletedText).toBe('original'); }); + it('preserves deletedText on replacement update when transaction meta only carries insertion mark', () => { + const schema = createCommentSchema(); + const insertMark = schema.marks[TrackInsertMarkName].create({ + id: 'replace-update-1', + author: 'Author', + authorEmail: 'author@example.com', + date: 'today', + }); + const deleteMark = schema.marks[TrackDeleteMarkName].create({ + id: 'replace-update-1', + author: 'Author', + authorEmail: 'author@example.com', + date: 'today', + }); + + const docInsertNode = schema.text('replacement', [insertMark]); + const docDeleteNode = schema.text('original', [deleteMark]); + const doc = schema.node('doc', null, [schema.node('paragraph', null, [docInsertNode, docDeleteNode])]); + const state = EditorState.create({ schema, doc }); + + // Simulate an update transaction where meta has only insertedMark, but the + // document still has both insert+delete marks under the same tracked-change id. + const payload = createOrUpdateTrackedChangeComment({ + event: 'update', + marks: { insertedMark: insertMark, deletionMark: null, formatMark: null }, + deletionNodes: [], + nodes: [schema.text('replacement', [insertMark])], + newEditorState: state, + documentId: 'doc-1', + trackedChangesForId: [ + { mark: insertMark, from: 1, to: doc.content.size }, + { mark: deleteMark, from: 1, to: doc.content.size }, + ], + }); + + expect(payload?.event).toBe(comments_module_events.UPDATE); + expect(payload?.trackedChangeType).toBe('both'); + expect(payload?.trackedChangeText).toBe('replacement'); + expect(payload?.deletedText).toBe('original'); + expect(payload?.changeId).toBe('replace-update-1'); + expect(payload?.trackedChangeDisplayType).toBeNull(); + }); + it('createOrUpdateTrackedChangeComment builds add and update payloads', () => { const schema = createCommentSchema(); const insertMark = schema.marks[TrackInsertMarkName].create({ @@ -1152,9 +1195,11 @@ describe('internal helper functions', () => { changeId: 'create-1', trackedChangeText: 'Body', }); + expect(addPayload.deletedText).toBeNull(); const updatePayload = createOrUpdateTrackedChangeComment({ event: 'update', ...baseArgs }); expect(updatePayload.event).toBe(comments_module_events.UPDATE); + expect(updatePayload.deletedText).toBeNull(); const emptyState = EditorState.create({ schema, diff --git a/tests/behavior/tests/comments/sd-2509-replacement-update-preserves-deleted-text.spec.ts b/tests/behavior/tests/comments/sd-2509-replacement-update-preserves-deleted-text.spec.ts new file mode 100644 index 0000000000..da6be9553f --- /dev/null +++ b/tests/behavior/tests/comments/sd-2509-replacement-update-preserves-deleted-text.spec.ts @@ -0,0 +1,54 @@ +import { test, expect } from '../../fixtures/superdoc.js'; +import { assertDocumentApiReady, listTrackChanges } from '../../helpers/document-api.js'; + +test.use({ config: { toolbar: 'full', comments: 'on', trackChanges: true } }); + +test('SD-2509 replacement bubble preserves deleted text after follow-up edits', async ({ superdoc }) => { + await assertDocumentApiReady(superdoc.page); + + // Type baseline text in editing mode + await superdoc.type('original'); + await superdoc.waitForStable(); + + // Switch to suggesting mode and replace + await superdoc.setDocumentMode('suggesting'); + await superdoc.waitForStable(); + + await superdoc.tripleClickLine(0); + await superdoc.waitForStable(); + await superdoc.type('replacement'); + await superdoc.waitForStable(); + + // Wait for the tracked change to appear + await expect + .poll(async () => (await listTrackChanges(superdoc.page, { type: 'insert' })).total) + .toBeGreaterThanOrEqual(1); + + // The bubble should show both the deleted and inserted text + const dialog = superdoc.page.locator('.comment-placeholder .comments-dialog', { + has: superdoc.page.locator('.tracked-change-text'), + }); + await expect(dialog).toBeVisible({ timeout: 10_000 }); + + const deletedText = dialog.locator('.tracked-change-text.is-deleted'); + const insertedText = dialog.locator('.tracked-change-text.is-inserted'); + + // Both sides of the replacement must be visible + await expect(deletedText).toBeVisible(); + await expect(deletedText).toContainText('original'); + await expect(insertedText).toContainText('replacement'); + + // Type more inside the insertion to trigger update transactions with insert-only meta. + // This is the SD-2509 bug scenario: subsequent keystrokes fire updates where only + // insertedMark is in the transaction meta, but both marks still exist in the document. + await superdoc.page.keyboard.press('End'); + await superdoc.page.keyboard.type(' extra'); + await superdoc.waitForStable(); + + // The deleted text must still be visible after the update + await expect(deletedText).toBeVisible(); + await expect(deletedText).toContainText('original'); + await expect(insertedText).toContainText('replacement extra'); + + await superdoc.snapshot('sd-2509-replacement-update-preserves-deleted-text'); +});