Skip to content

Commit f87cd08

Browse files
committed
refactor(super-editor): route mixed-bidi Backspace through handleBackspace chain
The MixedBidiBackspace extension previously bound Backspace via addShortcuts and dispatched its own transaction. That bypassed the canonical handleBackspace chain in core/extensions/keymap.js, skipping: - dispatchHistoryBoundary (so undo grouping diverged from regular Backspace), - the tr.setMeta('inputType', 'deleteContentBackward') hop (tracked-changes helpers gate Backspace-specific wrapping on this meta - see trackChangesHelpers/trackedTransaction.js:447, trackChangesHelpers/replaceAroundStep.js:162), - deleteBlockSdtAtTextBlockStart for SDT block boundaries, - the rest of the specialized run-aware ladder. Refactor: - Expose mixedBidiBackspace as a chain command via addCommands(); shape matches the existing backspaceAcrossRuns / backspaceNextToRun pattern (factory returning ({state, view, tr, dispatch}) => boolean). - Insert into the Backspace chain in keymap.js after backspaceAcrossRuns (specialized cross-run handling) and before deleteSelection (generic fallback), guarded with `?? false` so the chain works when the extension is unregistered. - Split the detection logic into pure resolveMixedBidiBackspaceRange so the command body stays tiny. - Tests rewritten to call the chain command shape directly and pin: chain dry-run mode (dispatch=undefined) returns true without mutating tr, non-mixed boundaries fall through, both RTL+LTR and LTR+RTL boundaries handled, posAtDOM is skipped early for pure-LTR/RTL lines.
1 parent 8ce8602 commit f87cd08

3 files changed

Lines changed: 102 additions & 91 deletions

File tree

