diff --git a/packages/super-editor/src/core/commands/backspaceNextToRun.js b/packages/super-editor/src/core/commands/backspaceNextToRun.js index d3626fd9bd..24584049fc 100644 --- a/packages/super-editor/src/core/commands/backspaceNextToRun.js +++ b/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; }; diff --git a/packages/super-editor/src/core/commands/backspaceNextToRun.test.js b/packages/super-editor/src/core/commands/backspaceNextToRun.test.js index 2302510b14..215b6e1568 100644 --- a/packages/super-editor/src/core/commands/backspaceNextToRun.test.js +++ b/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'); + }); }); diff --git a/packages/super-editor/src/core/commands/deleteSelection.js b/packages/super-editor/src/core/commands/deleteSelection.js index e2a1678415..02cd0e8817 100644 --- a/packages/super-editor/src/core/commands/deleteSelection.js +++ b/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; } } diff --git a/packages/super-editor/src/core/commands/deleteSelection.test.js b/packages/super-editor/src/core/commands/deleteSelection.test.js index 82c258f6a0..47719a4180 100644 --- a/packages/super-editor/src/core/commands/deleteSelection.test.js +++ b/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; 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 f1e35335bb..2b9d4f62a7 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.js +++ b/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); } 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 ab96e32b83..759d415281 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 @@ -1,6 +1,7 @@ import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest'; import { EditorState, TextSelection } from 'prosemirror-state'; import { DOMParser as PMDOMParser, Slice } from 'prosemirror-model'; +import { ReplaceStep } from 'prosemirror-transform'; import { trackedTransaction, documentHelpers } from './index.js'; import { TrackInsertMarkName, TrackDeleteMarkName } from '../constants.js'; import { TrackChangesBasePluginKey } from '../plugins/trackChangesBasePlugin.js'; @@ -350,6 +351,131 @@ describe('trackChangesHelpers replaceStep', () => { expect(meta.insertedMark.attrs.id).toBe(meta.deletionMark.attrs.id); }); + it('normalizes broad replacement steps and tracks only terminal period deletion', () => { + const oldDeleteMark = schema.marks[TrackDeleteMarkName].create({ + id: 'imported-del-id', + author: 'Imported Author', + authorEmail: 'imported@example.com', + date: '2024-01-01T00:00:00.000Z', + }); + + const paragraph = schema.nodes.paragraph.create({}, [ + schema.nodes.run.create({}, [schema.text('Current sentence')]), + schema.nodes.run.create({}, [schema.text(' old redline', [oldDeleteMark])]), + schema.nodes.run.create({ styleId: 'PeriodRun' }, [schema.text('.')]), + ]); + let state = createState(schema.nodes.doc.create({}, [paragraph])); + + const paragraphRange = getParagraphRange(state.doc, 0); + expect(paragraphRange).toBeTruthy(); + + const replacementParagraph = schema.nodes.paragraph.create({}, [ + schema.nodes.run.create({}, [schema.text('Current sentence')]), + schema.nodes.run.create({}, [schema.text(' old redline', [oldDeleteMark])]), + ]); + + const tr = state.tr.replace(paragraphRange.from, paragraphRange.to, new Slice(replacementParagraph.content, 0, 0)); + const tracked = trackedTransaction({ tr, state, user }); + const finalState = state.apply(tracked); + + /** @type {Record} */ + const deleteTextById = {}; + finalState.doc.descendants((node) => { + if (!node.isText || !node.text) return; + for (const mark of node.marks ?? []) { + if (mark.type.name !== TrackDeleteMarkName) continue; + const id = mark.attrs?.id; + if (!id) continue; + deleteTextById[id] = (deleteTextById[id] ?? '') + node.text; + } + }); + + if (deleteTextById['imported-del-id']) { + expect(deleteTextById['imported-del-id']).toContain('old redline'); + expect(deleteTextById['imported-del-id']).not.toContain('.'); + } + + const periodDeleteEntries = Object.entries(deleteTextById).filter( + ([id, text]) => id !== 'imported-del-id' && text.includes('.'), + ); + expect(periodDeleteEntries).toHaveLength(1); + expect(periodDeleteEntries[0]?.[1]).toBe('.'); + }); + + it('keeps caret near deletion point after normalized broad replacement so consecutive backspace works', () => { + const paragraph = schema.nodes.paragraph.create({}, [ + schema.nodes.run.create({}, [schema.text('Current sentence')]), + schema.nodes.run.create({}, [schema.text(' old redline')]), + schema.nodes.run.create({ styleId: 'PeriodRun' }, [schema.text('.')]), + ]); + let state = createState(schema.nodes.doc.create({}, [paragraph])); + + const periodPos = findTextPos(state.doc, '.'); + expect(periodPos).toBeTypeOf('number'); + state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, periodPos + 1))); + + const paragraphRange = getParagraphRange(state.doc, 0); + const replacementParagraph = schema.nodes.paragraph.create({}, [ + schema.nodes.run.create({}, [schema.text('Current sentence')]), + schema.nodes.run.create({}, [schema.text(' old redline')]), + ]); + + const tr1 = state.tr.replace(paragraphRange.from, paragraphRange.to, new Slice(replacementParagraph.content, 0, 0)); + tr1.setSelection(TextSelection.create(tr1.doc, periodPos)); + const tracked1 = trackedTransaction({ tr: tr1, state, user }); + state = state.apply(tracked1); + + const periodDeletePos = findTextPos(state.doc, '.'); + expect(periodDeletePos).toBeTypeOf('number'); + expect(Math.abs(state.selection.from - (periodDeletePos + 1))).toBeLessThanOrEqual(2); + const selectionAfterFirstDelete = state.selection.from; + + const tr2 = state.tr.delete(state.selection.from - 1, state.selection.from); + tr2.setMeta('inputType', 'deleteContentBackward'); + const tracked2 = trackedTransaction({ tr: tr2, state, user }); + const finalState = state.apply(tracked2); + + const deletedChars = []; + finalState.doc.descendants((node) => { + if (!node.isText || !node.text) return; + if (node.marks.some((mark) => mark.type.name === TrackDeleteMarkName)) { + deletedChars.push(node.text); + } + }); + + expect(deletedChars.join('')).toContain('.'); + expect(finalState.selection.from).toBeLessThanOrEqual(selectionAfterFirstDelete); + expect(Math.abs(finalState.selection.from - periodDeletePos)).toBeLessThanOrEqual(3); + }); + + it('prefers normalized deletion caret over broad original selection after normalization', () => { + const paragraph = schema.nodes.paragraph.create({}, [ + schema.nodes.run.create({}, [schema.text('Current sentence')]), + schema.nodes.run.create({}, [schema.text(' old redline')]), + schema.nodes.run.create({ styleId: 'PeriodRun' }, [schema.text('.')]), + ]); + let state = createState(schema.nodes.doc.create({}, [paragraph])); + + const periodPos = findTextPos(state.doc, '.'); + expect(periodPos).toBeTypeOf('number'); + const paragraphRange = getParagraphRange(state.doc, 0); + + const replacementParagraph = schema.nodes.paragraph.create({}, [ + schema.nodes.run.create({}, [schema.text('Current sentence')]), + schema.nodes.run.create({}, [schema.text(' old redline')]), + ]); + + const tr = state.tr.replace(paragraphRange.from, paragraphRange.to, new Slice(replacementParagraph.content, 0, 0)); + tr.setSelection(TextSelection.create(tr.doc, periodPos - 5)); + + const tracked = trackedTransaction({ tr, state, user }); + const finalState = state.apply(tracked); + const trackedPeriodPos = findTextPos(finalState.doc, '.'); + expect(trackedPeriodPos).toBeTypeOf('number'); + + expect(Math.abs(finalState.selection.from - trackedPeriodPos)).toBeLessThanOrEqual(1); + }); + it('prefers original paste slice before maxOpen fallback for collapsed insertions', () => { const doc = schema.nodes.doc.create( {}, @@ -382,6 +508,44 @@ describe('trackChangesHelpers replaceStep', () => { expect(text).not.toContain('Flattened Fallback'); }); + it('does not re-map the inverse of a normalized replace step when prior maps already exist', () => { + const paragraphOne = schema.nodes.paragraph.create({}, [schema.nodes.run.create({}, [schema.text('Prefix')])]); + const paragraphTwo = schema.nodes.paragraph.create({}, [ + schema.nodes.run.create({}, [schema.text('Current sentence old redline.')]), + ]); + const state = createState(schema.nodes.doc.create({}, [paragraphOne, paragraphTwo])); + + const inverseMapInputSizes = []; + const originalInvert = ReplaceStep.prototype.invert; + vi.spyOn(ReplaceStep.prototype, 'invert').mockImplementation(function invertSpy(docNode) { + const inverseStep = originalInvert.call(this, docNode); + const isSingleCharDelete = this.slice.content.size === 0 && this.to - this.from === 1; + if (!isSingleCharDelete) return inverseStep; + const originalMap = inverseStep.map.bind(inverseStep); + inverseStep.map = (mapping) => { + inverseMapInputSizes.push(mapping.maps.length); + return originalMap(mapping); + }; + return inverseStep; + }); + + const prefixPos = findTextPos(state.doc, 'Prefix'); + expect(prefixPos).toBeTypeOf('number'); + let tr = state.tr.insertText('!', prefixPos + 'Prefix'.length); + + const secondParagraphRange = getParagraphRange(tr.doc, 1); + expect(secondParagraphRange).toBeTruthy(); + const replacementParagraph = schema.nodes.paragraph.create({}, [ + schema.nodes.run.create({}, [schema.text('Current sentence old redline')]), + ]); + tr = tr.replace(secondParagraphRange.from, secondParagraphRange.to, new Slice(replacementParagraph.content, 0, 0)); + tr.setMeta('inputType', 'insertText'); + + trackedTransaction({ tr, state, user }); + + expect(inverseMapInputSizes).toHaveLength(0); + }); + it('deletes empty paragraph on Backspace in suggesting mode', () => { // When the cursor is inside an empty paragraph and the user presses Backspace, // ProseMirror creates a ReplaceStep that removes the empty paragraph node. 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 e2c188a07a..830ca2f55c 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackedTransaction.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackedTransaction.js @@ -101,7 +101,19 @@ export const trackedTransaction = ({ tr, state, user }) => { const trackMeta = newTr.getMeta(TrackChangesBasePluginKey); if (tr.selectionSet) { - if ( + // When replaceStep normalizes a broad replace to a single-char delete (e.g. terminal period), + // it sets selectionPos so the caret lands at the deletion edge. Honor it so the next backspace + // targets the right character instead of a block/run boundary. + if (trackMeta?.selectionPos !== undefined && trackMeta?.selectionPos !== null) { + const boundedPos = Math.max(0, Math.min(trackMeta.selectionPos, newTr.doc.content.size)); + const $pos = newTr.doc.resolve(boundedPos); + if ($pos.parent.inlineContent) { + newTr.setSelection(TextSelection.create(newTr.doc, boundedPos)); + } else { + // Normalized delete flows should keep the caret on the deletion side. + newTr.setSelection(TextSelection.near($pos, -1)); + } + } else if ( tr.selection instanceof TextSelection && (tr.selection.from < state.selection.from || tr.getMeta('inputType') === 'deleteContentBackward') ) { diff --git a/tests/behavior/tests/comments/delete-terminal-period-separate-run.spec.ts b/tests/behavior/tests/comments/delete-terminal-period-separate-run.spec.ts new file mode 100644 index 0000000000..c458ae9f02 --- /dev/null +++ b/tests/behavior/tests/comments/delete-terminal-period-separate-run.spec.ts @@ -0,0 +1,105 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { test, expect } from '../../fixtures/superdoc.js'; + +test.use({ config: { toolbar: 'full', comments: 'off', trackChanges: true } }); + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const DOC_PATH = path.resolve(__dirname, '../../test-data/comments-tcs/redline-full-paragraph.docx'); + +test.skip(!fs.existsSync(DOC_PATH), 'Test document not available — run pnpm corpus:pull'); + +const snapshotTrackDeletesAndBookmarks = async (superdoc: any) => + superdoc.page.evaluate(() => { + const editor = (window as any).editor; + const deleteById: Record = {}; + let bookmarkStartCount = 0; + let bookmarkEndCount = 0; + + editor.state.doc.descendants((node: any) => { + if (node.type?.name === 'bookmarkStart') bookmarkStartCount += 1; + if (node.type?.name === 'bookmarkEnd') bookmarkEndCount += 1; + if (!node.isText || !node.text) return; + for (const mark of node.marks ?? []) { + if (mark.type?.name !== 'trackDelete') continue; + const id = String(mark.attrs?.id ?? ''); + if (!id) continue; + deleteById[id] = (deleteById[id] ?? '') + node.text; + } + }); + + return { deleteById, bookmarkStartCount, bookmarkEndCount }; + }); + +test('two backspaces track period and l for bookmark-wrapped runs', async ({ superdoc }) => { + await superdoc.loadDocument(DOC_PATH); + await superdoc.waitForStable(); + await superdoc.page.evaluate(() => { + const editor = (window as any).editor; + editor.setOptions({ user: { name: 'Guest Reviewer', email: 'track@example.com' } }); + }); + + await superdoc.setDocumentMode('suggesting'); + await superdoc.waitForStable(); + + const before = await snapshotTrackDeletesAndBookmarks(superdoc); + const targetMarker = await superdoc.page.evaluate(() => { + const editor = (window as any).editor; + const { doc } = editor.state; + let marker: string | null = null; + + doc.descendants((node: any) => { + if (node.type?.name !== 'paragraph') return; + const normalized = String(node.textContent ?? '') + .replace(/\s+/g, ' ') + .trim(); + if (!normalized.includes('any and all such Confidential Material.')) return; + marker = String(node.attrs?.listRendering?.markerText ?? '').trim(); + return false; + }); + + if (!marker) { + throw new Error('Target numbered paragraph not found'); + } + return marker; + }); + expect(targetMarker).toBe('1.'); + + const periodPos = await superdoc.page.evaluate(() => { + const editor = (window as any).editor; + const { doc } = editor.state; + let matchPos = -1; + + doc.descendants((node: any, pos: number) => { + if (!node.isText || node.text !== '.') return; + const left = doc.textBetween(Math.max(0, pos - 80), pos, '', ''); + if (left.endsWith('any and all such Confidential Material')) { + matchPos = pos; + return false; + } + return; + }); + + if (matchPos === -1) { + throw new Error('Terminal period for Confidential Material sentence not found'); + } + return matchPos; + }); + await superdoc.setTextSelection(periodPos + 1); + await superdoc.press('Backspace'); + await superdoc.waitForStable(); + await superdoc.press('Backspace'); + await superdoc.waitForStable(); + + const snapshot = await snapshotTrackDeletesAndBookmarks(superdoc); + + const newDeletedCombined = Object.entries(snapshot.deleteById) + .filter(([id]) => !before.deleteById[id]) + .map(([, text]) => text) + .join(''); + + expect(newDeletedCombined).toBe('l.'); + expect(snapshot.bookmarkStartCount).toBe(before.bookmarkStartCount); + expect(snapshot.bookmarkEndCount).toBe(before.bookmarkEndCount); +}); diff --git a/tests/visual/tests/behavior/tracked-delete-terminal-period.spec.ts b/tests/visual/tests/behavior/tracked-delete-terminal-period.spec.ts new file mode 100644 index 0000000000..ce5f93531e --- /dev/null +++ b/tests/visual/tests/behavior/tracked-delete-terminal-period.spec.ts @@ -0,0 +1,112 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { test, expect } from '../fixtures/superdoc.js'; + +test.use({ + config: { + toolbar: 'full', + comments: 'off', + trackChanges: true, + hideCaret: true, + hideSelection: true, + }, +}); + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const DOC_PATH_CANDIDATES = [ + path.resolve(__dirname, '../../test-data/comments-tcs/redline-full-paragraph.docx'), + path.resolve(__dirname, '../../../../test-corpus/comments-tcs/redline-full-paragraph.docx'), +]; +const DOC_PATH = DOC_PATH_CANDIDATES.find((candidate) => fs.existsSync(candidate)) ?? DOC_PATH_CANDIDATES[0]; + +test.skip(!fs.existsSync(DOC_PATH), 'Test document not available — run pnpm corpus:pull'); + +const snapshotTrackDeletes = async (superdoc: any) => + superdoc.page.evaluate(() => { + const editor = (window as any).editor; + const deleteById: 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 = String(mark.attrs?.id ?? ''); + if (!id) continue; + deleteById[id] = (deleteById[id] ?? '') + node.text; + } + }); + return { deleteById }; + }); + +test('suggesting double backspace with bookmark-wrapped runs tracks period and preceding character', async ({ + superdoc, +}) => { + await superdoc.loadDocument(DOC_PATH); + await superdoc.waitForStable(); + await superdoc.page.evaluate(() => { + const editor = (window as any).editor; + editor.setOptions({ user: { name: 'Guest Reviewer', email: 'track@example.com' } }); + }); + + await superdoc.setDocumentMode('suggesting'); + await superdoc.waitForStable(); + + const before = await snapshotTrackDeletes(superdoc); + const targetMarker = await superdoc.page.evaluate(() => { + const editor = (window as any).editor; + const { doc } = editor.state; + let marker: string | null = null; + + doc.descendants((node: any) => { + if (node.type?.name !== 'paragraph') return; + const normalized = String(node.textContent ?? '') + .replace(/\s+/g, ' ') + .trim(); + if (!normalized.includes('any and all such Confidential Material.')) return; + marker = String(node.attrs?.listRendering?.markerText ?? '').trim(); + return false; + }); + + if (!marker) { + throw new Error('Target numbered paragraph not found'); + } + return marker; + }); + expect(targetMarker).toBe('1.'); + + const periodPos = await superdoc.page.evaluate(() => { + const editor = (window as any).editor; + const { doc } = editor.state; + let matchPos = -1; + + doc.descendants((node: any, pos: number) => { + if (!node.isText || node.text !== '.') return; + const left = doc.textBetween(Math.max(0, pos - 80), pos, '', ''); + if (left.endsWith('any and all such Confidential Material')) { + matchPos = pos; + return false; + } + return; + }); + + if (matchPos === -1) { + throw new Error('Terminal period for Confidential Material sentence not found'); + } + return matchPos; + }); + + await superdoc.setTextSelection(periodPos + 1); + await superdoc.press('Backspace'); + await superdoc.waitForStable(); + await superdoc.press('Backspace'); + await superdoc.waitForStable(); + + const after = await snapshotTrackDeletes(superdoc); + const newDeletedCombined = Object.entries(after.deleteById) + .filter(([id]) => !before.deleteById[id]) + .map(([, text]) => text) + .join(''); + + expect(newDeletedCombined).toBe('l.'); +});