fix: improve backspace behavior near run boundaries for tracked changes (#2175)

* fix: improve backspace behavior near run boundaries for tracked changes

* fix: cross boundry highlight and delete is broken

* fix: bound backspace text scan and prevent double-mapping in tracked replace steps

* test: push remote doc tests

Author: palmer-cl

packages/super-editor/src/core/commands/backspaceNextToRun.js

@@ -1,5 +1,15 @@
 import { Selection } from 'prosemirror-state';
 
+const findPreviousTextDeleteRange = (doc, cursorPos, minPos) => {
+  for (let pos = cursorPos - 1; pos >= minPos; pos -= 1) {
+    const $probe = doc.resolve(pos);
+    const nodeBefore = $probe.nodeBefore;
+    if (!nodeBefore?.isText || !nodeBefore.text?.length) continue;
+    return { from: pos - 1, to: pos };
+  }
+  return null;
+};
+
 /**
  * Backspaces a single character when the cursor sits adjacent to a run boundary.
  * Deletes the last character of the previous run (or the previous sibling run) without removing the whole run node.
@@ -16,21 +26,28 @@ export const backspaceNextToRun =
     if ($pos.nodeBefore?.type !== runType && $pos.pos !== $pos.start()) return false;
 
     if ($pos.nodeBefore) {
-      // Should delete the last character in the run before
-      // and not the entire run.
       if ($pos.nodeBefore.content.size === 0) return false;
-
-      tr.delete($pos.pos - 2, $pos.pos - 1).setSelection(Selection.near(tr.doc.resolve($pos.pos - 2)));
-      if (dispatch) {
-        dispatch(tr.scrollIntoView());
-      }
     } else {
       const prevNode = state.doc.resolve($pos.start() - 1).nodeBefore;
       if (prevNode?.type !== runType || prevNode.content.size === 0) return false;
-      tr.delete($pos.pos - 3, $pos.pos - 2).setSelection(Selection.near(tr.doc.resolve($pos.pos - 3)));
-      if (dispatch) {
-        dispatch(tr.scrollIntoView());
-      }
+    }
+
+    // Constrain the text scan to the adjacent run so we never delete
+    // text from a previous paragraph or an unrelated run.
+    let runContentStart;
+    if ($pos.nodeBefore) {
+      runContentStart = $pos.pos - $pos.nodeBefore.nodeSize + 1;
+    } else {
+      const prevNode = state.doc.resolve($pos.start() - 1).nodeBefore;
+      runContentStart = $pos.start() - 1 - prevNode.nodeSize + 1;
+    }
+
+    const deleteRange = findPreviousTextDeleteRange(state.doc, $pos.pos, runContentStart);
+    if (!deleteRange) return false;
+
+    tr.delete(deleteRange.from, deleteRange.to).setSelection(Selection.near(tr.doc.resolve(deleteRange.from)));
+    if (dispatch) {
+      dispatch(tr.scrollIntoView());
     }
     return true;
   };

packages/super-editor/src/core/commands/backspaceNextToRun.test.js

@@ -9,6 +9,7 @@ const makeSchema = () =>
       doc: { content: 'block+' },
       paragraph: { group: 'block', content: 'inline*' },
       run: { inline: true, group: 'inline', content: 'inline*' },
+      bookmarkEnd: { inline: true, group: 'inline', atom: true },
       text: { group: 'inline' },
     },
     marks: {},
@@ -81,4 +82,54 @@ describe('backspaceNextToRun', () => {
     expect(ok).toBe(false);
     expect(dispatch).not.toHaveBeenCalled();
   });
+
+  it('skips non-text inline nodes and deletes the previous text character', () => {
+    const schema = makeSchema();
+    const doc = schema.node('doc', null, [
+      schema.node('paragraph', null, [
+        schema.node('run', null, [schema.text('A'), schema.node('bookmarkEnd')]),
+        schema.node('run', null, schema.text('.')),
+      ]),
+    ]);
+
+    const boundary = posBetweenRuns(doc, 'A');
+    expect(boundary).not.toBeNull();
+
+    const state = EditorState.create({ schema, doc, selection: TextSelection.create(doc, boundary ?? 1) });
+    let dispatched;
+    const ok = backspaceNextToRun()({ state, tr: state.tr, dispatch: (t) => (dispatched = t) });
+
+    expect(ok).toBe(true);
+    expect(dispatched).toBeDefined();
+    // Should remove "A", not the bookmark node.
+    expect(dispatched.doc.textContent).toBe('.');
+    let bookmarkCount = 0;
+    dispatched.doc.descendants((node) => {
+      if (node.type.name === 'bookmarkEnd') bookmarkCount += 1;
+    });
+    expect(bookmarkCount).toBe(1);
+  });
+
+  it('does not scan into previous paragraphs when adjacent run has only non-text inline content', () => {
+    const schema = makeSchema();
+    const doc = schema.node('doc', null, [
+      schema.node('paragraph', null, [schema.node('run', null, schema.text('A'))]),
+      schema.node('paragraph', null, [
+        schema.node('run', null, [schema.node('bookmarkEnd')]),
+        schema.node('run', null, schema.text('B')),
+      ]),
+    ]);
+
+    const boundary = posBetweenRuns(doc, '');
+    expect(boundary).not.toBeNull();
+
+    const state = EditorState.create({ schema, doc, selection: TextSelection.create(doc, boundary ?? 1) });
+    const dispatch = vi.fn();
+
+    const ok = backspaceNextToRun()({ state, tr: state.tr, dispatch });
+
+    expect(ok).toBe(false);
+    expect(dispatch).not.toHaveBeenCalled();
+    expect(state.doc.textContent).toBe('AB');
+  });
 });

packages/super-editor/src/core/commands/deleteSelection.js

@@ -38,13 +38,13 @@ export const deleteSelection =
     const { from, to, empty } = state.selection;
 
     // Fix for SD-1013
-    // Docs that are loaded into SuperDoc, when user selects text from right to left and replace it with a single char:
-    // Prosemirror will interpret this as a backspace operation, which will delete the character.
-    // This is a workaround to prevent this from happening, by checking if the current DOM selection is a single character.
+    // Docs loaded into SuperDoc can emit a stray Backspace command while replacing
+    // a selection with a single character (right-to-left selection case).
+    // Apply this guard only for collapsed selections so real range deletion
+    // (highlight + Backspace/Delete) still works across run boundaries.
     if (typeof document !== 'undefined' && document.getSelection) {
       const currentDomSelection = document.getSelection();
-      // If the current DOM selection is a single character, we don't want to delete it.
-      if (currentDomSelection?.baseNode?.data?.length === 1) {
+      if (empty && currentDomSelection?.baseNode?.data?.length === 1) {
         return false;
       }
     }

packages/super-editor/src/core/commands/deleteSelection.test.js

@@ -149,9 +149,9 @@ describe('deleteSelection', () => {
   // When user selects text from right to left and replace it with a single char,
   // Prosemirror will interpret this as a backspace operation, which will delete the character.
   // This is a workaround to prevent this from happening, by checking if the current DOM selection is a single character.
-  it('returns false when current dom selection is a single character', () => {
+  it('returns false for collapsed selection when current dom selection is a single character', () => {
     const doc = schema.node('doc', null, [schema.node('paragraph', null, schema.text('abc def ghi'))]);
-    const sel = TextSelection.create(doc, 2, 5);
+    const sel = TextSelection.create(doc, 2, 2);
     const state = EditorState.create({ schema, doc, selection: sel });
 
     vi.spyOn(document, 'getSelection').mockReturnValue({
@@ -165,6 +165,28 @@ describe('deleteSelection', () => {
     expect(ok).toBe(false);
   });
 
+  it('does not short-circuit non-empty selection when dom baseNode length is 1', () => {
+    const doc = schema.node('doc', null, [schema.node('paragraph', null, schema.text('abc def ghi'))]);
+    const sel = TextSelection.create(doc, 2, 5);
+    const state = EditorState.create({ schema, doc, selection: sel });
+
+    vi.spyOn(document, 'getSelection').mockReturnValue({
+      baseNode: {
+        data: 'a',
+      },
+    });
+
+    pmDeleteSelection.mockReturnValueOnce('delegated-single-char-node');
+
+    const cmd = deleteSelection();
+    const dispatch = vi.fn();
+    const res = cmd({ state, tr: state.tr, dispatch });
+
+    expect(pmDeleteSelection).toHaveBeenCalledTimes(1);
+    expect(pmDeleteSelection).toHaveBeenCalledWith(state, dispatch);
+    expect(res).toBe('delegated-single-char-node');
+  });
+
   it('handles SSR environment when document is undefined', () => {
     // Save original document reference
     const originalDocument = globalThis.document;

packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.js

@@ -7,6 +7,137 @@ import { TrackChangesBasePluginKey } from '../plugins/index.js';
 import { CommentsPluginKey } from '../../comment/comments-plugin.js';
 import { findMarkPosition } from './documentHelpers.js';
 
+/**
+ * Given a range (from..to) and a count of characters ("the Nth character in that range"),
+ * returns the exact index in the document where that character sits. We only count
+ * real text—things like embedded widgets or block boundaries are skipped. Returns
+ * null if the count is beyond the end of the text in the range.
+ *
+ * @param {{ doc: import('prosemirror-model').Node, from: number, to: number, textOffset: number }} options
+ * @returns {number | null}
+ */
+const findDocPosByTextOffset = ({ doc, from, to, textOffset }) => {
+  let remaining = textOffset;
+  let foundPos = null;
+
+  doc.nodesBetween(from, to, (node, pos) => {
+    if (foundPos !== null) {
+      return false;
+    }
+    if (!node.isText || !node.text) {
+      return;
+    }
+
+    const nodeStart = Math.max(from, pos);
+    const nodeEnd = Math.min(to, pos + node.text.length);
+    if (nodeStart >= nodeEnd) {
+      return;
+    }
+
+    const nodeLen = nodeEnd - nodeStart;
+    if (remaining < nodeLen) {
+      foundPos = nodeStart + remaining;
+      return false;
+    }
+
+    remaining -= nodeLen;
+  });
+
+  return foundPos;
+};
+
+/**
+ * When the user deletes one character (e.g. backspace), the editor sometimes
+ * reports a change that spans a whole range—for example when the cursor is at
+ * the end of a paragraph. If the only real change is one character removed, we
+ * rewrite that into a simple "delete one character at position X" so we can
+ * show the right red strikethrough and put the cursor in the right place.
+ * We first try to see that from the changed range alone; if that fails (e.g. the
+ * range includes bookmarks or paragraph boundaries), we compare the full document
+ * text before and after to find the single deleted character. Returns the
+ * original change unchanged if it isn't actually a one-character delete or if
+ * we can't safely rewrite it.
+ *
+ * @param {{ step: import('prosemirror-transform').ReplaceStep, doc: import('prosemirror-model').Node }} options
+ * @returns {import('prosemirror-transform').ReplaceStep}
+ */
+const normalizeReplaceStepSingleCharDelete = ({ step, doc }) => {
+  if (
+    !(step instanceof ReplaceStep) ||
+    step.from === step.to ||
+    step.to - step.from <= 1 ||
+    step.slice.content.size === 0
+  ) {
+    return step;
+  }
+
+  const findSingleDeletedCharPos = ({ oldText, newText, from, to }) => {
+    if (oldText.length - newText.length !== 1) {
+      return null;
+    }
+
+    let prefix = 0;
+    while (prefix < newText.length && oldText.charCodeAt(prefix) === newText.charCodeAt(prefix)) {
+      prefix += 1;
+    }
+
+    let suffix = 0;
+    while (
+      suffix < newText.length - prefix &&
+      oldText.charCodeAt(oldText.length - 1 - suffix) === newText.charCodeAt(newText.length - 1 - suffix)
+    ) {
+      suffix += 1;
+    }
+
+    if (prefix + suffix !== newText.length) {
+      return null;
+    }
+
+    return findDocPosByTextOffset({ doc, from, to, textOffset: prefix });
+  };
+
+  // First try: only look at the text in the range that changed.
+  const rangeOldText = doc.textBetween(step.from, step.to);
+  const rangeNewText = step.slice.content.textBetween(0, step.slice.content.size);
+  let deleteFrom = findSingleDeletedCharPos({
+    oldText: rangeOldText,
+    newText: rangeNewText,
+    from: step.from,
+    to: step.to,
+  });
+
+  // If that didn't work—the range can include things that aren't plain text
+  // (e.g. bookmarks or paragraph boundaries)—compare the whole document before
+  // and after the change to find the one character that was removed. This path
+  // is rare and O(doc size); acceptable for normal docs.
+  if (deleteFrom === null) {
+    const applied = step.apply(doc);
+    if (applied.failed || !applied.doc) {
+      return step;
+    }
+    const oldDocText = doc.textBetween(0, doc.content.size);
+    const newDocText = applied.doc.textBetween(0, applied.doc.content.size);
+    deleteFrom = findSingleDeletedCharPos({
+      oldText: oldDocText,
+      newText: newDocText,
+      from: 0,
+      to: doc.content.size,
+    });
+    if (deleteFrom === null || deleteFrom < step.from || deleteFrom >= step.to) {
+      return step;
+    }
+  }
+
+  try {
+    const deleteTo = deleteFrom + 1;
+    const candidate = new ReplaceStep(deleteFrom, deleteTo, Slice.empty, step.structure);
+    const result = candidate.apply(doc);
+    return result.failed ? step : candidate;
+  } catch {
+    return step;
+  }
+};
+
 /**
  * Replace step.
  * @param {import('prosemirror-state').EditorState} options.state Editor state.
@@ -21,6 +152,13 @@ import { findMarkPosition } from './documentHelpers.js';
  * @param {number} options.originalStepIndex Original step index.
  */
 export const replaceStep = ({ state, tr, step, newTr, map, user, date, originalStep, originalStepIndex }) => {
+  const originalRange = { from: step.from, to: step.to, sliceSize: step.slice.content.size };
+  step = normalizeReplaceStepSingleCharDelete({ step, doc: newTr.doc });
+  const stepWasNormalized =
+    step.from !== originalRange.from ||
+    step.to !== originalRange.to ||
+    step.slice.content.size !== originalRange.sliceSize;
+
   // Handle structural deletions with no inline content (e.g., empty paragraph removal,
   // paragraph joins). When there's no content being inserted and no inline content in
   // the deletion range, markDeletion has nothing to mark — apply the step directly.
@@ -118,6 +256,7 @@ export const replaceStep = ({ state, tr, step, newTr, map, user, date, originalS
   }
 
   // Condense insertion down to a single replace step (so this tracked transaction remains a single-step insertion).
+  const docBeforeCondensedStep = newTr.doc;
   const condensedStep = new ReplaceStep(positionTo, positionTo, trackedInsertedSlice, false);
   if (newTr.maybeStep(condensedStep).failed) {
     // If the condensed step can't be applied, fall back to the original step and skip deletion tracking.
@@ -128,7 +267,15 @@ export const replaceStep = ({ state, tr, step, newTr, map, user, date, originalS
   }
 
   // We didn't apply the original step in its original place. We adjust the map accordingly.
-  const invertStep = originalStep.invert(tr.docs[originalStepIndex]).map(map);
+  // When stepWasNormalized is true, `step` is already in the mapped position space
+  // (originalStep.map(map) was applied before entering replaceStep). Calling .map(map)
+  // again would double-map positions and corrupt subsequent step/selection mapping
+  // in multi-step transactions.
+  const invertSourceStep = stepWasNormalized ? step : originalStep;
+  const invertSourceDoc = stepWasNormalized ? docBeforeCondensedStep : tr.docs[originalStepIndex];
+  const invertStep = stepWasNormalized
+    ? invertSourceStep.invert(invertSourceDoc)
+    : invertSourceStep.invert(invertSourceDoc).map(map);
   map.appendMap(invertStep.getMap());
   const mirrorIndex = map.maps.length - 1;
   map.appendMap(condensedStep.getMap(), mirrorIndex);
@@ -174,6 +321,13 @@ export const replaceStep = ({ state, tr, step, newTr, map, user, date, originalS
       meta.insertedTo = deletionMap.map(meta.insertedTo, 1);
     }
 
+    // Normalized broad -> single-char deletions should keep the caret at the
+    // normalized deletion edge, not the original broad transaction selection.
+    // This avoids follow-up Backspace events targeting structural boundaries.
+    if (stepWasNormalized && !meta.insertedMark) {
+      meta.selectionPos = deletionMap.map(step.from, -1);
+    }
+
     map.appendMapping(deletionMap);
   }