Skip to content

Commit 8f3cbe4

Browse files
authored
fix(track-changes): handle ReplaceAroundStep in tracked changes mode (SD-2061) (#2225)
* fix(track-changes): handle ReplaceAroundStep in tracked changes mode SD-2061: When backspacing in tracked changes mode triggers a ReplaceAroundStep (e.g. at paragraph/list boundaries), convert it to a tracked single-character deletion instead of silently applying the structural change. This preserves paragraph numbering, font, and alignment. Also fixes cursor positioning for ReplaceAroundStep transactions by checking selectionPos before tr.selectionSet, and merges adjacent trackDelete marks across run node boundaries so consecutive deletions appear as a single tracked change. * test(track-changes): add unit tests for replaceAroundStep handler Add 9 unit tests covering: - Non-backspace ReplaceAroundStep blocking - Backspace conversion to tracked single-char deletion - selectionPos meta for cursor positioning - findPreviousLiveCharPos skipping tracked-deleted text - Mark merging across run node boundaries (same/different authors) - selectionPos honored in trackedTransaction when tr.selectionSet is false Also add numbering preservation assertion to the existing behavior test for double-backspace at terminal period with bookmark-wrapped runs. * fix(track-changes): only merge contiguous trackDelete marks Stop the merge loop at the first live (non-deleted) text node so non-adjacent deletions within the search window are not incorrectly collapsed into a single tracked change. * fix(tests): reduce webkit flakiness in comment highlight assertions Increase default assertCommentHighlightExists timeout from 15s to 20s to match what most callers already pass explicitly. Consolidate 4 sequential post-italic assertions into a single composite poll to avoid multiplicative timeouts in webkit's slower rendering pipeline.
1 parent e6a0dab commit 8f3cbe4

7 files changed

Lines changed: 714 additions & 28 deletions

File tree

Lines changed: 157 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,158 @@
1-
export const replaceAroundStep = () => {
2-
// Skip for now.
1+
import { ReplaceStep } from 'prosemirror-transform';
2+
import { Slice } from 'prosemirror-model';
3+
import { replaceStep } from './replaceStep.js';
4+
import { TrackDeleteMarkName } from '../constants.js';
5+
import { TrackChangesBasePluginKey } from '../plugins/index.js';
6+
7+
/**
8+
* Find the closest live (non-tracked-deleted) text character position before
9+
* the cursor, within the same paragraph.
10+
*
11+
* @param {import('prosemirror-model').Node} doc
12+
* @param {number} cursorPos
13+
* @param {import('prosemirror-model').MarkType} trackDeleteMarkType
14+
* @returns {number | null} The document position of the character, or null.
15+
*/
16+
const findPreviousLiveCharPos = (doc, cursorPos, trackDeleteMarkType) => {
17+
const $cursor = doc.resolve(cursorPos);
18+
19+
// Find the enclosing paragraph (may need to go up past run nodes).
20+
let paraDepth = $cursor.depth;
21+
while (paraDepth > 0 && $cursor.node(paraDepth).type.name !== 'paragraph') {
22+
paraDepth--;
23+
}
24+
if (paraDepth <= 0) return null;
25+
26+
const paraStart = $cursor.before(paraDepth) + 1;
27+
28+
// Walk through inline nodes from paragraph start to cursor,
29+
// keeping track of the last live character position found.
30+
let lastLiveCharPos = null;
31+
32+
doc.nodesBetween(paraStart, cursorPos, (node, pos) => {
33+
if (!node.isText) return;
34+
35+
const hasDeleteMark = node.marks.some((m) => m.type === trackDeleteMarkType);
36+
if (hasDeleteMark) return;
37+
38+
// This is a live text node. Its last character within our range is
39+
// at min(nodeEnd, cursorPos) - 1.
40+
const nodeEnd = pos + node.nodeSize;
41+
const relevantEnd = Math.min(nodeEnd, cursorPos);
42+
if (relevantEnd > pos) {
43+
lastLiveCharPos = relevantEnd - 1;
44+
}
45+
});
46+
47+
return lastLiveCharPos;
48+
};
49+
50+
/**
51+
* Handle a ReplaceAroundStep in tracked changes mode.
52+
*
53+
* ReplaceAroundStep is ProseMirror's structural "change wrapper" operation
54+
* (e.g. lifting content out of a list item, changing block type). In tracked
55+
* changes mode we must never silently apply structural changes — they would
56+
* alter paragraph properties (numbering, font, alignment) without tracking.
57+
*
58+
* For backspace/delete, the user's intent is to delete a character, not change
59+
* paragraph structure. We convert the step to a tracked single-character
60+
* deletion using the existing replaceStep handler.
61+
*
62+
* @param {object} options
63+
* @param {import('prosemirror-state').EditorState} options.state
64+
* @param {import('prosemirror-state').Transaction} options.tr
65+
* @param {import('prosemirror-transform').ReplaceAroundStep} options.step
66+
* @param {import('prosemirror-state').Transaction} options.newTr
67+
* @param {import('prosemirror-transform').Mapping} options.map
68+
* @param {import('prosemirror-model').Node} options.doc
69+
* @param {object} options.user
70+
* @param {string} options.date
71+
* @param {import('prosemirror-transform').Step} options.originalStep
72+
* @param {number} options.originalStepIndex
73+
*/
74+
export const replaceAroundStep = ({
75+
state,
76+
tr,
77+
step,
78+
newTr,
79+
map,
80+
doc,
81+
user,
82+
date,
83+
originalStep,
84+
originalStepIndex,
85+
}) => {
86+
const inputType = tr.getMeta('inputType');
87+
const isBackspace = inputType === 'deleteContentBackward';
88+
89+
if (!isBackspace) {
90+
// Non-backspace ReplaceAroundStep in tracked changes: block it.
91+
// Structural wrapper changes (list toggle, block type change) should be
92+
// implemented as tracked format changes in the future. For now, silently
93+
// dropping them is safer than applying them untracked.
94+
return;
95+
}
96+
97+
// For backspace: find the previous live character and track its deletion.
98+
const trackDeleteMarkType = state.schema.marks[TrackDeleteMarkName];
99+
const deleteFrom = findPreviousLiveCharPos(doc, state.selection.from, trackDeleteMarkType);
100+
101+
if (deleteFrom === null) {
102+
// No live character found — nothing to delete. Skip the structural change.
103+
return;
104+
}
105+
106+
const charStep = new ReplaceStep(deleteFrom, deleteFrom + 1, Slice.empty);
107+
108+
replaceStep({
109+
state,
110+
tr,
111+
step: charStep,
112+
newTr,
113+
map,
114+
doc,
115+
user,
116+
date,
117+
originalStep: charStep,
118+
originalStepIndex,
119+
});
120+
121+
// Position the cursor at the deletion edge. The original transaction's
122+
// selection was computed for the structural ReplaceAroundStep, not our
123+
// fabricated character deletion. Override it so the cursor visually
124+
// moves left with each backspace.
125+
const trackMeta = newTr.getMeta(TrackChangesBasePluginKey) || {};
126+
trackMeta.selectionPos = deleteFrom;
127+
newTr.setMeta(TrackChangesBasePluginKey, trackMeta);
128+
129+
// Merge adjacent trackDelete marks that have the same author/date but different IDs.
130+
// When backspace first deletes a character (e.g. ".") via a normal ReplaceStep and
131+
// subsequent presses delete further characters (e.g. "l") via ReplaceAroundStep,
132+
// the deletion marks end up with different IDs because run node boundaries create
133+
// a position gap larger than findTrackedMarkBetween's ±1 offset. Re-mark the
134+
// earlier deletion with the current ID so they merge into a single tracked change.
135+
if (trackMeta.deletionMark) {
136+
const ourId = trackMeta.deletionMark.attrs.id;
137+
const ourEmail = trackMeta.deletionMark.attrs.authorEmail;
138+
const ourDate = trackMeta.deletionMark.attrs.date;
139+
const searchTo = Math.min(newTr.doc.content.size, deleteFrom + 20);
140+
141+
let contiguous = true;
142+
newTr.doc.nodesBetween(deleteFrom, searchTo, (node, pos) => {
143+
if (!contiguous) return false;
144+
if (!node.isText) return;
145+
const delMark = node.marks.find((m) => m.type.name === TrackDeleteMarkName);
146+
if (!delMark) {
147+
contiguous = false; // Live text — stop, deletions are no longer contiguous.
148+
return;
149+
}
150+
if (delMark.attrs.id !== ourId && delMark.attrs.authorEmail === ourEmail && delMark.attrs.date === ourDate) {
151+
const markType = state.schema.marks[TrackDeleteMarkName];
152+
const merged = markType.create({ ...delMark.attrs, id: ourId });
153+
newTr.removeMark(pos, pos + node.nodeSize, delMark);
154+
newTr.addMark(pos, pos + node.nodeSize, merged);
155+
}
156+
});
157+
}
3158
};

0 commit comments

Comments
 (0)