From 811dbe59e0af7cbcb3bd3c87b85b58bf7f08006a Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Sat, 24 Jan 2026 11:16:24 -0300 Subject: [PATCH 1/4] fix: correct cursor position when typing after fully track-deleted content When all content in a text node is track-deleted and the user types new text, characters were appearing in reverse order. This happened because: 1. Each new character was inserted after the deletion span (correct) 2. But the cursor was mapped back to the original position (incorrect) 3. So each subsequent character was also inserted at the same position The fix stores the actual insertion end position in transaction meta and uses it for cursor placement when content is inserted after a deletion span. --- .../track-changes/trackChangesHelpers/replaceStep.js | 3 +++ .../trackChangesHelpers/trackedTransaction.js | 7 +++++++ 2 files changed, 10 insertions(+) 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 161ac71823..27291dbbc4 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.js @@ -81,6 +81,9 @@ 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. + // This is needed when insertion was moved after a deletion span (SD-1624). + meta.insertedTo = map.map(positionTo + (insertion.insertedTo - insertion.insertedFrom), 1); } if (!newTr.selection.eq(trTemp.selection)) { 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 1160275196..c59f00ff14 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); @@ -109,6 +113,9 @@ export const trackedTransaction = ({ tr, state, user }) => { } 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))); + } else if (trackMeta?.insertedTo !== undefined) { + // SD-1624: When content was inserted after a deletion span, position cursor after the insertion. + newTr.setSelection(new TextSelection(newTr.doc.resolve(trackMeta.insertedTo))); } else { newTr.setSelection(tr.selection.map(newTr.doc, map)); } From caef38f062d099978bbd55bb5231df797dcaa3d1 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Sat, 24 Jan 2026 11:26:26 -0300 Subject: [PATCH 2/4] test: add regression test for SD-1624 cursor positioning --- .../trackChangesHelpers/replaceStep.test.js | 47 +++++++++++++++++++ .../trackChangesHelpers/trackedTransaction.js | 7 +-- 2 files changed, 51 insertions(+), 3 deletions(-) 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 4f07d4cd90..a6c8d2ee50 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,53 @@ 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('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 c59f00ff14..ed074f62e5 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackedTransaction.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackedTransaction.js @@ -110,12 +110,13 @@ export const trackedTransaction = ({ tr, state, user }) => { ) { const caretPos = map.map(tr.selection.from, -1); newTr.setSelection(new TextSelection(newTr.doc.resolve(caretPos))); - } 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))); } 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))); } else { newTr.setSelection(tr.selection.map(newTr.doc, map)); } From 9182f9dfcce0bb2471785c216ead824fdec0ea4a Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Sat, 24 Jan 2026 11:38:50 -0300 Subject: [PATCH 3/4] fix: limit position adjustment to single-step transactions --- .../trackChangesHelpers/replaceStep.js | 27 ++++++++++---- .../trackChangesHelpers/replaceStep.test.js | 37 +++++++++++++++++++ 2 files changed, 57 insertions(+), 7 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 27291dbbc4..9f23b43ff0 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,9 +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. - // This is needed when insertion was moved after a deletion span (SD-1624). - meta.insertedTo = map.map(positionTo + (insertion.insertedTo - insertion.insertedFrom), 1); + // 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)) { 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 a6c8d2ee50..157ab6db21 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 @@ -89,6 +89,43 @@ describe('trackChangesHelpers replaceStep', () => { expect(insertedText).toBe('xy'); }); + 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({ From 1f1de6f642e354be8d1230a13f2984e5741063f2 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Sat, 24 Jan 2026 08:03:01 -0800 Subject: [PATCH 4/4] fix: map insertedTo through deletionMap for correct cursor positioning --- .../trackChangesHelpers/replaceStep.js | 6 ++ .../trackChangesHelpers/replaceStep.test.js | 81 +++++++++++++++++++ 2 files changed, 87 insertions(+) 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 9f23b43ff0..a9a0f65825 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.js @@ -120,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 157ab6db21..929d322b2d 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 @@ -89,6 +89,87 @@ describe('trackChangesHelpers replaceStep', () => { 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