packages/super-editor/src/editors/v1/core/extensions/keymap.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ export const handleBackspace = (editor) => {
4343
() => commands.backspaceAtomBefore(),
4444
() => commands.backspaceNextToRun(),
4545
() => commands.backspaceAcrossRuns(),
46+
() => commands.mixedBidiBackspace?.() ?? false,
4647
() => commands.deleteSelection(),
4748
() => commands.removeNumberingProperties(),
4849
() => commands.joinBackward(),

packages/super-editor/src/editors/v1/extensions/mixed-bidi-backspace/mixed-bidi-backspace.js

Lines changed: 50 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -108,55 +108,87 @@ const resolveBoundaryChars = (chars, caretPoint) => {
108108
return { left, right };
109109
};
110110

111-
export const handleMixedBidiBackspace = (editor) => {
112-
const { state, view } = editor;
111+
/**
112+
* Compute the visual-left delete range at a mixed-bidi RTL/LTR boundary.
113+
*
114+
* Returns the PM-position range to delete, or `null` when the caret is not
115+
* on a mixed-direction boundary. Pure: does not mutate state or dispatch.
116+
*
117+
* @param {{ state: any, view: any }} args
118+
* @returns {{ from: number, to: number } | null}
119+
*/
120+
const resolveMixedBidiBackspaceRange = ({ state, view }) => {
113121
const { selection } = state;
114-
if (!selection?.empty) return false;
122+
if (!selection?.empty) return null;
115123

116-
const doc = view.dom.ownerDocument;
117-
const nativeSelection = doc.getSelection();
118-
if (!nativeSelection || nativeSelection.rangeCount === 0) return false;
124+
const doc = view?.dom?.ownerDocument;
125+
const nativeSelection = doc?.getSelection?.();
126+
if (!nativeSelection || nativeSelection.rangeCount === 0) return null;
119127

120128
const range = nativeSelection.getRangeAt(0);
121-
if (!range.collapsed) return false;
129+
if (!range.collapsed) return null;
122130

123131
const caretPoint = resolveCaretPoint(doc, range);
124-
if (!caretPoint) return false;
132+
if (!caretPoint) return null;
125133

126134
const lineEl = resolveLineElement(doc, caretPoint);
127-
if (!lineEl) return false;
135+
if (!lineEl) return null;
128136
const lineText = lineEl.textContent ?? '';
129137
const hasRtl = STRONG_RTL_CHAR_RE.test(lineText);
130138
const hasLtr = STRONG_LTR_CHAR_RE.test(lineText);
131-
if (!hasRtl || !hasLtr) return false;
139+
if (!hasRtl || !hasLtr) return null;
132140

133141
let chars = collectVisualChars(lineEl, view, caretPoint.x);
134142
if (chars.length < 2) {
135143
// Fallback to a full scan for correctness when the local window is too narrow.
136144
chars = collectVisualChars(lineEl, view, null);
137145
}
138146
const boundary = resolveBoundaryChars(chars, caretPoint);
139-
if (!boundary) return false;
147+
if (!boundary) return null;
140148

141-
if (!hasMixedDirectionBoundary(boundary.left.char, boundary.right.char)) return false;
142-
if (selection.from !== boundary.right.pmStart && selection.from !== boundary.left.pmEnd) return false;
149+
if (!hasMixedDirectionBoundary(boundary.left.char, boundary.right.char)) return null;
150+
if (selection.from !== boundary.right.pmStart && selection.from !== boundary.left.pmEnd) return null;
143151

144-
const tr = state.tr.delete(boundary.left.pmStart, boundary.left.pmEnd).scrollIntoView();
145-
view.dispatch(tr);
146-
return true;
152+
return { from: boundary.left.pmStart, to: boundary.left.pmEnd };
147153
};
148154

155+
/**
156+
* Mixed-bidi Backspace command. Slotted into the keymap Backspace chain so it
157+
* inherits the chain's history boundary, inputType: deleteContentBackward meta,
158+
* track-changes wrapping, protected-range guards, and SDT handling, instead of
159+
* dispatching its own transaction.
160+
*
161+
* Returns true (chain stops) only when the caret is at a strong-RTL/strong-LTR
162+
* boundary and the visual-left character is targeted for deletion. Otherwise
163+
* returns false so the chain falls through to deleteSelection / joinBackward.
164+
*
165+
* @returns {import('@core/commands/types/index.js').Command}
166+
*/
167+
export const mixedBidiBackspace =
168+
() =>
169+
({ state, view, tr, dispatch }) => {
170+
const range = resolveMixedBidiBackspaceRange({ state, view });
171+
if (!range) return false;
172+
173+
if (dispatch) {
174+
tr.delete(range.from, range.to);
175+
tr.scrollIntoView();
176+
}
177+
return true;
178+
};
179+
149180
export const MixedBidiBackspace = Extension.create({
150181
name: 'mixedBidiBackspace',
151182

152-
addShortcuts() {
183+
addCommands() {
153184
return {
154-
Backspace: () => handleMixedBidiBackspace(this.editor),
185+
mixedBidiBackspace,
155186
};
156187
},
157188
});
158189

159190
export const __TEST_ONLY__ = {
160191
resolveCaretPoint,
161192
hasMixedDirectionBoundary,
193+
resolveMixedBidiBackspaceRange,
162194
};
Lines changed: 51 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it, vi } from 'vitest';
2-
import { __TEST_ONLY__, handleMixedBidiBackspace } from './mixed-bidi-backspace.js';
2+
import { __TEST_ONLY__, mixedBidiBackspace } from './mixed-bidi-backspace.js';
33

44
const makeRect = (left, top = 10, width = 8, height = 12) => ({
55
left,
@@ -8,7 +8,7 @@ const makeRect = (left, top = 10, width = 8, height = 12) => ({
88
height,
99
});
1010

11-
const setupEditor = ({ text, charLefts, caretRect, selectionFrom, pmBase = 10 }) => {
11+
const setupContext = ({ text, charLefts, caretRect, selectionFrom, pmBase = 10 }) => {
1212
const doc = document.implementation.createHTMLDocument('mixed-bidi-backspace');
1313
Object.defineProperty(doc, 'defaultView', {
1414
value: { NodeFilter: { SHOW_TEXT: 4 } },
@@ -64,137 +64,115 @@ const setupEditor = ({ text, charLefts, caretRect, selectionFrom, pmBase = 10 })
6464
};
6565
const view = {
6666
dom: { ownerDocument: doc },
67-
dispatch,
6867
posAtDOM: vi.fn((node, offset) => {
6968
if (node !== textNode) throw new Error('unexpected node');
7069
return pmBase + offset;
7170
}),
7271
};
73-
const editor = {
74-
state: {
75-
selection: { empty: true, from: selectionFrom },
76-
tr,
77-
},
78-
view,
72+
const state = {
73+
selection: { empty: true, from: selectionFrom },
7974
};
8075

81-
return { editor, tr, dispatch };
76+
return { state, view, tr, dispatch };
8277
};
8378

84-
describe('mixed-bidi-backspace', () => {
85-
it('deletes visual-left char on mixed-direction boundary', () => {
86-
const { editor, tr, dispatch } = setupEditor({
79+
describe('mixedBidiBackspace (chain command)', () => {
80+
it('returns true and mutates the chain tr on RTL+LTR boundary', () => {
81+
const { state, view, tr, dispatch } = setupContext({
8782
text: 'אA',
8883
charLefts: [10, 20],
8984
caretRect: makeRect(20, 10, 1, 12),
9085
selectionFrom: 11,
9186
pmBase: 10,
9287
});
9388

94-
const handled = handleMixedBidiBackspace(editor);
89+
const handled = mixedBidiBackspace()({ state, view, tr, dispatch });
9590
expect(handled).toBe(true);
9691
expect(tr.delete).toHaveBeenCalledWith(10, 11);
97-
expect(dispatch).toHaveBeenCalledTimes(1);
92+
expect(tr.scrollIntoView).toHaveBeenCalledTimes(1);
93+
expect(dispatch).not.toHaveBeenCalled(); // chain owns dispatch
9894
});
9995

100-
it('fails open on non-mixed boundary (pure LTR)', () => {
101-
const { editor, tr, dispatch } = setupEditor({
102-
text: 'AB',
103-
charLefts: [10, 20],
104-
caretRect: makeRect(20, 10, 1, 12),
105-
selectionFrom: 11,
106-
pmBase: 10,
107-
});
108-
109-
const handled = handleMixedBidiBackspace(editor);
110-
expect(handled).toBe(false);
111-
expect(tr.delete).not.toHaveBeenCalled();
112-
expect(dispatch).not.toHaveBeenCalled();
113-
expect(editor.view.posAtDOM).not.toHaveBeenCalled();
114-
});
115-
116-
it('deletes visual-left char on inverse mixed boundary (LTR + RTL)', () => {
117-
const { editor, tr, dispatch } = setupEditor({
96+
it('returns true and mutates the chain tr on LTR+RTL boundary', () => {
97+
const { state, view, tr } = setupContext({
11898
text: 'Aא',
11999
charLefts: [10, 20],
120100
caretRect: makeRect(20, 10, 1, 12),
121101
selectionFrom: 11,
122102
pmBase: 10,
123103
});
124104

125-
const handled = handleMixedBidiBackspace(editor);
105+
const handled = mixedBidiBackspace()({ state, view, tr, dispatch: vi.fn() });
126106
expect(handled).toBe(true);
127107
expect(tr.delete).toHaveBeenCalledWith(10, 11);
128-
expect(dispatch).toHaveBeenCalledTimes(1);
129108
});
130109

131-
it('fails open when selectionFrom does not match boundary PM position', () => {
132-
const { editor, tr, dispatch } = setupEditor({
133-
text: 'אA',
110+
it('returns false without mutating tr on pure LTR (chain falls through)', () => {
111+
const { state, view, tr, dispatch } = setupContext({
112+
text: 'AB',
134113
charLefts: [10, 20],
135114
caretRect: makeRect(20, 10, 1, 12),
136-
selectionFrom: 999,
115+
selectionFrom: 11,
137116
pmBase: 10,
138117
});
139118

140-
const handled = handleMixedBidiBackspace(editor);
119+
const handled = mixedBidiBackspace()({ state, view, tr, dispatch });
141120
expect(handled).toBe(false);
142121
expect(tr.delete).not.toHaveBeenCalled();
143-
expect(dispatch).not.toHaveBeenCalled();
122+
expect(tr.scrollIntoView).not.toHaveBeenCalled();
123+
expect(view.posAtDOM).not.toHaveBeenCalled(); // early-out skips DOM scan
144124
});
145125

146-
it('fails open for punctuation bridge between RTL and LTR', () => {
147-
const { editor, tr, dispatch } = setupEditor({
148-
text: 'א.A',
149-
charLefts: [10, 20, 30],
150-
caretRect: makeRect(30, 10, 1, 12),
151-
selectionFrom: 12,
126+
it('returns false on non-empty selection (chain falls through)', () => {
127+
const { state, view, tr } = setupContext({
128+
text: 'אA',
129+
charLefts: [10, 20],
130+
caretRect: makeRect(20, 10, 1, 12),
131+
selectionFrom: 11,
152132
pmBase: 10,
153133
});
134+
state.selection.empty = false;
154135

155-
const handled = handleMixedBidiBackspace(editor);
136+
const handled = mixedBidiBackspace()({ state, view, tr, dispatch: vi.fn() });
156137
expect(handled).toBe(false);
157138
expect(tr.delete).not.toHaveBeenCalled();
158-
expect(dispatch).not.toHaveBeenCalled();
159139
});
160140

161-
it('fails open when caret is at visual start (no left char)', () => {
162-
const { editor, tr, dispatch } = setupEditor({
141+
// SD-2933 / SD-2767: the chain's `dispatch` parameter is undefined during dry-run
142+
// probing. The command must still return true without mutating tr in that mode.
143+
// chain.first uses this to detect whether a command CAN handle the operation.
144+
it('does not mutate tr when dispatch is undefined (chain dry-run probe)', () => {
145+
const { state, view, tr } = setupContext({
163146
text: 'אA',
164147
charLefts: [10, 20],
165-
caretRect: makeRect(5, 10, 1, 12),
166-
selectionFrom: 10,
148+
caretRect: makeRect(20, 10, 1, 12),
149+
selectionFrom: 11,
167150
pmBase: 10,
168151
});
169152

170-
const handled = handleMixedBidiBackspace(editor);
171-
expect(handled).toBe(false);
153+
const handled = mixedBidiBackspace()({ state, view, tr, dispatch: undefined });
154+
expect(handled).toBe(true);
172155
expect(tr.delete).not.toHaveBeenCalled();
173-
expect(dispatch).not.toHaveBeenCalled();
156+
expect(tr.scrollIntoView).not.toHaveBeenCalled();
174157
});
175158

176-
it('fails open for non-collapsed selection', () => {
177-
const { editor, tr, dispatch } = setupEditor({
178-
text: 'אA',
179-
charLefts: [10, 20],
180-
caretRect: makeRect(20, 10, 1, 12),
181-
selectionFrom: 11,
159+
it('returns false when caret is not at the boundary (e.g. mid-word)', () => {
160+
const { state, view, tr } = setupContext({
161+
text: 'אAB',
162+
charLefts: [10, 20, 30],
163+
caretRect: makeRect(30, 10, 1, 12),
164+
selectionFrom: 12, // past the boundary
182165
pmBase: 10,
183166
});
184-
editor.state.selection.empty = false;
185167

186-
const handled = handleMixedBidiBackspace(editor);
168+
const handled = mixedBidiBackspace()({ state, view, tr, dispatch: vi.fn() });
187169
expect(handled).toBe(false);
188-
expect(tr.delete).not.toHaveBeenCalled();
189-
expect(dispatch).not.toHaveBeenCalled();
190170
});
191171

192-
it('resolveCaretPoint returns null for zero-size rect', () => {
193-
const doc = document.implementation.createHTMLDocument('caret-rect');
194-
const result = __TEST_ONLY__.resolveCaretPoint(doc, {
195-
getBoundingClientRect: () => makeRect(0, 0, 0, 0),
196-
startContainer: doc.body,
197-
});
198-
expect(result).toBeNull();
172+
it('exposes hasMixedDirectionBoundary helper for direct testing', () => {
173+
expect(__TEST_ONLY__.hasMixedDirectionBoundary('א', 'A')).toBe(true);
174+
expect(__TEST_ONLY__.hasMixedDirectionBoundary('A', 'א')).toBe(true);
175+
expect(__TEST_ONLY__.hasMixedDirectionBoundary('A', 'B')).toBe(false);
176+
expect(__TEST_ONLY__.hasMixedDirectionBoundary('א', 'ש')).toBe(false);
199177
});
200178
});

0 commit comments

Comments
 (0)