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 161ac7182..a9a0f6582 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.js @@ -25,11 +25,19 @@ export const replaceStep = ({ state, tr, step, newTr, map, user, date, originalS // Default: insert replacement after the selected range (Word-like replace behavior). // If the selection ends inside an existing deletion, move insertion to after that deletion span. + // 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; - const probePos = Math.max(step.from, step.to - 1); - const deletionSpan = findMarkPosition(trTemp.doc, probePos, TrackDeleteMarkName); - if (deletionSpan && deletionSpan.to > positionTo) { - positionTo = deletionSpan.to; + let positionAdjusted = false; + const isSingleStep = tr.steps.length === 1; + + if (isSingleStep) { + const probePos = Math.max(step.from, step.to - 1); + const deletionSpan = findMarkPosition(trTemp.doc, probePos, TrackDeleteMarkName); + if (deletionSpan && deletionSpan.to > positionTo) { + positionTo = deletionSpan.to; + positionAdjusted = true; + } } const tryInsert = (slice) => { @@ -81,6 +89,14 @@ export const replaceStep = ({ state, tr, step, newTr, map, user, date, originalS if (insertion.insertedFrom !== insertion.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 = insertion.insertedTo - insertion.insertedFrom; + meta.insertedTo = positionTo + insertionLength; + } } if (!newTr.selection.eq(trTemp.selection)) { @@ -104,6 +120,12 @@ export const replaceStep = ({ state, tr, step, newTr, map, user, date, originalS meta.deletionNodes = deletionNodes; meta.deletionMark = deletionMark; + // Map insertedTo through deletionMap to account for position shifts from removing + // the user's own prior insertions (which markDeletion deletes instead of marking). + if (meta.insertedTo !== undefined) { + meta.insertedTo = deletionMap.map(meta.insertedTo, 1); + } + map.appendMapping(deletionMap); } 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 4f07d4cd9..929d322b2 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 @@ -42,6 +42,171 @@ describe('trackChangesHelpers replaceStep', () => { return found; }; + 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({ + id: 'del-existing', + author: user.name, + authorEmail: user.email, + date: '2024-01-01T00:00:00.000Z', + }); + + const run = schema.nodes.run.create({}, [schema.text('AB', [deletionMark])]); + const doc = schema.nodes.doc.create({}, schema.nodes.paragraph.create({}, run)); + let state = createState(doc); + + // Position cursor at the start of the paragraph (position 2, after doc and paragraph open tags) + state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, 2))); + + // Simulate typing "xy" one character at a time + // Note: We must explicitly setSelection to match real browser input behavior + // (replaceWith alone doesn't set tr.selectionSet = true) + + // First character: "x" + let tr = state.tr.replaceWith(state.selection.from, state.selection.from, schema.text('x')); + // Browser input places cursor after inserted text + tr.setSelection(TextSelection.create(tr.doc, tr.selection.from)); + tr.setMeta('inputType', 'insertText'); + let tracked = trackedTransaction({ tr, state, user }); + state = state.apply(tracked); + + // Second character: "y" + tr = state.tr.replaceWith(state.selection.from, state.selection.from, schema.text('y')); + tr.setSelection(TextSelection.create(tr.doc, tr.selection.from)); + tr.setMeta('inputType', 'insertText'); + tracked = trackedTransaction({ tr, state, user }); + state = state.apply(tracked); + + // Extract the inserted text (text with trackInsert mark) + let insertedText = ''; + state.doc.descendants((node) => { + if (node.isText && node.marks.some((mark) => mark.type.name === TrackInsertMarkName)) { + insertedText += node.text; + } + }); + + // The bug would cause "yx" (reversed), the fix ensures "xy" (correct order) + expect(insertedText).toBe('xy'); + }); + + it('should map insertedTo through deletionMap when replacing own insertions near deletion spans', () => { + // Edge case: User has their own prior insertion adjacent to a deletion span. + // When selecting across both and replacing, markDeletion removes the user's own + // insertion (shifting positions), but insertedTo was calculated before this shift. + // The cursor would land too far to the right if insertedTo isn't remapped. + // + // Document: [inserted:"XY"][deleted:"ABC"] + // User selects "XY" + part of "ABC" and types "Q" + // Expected: cursor lands right after "Q" + // Bug: cursor lands 2 positions too far right (length of removed "XY") + + const insertionMark = schema.marks[TrackInsertMarkName].create({ + id: 'ins-own', + author: user.name, + authorEmail: user.email, + date: '2024-01-01T00:00:00.000Z', + }); + + const deletionMark = schema.marks[TrackDeleteMarkName].create({ + id: 'del-existing', + author: user.name, + authorEmail: user.email, + date: '2024-01-01T00:00:00.000Z', + }); + + // "XY" with insertion mark, "ABC" with deletion mark + const run = schema.nodes.run.create({}, [schema.text('XY', [insertionMark]), schema.text('ABC', [deletionMark])]); + const doc = schema.nodes.doc.create({}, schema.nodes.paragraph.create({}, run)); + let state = createState(doc); + + const posXY = findTextPos(state.doc, 'XY'); + const posABC = findTextPos(state.doc, 'ABC'); + + // Select from start of "XY" into the deletion span (selecting "XY" + "A") + // This triggers positionAdjusted=true because selection ends inside deletion span. + const from = posXY; + const to = posABC + 1; + state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, from, to))); + + // Replace selection with "Q" + let tr = state.tr.replaceWith(from, to, schema.text('Q')); + tr.setSelection(TextSelection.create(tr.doc, from + 1)); // Browser would place cursor after "Q" + tr.setMeta('inputType', 'insertText'); + + const tracked = trackedTransaction({ tr, state, user }); + const finalState = state.apply(tracked); + + // After the transaction: + // - "XY" (user's own insertion) is removed entirely by markDeletion + // - "A" already has delete mark, stays as deleted + // - "Q" is inserted after the deletion span + // - Final doc should be: [deleted:"ABC"][inserted:"Q"] + // + // The cursor should be right after "Q" + // Bug would place it 2 positions too far right (length of removed "XY") + + // Verify the document structure + let deletedText = ''; + let insertedText = ''; + finalState.doc.descendants((node) => { + if (node.isText) { + if (node.marks.some((mark) => mark.type.name === TrackDeleteMarkName)) { + deletedText += node.text; + } + if (node.marks.some((mark) => mark.type.name === TrackInsertMarkName)) { + insertedText += node.text; + } + } + }); + + expect(deletedText).toBe('ABC'); // Already-deleted text is preserved + expect(insertedText).toBe('Q'); + + // The critical assertion: cursor position + // With the bug, this would fail because cursor is at wrong position + const cursorPos = finalState.selection.from; + const expectedCursorPos = findTextPos(finalState.doc, 'Q') + 1; // Right after "Q" + + expect(cursorPos).toBe(expectedCursorPos); + }); + + it('handles multi-step transactions without losing content (SD-1624 fix)', () => { + // Multi-step transactions (like input rules) should preserve all content. + // The position adjustment for insertion after deletion spans is only applied + // to single-step transactions to avoid breaking multi-step mapping. + const deletionMark = schema.marks[TrackDeleteMarkName].create({ + id: 'del-existing', + author: user.name, + authorEmail: user.email, + date: '2024-01-01T00:00:00.000Z', + }); + + const run = schema.nodes.run.create({}, [schema.text('AB', [deletionMark])]); + const doc = schema.nodes.doc.create({}, schema.nodes.paragraph.create({}, run)); + let state = createState(doc); + state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, 2))); + + // Two steps in one transaction (like input rules or batched typing) + let tr = state.tr; + tr = tr.replaceWith(2, 2, schema.text('x')); + tr = tr.replaceWith(3, 3, schema.text('y')); + tr.setSelection(TextSelection.create(tr.doc, 4)); + tr.setMeta('inputType', 'insertText'); + + const tracked = trackedTransaction({ tr, state, user }); + const finalState = state.apply(tracked); + + let insertedText = ''; + finalState.doc.descendants((node) => { + if (node.isText && node.marks.some((mark) => mark.type.name === TrackInsertMarkName)) { + insertedText += node.text; + } + }); + + // Both characters should be tracked + expect(insertedText).toBe('xy'); + }); + it('tracks replace even when selection contains existing deletions and links', () => { const linkMark = schema.marks.link.create({ href: 'https://example.com' }); const existingDeletion = schema.marks[TrackDeleteMarkName].create({ 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 116027519..ed074f62e 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackedTransaction.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackedTransaction.js @@ -5,6 +5,7 @@ import { replaceStep } from './replaceStep.js'; import { addMarkStep } from './addMarkStep.js'; import { removeMarkStep } from './removeMarkStep.js'; import { TrackDeleteMarkName } from '../constants.js'; +import { TrackChangesBasePluginKey } from '../plugins/index.js'; import { findMark } from '@core/helpers/index.js'; import { CommentsPluginKey } from '../../comment/comments-plugin.js'; @@ -96,6 +97,9 @@ export const trackedTransaction = ({ tr, state, user }) => { newTr.setMeta('addToHistory', tr.getMeta('addToHistory')); } + // Get the track changes meta to check if we have an adjusted insertion position (SD-1624). + const trackMeta = newTr.getMeta(TrackChangesBasePluginKey); + if (tr.selectionSet) { const deletionMarkSchema = state.schema.marks[TrackDeleteMarkName]; const deletionMark = findMark(state, deletionMarkSchema, false); @@ -106,6 +110,10 @@ 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)));