diff --git a/packages/layout-engine/contracts/src/direction-context.ts b/packages/layout-engine/contracts/src/direction-context.ts index b4f956333a..e533d52c5e 100644 --- a/packages/layout-engine/contracts/src/direction-context.ts +++ b/packages/layout-engine/contracts/src/direction-context.ts @@ -114,7 +114,23 @@ export type ParagraphDirectionContext = { * w:bdo (§17.3.2.3 override). */ export type RunBidiContext = { - /** w:rPr/w:rtl. Forces complex-script formatting; see RunScriptContext. */ + /** + * w:rPr/w:rtl. Preserves the source OOXML signal that the run carries + * the `w:rtl` flag. Per §17.3.2.30, `w:rtl` does two things at the model + * level: + * 1. Forces the complex-script formatting stack (bCs, iCs, szCs, + * rFonts/@cs). See RunScriptContext for the formatting half. + * 2. Acts as a Character Directionality Override for weak/neutral + * characters in the run (NOT a forced visual flip of strong-LTR text; + * §17.3.2.30 explicitly says behavior on strong-LTR is unspecified). + * + * `rtl: true` is the source signal, NOT a directive that every consumer + * must project to `dir="rtl"` in the rendered DOM. The painter decides + * the DOM projection per its Word-parity rules (see + * `features/inline-direction/resolveRunDirectionAttribute`). Exporters + * must preserve `rtl: true` on round-trip regardless of paint decisions, + * since dropping it would lose the source `w:rPr/w:rtl` semantics. + */ rtl: boolean; /** w:dir; bidi embedding direction (RLE/LRE). Wave 1c. */ embedding?: BaseDirection; diff --git a/packages/layout-engine/painters/dom/src/features/inline-direction/index.ts b/packages/layout-engine/painters/dom/src/features/inline-direction/index.ts index 14ab684072..2492bf20f2 100644 --- a/packages/layout-engine/painters/dom/src/features/inline-direction/index.ts +++ b/packages/layout-engine/painters/dom/src/features/inline-direction/index.ts @@ -20,3 +20,11 @@ */ export { applyRtlStyles, shouldUseSegmentPositioning } from './rtl-styles.js'; +export { + resolveRunDirectionAttribute, + normalizeRtlDateTokenForWordParity, + RTL_DATE_LIKE_TOKEN_RE, + STRONG_RTL_CHAR_RE, + LATIN_DIGIT_NEUTRAL_ONLY_RE, + type RunDirAttribute, +} from './run-direction.js'; diff --git a/packages/layout-engine/painters/dom/src/features/inline-direction/run-direction.test.ts b/packages/layout-engine/painters/dom/src/features/inline-direction/run-direction.test.ts new file mode 100644 index 0000000000..417c2229f7 --- /dev/null +++ b/packages/layout-engine/painters/dom/src/features/inline-direction/run-direction.test.ts @@ -0,0 +1,211 @@ +import { describe, expect, it } from 'vitest'; +import { + resolveRunDirectionAttribute, + normalizeRtlDateTokenForWordParity, + RTL_DATE_LIKE_TOKEN_RE, + STRONG_RTL_CHAR_RE, + LATIN_DIGIT_NEUTRAL_ONLY_RE, +} from './run-direction.js'; + +describe('resolveRunDirectionAttribute', () => { + describe('rtl-tagged runs', () => { + it('returns "rtl" for Hebrew text', () => { + expect( + resolveRunDirectionAttribute({ + runText: 'שלום', + effectiveText: 'שלום', + isRtlTagged: true, + }), + ).toBe('rtl'); + }); + + it('returns "rtl" for Arabic text', () => { + expect( + resolveRunDirectionAttribute({ + runText: 'مرحبا', + effectiveText: 'مرحبا', + isRtlTagged: true, + }), + ).toBe('rtl'); + }); + + it('returns null for Latin-only text (Word-parity: §17.3.2.30 unspecified)', () => { + expect( + resolveRunDirectionAttribute({ + runText: 'Hello', + effectiveText: 'Hello', + isRtlTagged: true, + }), + ).toBe(null); + }); + + it('returns null for digit-only text', () => { + expect( + resolveRunDirectionAttribute({ + runText: '2026', + effectiveText: '2026', + isRtlTagged: true, + }), + ).toBe(null); + }); + + it('returns "rtl" for date-like numeric (isolates the date as RTL unit)', () => { + expect( + resolveRunDirectionAttribute({ + runText: '2026-03-15', + effectiveText: '2026-03-15', + isRtlTagged: true, + }), + ).toBe('rtl'); + }); + + it('returns "rtl" for mixed strong-RTL + Latin (Hebrew present)', () => { + expect( + resolveRunDirectionAttribute({ + runText: 'first שלום', + effectiveText: 'first שלום', + isRtlTagged: true, + }), + ).toBe('rtl'); + }); + + it('returns "rtl" for empty text (honor source signal when no content)', () => { + expect( + resolveRunDirectionAttribute({ + runText: '', + effectiveText: '', + isRtlTagged: true, + }), + ).toBe('rtl'); + }); + + it('returns "rtl" for whitespace-only text', () => { + expect( + resolveRunDirectionAttribute({ + runText: ' ', + effectiveText: ' ', + isRtlTagged: true, + }), + ).toBe('rtl'); + }); + + // Fail-safe: anything that doesn't match the Latin/digit/neutral set OR the + // strong-RTL set still honors the source signal. East Asian, presentation + // forms, symbols outside the neutral set all fall into this branch. + it('returns "rtl" for text that is neither Latin nor strong-RTL', () => { + expect( + resolveRunDirectionAttribute({ + runText: '世界', + effectiveText: '世界', + isRtlTagged: true, + }), + ).toBe('rtl'); + }); + + it('uses effectiveText when runText is undefined', () => { + expect( + resolveRunDirectionAttribute({ + runText: undefined, + effectiveText: 'שלום', + isRtlTagged: true, + }), + ).toBe('rtl'); + }); + }); + + describe('non-rtl-tagged runs', () => { + it('returns "ltr" for date-like numeric (Word-parity in RTL paragraph)', () => { + expect( + resolveRunDirectionAttribute({ + runText: '2026-03-15', + effectiveText: '2026-03-15', + isRtlTagged: false, + }), + ).toBe('ltr'); + }); + + it('returns null for plain Latin (let paragraph + UBA decide)', () => { + expect( + resolveRunDirectionAttribute({ + runText: 'Hello', + effectiveText: 'Hello', + isRtlTagged: false, + }), + ).toBe(null); + }); + + it('returns null for Hebrew text without w:rtl (paragraph context resolves)', () => { + expect( + resolveRunDirectionAttribute({ + runText: 'שלום', + effectiveText: 'שלום', + isRtlTagged: false, + }), + ).toBe(null); + }); + + it('returns null when runText is undefined (no date pattern to match)', () => { + expect( + resolveRunDirectionAttribute({ + runText: undefined, + effectiveText: '2026-03-15', + isRtlTagged: false, + }), + ).toBe(null); + }); + }); +}); + +describe('normalizeRtlDateTokenForWordParity', () => { + const RLM = '\u200F'; + + it('wraps separators with RLM in date-like text', () => { + expect(normalizeRtlDateTokenForWordParity('2026-03-15')).toBe(`2026${RLM}-${RLM}03${RLM}-${RLM}15`); + }); + + it('handles slash separators', () => { + expect(normalizeRtlDateTokenForWordParity('15/03/2026')).toBe(`15${RLM}/${RLM}03${RLM}/${RLM}2026`); + }); + + it('handles dot separators', () => { + expect(normalizeRtlDateTokenForWordParity('1.2.3')).toBe(`1${RLM}.${RLM}2${RLM}.${RLM}3`); + }); + + it('wraps the leading sign too (no special-case for leading "-")', () => { + // Implementation is text.replace(/[./-]/g, ...). The leading sign is also + // a `-`, so it gets RLM-wrapped. This matches the pre-extraction behavior. + expect(normalizeRtlDateTokenForWordParity('-2026-03')).toBe(`${RLM}-${RLM}2026${RLM}-${RLM}03`); + }); + + it('returns unchanged for non-date text', () => { + expect(normalizeRtlDateTokenForWordParity('Hello world')).toBe('Hello world'); + expect(normalizeRtlDateTokenForWordParity('2026')).toBe('2026'); // no separator + expect(normalizeRtlDateTokenForWordParity('שלום')).toBe('שלום'); + }); +}); + +describe('regex coverage smoke tests', () => { + it('RTL_DATE_LIKE_TOKEN_RE matches numeric dates', () => { + expect(RTL_DATE_LIKE_TOKEN_RE.test('2026-03-15')).toBe(true); + expect(RTL_DATE_LIKE_TOKEN_RE.test('15/03/2026')).toBe(true); + expect(RTL_DATE_LIKE_TOKEN_RE.test('1.2.3')).toBe(true); + expect(RTL_DATE_LIKE_TOKEN_RE.test('-2026-03')).toBe(true); + expect(RTL_DATE_LIKE_TOKEN_RE.test('2026')).toBe(false); // no separator + expect(RTL_DATE_LIKE_TOKEN_RE.test('a-b-c')).toBe(false); + }); + + it('STRONG_RTL_CHAR_RE matches Hebrew and Arabic core blocks', () => { + expect(STRONG_RTL_CHAR_RE.test('שלום')).toBe(true); + expect(STRONG_RTL_CHAR_RE.test('مرحبا')).toBe(true); + expect(STRONG_RTL_CHAR_RE.test('Hello')).toBe(false); + expect(STRONG_RTL_CHAR_RE.test('2026')).toBe(false); + }); + + it('LATIN_DIGIT_NEUTRAL_ONLY_RE matches Latin + digit + neutral chars', () => { + expect(LATIN_DIGIT_NEUTRAL_ONLY_RE.test('Hello world')).toBe(true); + expect(LATIN_DIGIT_NEUTRAL_ONLY_RE.test('copy 2')).toBe(true); + expect(LATIN_DIGIT_NEUTRAL_ONLY_RE.test('a/b-c.d')).toBe(true); + expect(LATIN_DIGIT_NEUTRAL_ONLY_RE.test('שלום')).toBe(false); + expect(LATIN_DIGIT_NEUTRAL_ONLY_RE.test('Hello שלום')).toBe(false); + }); +}); diff --git a/packages/layout-engine/painters/dom/src/features/inline-direction/run-direction.ts b/packages/layout-engine/painters/dom/src/features/inline-direction/run-direction.ts new file mode 100644 index 0000000000..291bfdaf03 --- /dev/null +++ b/packages/layout-engine/painters/dom/src/features/inline-direction/run-direction.ts @@ -0,0 +1,99 @@ +/** + * Run-level direction helpers for DomPainter. + * + * These helpers encode paint-time decisions about how to project the OOXML + * `w:rPr/w:rtl` signal onto a rendered span's `dir` attribute, plus a narrow + * Word-parity workaround for RTL-tagged date-like numeric runs. + * + * The heuristic is intentionally scoped to current Word-parity fixtures + * (SD-3098 mixed-bidi date tokens). It is NOT a full implementation of + * §17.3.2.30 semantics - notably absent: `w:dir` embedding (§17.3.2.8), + * `w:bdo` override (§17.3.2.3), and `w:lang/@bidi` Hebrew vs Arabic numeric + * differences. Those gaps are tracked separately; see SD-2767 follow-ups. + * + * @spec ECMA-376 §17.3.2.30 (rtl), §17.17.4 (boolean property) + */ + +/** + * Matches numeric date-like tokens such as `2026-03-15`, `15/03/2026`, `1.2.3`. + * Used by both the run direction resolver and the paint-time RLM injection + * for Word parity on RTL date strings. + */ +export const RTL_DATE_LIKE_TOKEN_RE = /^-?\d+(?:[./-]\d+)+$/; + +/** + * Matches strong-RTL characters in the Hebrew / Arabic / Syriac core blocks. + * + * Known gap: misses Hebrew presentation forms (FB1D-FB4F) and Arabic + * presentation forms (FB50-FDFF, FE70-FEFF). Tracked under SD-2767 follow-ups. + */ +export const STRONG_RTL_CHAR_RE = /[\u0590-\u08FF]/; + +/** + * Matches runs whose content is exclusively Latin / digit / neutral. Used as + * the "skip per-run dir=rtl" guard: per §17.3.2.30, behavior of w:rtl on + * strongly LTR text is unspecified, and Word's empirical output for these + * runs does not visually reorder. + */ +export const LATIN_DIGIT_NEUTRAL_ONLY_RE = /^[\s0-9A-Za-z./\-_:,+()]+$/; + +const RLM = '\u200F'; + +/** + * Word-parity workaround for RTL date-like tokens. + * + * Word internally injects RLM around numeric separators in RTL date strings, + * preserving LTR order for the digits while keeping the run RTL. The browser's + * UBA alone does not match this. We mirror Word by injecting RLM at paint + * time only - the DOM text differs from the PM model and from the exported + * OOXML, which both keep the original separators. + * + * Intentionally narrow: only matches numeric date-like patterns so other + * numeric content is unaffected. Scope is current SD-3098 fixtures. + */ +export const normalizeRtlDateTokenForWordParity = (text: string): string => { + if (!RTL_DATE_LIKE_TOKEN_RE.test(text)) { + return text; + } + return text.replace(/[./-]/g, (separator) => `${RLM}${separator}${RLM}`); +}; + +/** + * Compute the `dir` attribute (if any) to apply to a rendered run span. + * + * Decision table: + * - rtl-tagged + empty text -> 'rtl' (no content to classify, honor source signal) + * - rtl-tagged + date-like numeric -> 'rtl' (isolates the date as a unit) + * - rtl-tagged + contains strong-RTL chars -> 'rtl' (standard case) + * - rtl-tagged + only Latin/digit/neutral -> null (per §17.3.2.30, unspecified; + * Word does not visually reorder these, so omit dir to inherit paragraph) + * - rtl-tagged + other (e.g. East Asian, presentation forms) -> 'rtl' (fail-safe) + * - NOT rtl-tagged + date-like numeric text -> 'ltr' (Word-parity: keeps date + * LTR-classified within an RTL paragraph context so digits don't drift) + * - NOT rtl-tagged + anything else -> null (let paragraph + UBA decide) + */ +export type RunDirAttribute = 'rtl' | 'ltr' | null; + +export const resolveRunDirectionAttribute = (opts: { + /** Original run text from the model. */ + runText: string | undefined; + /** Post-token-resolution text used for rendering (e.g. field token expansion). */ + effectiveText: string; + /** True when the source OOXML carries `w:rPr/w:rtl`. */ + isRtlTagged: boolean; +}): RunDirAttribute => { + if (opts.isRtlTagged) { + const sample = (opts.runText ?? opts.effectiveText).trim(); + if (!sample) return 'rtl'; + if (RTL_DATE_LIKE_TOKEN_RE.test(sample)) return 'rtl'; + if (STRONG_RTL_CHAR_RE.test(sample)) return 'rtl'; + if (LATIN_DIGIT_NEUTRAL_ONLY_RE.test(sample)) return null; + return 'rtl'; + } + + if (typeof opts.runText === 'string' && RTL_DATE_LIKE_TOKEN_RE.test(opts.runText)) { + return 'ltr'; + } + + return null; +}; diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index d6438595d0..44ed6f6c4a 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -125,7 +125,12 @@ import { stampBetweenBorderDataset, type BetweenBorderInfo, } from './features/paragraph-borders/index.js'; -import { applyRtlStyles, shouldUseSegmentPositioning } from './features/inline-direction/index.js'; +import { + applyRtlStyles, + shouldUseSegmentPositioning, + resolveRunDirectionAttribute, + normalizeRtlDateTokenForWordParity, +} from './features/inline-direction/index.js'; import { convertOmmlToMathml } from './features/math/index.js'; /** @@ -5597,12 +5602,16 @@ export class DomPainter { // Pass isLink flag to skip applying inline color/decoration styles for links applyRunStyles(elem as HTMLElement, run, isActiveLink); - // SD-3098 Word-parity: rtl-tagged runs get dir="rtl" so per-run bidi is isolated; - // non-rtl date-like runs in RTL context get dir="ltr" to prevent separator drift. - if (textRun.bidi?.rtl === true) { - elem.setAttribute('dir', 'rtl'); - } else if (typeof textRun.text === 'string' && RTL_DATE_LIKE_TOKEN_RE.test(textRun.text)) { - elem.setAttribute('dir', 'ltr'); + // SD-3098 Word-parity: run-level dir attribute decision lives in the + // inline-direction feature. See resolveRunDirectionAttribute for the + // decision table (rtl-tagged + content classification + date-like). + const dirAttr = resolveRunDirectionAttribute({ + runText: textRun.text, + effectiveText, + isRtlTagged: textRun.bidi?.rtl === true, + }); + if (dirAttr) { + elem.setAttribute('dir', dirAttr); } const commentAnnotations = textRun.comments; const hasAnyComment = !!commentAnnotations?.length; @@ -7322,8 +7331,7 @@ export class DomPainter { wrapper.dataset.layoutEpoch = String(this.layoutEpoch); this.applySdtDataset(wrapper, sdt); - const appearance = - sdt.type === 'structuredContent' ? (sdt as { appearance?: string }).appearance : undefined; + const appearance = sdt.type === 'structuredContent' ? (sdt as { appearance?: string }).appearance : undefined; if (appearance === 'hidden') { wrapper.dataset.appearance = 'hidden'; // No alias label and no chrome: see CSS rule keyed off @@ -8294,18 +8302,3 @@ const resolveRunText = (run: Run, context: FragmentRenderContext): string => { } return run.text ?? ''; }; - -const RTL_DATE_LIKE_TOKEN_RE = /^-?\d+(?:[./-]\d+)+$/; -const RLM = '\u200F'; - -// AIDEV-NOTE: SD-3098 Word-parity workaround for RTL date-like tokens. We inject -// RLM around separators at paint time only (DOM text), never into PM/model/export. -// Word reorders numerics inside RTL date strings via internal RLM treatment; the -// browser's UBA does not. This is intentionally narrow - only matches date-like -// numeric patterns - so non-date numeric content is unaffected. -const normalizeRtlDateTokenForWordParity = (text: string): string => { - if (!RTL_DATE_LIKE_TOKEN_RE.test(text)) { - return text; - } - return text.replace(/[./-]/g, (separator) => `${RLM}${separator}${RLM}`); -}; diff --git a/packages/layout-engine/painters/dom/src/rtl-date-parity.test.ts b/packages/layout-engine/painters/dom/src/rtl-date-parity.test.ts index dc278e4f1c..8d65060e1f 100644 --- a/packages/layout-engine/painters/dom/src/rtl-date-parity.test.ts +++ b/packages/layout-engine/painters/dom/src/rtl-date-parity.test.ts @@ -68,9 +68,10 @@ describe('RTL date parity', () => { expect(span?.textContent).toBe(runText); }); - // SD-3098: mixed runs on the same line - the bidiCompatible merge guard keeps - // them as separate spans, so each can carry its own dir attribute. - it('paints mixed rtl + ltr runs on the same line as separate spans with distinct dir attrs', () => { + // Mixed runs remain separate spans. In RTL paragraphs we now avoid forcing + // per-run dir="rtl" for plain Latin/digit tokens, so numeric rtl-tagged runs + // can inherit paragraph direction without isolated run reordering. + it('paints mixed rtl + ltr runs on the same line as separate spans with date-only ltr override', () => { const blockId = 'mixed'; const ltrText = '-03-23'; const rtlText = '2026'; @@ -110,13 +111,13 @@ describe('RTL date parity', () => { expect(spans.length).toBe(2); expect(spans[0].getAttribute('dir')).toBe('ltr'); expect(spans[0].textContent).toBe(ltrText); - expect(spans[1].getAttribute('dir')).toBe('rtl'); + expect(spans[1].getAttribute('dir')).toBeNull(); expect(spans[1].textContent).toBe(rtlText); }); - // SD-3098: rtl-tagged runs that are NOT date-like keep dir="rtl" but get no - // RLM injection. Plain integers (`2026`) don't match the date regex. - it('does not inject RLM into rtl runs whose text is not date-like', () => { + // Rtl-tagged numeric runs in RTL paragraphs do not force dir="rtl" anymore. + // This avoids undesired token inversion in mixed header/footer runs like "copy 2". + it('does not force per-run rtl dir for non-date numeric runs in RTL paragraphs', () => { const blockId = 'rtl-numeric'; const runText = '2026'; const block: FlowBlock = { @@ -133,7 +134,7 @@ describe('RTL date parity', () => { painter.paint(makeLayout(blockId), mount); const span = mount.querySelector('.superdoc-line span'); - expect(span?.getAttribute('dir')).toBe('rtl'); + expect(span?.getAttribute('dir')).toBeNull(); expect(span?.textContent).toBe(runText); expect(span?.textContent).not.toContain('\u200F'); }); diff --git a/packages/super-editor/src/editors/v1/core/extensions/keymap-backspace-chain.test.js b/packages/super-editor/src/editors/v1/core/extensions/keymap-backspace-chain.test.js new file mode 100644 index 0000000000..cd9d480fcc --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/extensions/keymap-backspace-chain.test.js @@ -0,0 +1,157 @@ +import { describe, it, expect, vi } from 'vitest'; +import { handleBackspace } from './keymap.js'; + +/** + * Pins the ordering of commands in the Backspace chain. + * + * The chain shape matters because: + * - `inputType: deleteContentBackward` meta must be set before any specialized + * handler runs (track-changes Backspace gating depends on it). + * - The mixed-bidi handler must run after the run-aware ladder so it does not + * intercept Backspace inside SDT blocks or atomic-before cases. + * - It must run before the generic `deleteSelection` / `joinBackward` fallbacks. + * + * If the chain order changes, this test fails loudly and the author must + * justify the new ordering. + */ +describe('handleBackspace chain ordering', () => { + const makeEditor = () => { + const callLog = []; + const setMetaLog = []; + + const tr = { + setMeta: vi.fn((key, value) => { + setMetaLog.push({ key, value }); + return tr; + }), + }; + + // Each command spy records its name and returns false so the chain + // walks through every entry; the dispatchHistoryBoundary helper at the + // top of handleBackspace dispatches the closeHistory tr separately. + const make = (name) => () => { + callLog.push(name); + return false; + }; + + const commands = { + undoInputRule: make('undoInputRule'), + deleteBlockSdtAtTextBlockStart: make('deleteBlockSdtAtTextBlockStart'), + backspaceEmptyRunParagraph: make('backspaceEmptyRunParagraph'), + backspaceSkipEmptyRun: make('backspaceSkipEmptyRun'), + backspaceAtomBefore: make('backspaceAtomBefore'), + backspaceNextToRun: make('backspaceNextToRun'), + backspaceAcrossRuns: make('backspaceAcrossRuns'), + mixedBidiBackspace: make('mixedBidiBackspace'), + deleteSelection: make('deleteSelection'), + removeNumberingProperties: make('removeNumberingProperties'), + joinBackward: make('joinBackward'), + selectNodeBackward: make('selectNodeBackward'), + }; + + const editor = { + view: { state: { tr }, dispatch: vi.fn() }, + commands: { + first: vi.fn((build) => { + const fns = build({ commands, tr }); + for (const fn of fns) { + const result = fn(); + if (result) return result; + } + return false; + }), + }, + }; + + return { editor, callLog, setMetaLog }; + }; + + it('walks the chain in the expected order when no command handles', () => { + const { editor, callLog } = makeEditor(); + handleBackspace(editor); + expect(callLog).toEqual([ + 'undoInputRule', + // step 2 sets inputType meta and returns false (no command call) + 'deleteBlockSdtAtTextBlockStart', + 'backspaceEmptyRunParagraph', + 'backspaceSkipEmptyRun', + 'backspaceAtomBefore', + 'backspaceNextToRun', + 'backspaceAcrossRuns', + 'mixedBidiBackspace', + 'deleteSelection', + 'removeNumberingProperties', + 'joinBackward', + 'selectNodeBackward', + ]); + }); + + it('sets inputType: deleteContentBackward meta before specialized handlers', () => { + const { editor, callLog, setMetaLog } = makeEditor(); + handleBackspace(editor); + + expect(setMetaLog).toContainEqual({ key: 'inputType', value: 'deleteContentBackward' }); + + // Meta must be set BEFORE the run-aware handlers run, otherwise track-changes + // Backspace wrapping in trackChangesHelpers/trackedTransaction.js cannot + // identify the tr as a Backspace. + const sdtIndex = callLog.indexOf('deleteBlockSdtAtTextBlockStart'); + expect(sdtIndex).toBeGreaterThanOrEqual(0); + // Spy log only records command calls, not the meta-setter step; verify + // meta-setter happens at chain position 1 by reconstructing the chain + // walk (undoInputRule at 0, meta-setter at 1, then SDT at 2). + expect(callLog[0]).toBe('undoInputRule'); + expect(callLog[1]).toBe('deleteBlockSdtAtTextBlockStart'); + }); + + it('places mixedBidiBackspace after backspaceAcrossRuns and before deleteSelection', () => { + const { editor, callLog } = makeEditor(); + handleBackspace(editor); + + const acrossRunsIndex = callLog.indexOf('backspaceAcrossRuns'); + const mixedIndex = callLog.indexOf('mixedBidiBackspace'); + const deleteSelectionIndex = callLog.indexOf('deleteSelection'); + + expect(acrossRunsIndex).toBeGreaterThanOrEqual(0); + expect(mixedIndex).toBeGreaterThan(acrossRunsIndex); + expect(deleteSelectionIndex).toBeGreaterThan(mixedIndex); + }); + + it('tolerates missing mixedBidiBackspace command (extension not registered)', () => { + const { editor, callLog } = makeEditor(); + // Simulate the extension being absent: drop the mixedBidiBackspace key + // from the commands map. The chain uses `?? false` so it should keep walking. + delete editor.commands.first.mock.calls; // reset just in case + const editorWithoutCommand = { + ...editor, + commands: { + first: vi.fn((build) => { + const tr = editor.view.state.tr; + const commands = new Proxy( + {}, + { + get(_, name) { + if (name === 'mixedBidiBackspace') return undefined; + return () => { + callLog.push(name); + return false; + }; + }, + }, + ); + const fns = build({ commands, tr }); + for (const fn of fns) { + const result = fn(); + if (result) return result; + } + return false; + }), + }, + }; + + expect(() => handleBackspace(editorWithoutCommand)).not.toThrow(); + expect(callLog).toContain('backspaceAcrossRuns'); + expect(callLog).toContain('deleteSelection'); + expect(callLog).not.toContain('mixedBidiBackspace'); + }); +}); diff --git a/packages/super-editor/src/editors/v1/core/extensions/keymap.js b/packages/super-editor/src/editors/v1/core/extensions/keymap.js index 3cd3c93eb1..f094d10b7e 100644 --- a/packages/super-editor/src/editors/v1/core/extensions/keymap.js +++ b/packages/super-editor/src/editors/v1/core/extensions/keymap.js @@ -43,6 +43,7 @@ export const handleBackspace = (editor) => { () => commands.backspaceAtomBefore(), () => commands.backspaceNextToRun(), () => commands.backspaceAcrossRuns(), + () => commands.mixedBidiBackspace?.() ?? false, () => commands.deleteSelection(), () => commands.removeNumberingProperties(), () => commands.joinBackward(), diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts index 9ceea34d17..1fc9cb50a1 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts @@ -75,6 +75,7 @@ import { debugLog, updateSelectionDebugHud, type SelectionDebugHudState } from ' import { renderCellSelectionOverlay } from './selection/CellSelectionOverlay.js'; import { renderCaretOverlay, renderSelectionRects } from './selection/LocalSelectionOverlayRendering.js'; import { computeCaretLayoutRectGeometry as computeCaretLayoutRectGeometryFromHelper } from './selection/CaretGeometry.js'; +import { shouldUseNativeCaretFallback } from './selection/native-caret-fallback.js'; import { computeCaretRectFromVisibleTextOffset as computeCaretRectFromVisibleTextOffsetFromHelper, computeSelectionRectsFromVisibleTextOffsets as computeSelectionRectsFromVisibleTextOffsetsFromHelper, @@ -9785,9 +9786,16 @@ export class PresentationEditor extends EventEmitter { /** * Compute caret position, preferring DOM when available, falling back to geometry. + * + * SD-3170: the native-selection refinement inside computeCaretLayoutRectGeometry + * reads the browser's collapsed selection rect and prefers it over geometry. + * That's only sound when the requested `pos` is the local user's actual caret. + * Arbitrary-position queries (remote collaborator cursors, vertical-arrow + * navigation binary search) must not get the local rect substituted in. */ #computeCaretLayoutRect(pos: number): { pageIndex: number; x: number; y: number; height: number } | null { - const geometry = this.#computeCaretLayoutRectGeometry(pos, true); + const useNativeFallback = shouldUseNativeCaretFallback(this.editor?.state?.selection, pos); + const geometry = this.#computeCaretLayoutRectGeometry(pos, useNativeFallback); let dom: { pageIndex: number; x: number; y: number } | null = null; try { dom = this.#computeDomCaretPageLocal(pos); diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/selection/CaretGeometry.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/selection/CaretGeometry.ts index 353bee91e2..903ca2125d 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/selection/CaretGeometry.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/selection/CaretGeometry.ts @@ -123,6 +123,7 @@ export function computeCaretLayoutRectGeometry( includeDomFallback = true, ): CaretLayoutRect | null { if (!layout) return null; + const originalPos = pos; // Geometry-based calculation from layout engine let effectivePos = pos; @@ -214,30 +215,116 @@ export function computeCaretLayoutRectGeometry( const pageEl = getPageElementByIndex(painterHost ?? null, hit.pageIndex); const pageRect = pageEl?.getBoundingClientRect(); - // Find span containing this pos and measure actual DOM position + if (includeDomFallback && pageRect) { + const selection = pageEl?.ownerDocument?.getSelection(); + if (selection?.rangeCount && selection.isCollapsed) { + const nativeRange = selection.getRangeAt(0); + if (typeof nativeRange.getBoundingClientRect === 'function') { + const nativeRect = nativeRange.getBoundingClientRect(); + const inPageBounds = + Number.isFinite(nativeRect.left) && + Number.isFinite(nativeRect.top) && + Number.isFinite(nativeRect.height) && + nativeRect.height > 0 && + nativeRect.left >= pageRect.left - 2 && + nativeRect.left <= pageRect.right + 2 && + nativeRect.top >= pageRect.top - 2 && + nativeRect.top <= pageRect.bottom + 2; + const nativeX = (nativeRect.left - pageRect.left) / zoom; + const nativeY = (nativeRect.top - pageRect.top) / zoom; + const withinGeometrySanity = Math.abs(nativeX - result.x) <= 80; + if (inPageBounds && withinGeometrySanity) { + return { + pageIndex: hit.pageIndex, + x: nativeX, + y: nativeY, + height: line.lineHeight, + }; + } + } + } + } + + // Find span containing this pos and measure actual DOM position. + // Prefer a local line-scoped lookup first (fast path), then fall back to a + // bounded page-level probe near the target PM position. let domCaretX: number | null = null; let domCaretY: number | null = null; - const spanEls = pageEl?.querySelectorAll('span[data-pm-start][data-pm-end]'); - for (const spanEl of Array.from(spanEls ?? [])) { - const pmStart = Number((spanEl as HTMLElement).dataset.pmStart); - const pmEnd = Number((spanEl as HTMLElement).dataset.pmEnd); - if (effectivePos >= pmStart && effectivePos <= pmEnd && spanEl.firstChild?.nodeType === Node.TEXT_NODE) { - const textNode = spanEl.firstChild as Text; - const charIndex = Math.min(effectivePos - pmStart, textNode.length); - const rangeObj = document.createRange(); - rangeObj.setStart(textNode, charIndex); - rangeObj.setEnd(textNode, charIndex); - if (typeof rangeObj.getBoundingClientRect !== 'function') { - break; - } - const rangeRect = rangeObj.getBoundingClientRect(); - if (pageRect) { - domCaretX = (rangeRect.left - pageRect.left) / zoom; - domCaretY = (rangeRect.top - pageRect.top) / zoom; + const spanCandidates: HTMLElement[] = []; + const pushUnique = (el: HTMLElement) => { + if (!spanCandidates.includes(el)) spanCandidates.push(el); + }; + const collectSpans = (root: ParentNode | null | undefined) => { + if (!root) return; + const spans = root.querySelectorAll('span[data-pm-start][data-pm-end]'); + for (const span of Array.from(spans)) { + if (span instanceof HTMLElement) { + pushUnique(span); } + } + }; + + const lineEls = pageEl?.querySelectorAll('.superdoc-line[data-pm-start][data-pm-end]'); + let localLineEl: HTMLElement | null = null; + for (const lineEl of Array.from(lineEls ?? [])) { + if (!(lineEl instanceof HTMLElement)) continue; + const lineStart = Number(lineEl.dataset.pmStart); + const lineEnd = Number(lineEl.dataset.pmEnd); + if (!Number.isFinite(lineStart) || !Number.isFinite(lineEnd)) continue; + if (effectivePos >= lineStart && effectivePos <= lineEnd) { + localLineEl = lineEl; break; } } + collectSpans(localLineEl); + + if (spanCandidates.length === 0) { + const MAX_PM_DISTANCE = 8; + const pageSpans = pageEl?.querySelectorAll('span[data-pm-start][data-pm-end]'); + for (const span of Array.from(pageSpans ?? [])) { + if (!(span instanceof HTMLElement)) continue; + const pmStart = Number(span.dataset.pmStart); + const pmEnd = Number(span.dataset.pmEnd); + if (!Number.isFinite(pmStart) || !Number.isFinite(pmEnd)) continue; + if (effectivePos >= pmStart && effectivePos <= pmEnd) { + pushUnique(span); + continue; + } + if (Math.abs(pmStart - effectivePos) <= MAX_PM_DISTANCE || Math.abs(pmEnd - effectivePos) <= MAX_PM_DISTANCE) { + pushUnique(span); + } + } + } + + const domProbePositions = Array.from(new Set([originalPos, effectivePos, originalPos - 1, originalPos + 1])).filter( + (candidate) => candidate >= 0, + ); + + let resolved = false; + for (const probePos of domProbePositions) { + for (const spanEl of spanCandidates) { + const pmStart = Number((spanEl as HTMLElement).dataset.pmStart); + const pmEnd = Number((spanEl as HTMLElement).dataset.pmEnd); + if (probePos >= pmStart && probePos <= pmEnd && spanEl.firstChild?.nodeType === Node.TEXT_NODE) { + const textNode = spanEl.firstChild as Text; + const charIndex = Math.min(probePos - pmStart, textNode.length); + const rangeObj = document.createRange(); + rangeObj.setStart(textNode, charIndex); + rangeObj.setEnd(textNode, charIndex); + if (typeof rangeObj.getBoundingClientRect !== 'function') { + continue; + } + const rangeRect = rangeObj.getBoundingClientRect(); + if (pageRect) { + domCaretX = (rangeRect.left - pageRect.left) / zoom; + domCaretY = (rangeRect.top - pageRect.top) / zoom; + resolved = true; + break; + } + } + } + if (resolved) break; + } // If we found a DOM caret position, prefer it to avoid residual drift if (includeDomFallback && domCaretX != null && domCaretY != null) { diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/selection/native-caret-fallback.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/selection/native-caret-fallback.test.ts new file mode 100644 index 0000000000..a3e13d3827 --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/selection/native-caret-fallback.test.ts @@ -0,0 +1,38 @@ +import { describe, it, expect } from 'vitest'; +import { shouldUseNativeCaretFallback } from './native-caret-fallback.js'; + +describe('shouldUseNativeCaretFallback', () => { + it('returns false when selection is null', () => { + expect(shouldUseNativeCaretFallback(null, 5)).toBe(false); + }); + + it('returns false when selection is undefined', () => { + expect(shouldUseNativeCaretFallback(undefined, 5)).toBe(false); + }); + + it('returns false when selection is not collapsed', () => { + // Even if the requested pos equals one of the selection endpoints, a range + // selection means the user has multiple positions selected. The native + // refinement should not fire. + expect(shouldUseNativeCaretFallback({ empty: false, head: 5 }, 5)).toBe(false); + }); + + it('returns false when requested pos differs from selection head', () => { + // SD-3170: arbitrary-position queries (remote cursors, vertical-nav + // binary-search probes) must not get the native-selection rect. + expect(shouldUseNativeCaretFallback({ empty: true, head: 5 }, 6)).toBe(false); + expect(shouldUseNativeCaretFallback({ empty: true, head: 10 }, 4)).toBe(false); + }); + + it('returns true only for the local collapsed caret', () => { + expect(shouldUseNativeCaretFallback({ empty: true, head: 5 }, 5)).toBe(true); + expect(shouldUseNativeCaretFallback({ empty: true, head: 0 }, 0)).toBe(true); + }); + + it('treats head 0 distinctly from no selection', () => { + // Boundary check: head: 0 with pos: 0 is a valid local caret. + expect(shouldUseNativeCaretFallback({ empty: true, head: 0 }, 0)).toBe(true); + // pos -1 (impossible PM position) is never the local caret. + expect(shouldUseNativeCaretFallback({ empty: true, head: 0 }, -1)).toBe(false); + }); +}); diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/selection/native-caret-fallback.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/selection/native-caret-fallback.ts new file mode 100644 index 0000000000..448873a356 --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/selection/native-caret-fallback.ts @@ -0,0 +1,42 @@ +/** + * Pure gate for the native-selection refinement in + * {@link computeCaretLayoutRectGeometry}. + * + * The native-selection refinement (added in SD-2933) reads the browser's + * collapsed selection rect and prefers it over the geometry-computed caret + * when within a sanity bound. That refinement is only sound when the + * requested `pos` IS the local user's actual caret. + * + * Two callers in PresentationEditor ask {@link computeCaretLayoutRect} about + * positions that are NOT the local caret: + * + * - {@link RemoteCursorManager} queries each remote collaborator's head. + * - Vertical-arrow navigation binary-searches candidate positions on the + * next line to pick the one closest to the previous horizontal X. + * + * Without this gate, the native-selection refinement would substitute the + * LOCAL caret's rect for those queries when they happen to fall within the + * sanity window. Remote cursors would render at the local caret's position; + * vertical navigation would converge to the local caret. + * + * SD-3170 tracks this. + * + * @param selection - The current ProseMirror selection (or null/undefined + * when no editor is attached). + * @param pos - The position the caller is asking about. + * @returns true only when the selection is collapsed AND its caret head is + * exactly the requested position. + */ +export type CaretFallbackSelection = { + empty: boolean; + head: number; +}; + +export const shouldUseNativeCaretFallback = ( + selection: CaretFallbackSelection | null | undefined, + pos: number, +): boolean => { + if (!selection) return false; + if (!selection.empty) return false; + return selection.head === pos; +}; diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/CaretGeometry.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/CaretGeometry.test.ts index 74194d9c28..1437799495 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/CaretGeometry.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/CaretGeometry.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, beforeEach } from 'vitest'; +import { describe, expect, it, beforeEach, vi } from 'vitest'; import type { FlowBlock, Layout, Line, Measure, ParaFragment } from '@superdoc/contracts'; import { computeCaretLayoutRectGeometry, type ComputeCaretLayoutRectGeometryDeps } from '../selection/CaretGeometry.js'; @@ -329,6 +329,184 @@ describe('CaretGeometry', () => { expect(result?.pageIndex).toBe(0); }); + it('uses native collapsed selection rect when in page bounds', () => { + const block = createMockParagraphBlock('1-para', 1, 12); + const line = createMockLine(1, 12, 16); + const measure = createMockParagraphMeasure([line]); + const fragment = createMockParaFragment('1-para', 10, 10, 200, 16, 0, 1, 1, 12); + const layout = createMockLayout(fragment); + + const deps: ComputeCaretLayoutRectGeometryDeps = { + layout, + blocks: [block], + measures: [measure], + painterHost: mockDom.painterHost, + viewportHost: mockDom.viewportHost, + visibleHost: mockDom.visibleHost, + zoom: 1, + }; + + const pageEl = mockDom.painterHost.querySelector('.superdoc-page') as HTMLElement; + expect(pageEl).toBeTruthy(); + + vi.spyOn(pageEl, 'getBoundingClientRect').mockReturnValue({ + left: 100, + top: 200, + right: 500, + bottom: 700, + width: 400, + height: 500, + x: 100, + y: 200, + toJSON: () => ({}), + } as DOMRect); + + const nativeRange = { + getBoundingClientRect: () => + ({ + left: 150, + top: 230, + right: 150, + bottom: 246, + width: 0, + height: 16, + x: 150, + y: 230, + toJSON: () => ({}), + }) as DOMRect, + }; + + vi.spyOn(pageEl.ownerDocument, 'getSelection').mockReturnValue({ + rangeCount: 1, + isCollapsed: true, + getRangeAt: () => nativeRange as Range, + } as Selection); + + const result = computeCaretLayoutRectGeometry(deps, 5, true); + expect(result).not.toBe(null); + expect(result?.x).toBeCloseTo(50, 3); + expect(result?.y).toBeCloseTo(30, 3); + }); + + it('does not use native selection rect when includeDomFallback is false', () => { + const block = createMockParagraphBlock('1-para', 1, 12); + const line = createMockLine(1, 12, 16); + const measure = createMockParagraphMeasure([line]); + const fragment = createMockParaFragment('1-para', 10, 10, 200, 16, 0, 1, 1, 12); + const layout = createMockLayout(fragment); + + const deps: ComputeCaretLayoutRectGeometryDeps = { + layout, + blocks: [block], + measures: [measure], + painterHost: mockDom.painterHost, + viewportHost: mockDom.viewportHost, + visibleHost: mockDom.visibleHost, + zoom: 1, + }; + + const pageEl = mockDom.painterHost.querySelector('.superdoc-page') as HTMLElement; + expect(pageEl).toBeTruthy(); + + vi.spyOn(pageEl, 'getBoundingClientRect').mockReturnValue({ + left: 100, + top: 200, + right: 500, + bottom: 700, + width: 400, + height: 500, + x: 100, + y: 200, + toJSON: () => ({}), + } as DOMRect); + + const nativeRange = { + getBoundingClientRect: () => + ({ + left: 150, + top: 230, + right: 150, + bottom: 246, + width: 0, + height: 16, + x: 150, + y: 230, + toJSON: () => ({}), + }) as DOMRect, + }; + + vi.spyOn(pageEl.ownerDocument, 'getSelection').mockReturnValue({ + rangeCount: 1, + isCollapsed: true, + getRangeAt: () => nativeRange as Range, + } as Selection); + + const result = computeCaretLayoutRectGeometry(deps, 5, false); + expect(result).not.toBe(null); + // includeDomFallback=false should keep geometry-path output, not native DOM selection coordinates. + expect(result?.x).not.toBeCloseTo(50, 3); + expect(result?.y).not.toBeCloseTo(30, 3); + }); + + it('ignores in-bounds native selection rect when it is far from geometry result', () => { + const block = createMockParagraphBlock('1-para', 1, 12); + const line = createMockLine(1, 12, 16); + const measure = createMockParagraphMeasure([line]); + const fragment = createMockParaFragment('1-para', 10, 10, 200, 16, 0, 1, 1, 12); + const layout = createMockLayout(fragment); + + const deps: ComputeCaretLayoutRectGeometryDeps = { + layout, + blocks: [block], + measures: [measure], + painterHost: mockDom.painterHost, + viewportHost: mockDom.viewportHost, + visibleHost: mockDom.visibleHost, + zoom: 1, + }; + + const pageEl = mockDom.painterHost.querySelector('.superdoc-page') as HTMLElement; + expect(pageEl).toBeTruthy(); + + vi.spyOn(pageEl, 'getBoundingClientRect').mockReturnValue({ + left: 100, + top: 200, + right: 500, + bottom: 700, + width: 400, + height: 500, + x: 100, + y: 200, + toJSON: () => ({}), + } as DOMRect); + + const nativeRange = { + getBoundingClientRect: () => + ({ + left: 480, + top: 230, + right: 480, + bottom: 246, + width: 0, + height: 16, + x: 480, + y: 230, + toJSON: () => ({}), + }) as DOMRect, + }; + + vi.spyOn(pageEl.ownerDocument, 'getSelection').mockReturnValue({ + rangeCount: 1, + isCollapsed: true, + getRangeAt: () => nativeRange as Range, + } as Selection); + + const result = computeCaretLayoutRectGeometry(deps, 5, true); + expect(result).not.toBe(null); + // Native rect is in page bounds but implausibly far from geometry; should be ignored. + expect(result?.x).not.toBeCloseTo(380, 3); + }); + it('handles virtualized content (no DOM element available)', () => { const block = createMockParagraphBlock('1-para', 1, 12); const line = createMockLine(1, 12, 16); diff --git a/packages/super-editor/src/editors/v1/extensions/index.js b/packages/super-editor/src/editors/v1/extensions/index.js index 5940506ddc..acf4a1e655 100644 --- a/packages/super-editor/src/editors/v1/extensions/index.js +++ b/packages/super-editor/src/editors/v1/extensions/index.js @@ -26,7 +26,7 @@ import { Text } from './text/index.js'; import { Run } from './run/index.js'; import { Paragraph } from './paragraph/index.js'; import { Heading } from './heading/index.js'; -import { CommentRangeStart, CommentRangeEnd, CommentReference, CommentsMark } from './comment/index.js'; +import { CommentRangeStart, CommentRangeEnd, CommentReference, CommentsMark, CommentsPlugin } from './comment/index.js'; import { FootnoteReference } from './footnote/index.js'; import { EndnoteReference } from './endnote/index.js'; import { TabNode } from './tab/index.js'; @@ -72,11 +72,10 @@ import { Underline } from './underline/index.js'; import { Highlight } from './highlight/index.js'; import { Strike } from './strike/index.js'; import { Link } from './link/index.js'; -import { TrackInsert, TrackDelete, TrackFormat, TrackChanges } from './track-changes/index.js'; +import { TrackInsert, TrackDelete, TrackFormat, TrackChanges, trackChangesHelpers } from './track-changes/index.js'; import { TextTransform } from './text-transform/index.js'; // Plugins -import { CommentsPlugin } from './comment/index.js'; import { Placeholder } from './placeholder/index.js'; import { PopoverPlugin } from './popover-plugin/index.js'; import { LinkedStyles } from './linked-styles/linked-styles.js'; @@ -86,13 +85,13 @@ import { CustomSelection } from './custom-selection/index.js'; import { PermissionRanges } from './permission-ranges/index.js'; import { Protection } from './protection/index.js'; import { VerticalNavigation } from './vertical-navigation/index.js'; +import { MixedBidiBackspace } from './mixed-bidi-backspace/index.js'; // Permissions import { PermStart, PermStartBlock } from './perm-start/index.js'; import { PermEnd, PermEndBlock } from './perm-end/index.js'; // Helpers -import { trackChangesHelpers } from './track-changes/index.js'; import { Diffing } from './diffing/index.js'; const getRichTextExtensions = () => { @@ -133,6 +132,7 @@ const getRichTextExtensions = () => { Image, NodeResizer, CustomSelection, + MixedBidiBackspace, MathInline, MathBlock, PassthroughInline, @@ -234,6 +234,7 @@ const getStarterExtensions = () => { PermissionRanges, Protection, VerticalNavigation, + MixedBidiBackspace, MathInline, MathBlock, PassthroughInline, @@ -323,6 +324,7 @@ export { PassthroughBlock, PermissionRanges, Protection, + MixedBidiBackspace, CrossReference, SequenceField, DocumentStatField, diff --git a/packages/super-editor/src/editors/v1/extensions/mixed-bidi-backspace/index.js b/packages/super-editor/src/editors/v1/extensions/mixed-bidi-backspace/index.js new file mode 100644 index 0000000000..6918605f83 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/mixed-bidi-backspace/index.js @@ -0,0 +1 @@ +export * from './mixed-bidi-backspace.js'; diff --git a/packages/super-editor/src/editors/v1/extensions/mixed-bidi-backspace/mixed-bidi-backspace.js b/packages/super-editor/src/editors/v1/extensions/mixed-bidi-backspace/mixed-bidi-backspace.js new file mode 100644 index 0000000000..1a8b0f5fe2 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/mixed-bidi-backspace/mixed-bidi-backspace.js @@ -0,0 +1,194 @@ +// @ts-nocheck +import { Extension } from '@core/Extension.js'; + +const STRONG_RTL_CHAR_RE = /[\u0590-\u08FF]/; +const STRONG_LTR_CHAR_RE = /[A-Za-z\u00C0-\u024F]/; + +const isStrongRtl = (char) => STRONG_RTL_CHAR_RE.test(char); +const isStrongLtr = (char) => STRONG_LTR_CHAR_RE.test(char); + +const hasMixedDirectionBoundary = (leftChar, rightChar) => + (isStrongRtl(leftChar) && isStrongLtr(rightChar)) || (isStrongLtr(leftChar) && isStrongRtl(rightChar)); + +const resolveCaretPoint = (doc, range) => { + const rect = range.getBoundingClientRect(); + if (rect && Number.isFinite(rect.left) && Number.isFinite(rect.top)) { + // Collapsed ranges may transiently report a zero rect during render lag. + // In that case, fail-open instead of falling back to the parent box, + // which would produce an imprecise X and potentially a wrong boundary. + if (rect.width === 0 && rect.height === 0) { + return null; + } + const midY = rect.height > 0 ? rect.top + rect.height / 2 : rect.top; + return { x: rect.left, y: midY }; + } + + const node = + range.startContainer?.nodeType === Node.TEXT_NODE ? range.startContainer.parentElement : range.startContainer; + if (!node || !(node instanceof HTMLElement)) return null; + const fallbackRect = node.getBoundingClientRect(); + if (!fallbackRect) return null; + return { x: fallbackRect.left, y: fallbackRect.top + fallbackRect.height / 2 }; +}; + +const resolveLineElement = (doc, point) => { + const hit = doc.elementsFromPoint(point.x, point.y); + return hit.find((el) => (el instanceof HTMLElement ? el.classList.contains('superdoc-line') : false)); +}; + +const collectVisualChars = (lineEl, view, targetX = null) => { + const doc = lineEl.ownerDocument; + const nodeFilter = doc.defaultView?.NodeFilter; + if (!nodeFilter) return []; + const chars = []; + const walker = doc.createTreeWalker(lineEl, nodeFilter.SHOW_TEXT); + const RANGE_WINDOW_PX = 96; + const hasTargetX = Number.isFinite(targetX); + let node = walker.nextNode(); + + while (node) { + const textNode = /** @type {Text} */ (node); + const text = textNode.textContent ?? ''; + + for (let i = 0; i < text.length; i += 1) { + const char = text[i]; + if (!char || /\s/.test(char)) continue; + + let pmStart; + let pmEnd; + try { + pmStart = view.posAtDOM(textNode, i); + pmEnd = view.posAtDOM(textNode, i + 1); + } catch { + continue; + } + if (!Number.isFinite(pmStart) || !Number.isFinite(pmEnd) || pmEnd <= pmStart) continue; + + const range = doc.createRange(); + range.setStart(textNode, i); + range.setEnd(textNode, i + 1); + const rect = range.getBoundingClientRect(); + if (rect.width <= 0 || rect.height <= 0) continue; + if (hasTargetX && rect.right < targetX - RANGE_WINDOW_PX) continue; + if (hasTargetX && rect.left > targetX + RANGE_WINDOW_PX) continue; + + chars.push({ + char, + pmStart, + pmEnd, + centerX: rect.left + rect.width / 2, + centerY: rect.top + rect.height / 2, + }); + } + + node = walker.nextNode(); + } + + return chars; +}; + +const resolveBoundaryChars = (chars, caretPoint) => { + if (chars.length === 0) return null; + const sameBand = chars.filter((c) => Math.abs(c.centerY - caretPoint.y) <= 8); + const band = sameBand.length > 0 ? sameBand : chars; + band.sort((a, b) => a.centerX - b.centerX); + + let left = null; + let right = null; + for (const c of band) { + if (c.centerX < caretPoint.x) { + left = c; + continue; + } + right = c; + break; + } + + if (!left || !right) return null; + return { left, right }; +}; + +/** + * Compute the visual-left delete range at a mixed-bidi RTL/LTR boundary. + * + * Returns the PM-position range to delete, or `null` when the caret is not + * on a mixed-direction boundary. Pure: does not mutate state or dispatch. + * + * @param {{ state: any, view: any }} args + * @returns {{ from: number, to: number } | null} + */ +const resolveMixedBidiBackspaceRange = ({ state, view }) => { + const { selection } = state; + if (!selection?.empty) return null; + + const doc = view?.dom?.ownerDocument; + const nativeSelection = doc?.getSelection?.(); + if (!nativeSelection || nativeSelection.rangeCount === 0) return null; + + const range = nativeSelection.getRangeAt(0); + if (!range.collapsed) return null; + + const caretPoint = resolveCaretPoint(doc, range); + if (!caretPoint) return null; + + const lineEl = resolveLineElement(doc, caretPoint); + if (!lineEl) return null; + const lineText = lineEl.textContent ?? ''; + const hasRtl = STRONG_RTL_CHAR_RE.test(lineText); + const hasLtr = STRONG_LTR_CHAR_RE.test(lineText); + if (!hasRtl || !hasLtr) return null; + + let chars = collectVisualChars(lineEl, view, caretPoint.x); + if (chars.length < 2) { + // Fallback to a full scan for correctness when the local window is too narrow. + chars = collectVisualChars(lineEl, view, null); + } + const boundary = resolveBoundaryChars(chars, caretPoint); + if (!boundary) return null; + + if (!hasMixedDirectionBoundary(boundary.left.char, boundary.right.char)) return null; + if (selection.from !== boundary.right.pmStart && selection.from !== boundary.left.pmEnd) return null; + + return { from: boundary.left.pmStart, to: boundary.left.pmEnd }; +}; + +/** + * Mixed-bidi Backspace command. Slotted into the keymap Backspace chain so it + * inherits the chain's history boundary, inputType: deleteContentBackward meta, + * track-changes wrapping, protected-range guards, and SDT handling, instead of + * dispatching its own transaction. + * + * Returns true (chain stops) only when the caret is at a strong-RTL/strong-LTR + * boundary and the visual-left character is targeted for deletion. Otherwise + * returns false so the chain falls through to deleteSelection / joinBackward. + * + * @returns {import('@core/commands/types/index.js').Command} + */ +export const mixedBidiBackspace = + () => + ({ state, view, tr, dispatch }) => { + const range = resolveMixedBidiBackspaceRange({ state, view }); + if (!range) return false; + + if (dispatch) { + tr.delete(range.from, range.to); + tr.scrollIntoView(); + } + return true; + }; + +export const MixedBidiBackspace = Extension.create({ + name: 'mixedBidiBackspace', + + addCommands() { + return { + mixedBidiBackspace, + }; + }, +}); + +export const __TEST_ONLY__ = { + resolveCaretPoint, + hasMixedDirectionBoundary, + resolveMixedBidiBackspaceRange, +}; diff --git a/packages/super-editor/src/editors/v1/extensions/mixed-bidi-backspace/mixed-bidi-backspace.test.js b/packages/super-editor/src/editors/v1/extensions/mixed-bidi-backspace/mixed-bidi-backspace.test.js new file mode 100644 index 0000000000..2b6497300a --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/mixed-bidi-backspace/mixed-bidi-backspace.test.js @@ -0,0 +1,178 @@ +import { describe, expect, it, vi } from 'vitest'; +import { __TEST_ONLY__, mixedBidiBackspace } from './mixed-bidi-backspace.js'; + +const makeRect = (left, top = 10, width = 8, height = 12) => ({ + left, + top, + width, + height, +}); + +const setupContext = ({ text, charLefts, caretRect, selectionFrom, pmBase = 10 }) => { + const doc = document.implementation.createHTMLDocument('mixed-bidi-backspace'); + Object.defineProperty(doc, 'defaultView', { + value: { NodeFilter: { SHOW_TEXT: 4 } }, + configurable: true, + }); + const lineEl = doc.createElement('div'); + lineEl.className = 'superdoc-line'; + const textNode = doc.createTextNode(text); + lineEl.appendChild(textNode); + doc.body.appendChild(lineEl); + + doc.elementsFromPoint = vi.fn(() => [lineEl]); + + doc.createRange = vi.fn(() => { + const range = { + _node: null, + _start: 0, + _end: 0, + setStart(node, offset) { + this._node = node; + this._start = offset; + }, + setEnd(node, offset) { + this._node = node; + this._end = offset; + }, + getBoundingClientRect() { + if (this._node !== textNode) return makeRect(0, 0, 0, 0); + const chIndex = this._start; + const left = charLefts[chIndex]; + if (typeof left !== 'number') return makeRect(0, 0, 0, 0); + return makeRect(left); + }, + }; + return range; + }); + + const nativeRange = { + collapsed: true, + getBoundingClientRect: () => caretRect, + startContainer: textNode, + }; + + doc.getSelection = vi.fn(() => ({ + rangeCount: 1, + getRangeAt: () => nativeRange, + })); + + const dispatch = vi.fn(); + const tr = { + delete: vi.fn(() => tr), + scrollIntoView: vi.fn(() => tr), + }; + const view = { + dom: { ownerDocument: doc }, + posAtDOM: vi.fn((node, offset) => { + if (node !== textNode) throw new Error('unexpected node'); + return pmBase + offset; + }), + }; + const state = { + selection: { empty: true, from: selectionFrom }, + }; + + return { state, view, tr, dispatch }; +}; + +describe('mixedBidiBackspace (chain command)', () => { + it('returns true and mutates the chain tr on RTL+LTR boundary', () => { + const { state, view, tr, dispatch } = setupContext({ + text: 'אA', + charLefts: [10, 20], + caretRect: makeRect(20, 10, 1, 12), + selectionFrom: 11, + pmBase: 10, + }); + + const handled = mixedBidiBackspace()({ state, view, tr, dispatch }); + expect(handled).toBe(true); + expect(tr.delete).toHaveBeenCalledWith(10, 11); + expect(tr.scrollIntoView).toHaveBeenCalledTimes(1); + expect(dispatch).not.toHaveBeenCalled(); // chain owns dispatch + }); + + it('returns true and mutates the chain tr on LTR+RTL boundary', () => { + const { state, view, tr } = setupContext({ + text: 'Aא', + charLefts: [10, 20], + caretRect: makeRect(20, 10, 1, 12), + selectionFrom: 11, + pmBase: 10, + }); + + const handled = mixedBidiBackspace()({ state, view, tr, dispatch: vi.fn() }); + expect(handled).toBe(true); + expect(tr.delete).toHaveBeenCalledWith(10, 11); + }); + + it('returns false without mutating tr on pure LTR (chain falls through)', () => { + const { state, view, tr, dispatch } = setupContext({ + text: 'AB', + charLefts: [10, 20], + caretRect: makeRect(20, 10, 1, 12), + selectionFrom: 11, + pmBase: 10, + }); + + const handled = mixedBidiBackspace()({ state, view, tr, dispatch }); + expect(handled).toBe(false); + expect(tr.delete).not.toHaveBeenCalled(); + expect(tr.scrollIntoView).not.toHaveBeenCalled(); + expect(view.posAtDOM).not.toHaveBeenCalled(); // early-out skips DOM scan + }); + + it('returns false on non-empty selection (chain falls through)', () => { + const { state, view, tr } = setupContext({ + text: 'אA', + charLefts: [10, 20], + caretRect: makeRect(20, 10, 1, 12), + selectionFrom: 11, + pmBase: 10, + }); + state.selection.empty = false; + + const handled = mixedBidiBackspace()({ state, view, tr, dispatch: vi.fn() }); + expect(handled).toBe(false); + expect(tr.delete).not.toHaveBeenCalled(); + }); + + // SD-2933 / SD-2767: the chain's `dispatch` parameter is undefined during dry-run + // probing. The command must still return true without mutating tr in that mode. + // chain.first uses this to detect whether a command CAN handle the operation. + it('does not mutate tr when dispatch is undefined (chain dry-run probe)', () => { + const { state, view, tr } = setupContext({ + text: 'אA', + charLefts: [10, 20], + caretRect: makeRect(20, 10, 1, 12), + selectionFrom: 11, + pmBase: 10, + }); + + const handled = mixedBidiBackspace()({ state, view, tr, dispatch: undefined }); + expect(handled).toBe(true); + expect(tr.delete).not.toHaveBeenCalled(); + expect(tr.scrollIntoView).not.toHaveBeenCalled(); + }); + + it('returns false when caret is not at the boundary (e.g. mid-word)', () => { + const { state, view, tr } = setupContext({ + text: 'אAB', + charLefts: [10, 20, 30], + caretRect: makeRect(30, 10, 1, 12), + selectionFrom: 12, // past the boundary + pmBase: 10, + }); + + const handled = mixedBidiBackspace()({ state, view, tr, dispatch: vi.fn() }); + expect(handled).toBe(false); + }); + + it('exposes hasMixedDirectionBoundary helper for direct testing', () => { + expect(__TEST_ONLY__.hasMixedDirectionBoundary('א', 'A')).toBe(true); + expect(__TEST_ONLY__.hasMixedDirectionBoundary('A', 'א')).toBe(true); + expect(__TEST_ONLY__.hasMixedDirectionBoundary('A', 'B')).toBe(false); + expect(__TEST_ONLY__.hasMixedDirectionBoundary('א', 'ש')).toBe(false); + }); +}); diff --git a/tests/behavior/tests/formatting/rtl-dates-word-parity.spec.ts b/tests/behavior/tests/formatting/rtl-dates-word-parity.spec.ts index fb0dc361b5..c97ff9446a 100644 --- a/tests/behavior/tests/formatting/rtl-dates-word-parity.spec.ts +++ b/tests/behavior/tests/formatting/rtl-dates-word-parity.spec.ts @@ -14,14 +14,19 @@ test('rtl dates render in the same visual order as Word', async ({ superdoc }) = const headerText = await headerRuns.last().evaluate((el) => el.textContent ?? ''); expect(headerText.includes('\u200F/\u200F')).toBe(true); - const bodyDateRuns = superdoc.page - .locator('.superdoc-page .superdoc-fragment .superdoc-line span') - .filter({ hasText: '-03-23' }); + const bodyFragments = superdoc.page.locator('.superdoc-page > .superdoc-fragment'); + const bodyDateRuns = bodyFragments.locator('.superdoc-line span').filter({ hasText: '-03-23' }); await expect(bodyDateRuns.first()).toHaveAttribute('dir', 'ltr'); - const bodyRtlNumericRun = superdoc.page - .locator('.superdoc-page .superdoc-fragment .superdoc-line span[dir="rtl"]') - .filter({ hasText: '2026' }) + // SD-2933: rtl-tagged digit-only runs (e.g. a standalone "2026") fall into the + // latin-only branch of resolveRunDirectionAttribute and intentionally do NOT + // receive a per-run dir attribute. The paragraph direction carries them via + // UBA, matching Word's empirical rendering. Per ECMA-376 §17.3.2.30, w:rtl on + // strongly-LTR text is unspecified behavior. + const bodyNumericRun = bodyFragments + .locator('.superdoc-line span') + .filter({ hasText: /^2026$/ }) .first(); - await expect(bodyRtlNumericRun).toBeVisible(); + await expect(bodyNumericRun).toBeVisible(); + await expect(bodyNumericRun).not.toHaveAttribute('dir', 'rtl'); }); diff --git a/tests/behavior/tests/selection/fixtures/mixed-bidi-2.docx b/tests/behavior/tests/selection/fixtures/mixed-bidi-2.docx new file mode 100644 index 0000000000..96b367db35 Binary files /dev/null and b/tests/behavior/tests/selection/fixtures/mixed-bidi-2.docx differ diff --git a/tests/behavior/tests/selection/rtl-arrow-key-movement.spec.ts b/tests/behavior/tests/selection/rtl-arrow-key-movement.spec.ts index 164e12995a..817979f10b 100644 --- a/tests/behavior/tests/selection/rtl-arrow-key-movement.spec.ts +++ b/tests/behavior/tests/selection/rtl-arrow-key-movement.spec.ts @@ -5,6 +5,7 @@ import { test, expect } from '../../fixtures/superdoc.js'; const __dirname = path.dirname(fileURLToPath(import.meta.url)); const DOC_PATH = path.resolve(__dirname, 'fixtures/rtl-mixed-bidi.docx'); +const MIXED_BIDI_DOC_PATH = path.resolve(__dirname, 'fixtures/mixed-bidi-2.docx'); test.skip(!fs.existsSync(DOC_PATH), 'RTL fixture not available'); @@ -295,4 +296,214 @@ test.describe('RTL arrow key cursor movement (SD-2390)', () => { const direction = await insertedLine.evaluate((el) => getComputedStyle(el).direction); expect(direction).toBe('rtl'); }); + + test('ArrowLeft/ArrowRight at mixed-bidi boundary moves one visual step', async ({ superdoc }) => { + await superdoc.loadDocument(MIXED_BIDI_DOC_PATH); + await superdoc.waitForStable(); + + const boundaryPoint = await superdoc.page.evaluate(() => { + const lines = Array.from(document.querySelectorAll('.superdoc-line')); + for (const line of lines) { + const spans = Array.from(line.querySelectorAll('span[data-pm-start][data-pm-end]')) as HTMLElement[]; + const rtlSpan = spans.find((span) => /[\u0590-\u05FF\u0600-\u06FF]/.test(span.textContent ?? '')); + const ltrSpan = spans.find((span) => /[A-Za-z]/.test(span.textContent ?? '')); + if (!rtlSpan || !ltrSpan) continue; + const ltrRect = ltrSpan.getBoundingClientRect(); + return { x: ltrRect.left + 2, y: ltrRect.top + ltrRect.height / 2 }; + } + return null; + }); + + expect(boundaryPoint).not.toBeNull(); + if (!boundaryPoint) return; + + await superdoc.page.mouse.click(boundaryPoint.x, boundaryPoint.y); + await superdoc.waitForStable(); + + const before = await superdoc.getSelection(); + const xBefore = await superdoc.page.evaluate((pos) => { + const pe = (window as any).superdoc?.activeEditor?.presentationEditor; + return pe?.computeCaretLayoutRect(pos)?.x; + }, before.from); + + await superdoc.page.keyboard.press('ArrowRight'); + await superdoc.waitForStable(); + + const afterRight = await superdoc.getSelection(); + const xAfterRight = await superdoc.page.evaluate((pos) => { + const pe = (window as any).superdoc?.activeEditor?.presentationEditor; + return pe?.computeCaretLayoutRect(pos)?.x; + }, afterRight.from); + + await superdoc.page.keyboard.press('ArrowLeft'); + await superdoc.waitForStable(); + + const afterLeft = await superdoc.getSelection(); + const xAfterLeft = await superdoc.page.evaluate((pos) => { + const pe = (window as any).superdoc?.activeEditor?.presentationEditor; + return pe?.computeCaretLayoutRect(pos)?.x; + }, afterLeft.from); + + expect(afterRight.from).not.toBe(before.from); + expect(Math.abs(xAfterRight - xBefore)).toBeGreaterThan(0.1); + expect(afterLeft.from).toBe(before.from); + expect(Math.abs(xAfterLeft - xBefore)).toBeLessThanOrEqual(1.0); + }); + + test('Shift+Arrow across mixed-bidi boundary keeps split non-overlapping selection rects', async ({ + superdoc, + browserName, + }) => { + test.fixme( + browserName === 'firefox', + 'Firefox paints mixed-bidi boundary selection with a different overlay geometry (no stable split-rect pattern).', + ); + + await superdoc.loadDocument(MIXED_BIDI_DOC_PATH); + await superdoc.waitForStable(); + + const boundaryPoint = await superdoc.page.evaluate(() => { + const lines = Array.from(document.querySelectorAll('.superdoc-line')); + for (const line of lines) { + const spans = Array.from(line.querySelectorAll('span[data-pm-start][data-pm-end]')) as HTMLElement[]; + const rtlSpan = spans.find((span) => /[\u0590-\u05FF\u0600-\u06FF]/.test(span.textContent ?? '')); + const ltrSpan = spans.find((span) => /[A-Za-z]/.test(span.textContent ?? '')); + if (!rtlSpan || !ltrSpan) continue; + const ltrRect = ltrSpan.getBoundingClientRect(); + return { x: ltrRect.left + 2, y: ltrRect.top + ltrRect.height / 2 }; + } + return null; + }); + + expect(boundaryPoint).not.toBeNull(); + if (!boundaryPoint) return; + + await superdoc.page.mouse.click(boundaryPoint.x, boundaryPoint.y); + await superdoc.waitForStable(); + + const evaluateSplitRects = async () => + superdoc.page.evaluate(() => { + const layer = document.querySelector('.presentation-editor__selection-layer--local'); + if (!layer) return { hasSplit: false, rectCount: 0 }; + + const rects = Array.from(layer.children) + .map((child) => (child as HTMLElement).getBoundingClientRect()) + .filter((r) => r.width > 0 && r.height > 0) + .map((r) => ({ x: r.x, y: r.y, width: r.width, height: r.height })); + + const Y_SAME_LINE_THRESHOLD_PX = 3; + for (let i = 0; i < rects.length; i++) { + for (let j = i + 1; j < rects.length; j++) { + const a = rects[i]!; + const b = rects[j]!; + if (Math.abs(a.y - b.y) > Y_SAME_LINE_THRESHOLD_PX) continue; + const aRight = a.x + a.width; + const bRight = b.x + b.width; + const overlap = Math.max(0, Math.min(aRight, bRight) - Math.max(a.x, b.x)); + if (overlap === 0) { + return { hasSplit: true, rectCount: rects.length }; + } + } + } + + return { hasSplit: false, rectCount: rects.length }; + }); + + await superdoc.page.keyboard.down('Shift'); + await superdoc.page.keyboard.press('ArrowRight'); + await superdoc.page.keyboard.press('ArrowRight'); + await superdoc.page.keyboard.up('Shift'); + await superdoc.waitForStable(); + + const selAfterRight = await superdoc.getSelection(); + expect(selAfterRight.to - selAfterRight.from).toBeGreaterThan(0); + const splitAfterRight = await evaluateSplitRects(); + + await superdoc.page.mouse.click(boundaryPoint.x, boundaryPoint.y); + await superdoc.waitForStable(); + + await superdoc.page.keyboard.down('Shift'); + await superdoc.page.keyboard.press('ArrowLeft'); + await superdoc.page.keyboard.press('ArrowLeft'); + await superdoc.page.keyboard.up('Shift'); + await superdoc.waitForStable(); + + const selAfterLeft = await superdoc.getSelection(); + expect(selAfterLeft.to - selAfterLeft.from).toBeGreaterThan(0); + const splitAfterLeft = await evaluateSplitRects(); + + const bestRectCount = Math.max(splitAfterRight.rectCount, splitAfterLeft.rectCount); + expect(bestRectCount).toBeGreaterThan(0); + expect(splitAfterRight.hasSplit || splitAfterLeft.hasSplit).toBe(true); + }); + + test('Typing Latin in RTL mixed-bidi boundary does not cause caret drift/snap-back', async ({ superdoc }) => { + await superdoc.loadDocument(MIXED_BIDI_DOC_PATH); + await superdoc.waitForStable(); + + const boundaryPoint = await superdoc.page.evaluate(() => { + const lines = Array.from(document.querySelectorAll('.superdoc-line')); + for (const line of lines) { + const spans = Array.from(line.querySelectorAll('span[data-pm-start][data-pm-end]')) as HTMLElement[]; + const rtlSpan = spans.find((span) => /[\u0590-\u05FF\u0600-\u06FF]/.test(span.textContent ?? '')); + const ltrSpan = spans.find((span) => /[A-Za-z]/.test(span.textContent ?? '')); + if (!rtlSpan || !ltrSpan) continue; + const ltrRect = ltrSpan.getBoundingClientRect(); + return { x: ltrRect.left + 2, y: ltrRect.top + ltrRect.height / 2 }; + } + return null; + }); + + expect(boundaryPoint).not.toBeNull(); + if (!boundaryPoint) return; + + const getCaret = async () => { + const sel = await superdoc.getSelection(); + const x = await superdoc.page.evaluate((pos) => { + const pe = (window as any).superdoc?.activeEditor?.presentationEditor; + return pe?.computeCaretLayoutRect(pos)?.x ?? null; + }, sel.from); + return { pos: sel.from, x }; + }; + + await superdoc.page.mouse.click(boundaryPoint.x, boundaryPoint.y); + await superdoc.waitForStable(); + + const c0 = await getCaret(); + expect(c0.x).not.toBeNull(); + if (c0.x == null) return; + + await superdoc.page.keyboard.insertText('A'); + await superdoc.waitForStable(); + const c1 = await getCaret(); + + await superdoc.page.keyboard.insertText('B'); + await superdoc.waitForStable(); + const c2 = await getCaret(); + + await superdoc.page.keyboard.insertText('C'); + await superdoc.waitForStable(); + const c3 = await getCaret(); + + expect(c1.x).not.toBeNull(); + expect(c2.x).not.toBeNull(); + expect(c3.x).not.toBeNull(); + if (c1.x == null || c2.x == null || c3.x == null) return; + + const d1 = c1.x - c0.x; + const d2 = c2.x - c1.x; + const d3 = c3.x - c2.x; + + // Boundary ambiguity can yield a zero delta for one keystroke; that's fine. + // Drift/snap-back signal is a direction reversal between non-zero steps. + const nonZeroSigns = [Math.sign(d1), Math.sign(d2), Math.sign(d3)].filter((s) => s !== 0); + if (nonZeroSigns.length >= 2) { + const first = nonZeroSigns[0]!; + expect(nonZeroSigns.every((s) => s === first)).toBe(true); + } + // PM position must still advance with typing even if visual X is near-stationary at boundary. + expect(c1.pos).toBeGreaterThan(c0.pos); + expect(c2.pos).toBeGreaterThan(c1.pos); + expect(c3.pos).toBeGreaterThan(c2.pos); + }); }); diff --git a/tests/behavior/tests/selection/rtl-click-positioning.spec.ts b/tests/behavior/tests/selection/rtl-click-positioning.spec.ts index 0b12e98b70..e4b241e2b3 100644 --- a/tests/behavior/tests/selection/rtl-click-positioning.spec.ts +++ b/tests/behavior/tests/selection/rtl-click-positioning.spec.ts @@ -5,6 +5,7 @@ import { test, expect } from '../../fixtures/superdoc.js'; const __dirname = path.dirname(fileURLToPath(import.meta.url)); const DOC_PATH = path.resolve(__dirname, 'fixtures/rtl-mixed-bidi.docx'); +const MIXED_BIDI_DOC_PATH = path.resolve(__dirname, 'fixtures/mixed-bidi-2.docx'); test.skip(!fs.existsSync(DOC_PATH), 'RTL fixture not available'); @@ -124,4 +125,194 @@ test.describe('RTL click-to-position mapping', () => { expect(selRight.from).toBeGreaterThan(0); expect(selLeft.from).not.toBe(selRight.from); }); + + test('Backspace at mixed-bidi boundary mutates content from a boundary caret', async ({ superdoc, browserName }) => { + test.fixme( + browserName === 'firefox', + 'Firefox mixed-bidi boundary caret/backspace geometry differs from Chromium/WebKit in this fixture.', + ); + + await superdoc.loadDocument(MIXED_BIDI_DOC_PATH); + await superdoc.waitForStable(); + + const beforeText = await superdoc.getTextContent(); + + const boundaryPoint = await superdoc.page.evaluate(() => { + const lines = Array.from(document.querySelectorAll('.superdoc-line')); + for (const line of lines) { + const spans = Array.from(line.querySelectorAll('span[data-pm-start][data-pm-end]')) as HTMLElement[]; + const rtlSpan = spans.find((span) => /[\u0590-\u05FF\u0600-\u06FF]/.test(span.textContent ?? '')); + const ltrSpan = spans.find((span) => /[A-Za-z]/.test(span.textContent ?? '')); + if (!rtlSpan || !ltrSpan) continue; + + const ltrRect = ltrSpan.getBoundingClientRect(); + return { + x: ltrRect.left + 2, + y: ltrRect.top + ltrRect.height / 2, + }; + } + return null; + }); + + expect(boundaryPoint).not.toBeNull(); + if (!boundaryPoint) return; + + await superdoc.page.mouse.click(boundaryPoint.x, boundaryPoint.y); + await superdoc.waitForStable(); + + const beforeSel = await superdoc.getSelection(); + expect(beforeSel.from).toBe(beforeSel.to); + + const boundaryChars = await superdoc.page.evaluate(({ x, y }) => { + const resolveAt = (probeX: number) => { + const lineEl = document + .elementsFromPoint(probeX, y) + .find((el) => (el as HTMLElement).classList?.contains('superdoc-line')) as HTMLElement | undefined; + if (!lineEl) return null; + + type CharBox = { char: string; left: number; right: number; centerX: number; centerY: number }; + const chars: CharBox[] = []; + const doc = document; + const walker = doc.createTreeWalker(lineEl, NodeFilter.SHOW_TEXT); + let node = walker.nextNode() as Text | null; + while (node) { + const text = node.textContent ?? ''; + for (let i = 0; i < text.length; i++) { + const ch = text[i]; + if (!ch || /\s/.test(ch)) continue; + const range = doc.createRange(); + range.setStart(node, i); + range.setEnd(node, i + 1); + const rect = range.getBoundingClientRect(); + if (rect.width > 0 && rect.height > 0) { + chars.push({ + char: ch, + left: rect.left, + right: rect.right, + centerX: rect.left + rect.width / 2, + centerY: rect.top + rect.height / 2, + }); + } + } + node = walker.nextNode() as Text | null; + } + + const sameVisualBand = chars.filter((c) => Math.abs(c.centerY - y) <= 6); + const band = sameVisualBand.length > 0 ? sameVisualBand : chars; + if (band.length === 0) return null; + + band.sort((a, b) => a.centerX - b.centerX); + + let leftChar: CharBox | null = null; + let rightChar: CharBox | null = null; + let leftIndex = -1; + for (const c of band) { + if (c.centerX < probeX) { + if (!leftChar || c.centerX > leftChar.centerX) { + leftChar = c; + leftIndex = band.indexOf(c); + } + } + if (c.centerX >= probeX) { + if (!rightChar || c.centerX < rightChar.centerX) rightChar = c; + } + } + + return { + linePmStart: lineEl.getAttribute('data-pm-start'), + linePmEnd: lineEl.getAttribute('data-pm-end'), + visualSequence: band.map((c) => c.char).join(''), + visualLeftIndex: leftIndex, + visualLeftChar: leftChar?.char ?? null, + visualRightChar: rightChar?.char ?? null, + }; + }; + + const probes = [0, -1, -2, 1]; + for (const dx of probes) { + const resolved = resolveAt(x + dx); + if (resolved?.visualLeftChar) return resolved; + } + return resolveAt(x); + }, boundaryPoint); + + expect(boundaryChars).not.toBeNull(); + expect(boundaryChars?.visualLeftChar).not.toBeNull(); + + await superdoc.press('Backspace'); + await superdoc.waitForStable(); + + const afterText = await superdoc.getTextContent(); + const afterSel = await superdoc.getSelection(); + + expect(afterText).not.toBe(beforeText); + expect(afterText.length).toBe(beforeText.length - 1); + expect(afterSel.from).toBe(afterSel.to); + + const countChar = (text: string, char: string): number => { + let count = 0; + for (const ch of text) if (ch === char) count++; + return count; + }; + + const deletedChar = boundaryChars?.visualLeftChar; + if (deletedChar) { + const beforeDeletedCount = countChar(beforeText, deletedChar); + const afterDeletedCount = countChar(afterText, deletedChar); + expect(afterDeletedCount).toBe(beforeDeletedCount - 1); + } + + const controlChar = boundaryChars?.visualRightChar; + if (controlChar && controlChar !== deletedChar) { + const beforeControlCount = countChar(beforeText, controlChar); + const afterControlCount = countChar(afterText, controlChar); + expect(afterControlCount).toBe(beforeControlCount); + } + + const afterBoundaryLine = await superdoc.page.evaluate((linePmStart) => { + const lineEl = Array.from(document.querySelectorAll('.superdoc-line')).find( + (line) => line.getAttribute('data-pm-start') === linePmStart, + ) as HTMLElement | undefined; + if (!lineEl) return null; + + type CharBox = { char: string; centerX: number; centerY: number }; + const chars: CharBox[] = []; + const doc = document; + const walker = doc.createTreeWalker(lineEl, NodeFilter.SHOW_TEXT); + let node = walker.nextNode() as Text | null; + while (node) { + const text = node.textContent ?? ''; + for (let i = 0; i < text.length; i++) { + const ch = text[i]; + if (!ch || /\s/.test(ch)) continue; + const range = doc.createRange(); + range.setStart(node, i); + range.setEnd(node, i + 1); + const rect = range.getBoundingClientRect(); + if (rect.width > 0 && rect.height > 0) { + chars.push({ char: ch, centerX: rect.left + rect.width / 2, centerY: rect.top + rect.height / 2 }); + } + } + node = walker.nextNode() as Text | null; + } + if (chars.length === 0) return ''; + + const centerY = chars.reduce((sum, c) => sum + c.centerY, 0) / chars.length; + const band = chars.filter((c) => Math.abs(c.centerY - centerY) <= 6); + const target = band.length > 0 ? band : chars; + target.sort((a, b) => a.centerX - b.centerX); + return target.map((c) => c.char).join(''); + }, boundaryChars?.linePmStart ?? null); + + if ( + boundaryChars?.visualSequence && + boundaryChars.visualLeftIndex >= 0 && + boundaryChars.visualLeftIndex < boundaryChars.visualSequence.length + ) { + const expectedSequence = + boundaryChars.visualSequence.slice(0, boundaryChars.visualLeftIndex) + + boundaryChars.visualSequence.slice(boundaryChars.visualLeftIndex + 1); + expect(afterBoundaryLine).toBe(expectedSequence); + } + }); });