diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.js index a0e9e9d11f..5f11e1fa3f 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.js @@ -1,3 +1,158 @@ -export const replaceAroundStep = () => { - // Skip for now. +import { ReplaceStep } from 'prosemirror-transform'; +import { Slice } from 'prosemirror-model'; +import { replaceStep } from './replaceStep.js'; +import { TrackDeleteMarkName } from '../constants.js'; +import { TrackChangesBasePluginKey } from '../plugins/index.js'; + +/** + * Find the closest live (non-tracked-deleted) text character position before + * the cursor, within the same paragraph. + * + * @param {import('prosemirror-model').Node} doc + * @param {number} cursorPos + * @param {import('prosemirror-model').MarkType} trackDeleteMarkType + * @returns {number | null} The document position of the character, or null. + */ +const findPreviousLiveCharPos = (doc, cursorPos, trackDeleteMarkType) => { + const $cursor = doc.resolve(cursorPos); + + // Find the enclosing paragraph (may need to go up past run nodes). + let paraDepth = $cursor.depth; + while (paraDepth > 0 && $cursor.node(paraDepth).type.name !== 'paragraph') { + paraDepth--; + } + if (paraDepth <= 0) return null; + + const paraStart = $cursor.before(paraDepth) + 1; + + // Walk through inline nodes from paragraph start to cursor, + // keeping track of the last live character position found. + let lastLiveCharPos = null; + + doc.nodesBetween(paraStart, cursorPos, (node, pos) => { + if (!node.isText) return; + + const hasDeleteMark = node.marks.some((m) => m.type === trackDeleteMarkType); + if (hasDeleteMark) return; + + // This is a live text node. Its last character within our range is + // at min(nodeEnd, cursorPos) - 1. + const nodeEnd = pos + node.nodeSize; + const relevantEnd = Math.min(nodeEnd, cursorPos); + if (relevantEnd > pos) { + lastLiveCharPos = relevantEnd - 1; + } + }); + + return lastLiveCharPos; +}; + +/** + * Handle a ReplaceAroundStep in tracked changes mode. + * + * ReplaceAroundStep is ProseMirror's structural "change wrapper" operation + * (e.g. lifting content out of a list item, changing block type). In tracked + * changes mode we must never silently apply structural changes — they would + * alter paragraph properties (numbering, font, alignment) without tracking. + * + * For backspace/delete, the user's intent is to delete a character, not change + * paragraph structure. We convert the step to a tracked single-character + * deletion using the existing replaceStep handler. + * + * @param {object} options + * @param {import('prosemirror-state').EditorState} options.state + * @param {import('prosemirror-state').Transaction} options.tr + * @param {import('prosemirror-transform').ReplaceAroundStep} options.step + * @param {import('prosemirror-state').Transaction} options.newTr + * @param {import('prosemirror-transform').Mapping} options.map + * @param {import('prosemirror-model').Node} options.doc + * @param {object} options.user + * @param {string} options.date + * @param {import('prosemirror-transform').Step} options.originalStep + * @param {number} options.originalStepIndex + */ +export const replaceAroundStep = ({ + state, + tr, + step, + newTr, + map, + doc, + user, + date, + originalStep, + originalStepIndex, +}) => { + const inputType = tr.getMeta('inputType'); + const isBackspace = inputType === 'deleteContentBackward'; + + if (!isBackspace) { + // Non-backspace ReplaceAroundStep in tracked changes: block it. + // Structural wrapper changes (list toggle, block type change) should be + // implemented as tracked format changes in the future. For now, silently + // dropping them is safer than applying them untracked. + return; + } + + // For backspace: find the previous live character and track its deletion. + const trackDeleteMarkType = state.schema.marks[TrackDeleteMarkName]; + const deleteFrom = findPreviousLiveCharPos(doc, state.selection.from, trackDeleteMarkType); + + if (deleteFrom === null) { + // No live character found — nothing to delete. Skip the structural change. + return; + } + + const charStep = new ReplaceStep(deleteFrom, deleteFrom + 1, Slice.empty); + + replaceStep({ + state, + tr, + step: charStep, + newTr, + map, + doc, + user, + date, + originalStep: charStep, + originalStepIndex, + }); + + // Position the cursor at the deletion edge. The original transaction's + // selection was computed for the structural ReplaceAroundStep, not our + // fabricated character deletion. Override it so the cursor visually + // moves left with each backspace. + const trackMeta = newTr.getMeta(TrackChangesBasePluginKey) || {}; + trackMeta.selectionPos = deleteFrom; + newTr.setMeta(TrackChangesBasePluginKey, trackMeta); + + // Merge adjacent trackDelete marks that have the same author/date but different IDs. + // When backspace first deletes a character (e.g. ".") via a normal ReplaceStep and + // subsequent presses delete further characters (e.g. "l") via ReplaceAroundStep, + // the deletion marks end up with different IDs because run node boundaries create + // a position gap larger than findTrackedMarkBetween's ±1 offset. Re-mark the + // earlier deletion with the current ID so they merge into a single tracked change. + if (trackMeta.deletionMark) { + const ourId = trackMeta.deletionMark.attrs.id; + const ourEmail = trackMeta.deletionMark.attrs.authorEmail; + const ourDate = trackMeta.deletionMark.attrs.date; + const searchTo = Math.min(newTr.doc.content.size, deleteFrom + 20); + + let contiguous = true; + newTr.doc.nodesBetween(deleteFrom, searchTo, (node, pos) => { + if (!contiguous) return false; + if (!node.isText) return; + const delMark = node.marks.find((m) => m.type.name === TrackDeleteMarkName); + if (!delMark) { + contiguous = false; // Live text — stop, deletions are no longer contiguous. + return; + } + if (delMark.attrs.id !== ourId && delMark.attrs.authorEmail === ourEmail && delMark.attrs.date === ourDate) { + const markType = state.schema.marks[TrackDeleteMarkName]; + const merged = markType.create({ ...delMark.attrs, id: ourId }); + newTr.removeMark(pos, pos + node.nodeSize, delMark); + newTr.addMark(pos, pos + node.nodeSize, merged); + } + }); + } }; diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.test.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.test.js new file mode 100644 index 0000000000..5764beddea --- /dev/null +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.test.js @@ -0,0 +1,474 @@ +import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest'; +import { EditorState, TextSelection } from 'prosemirror-state'; +import { Mapping, ReplaceAroundStep, ReplaceStep } from 'prosemirror-transform'; +import { Fragment, Slice } from 'prosemirror-model'; +import { trackedTransaction, documentHelpers } from './index.js'; +import { replaceAroundStep } from './replaceAroundStep.js'; +import { TrackDeleteMarkName, TrackInsertMarkName } from '../constants.js'; +import { TrackChangesBasePluginKey } from '../plugins/trackChangesBasePlugin.js'; +import { initTestEditor } from '@tests/helpers/helpers.js'; +import { findTextPos } from './testUtils.js'; + +describe('replaceAroundStep handler', () => { + let editor; + let schema; + let basePlugins; + + const user = { name: 'Track Tester', email: 'track@example.com' }; + const date = '2024-01-01T00:00:00.000Z'; + + beforeEach(() => { + ({ editor } = initTestEditor({ mode: 'text', content: '

' })); + schema = editor.schema; + basePlugins = editor.state.plugins; + }); + + afterEach(() => { + vi.restoreAllMocks(); + editor?.destroy(); + editor = null; + }); + + const createState = (doc) => + EditorState.create({ + schema, + doc, + plugins: basePlugins, + }); + + /** + * Create a ReplaceAroundStep that simulates ProseMirror's structural backspace. + * A ReplaceAroundStep replaces from..gapFrom and gapTo..to with a slice, + * preserving the content between gapFrom..gapTo (the "gap"). + * + * For backspace at a list/paragraph boundary, ProseMirror creates a step that + * lifts content out of its wrapper. We simulate this by wrapping the text + * in a listItem-like structure and creating a step that unwraps it. + */ + const createBackspaceReplaceAroundTr = (state, doc) => { + // Create a ReplaceAroundStep that covers the paragraph node. + // This simulates what ProseMirror generates when backspacing at a + // paragraph boundary to lift content out of its wrapper. + // + // We find the first paragraph and create a step that would "unwrap" it + // by replacing the paragraph's opening and closing tokens while preserving + // the content between them. + let paraStart = null; + let paraEnd = null; + doc.forEach((node, offset) => { + if (paraStart === null && node.type.name === 'paragraph') { + paraStart = offset; + paraEnd = offset + node.nodeSize; + } + }); + + if (paraStart === null) throw new Error('No paragraph found'); + + // Build a transaction with a ReplaceAroundStep. + // The step unwraps the paragraph: replaces the paragraph node but keeps its inline content. + // from=paraStart, to=paraEnd, gapFrom=paraStart+1, gapTo=paraEnd-1 + // This means: replace [paraStart..paraStart+1] (open tag) and [paraEnd-1..paraEnd] (close tag) + // with a new paragraph wrapper, keeping the content (the "gap") intact. + const tr = state.tr; + const step = new ReplaceAroundStep( + paraStart, + paraEnd, + paraStart + 1, + paraEnd - 1, + new Slice(Fragment.from(schema.nodes.paragraph.create()), 0, 0), + 1, + true, + ); + + if (!tr.maybeStep(step).failed) { + return tr; + } + + // Fallback: just create a simple structural step to test the handler + return null; + }; + + /** + * Helper to invoke replaceAroundStep handler directly. + * Sets up tr with a real step applied so tr.docs[0] is populated. + */ + const invokeHandler = ({ state, inputType = 'deleteContentBackward' }) => { + const doc = state.doc; + const trackDeleteMarkType = state.schema.marks[TrackDeleteMarkName]; + + // We need to supply a tr that has tr.docs populated (i.e., a step was applied to it). + // Apply a dummy ReplaceStep to the tr to populate tr.docs[0]. + const tr = state.tr; + + // Find cursor position and the char we'd delete + const cursorPos = state.selection.from; + let lastLiveCharPos = null; + + // Walk the paragraph to find the live char (same logic as findPreviousLiveCharPos) + const $cursor = doc.resolve(cursorPos); + let paraDepth = $cursor.depth; + while (paraDepth > 0 && $cursor.node(paraDepth).type.name !== 'paragraph') { + paraDepth--; + } + if (paraDepth > 0) { + const paraStart = $cursor.before(paraDepth) + 1; + doc.nodesBetween(paraStart, cursorPos, (node, pos) => { + if (!node.isText) return; + const hasDeleteMark = node.marks.some((m) => m.type === trackDeleteMarkType); + if (hasDeleteMark) return; + const nodeEnd = pos + node.nodeSize; + const relevantEnd = Math.min(nodeEnd, cursorPos); + if (relevantEnd > pos) lastLiveCharPos = relevantEnd - 1; + }); + } + + // Apply a ReplaceStep to tr so tr.docs gets populated + if (lastLiveCharPos !== null) { + tr.step(new ReplaceStep(lastLiveCharPos, lastLiveCharPos + 1, Slice.empty)); + } + tr.setMeta('inputType', inputType); + + const newTr = state.tr; + const map = new Mapping(); + const fakeStep = {}; + + replaceAroundStep({ + state, + tr, + step: fakeStep, + newTr, + map, + doc, + user, + date, + originalStep: fakeStep, + originalStepIndex: 0, + }); + + return newTr; + }; + + describe('non-backspace blocking', () => { + it('blocks non-backspace ReplaceAroundStep (no steps added to newTr)', () => { + const doc = schema.nodes.doc.create( + {}, + schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('Hello')])), + ); + const state = createState(doc); + const cursorPos = findTextPos(state.doc, 'Hello') + 5; + const stateWithSelection = state.apply(state.tr.setSelection(TextSelection.create(state.doc, cursorPos))); + + const newTr = invokeHandler({ state: stateWithSelection, inputType: 'insertParagraph' }); + + // No steps should be added — the structural change is blocked + expect(newTr.steps.length).toBe(0); + }); + + it('blocks ReplaceAroundStep without inputType meta', () => { + const doc = schema.nodes.doc.create( + {}, + schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('Hello')])), + ); + const state = createState(doc); + + const tr = state.tr; + // No inputType meta set + const newTr = state.tr; + const map = new Mapping(); + + replaceAroundStep({ + state, + tr, + step: {}, + newTr, + map, + doc: state.doc, + user, + date, + originalStep: {}, + originalStepIndex: 0, + }); + + expect(newTr.steps.length).toBe(0); + }); + }); + + describe('backspace character deletion', () => { + it('converts backspace ReplaceAroundStep to tracked single-char deletion', () => { + const doc = schema.nodes.doc.create( + {}, + schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('Hello')])), + ); + let state = createState(doc); + + // Position cursor after 'o' (end of "Hello") + const cursorPos = findTextPos(state.doc, 'Hello') + 5; + state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, cursorPos))); + + const newTr = invokeHandler({ state }); + + // Should have added steps (the tracked deletion) + expect(newTr.steps.length).toBeGreaterThan(0); + + // The last character 'o' should be marked as deleted + let hasDeleteMark = false; + newTr.doc.descendants((node) => { + if (node.isText && node.marks.some((m) => m.type.name === TrackDeleteMarkName)) { + hasDeleteMark = true; + } + }); + expect(hasDeleteMark).toBe(true); + }); + + it('sets selectionPos on track meta for cursor positioning', () => { + const doc = schema.nodes.doc.create( + {}, + schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('Hello')])), + ); + let state = createState(doc); + + const cursorPos = findTextPos(state.doc, 'Hello') + 5; + state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, cursorPos))); + + const newTr = invokeHandler({ state }); + const trackMeta = newTr.getMeta(TrackChangesBasePluginKey); + + expect(trackMeta).toBeDefined(); + expect(trackMeta.selectionPos).toBeDefined(); + // selectionPos should be the position of the deleted character (before cursor) + expect(trackMeta.selectionPos).toBeLessThan(cursorPos); + }); + + it('returns early when no live character exists before cursor', () => { + // All content is tracked-deleted — nothing to delete + const deleteMark = schema.marks[TrackDeleteMarkName].create({ + id: 'del-existing', + author: user.name, + authorEmail: user.email, + date, + }); + const doc = schema.nodes.doc.create( + {}, + schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('Deleted', [deleteMark])])), + ); + let state = createState(doc); + + const cursorPos = findTextPos(state.doc, 'Deleted') + 7; + state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, cursorPos))); + + const newTr = invokeHandler({ state }); + + // No steps — nothing to delete + expect(newTr.steps.length).toBe(0); + }); + }); + + describe('findPreviousLiveCharPos (tested through handler)', () => { + it('skips tracked-deleted text and deletes the last live character', () => { + // "Hel" (live) + "lo" (deleted by another author) — cursor at end + // Should delete 'l' (the last live char), not 'o' + const deleteMark = schema.marks[TrackDeleteMarkName].create({ + id: 'del-existing', + author: 'Other Author', + authorEmail: 'other@example.com', + date: '2023-01-01T00:00:00.000Z', + }); + const doc = schema.nodes.doc.create( + {}, + schema.nodes.paragraph.create( + {}, + schema.nodes.run.create({}, [schema.text('Hel'), schema.text('lo', [deleteMark])]), + ), + ); + let state = createState(doc); + + // Cursor after 'lo' (end of all text) + const hPos = findTextPos(state.doc, 'Hel'); + const cursorPos = hPos + 5; // After "Hello" = Hel + lo + state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, cursorPos))); + + const newTr = invokeHandler({ state }); + + // Should have tracked deletion + expect(newTr.steps.length).toBeGreaterThan(0); + + // 'l' from "Hel" should be the newly deleted character + let newlyDeletedText = ''; + newTr.doc.descendants((node) => { + if (!node.isText) return; + const mark = node.marks.find((m) => m.type.name === TrackDeleteMarkName); + if (mark && mark.attrs.id !== 'del-existing') { + newlyDeletedText += node.text; + } + }); + expect(newlyDeletedText).toBe('l'); + }); + }); + + describe('mark merging', () => { + it('merges adjacent trackDelete marks with same author/date but different IDs', () => { + // Simulate the scenario: "." was deleted previously (id A), now "l" is deleted (id B) + // They are in separate run nodes with a gap between them. + // After handler runs, both should have the same ID. + const existingDeleteMark = schema.marks[TrackDeleteMarkName].create({ + id: 'existing-period-id', + author: user.name, + authorEmail: user.email, + date, + }); + + // "Material" (live) in one run, "." (deleted, id=existing) in another run + const doc = schema.nodes.doc.create( + {}, + schema.nodes.paragraph.create({}, [ + schema.nodes.run.create({}, [schema.text('Material')]), + schema.nodes.run.create({}, [schema.text('.', [existingDeleteMark])]), + ]), + ); + let state = createState(doc); + + // Cursor after 'l' in "Material" (position to delete 'l') + const textPos = findTextPos(state.doc, 'Material'); + const cursorPos = textPos + 8; // After "Material" + state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, cursorPos))); + + const newTr = invokeHandler({ state }); + const trackMeta = newTr.getMeta(TrackChangesBasePluginKey); + + // The handler should have created a deletion mark + expect(trackMeta?.deletionMark).toBeDefined(); + + // Check that the existing "." deletion was re-marked with the new ID + const newId = trackMeta.deletionMark.attrs.id; + let periodMarkId = null; + newTr.doc.descendants((node) => { + if (!node.isText || node.text !== '.') return; + const delMark = node.marks.find((m) => m.type.name === TrackDeleteMarkName); + if (delMark) periodMarkId = delMark.attrs.id; + }); + + // Both the 'l' deletion and the '.' deletion should now share the same ID + expect(periodMarkId).toBe(newId); + }); + + it('does not merge marks from different authors', () => { + const otherAuthorMark = schema.marks[TrackDeleteMarkName].create({ + id: 'other-author-del', + author: 'Other Author', + authorEmail: 'other@example.com', + date, + }); + + const doc = schema.nodes.doc.create( + {}, + schema.nodes.paragraph.create({}, [ + schema.nodes.run.create({}, [schema.text('Material')]), + schema.nodes.run.create({}, [schema.text('.', [otherAuthorMark])]), + ]), + ); + let state = createState(doc); + + const textPos = findTextPos(state.doc, 'Material'); + const cursorPos = textPos + 8; + state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, cursorPos))); + + const newTr = invokeHandler({ state }); + + // The "." should keep the other author's ID (not merged) + let periodMarkId = null; + newTr.doc.descendants((node) => { + if (!node.isText || node.text !== '.') return; + const delMark = node.marks.find((m) => m.type.name === TrackDeleteMarkName); + if (delMark) periodMarkId = delMark.attrs.id; + }); + + expect(periodMarkId).toBe('other-author-del'); + }); + + it('does not merge non-contiguous marks separated by live text', () => { + // "Material" (live) + "." (deleted, same author) + "end" (live) + "!" (deleted, same author) + // Only "." should be merged with the new deletion; "!" should keep its own ID + // because "end" (live text) breaks contiguity. + const deleteMark1 = schema.marks[TrackDeleteMarkName].create({ + id: 'period-del', + author: user.name, + authorEmail: user.email, + date, + }); + const deleteMark2 = schema.marks[TrackDeleteMarkName].create({ + id: 'excl-del', + author: user.name, + authorEmail: user.email, + date, + }); + + const doc = schema.nodes.doc.create( + {}, + schema.nodes.paragraph.create({}, [ + schema.nodes.run.create({}, [schema.text('Material')]), + schema.nodes.run.create({}, [schema.text('.', [deleteMark1])]), + schema.nodes.run.create({}, [schema.text('end')]), + schema.nodes.run.create({}, [schema.text('!', [deleteMark2])]), + ]), + ); + let state = createState(doc); + + const textPos = findTextPos(state.doc, 'Material'); + const cursorPos = textPos + 8; + state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, cursorPos))); + + const newTr = invokeHandler({ state }); + const trackMeta = newTr.getMeta(TrackChangesBasePluginKey); + const newId = trackMeta?.deletionMark?.attrs?.id; + + // "." should be merged (same ID as new deletion) + let periodMarkId = null; + let exclMarkId = null; + newTr.doc.descendants((node) => { + if (!node.isText) return; + const delMark = node.marks.find((m) => m.type.name === TrackDeleteMarkName); + if (!delMark) return; + if (node.text === '.') periodMarkId = delMark.attrs.id; + if (node.text === '!') exclMarkId = delMark.attrs.id; + }); + + expect(periodMarkId).toBe(newId); // Adjacent — merged + expect(exclMarkId).toBe('excl-del'); // Non-contiguous — NOT merged + }); + }); + + describe('selectionPos in trackedTransaction', () => { + it('selectionPos is honored even when tr.selectionSet is false', () => { + // This tests the critical fix in trackedTransaction.js: + // ReplaceAroundStep transactions may not set selection on the original tr, + // but our handler sets selectionPos on the meta. trackedTransaction must + // check selectionPos BEFORE checking tr.selectionSet. + const doc = schema.nodes.doc.create( + {}, + schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('Hello')])), + ); + let state = createState(doc); + + const cursorPos = findTextPos(state.doc, 'Hello') + 5; + state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, cursorPos))); + + // Create a transaction with a ReplaceStep but do NOT set selection on tr, + // simulating the ReplaceAroundStep behavior where tr.selectionSet is false. + const deletePos = cursorPos - 1; + const tr = state.tr; + tr.step(new ReplaceStep(deletePos, cursorPos, Slice.empty)); + tr.setMeta('inputType', 'deleteContentBackward'); + // Critically: do NOT call tr.setSelection — this is what happens with ReplaceAroundStep + + // Set selectionPos meta to simulate what replaceAroundStep handler does + tr.setMeta(TrackChangesBasePluginKey, { selectionPos: deletePos }); + + // Now run through trackedTransaction + const tracked = trackedTransaction({ tr, state, user }); + const finalState = state.apply(tracked); + + // The selection should be at or near deletePos, not at the original cursor position + expect(finalState.selection.from).toBeLessThanOrEqual(cursorPos); + }); + }); +}); diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js index 170d1f9357..35700e19f3 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js @@ -559,7 +559,6 @@ describe('trackChangesHelpers', () => { it('no-op helpers exist for future implementations', () => { expect(markWrapping()).toBeUndefined(); - expect(replaceAroundStep()).toBeUndefined(); }); it('trackedTransaction keeps selection in sync', () => { 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 830ca2f55c..2f5ddc9f38 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackedTransaction.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackedTransaction.js @@ -1,9 +1,10 @@ -import { Mapping, ReplaceStep, AddMarkStep, RemoveMarkStep } from 'prosemirror-transform'; +import { Mapping, ReplaceStep, AddMarkStep, RemoveMarkStep, ReplaceAroundStep } from 'prosemirror-transform'; import { TextSelection } from 'prosemirror-state'; import { ySyncPluginKey } from 'y-prosemirror'; import { replaceStep } from './replaceStep.js'; import { addMarkStep } from './addMarkStep.js'; import { removeMarkStep } from './removeMarkStep.js'; +import { replaceAroundStep } from './replaceAroundStep.js'; import { TrackDeleteMarkName } from '../constants.js'; import { TrackChangesBasePluginKey } from '../plugins/index.js'; import { findMark } from '@core/helpers/index.js'; @@ -80,7 +81,23 @@ export const trackedTransaction = ({ tr, state, user }) => { user, date, }); + } else if (step instanceof ReplaceAroundStep) { + replaceAroundStep({ + state, + tr, + step, + newTr, + map, + doc, + user, + date, + originalStep, + originalStepIndex, + }); } else { + // Non-structural steps (AttrStep, SetNodeMarkupStep) are typically + // metadata updates from plugins (e.g. listRendering, sdBlockRev). + // These are safe to apply without tracking. newTr.step(step); } }); @@ -100,20 +117,20 @@ export const trackedTransaction = ({ tr, state, user }) => { // Get the track changes meta to check if we have an adjusted insertion position (SD-1624). const trackMeta = newTr.getMeta(TrackChangesBasePluginKey); - if (tr.selectionSet) { - // 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 ( + // selectionPos is an explicit cursor override from tracked change handlers (e.g. + // replaceAroundStep converting a structural step to a character deletion). It must + // be honored regardless of tr.selectionSet, because the original transaction may + // not have set a selection (e.g. ReplaceAroundStep transactions). + 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 { + newTr.setSelection(TextSelection.near($pos, -1)); + } + } else if (tr.selectionSet) { + if ( tr.selection instanceof TextSelection && (tr.selection.from < state.selection.from || tr.getMeta('inputType') === 'deleteContentBackward') ) { @@ -139,8 +156,6 @@ export const trackedTransaction = ({ tr, state, user }) => { } else if (state.selection.from - tr.selection.from > 1 && tr.selection.$head.depth > 1) { const caretPos = map.map(tr.selection.from - 2, -1); newTr.setSelection(new TextSelection(newTr.doc.resolve(caretPos))); - } else { - // Skip the other cases for now. } if (tr.storedMarksSet) { diff --git a/tests/behavior/fixtures/superdoc.ts b/tests/behavior/fixtures/superdoc.ts index 6081962962..7b525d396a 100644 --- a/tests/behavior/fixtures/superdoc.ts +++ b/tests/behavior/fixtures/superdoc.ts @@ -648,7 +648,7 @@ function createFixture(page: Page, editor: Locator, modKey: string) { async assertCommentHighlightExists(opts?: { text?: string; commentId?: string; timeoutMs?: number }) { const expectedText = opts?.text; const expectedCommentId = opts?.commentId; - const timeoutMs = opts?.timeoutMs ?? 15_000; + const timeoutMs = opts?.timeoutMs ?? 20_000; await expect .poll( () => 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 index c458ae9f02..f32b214111 100644 --- a/tests/behavior/tests/comments/delete-terminal-period-separate-run.spec.ts +++ b/tests/behavior/tests/comments/delete-terminal-period-separate-run.spec.ts @@ -102,4 +102,26 @@ test('two backspaces track period and l for bookmark-wrapped runs', async ({ sup expect(newDeletedCombined).toBe('l.'); expect(snapshot.bookmarkStartCount).toBe(before.bookmarkStartCount); expect(snapshot.bookmarkEndCount).toBe(before.bookmarkEndCount); + + // SD-2061: Verify numbered list marker is preserved after backspace. + // Before the fix, ReplaceAroundStep was applied untracked, which removed + // paragraph properties (numbering, font, alignment). + const markerAfter = 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; + }); + + return marker; + }); + expect(markerAfter).toBe('1.'); }); diff --git a/tests/behavior/tests/formatting/decoration-survives-mark-change.spec.ts b/tests/behavior/tests/formatting/decoration-survives-mark-change.spec.ts index eb93832352..e905ea59fd 100644 --- a/tests/behavior/tests/formatting/decoration-survives-mark-change.spec.ts +++ b/tests/behavior/tests/formatting/decoration-survives-mark-change.spec.ts @@ -73,13 +73,34 @@ test.describe('comment highlight survives mark changes', () => { await superdoc.waitForStable(); // Comment highlight must still exist — after the run split the highlight may - // span multiple elements, so check by commentId rather than full text - await superdoc.assertCommentHighlightExists({ commentId }); - - // Each part of the range should still carry the highlight - await superdoc.assertCommentHighlightExists({ text: 'quick', commentId }); - await superdoc.assertCommentHighlightExists({ text: 'brown', commentId }); - await superdoc.assertCommentHighlightExists({ text: 'fox', commentId }); + // span multiple elements, so check by commentId AND all text segments in a + // single poll to avoid multiplicative timeouts in slower engines (webkit). + await expect + .poll( + () => + superdoc.page.evaluate( + ({ cId }) => { + const normalize = (v: string) => v.replace(/\s+/g, ' ').trim(); + const highlights = Array.from(document.querySelectorAll('.superdoc-comment-highlight')); + if (highlights.length === 0) return 'no highlights'; + const hasId = highlights.some((el) => + (el.getAttribute('data-comment-ids') ?? '') + .split(/[\s,]+/) + .filter(Boolean) + .includes(cId), + ); + if (!hasId) return 'missing commentId'; + const texts = highlights.map((el) => normalize(el.textContent ?? '')); + for (const word of ['quick', 'brown', 'fox']) { + if (!texts.some((t) => t.includes(word))) return `missing "${word}"`; + } + return 'ok'; + }, + { cId: commentId }, + ), + { timeout: 25_000 }, + ) + .toBe('ok'); // Italic applied to "brown" await superdoc.assertTextHasMarks('brown', ['italic']);