diff --git a/devtools/visual-testing/tests/interactions/stories/comments-tcs/backspace-empty-paragraph-suggesting.ts b/devtools/visual-testing/tests/interactions/stories/comments-tcs/backspace-empty-paragraph-suggesting.ts new file mode 100644 index 0000000000..d4dca56c6b --- /dev/null +++ b/devtools/visual-testing/tests/interactions/stories/comments-tcs/backspace-empty-paragraph-suggesting.ts @@ -0,0 +1,70 @@ +import { defineStory } from '@superdoc-testing/helpers'; + +const WAIT_MS = 300; + +/** + * SD-1810: Backspace doesn't delete empty paragraph in suggesting mode + * + * Bug: When a user creates a new paragraph (Enter) in suggesting mode and + * immediately presses Backspace, nothing happens — the empty paragraph stays. + * + * Root cause: The track changes system intercepts the ReplaceStep via + * replaceStep() → markDeletion(). An empty paragraph has no inline content, + * so markDeletion finds nothing to mark and the step is silently swallowed. + * + * This test recreates the exact bug scenario: + * 1. Type text, press Enter to create an empty paragraph + * 2. Press Backspace — empty paragraph should be removed + * 3. Also tests the Enter → Enter → Backspace → Backspace flow (paragraph join) + */ +export default defineStory({ + name: 'backspace-empty-paragraph-suggesting', + description: 'Test Backspace on empty paragraphs and paragraph joins in suggesting mode', + tickets: ['SD-1810'], + startDocument: null, + layout: true, + comments: 'off', + hideCaret: false, + hideSelection: false, + + async run(_page, helpers): Promise { + const { type, press, setDocumentMode, waitForStable, milestone } = helpers; + + // Step 1: Type initial content + await type('Hello World'); + await waitForStable(WAIT_MS); + await milestone('initial-text', 'Document with initial text'); + + // Step 2: Switch to suggesting mode + await setDocumentMode('suggesting'); + await waitForStable(WAIT_MS); + await milestone('suggesting-mode', 'Switched to suggesting mode'); + + // Step 3: Press Enter to create an empty paragraph + await press('Enter'); + await waitForStable(WAIT_MS); + await milestone('after-enter', 'New empty paragraph created below'); + + // Step 4: Press Backspace — should delete the empty paragraph + await press('Backspace'); + await waitForStable(WAIT_MS); + await milestone('after-backspace', 'Empty paragraph removed, cursor back at end of "Hello World"'); + + // Step 5: Test Enter → Enter → Backspace → Backspace (join flow) + await press('Enter'); + await waitForStable(WAIT_MS); + await press('Enter'); + await waitForStable(WAIT_MS); + await milestone('after-two-enters', 'Two new paragraphs created'); + + // Step 6: First Backspace — removes the second empty paragraph + await press('Backspace'); + await waitForStable(WAIT_MS); + await milestone('after-first-backspace', 'One empty paragraph removed'); + + // Step 7: Second Backspace — joins back with the original paragraph + await press('Backspace'); + await waitForStable(WAIT_MS); + await milestone('after-second-backspace', 'Joined back to original paragraph — cursor at end of "Hello World"'); + }, +}); diff --git a/packages/super-editor/src/core/extensions/keymap-history.test.js b/packages/super-editor/src/core/extensions/keymap-history.test.js new file mode 100644 index 0000000000..c9afa24209 --- /dev/null +++ b/packages/super-editor/src/core/extensions/keymap-history.test.js @@ -0,0 +1,202 @@ +import { describe, it, expect, afterEach } from 'vitest'; +import { closeHistory, undoDepth } from 'prosemirror-history'; +import { initTestEditor } from '@tests/helpers/helpers.js'; +import { handleEnter, handleBackspace, handleDelete } from './keymap.js'; + +describe('keymap history grouping', () => { + let editor; + + afterEach(() => { + editor?.destroy(); + editor = null; + }); + + const insertText = (ed, text) => { + const { from } = ed.state.selection; + ed.view.dispatch(ed.state.tr.insertText(text, from)); + }; + + /** Simulate closeHistoryOnly (space / Opt+Backspace handler). */ + const closeHistoryGroup = (ed) => { + ed.view.dispatch(closeHistory(ed.view.state.tr)); + }; + + it('Enter creates a new undo group boundary', () => { + ({ editor } = initTestEditor({ mode: 'text', content: '

' })); + + insertText(editor, 'hello'); + const depthAfterFirstText = undoDepth(editor.state); + + handleEnter(editor); + + insertText(editor, 'world'); + const depthAfterSecondText = undoDepth(editor.state); + + expect(depthAfterSecondText).toBeGreaterThan(depthAfterFirstText); + }); + + it('undo after Enter restores text before Enter', () => { + ({ editor } = initTestEditor({ mode: 'text', content: '

' })); + + insertText(editor, 'hello'); + handleEnter(editor); + insertText(editor, 'world'); + + const textBefore = editor.state.doc.textContent; + expect(textBefore).toContain('hello'); + expect(textBefore).toContain('world'); + + editor.commands.undo(); + const textAfterUndo = editor.state.doc.textContent; + expect(textAfterUndo).toContain('hello'); + }); + + it('Enter creates boundary in suggesting mode', () => { + ({ editor } = initTestEditor({ + mode: 'text', + content: '

', + user: { name: 'Tester', email: 'test@test.com' }, + })); + + editor.commands.enableTrackChanges?.(); + + insertText(editor, 'hello'); + const depthAfterFirstText = undoDepth(editor.state); + + handleEnter(editor); + + insertText(editor, 'world'); + const depthAfterSecondText = undoDepth(editor.state); + + expect(depthAfterSecondText).toBeGreaterThan(depthAfterFirstText); + }); + + it('space creates a word-level undo boundary', () => { + ({ editor } = initTestEditor({ mode: 'text', content: '

' })); + + insertText(editor, 'hello'); + const depthAfterFirstWord = undoDepth(editor.state); + + // Simulate space handler: closeHistory then type space + closeHistoryGroup(editor); + insertText(editor, ' '); + + insertText(editor, 'world'); + const depthAfterSecondWord = undoDepth(editor.state); + + expect(depthAfterSecondWord).toBeGreaterThan(depthAfterFirstWord); + }); + + it('undo after space removes only the last word', () => { + ({ editor } = initTestEditor({ mode: 'text', content: '

' })); + + insertText(editor, 'hello'); + closeHistoryGroup(editor); + insertText(editor, ' world'); + + expect(editor.state.doc.textContent).toBe('hello world'); + + editor.commands.undo(); + expect(editor.state.doc.textContent).toBe('hello'); + }); + + it('closeHistory before deletion creates its own undo step', () => { + ({ editor } = initTestEditor({ mode: 'text', content: '

' })); + + insertText(editor, 'hello world'); + const depthAfterTyping = undoDepth(editor.state); + + // Simulate Opt+Backspace: closeHistory then delete last word + closeHistoryGroup(editor); + const { from } = editor.state.selection; + editor.view.dispatch(editor.state.tr.delete(from - 5, from)); + const depthAfterDelete = undoDepth(editor.state); + + expect(depthAfterDelete).toBeGreaterThan(depthAfterTyping); + + // Undo should restore the deleted word + editor.commands.undo(); + expect(editor.state.doc.textContent).toBe('hello world'); + }); + + it('Backspace creates a new undo group boundary', () => { + ({ editor } = initTestEditor({ mode: 'text', content: '

' })); + + // Create two paragraphs: type, Enter, type + insertText(editor, 'hello'); + handleEnter(editor); + insertText(editor, 'world'); + const depthBeforeBackspace = undoDepth(editor.state); + + // Move cursor to start of second paragraph so joinBackward succeeds + let secondParaStart = null; + editor.state.doc.forEach((_node, offset, index) => { + if (index === 1) secondParaStart = offset + 1; + }); + editor.view.dispatch( + editor.state.tr.setSelection(editor.state.selection.constructor.create(editor.state.doc, secondParaStart)), + ); + + // Backspace at start of second paragraph → joins paragraphs + handleBackspace(editor); + + insertText(editor, ' after'); + const depthAfterBackspace = undoDepth(editor.state); + + expect(depthAfterBackspace).toBeGreaterThan(depthBeforeBackspace); + }); + + it('undo after Backspace join restores paragraph break', () => { + ({ editor } = initTestEditor({ mode: 'text', content: '

' })); + + insertText(editor, 'hello'); + handleEnter(editor); + insertText(editor, 'world'); + + expect(editor.state.doc.childCount).toBe(2); + + // Move cursor to start of second paragraph + let secondParaStart = null; + editor.state.doc.forEach((_node, offset, index) => { + if (index === 1) secondParaStart = offset + 1; + }); + editor.view.dispatch( + editor.state.tr.setSelection(editor.state.selection.constructor.create(editor.state.doc, secondParaStart)), + ); + + // Backspace joins paragraphs + handleBackspace(editor); + expect(editor.state.doc.childCount).toBe(1); + + // Undo should restore the paragraph break + editor.commands.undo(); + expect(editor.state.doc.childCount).toBe(2); + }); + + it('Delete creates a new undo group boundary', () => { + ({ editor } = initTestEditor({ mode: 'text', content: '

' })); + + // Create two paragraphs + insertText(editor, 'hello'); + handleEnter(editor); + insertText(editor, 'world'); + const depthBeforeDelete = undoDepth(editor.state); + + // Move cursor to end of first paragraph so joinForward succeeds + let firstParaEnd = null; + editor.state.doc.forEach((node, offset, index) => { + if (index === 0) firstParaEnd = offset + node.nodeSize - 1; + }); + editor.view.dispatch( + editor.state.tr.setSelection(editor.state.selection.constructor.create(editor.state.doc, firstParaEnd)), + ); + + // Delete at end of first paragraph → joins paragraphs + handleDelete(editor); + + insertText(editor, ' after'); + const depthAfterDelete = undoDepth(editor.state); + + expect(depthAfterDelete).toBeGreaterThan(depthBeforeDelete); + }); +}); diff --git a/packages/super-editor/src/core/extensions/keymap.js b/packages/super-editor/src/core/extensions/keymap.js index 2ad353bc08..dbb467ed8d 100644 --- a/packages/super-editor/src/core/extensions/keymap.js +++ b/packages/super-editor/src/core/extensions/keymap.js @@ -1,8 +1,16 @@ +import { closeHistory } from 'prosemirror-history'; import { Extension } from '../Extension.js'; import { isIOS } from '../utilities/isIOS.js'; import { isMacOS } from '../utilities/isMacOS.js'; export const handleEnter = (editor) => { + const { view } = editor; + // Close the current undo group so this structural action becomes its own undo step. + // Note: this fires before the command chain, so if no command succeeds (rare — e.g. + // Enter with no valid split target) an empty undo boundary is created. Acceptable + // trade-off vs. the complexity of post-hoc closeHistory after commands.first. + view?.dispatch?.(closeHistory(view?.state?.tr)); + return editor.commands.first(({ commands }) => [ () => commands.splitRunToParagraph(), () => commands.newlineInCode(), @@ -13,6 +21,10 @@ export const handleEnter = (editor) => { }; export const handleBackspace = (editor) => { + const { view } = editor; + // Close undo group — see comment in handleEnter. + view?.dispatch?.(closeHistory(view?.state?.tr)); + return editor.commands.first(({ commands, tr }) => [ () => commands.undoInputRule(), () => { @@ -30,6 +42,10 @@ export const handleBackspace = (editor) => { }; export const handleDelete = (editor) => { + const { view } = editor; + // Close undo group — see comment in handleEnter. + view?.dispatch?.(closeHistory(view?.state?.tr)); + return editor.commands.first(({ commands }) => [ () => commands.deleteSkipEmptyRun(), () => commands.deleteNextToRun(), diff --git a/packages/super-editor/src/extensions/block-node/block-node.js b/packages/super-editor/src/extensions/block-node/block-node.js index ec15857294..d6777ed6e2 100644 --- a/packages/super-editor/src/extensions/block-node/block-node.js +++ b/packages/super-editor/src/extensions/block-node/block-node.js @@ -242,6 +242,26 @@ export const BlockNode = Extension.create({ tr.setNodeMarkup(pos, undefined, nextAttrs, node.marks); }; + /** + * Ensures a block node has a unique sdBlockId, assigning a new UUID if the + * current ID is missing or already seen. Tracks seen IDs in the provided Set + * to detect duplicates (e.g., when tr.split() copies the original paragraph's ID). + * @param {ProseMirrorNode} node - The node to check. + * @param {Object} nextAttrs - Mutable attrs object to update. + * @param {Set} seenIds - Set of IDs already encountered in this traversal. + * @returns {boolean} True if the sdBlockId was changed. + */ + const ensureUniqueSdBlockId = (node, nextAttrs, seenIds) => { + const currentId = node.attrs?.sdBlockId; + let changed = false; + if (nodeAllowsSdBlockIdAttr(node) && (nodeNeedsSdBlockId(node) || seenIds.has(currentId))) { + nextAttrs.sdBlockId = uuidv4(); + changed = true; + } + if (currentId) seenIds.add(currentId); + return changed; + }; + return [ new Plugin({ key: BlockNodePluginKey, @@ -258,14 +278,11 @@ export const BlockNode = Extension.create({ if (!hasInitialized) { // Initial pass: assign IDs to all block nodes in document + const seenIds = new Set(); newState.doc.descendants((node, pos) => { if (!nodeAllowsSdBlockIdAttr(node) && !nodeAllowsSdBlockRevAttr(node)) return; const nextAttrs = { ...node.attrs }; - let nodeChanged = false; - if (nodeAllowsSdBlockIdAttr(node) && nodeNeedsSdBlockId(node)) { - nextAttrs.sdBlockId = uuidv4(); - nodeChanged = true; - } + let nodeChanged = ensureUniqueSdBlockId(node, nextAttrs, seenIds); if (nodeAllowsSdBlockRevAttr(node)) { const rev = ensureBlockRev(node); if (nextAttrs.sdBlockRev !== rev) { @@ -331,6 +348,9 @@ export const BlockNode = Extension.create({ const docSize = newState.doc.content.size; const mergedRanges = mergeRanges(rangesToCheck, docSize); + // Track seen sdBlockIds across all ranges to detect duplicates + // (e.g., when tr.split() copies the original paragraph's sdBlockId to the new one). + const seenBlockIds = new Set(); for (const { from, to } of mergedRanges) { const clampedRange = clampRange(from, to, docSize); @@ -346,11 +366,7 @@ export const BlockNode = Extension.create({ if (!nodeAllowsSdBlockIdAttr(node) && !nodeAllowsSdBlockRevAttr(node)) return; if (updatedPositions.has(pos)) return; const nextAttrs = { ...node.attrs }; - let nodeChanged = false; - if (nodeAllowsSdBlockIdAttr(node) && nodeNeedsSdBlockId(node)) { - nextAttrs.sdBlockId = uuidv4(); - nodeChanged = true; - } + let nodeChanged = ensureUniqueSdBlockId(node, nextAttrs, seenBlockIds); if (nodeAllowsSdBlockRevAttr(node)) { nextAttrs.sdBlockRev = getNextBlockRev(node); nodeChanged = true; @@ -369,14 +385,11 @@ export const BlockNode = Extension.create({ } if (shouldFallbackToFullTraversal) { + const fallbackSeenIds = new Set(); newState.doc.descendants((node, pos) => { if (!nodeAllowsSdBlockIdAttr(node) && !nodeAllowsSdBlockRevAttr(node)) return; const nextAttrs = { ...node.attrs }; - let nodeChanged = false; - if (nodeAllowsSdBlockIdAttr(node) && nodeNeedsSdBlockId(node)) { - nextAttrs.sdBlockId = uuidv4(); - nodeChanged = true; - } + let nodeChanged = ensureUniqueSdBlockId(node, nextAttrs, fallbackSeenIds); if (nodeAllowsSdBlockRevAttr(node)) { nextAttrs.sdBlockRev = getNextBlockRev(node); nodeChanged = true; 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 5c40536ed7..36e0e4e2df 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceStep.js @@ -21,6 +21,32 @@ import { findMarkPosition } from './documentHelpers.js'; * @param {number} options.originalStepIndex Original step index. */ export const replaceStep = ({ state, tr, step, newTr, map, user, date, originalStep, originalStepIndex }) => { + // 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. + // + // Edge case: if a paragraph contains only TrackDelete-marked text, hasInlineContent + // returns true and the normal tracking flow runs. markDeletion skips already-deleted + // nodes, but the join still applies through the replace machinery — the delete is + // not swallowed. This is correct: the structural join merges the blocks while + // preserving the existing deletion marks on the text content. + if (step.from !== step.to && step.slice.content.size === 0) { + let hasInlineContent = false; + newTr.doc.nodesBetween(step.from, step.to, (node) => { + if (node.isInline) { + hasInlineContent = true; + return false; + } + }); + + if (!hasInlineContent) { + if (!newTr.maybeStep(step).failed) { + map.appendMap(step.getMap()); + } + return; + } + } + const trTemp = state.apply(newTr).tr; // Default: insert replacement after the selected range (Word-like replace behavior). 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 86704c5db1..d442fbbb0b 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 @@ -371,6 +371,82 @@ describe('trackChangesHelpers replaceStep', () => { expect(text).not.toContain('Flattened Fallback'); }); + 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. + // The track changes system should allow this deletion to proceed since there's + // no inline content to track. + + // Create doc with:

Hello

+ const run = schema.nodes.run.create({}, [schema.text('Hello')]); + const para1 = schema.nodes.paragraph.create({}, run); + const para2 = schema.nodes.paragraph.create(); + const doc = schema.nodes.doc.create({}, [para1, para2]); + let state = createState(doc); + + // Find empty paragraph position dynamically + let emptyParaOffset = null; + state.doc.forEach((node, offset) => { + if (node.type.name === 'paragraph' && node.content.size === 0) { + emptyParaOffset = offset; + } + }); + expect(emptyParaOffset).not.toBeNull(); + + // Cursor inside empty paragraph (offset + 1 for the opening position) + state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, emptyParaOffset + 1))); + + // Simulate Backspace: joinBackward creates a ReplaceStep that removes the empty paragraph + const tr = state.tr.delete(emptyParaOffset, emptyParaOffset + para2.nodeSize); + tr.setMeta('inputType', 'deleteContentBackward'); + + const tracked = trackedTransaction({ tr, state, user }); + const finalState = state.apply(tracked); + + // The empty paragraph should be deleted — only one paragraph should remain + let paragraphCount = 0; + finalState.doc.forEach((node) => { + if (node.type.name === 'paragraph') paragraphCount++; + }); + expect(paragraphCount).toBe(1); + + // The remaining paragraph should contain "Hello" + let textContent = ''; + finalState.doc.descendants((node) => { + if (node.isText) textContent += node.text; + }); + expect(textContent).toBe('Hello'); + }); + + it('applies paragraph join directly in suggesting mode (no inline content to track)', () => { + // Paragraph joins have no inline content in their step range (only block boundary + // tokens), so markDeletion has nothing to mark. The join is applied directly. + const para1 = schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('Hello')])); + const para2 = schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('World')])); + const doc = schema.nodes.doc.create({}, [para1, para2]); + let state = createState(doc); + + let joinPos = null; + state.doc.forEach((node, offset, index) => { + if (index === 0) joinPos = offset + node.nodeSize; + }); + expect(joinPos).not.toBeNull(); + + const tr = state.tr.join(joinPos); + tr.setMeta('inputType', 'deleteContentBackward'); + + const tracked = trackedTransaction({ tr, state, user }); + const finalState = state.apply(tracked); + + // The join should be applied — only one paragraph remains + let paragraphCount = 0; + finalState.doc.forEach(() => paragraphCount++); + expect(paragraphCount).toBe(1); + + // Both texts should be merged + expect(finalState.doc.textContent).toBe('HelloWorld'); + }); + 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({