From cc577ae2ab38a7d7a3ee40582996fbe7f819faec Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Fri, 27 Mar 2026 15:44:06 -0300 Subject: [PATCH 1/3] fix(super-editor): correct cursor movement direction in RTL paragraphs Set dir="rtl" on the hidden ProseMirror editor's paragraph DOM elements when paragraphProperties.rightToLeft is true. Without this, the browser's native bidi cursor handling treated all text as LTR, causing ArrowLeft to move the cursor visually right and ArrowRight visually left in Arabic text. Also fix resolvePositionAtGoalX binary search for vertical navigation (up/down arrows) to invert search direction in RTL paragraphs where X coordinates decrease with increasing PM positions. SD-2390 --- .../extensions/paragraph/ParagraphNodeView.js | 6 ++ .../paragraph/ParagraphNodeView.test.js | 41 +++++++++++++ .../vertical-navigation.js | 17 ++++-- .../vertical-navigation.test.js | 59 +++++++++++++++++++ 4 files changed, 118 insertions(+), 5 deletions(-) diff --git a/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.js b/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.js index 5919035f2b..94b3717102 100644 --- a/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.js +++ b/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.js @@ -141,6 +141,12 @@ export class ParagraphNodeView { if (paragraphProperties.styleId) { this.dom.setAttribute('styleid', paragraphProperties.styleId); } + + if (paragraphProperties.rightToLeft) { + this.dom.setAttribute('dir', 'rtl'); + } else { + this.dom.removeAttribute('dir'); + } } /** diff --git a/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.test.js b/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.test.js index eb46ceb100..4575db07f8 100644 --- a/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.test.js +++ b/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.test.js @@ -301,4 +301,45 @@ describe('ParagraphNodeView', () => { expect(nodeView.dom.getAttribute('data-level')).toBeNull(); expect(nodeView.dom.classList.contains('sd-editor-dropcap')).toBe(false); }); + + it('sets dir="rtl" on RTL paragraphs', () => { + isList.mockReturnValue(false); + resolveParagraphProperties.mockReturnValue({ rightToLeft: true }); + + const { nodeView } = mountNodeView({ + attrs: { + paragraphProperties: { rightToLeft: true }, + listRendering: {}, + }, + }); + + expect(nodeView.dom.getAttribute('dir')).toBe('rtl'); + }); + + it('does not set dir on LTR paragraphs', () => { + isList.mockReturnValue(false); + resolveParagraphProperties.mockReturnValue({}); + + const { nodeView } = mountNodeView(); + + expect(nodeView.dom.getAttribute('dir')).toBeNull(); + }); + + it('removes dir="rtl" when paragraph changes from RTL to LTR', () => { + isList.mockReturnValue(false); + resolveParagraphProperties.mockReturnValueOnce({ rightToLeft: true }).mockReturnValueOnce({}); + + const { nodeView } = mountNodeView({ + attrs: { + paragraphProperties: { rightToLeft: true }, + listRendering: {}, + }, + }); + expect(nodeView.dom.getAttribute('dir')).toBe('rtl'); + + const ltrNode = createNode({ attrs: { paragraphProperties: {}, listRendering: {} } }); + nodeView.update(ltrNode, []); + + expect(nodeView.dom.getAttribute('dir')).toBeNull(); + }); }); diff --git a/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.js b/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.js index 7f96f64fd9..17435a98e5 100644 --- a/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.js +++ b/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.js @@ -379,9 +379,12 @@ export function resolvePositionAtGoalX(editor, pmStart, pmEnd, goalX) { let bestPos = pmStart; let bestDist = Infinity; - // Binary search: characters within a single line have monotonically increasing X. - // NOTE: assumes LTR text. For RTL, X decreases with position so the search - // direction would be inverted. bestPos/bestDist tracking limits the impact. + // Detect RTL: in RTL lines, X decreases as PM position increases, so the + // binary search comparison must be inverted. + const $start = editor.state.doc.resolve(pmStart); + const paraNode = $start.parent.type.name === 'paragraph' ? $start.parent : $start.node(-1); + const isRtl = paraNode?.attrs?.paragraphProperties?.rightToLeft === true; + let lo = pmStart; let hi = pmEnd; @@ -403,9 +406,13 @@ export function resolvePositionAtGoalX(editor, pmStart, pmEnd, goalX) { } if (rect.x < goalX) { - lo = mid + 1; + // In LTR, X < goalX means search higher positions (further right). + // In RTL, X < goalX means search lower positions (further right in RTL). + if (isRtl) hi = mid - 1; + else lo = mid + 1; } else if (rect.x > goalX) { - hi = mid - 1; + if (isRtl) lo = mid + 1; + else hi = mid - 1; } else { // Exact match break; diff --git a/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.test.js b/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.test.js index 733da82109..865fe908c6 100644 --- a/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.test.js +++ b/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.test.js @@ -366,6 +366,16 @@ describe('VerticalNavigation', () => { describe('resolvePositionAtGoalX', () => { const makeEditor = (rectFn) => ({ presentationEditor: { computeCaretLayoutRect: rectFn }, + state: { + doc: { + resolve: () => ({ + parent: { + type: { name: 'paragraph' }, + attrs: { paragraphProperties: {} }, + }, + }), + }, + }, }); it('returns the position whose X is closest to goalX', () => { @@ -415,4 +425,53 @@ describe('resolvePositionAtGoalX', () => { const result = resolvePositionAtGoalX(editor, 10, 10, 50); expect(result).toEqual({ pos: 10 }); }); + + describe('RTL support', () => { + const makeRtlEditor = (rectFn, isRtl) => ({ + presentationEditor: { computeCaretLayoutRect: rectFn }, + state: { + doc: { + resolve: () => ({ + parent: { + type: { name: 'paragraph' }, + attrs: { + paragraphProperties: isRtl ? { rightToLeft: true } : {}, + }, + }, + }), + }, + }, + }); + + it('finds correct position in RTL line (X decreases with position)', () => { + // RTL: position 10 → x=40, position 14 → x=0 (X decreases with PM position) + const editor = makeRtlEditor((pos) => ({ x: (14 - pos) * 10 }), true); + const result = resolvePositionAtGoalX(editor, 10, 14, 25); + // goalX=25: pos 11 has x=30 (dist=5), pos 12 has x=20 (dist=5) + // Binary search with inverted direction should find pos 11 or 12 + expect(result.pos).toBeGreaterThanOrEqual(11); + expect(result.pos).toBeLessThanOrEqual(12); + }); + + it('returns pmStart for RTL when goalX matches the rightmost position', () => { + // RTL: pmStart has highest X + const editor = makeRtlEditor((pos) => ({ x: (14 - pos) * 10 }), true); + const result = resolvePositionAtGoalX(editor, 10, 14, 40); + expect(result).toEqual({ pos: 10 }); + }); + + it('returns pmEnd for RTL when goalX matches the leftmost position', () => { + // RTL: pmEnd has lowest X + const editor = makeRtlEditor((pos) => ({ x: (14 - pos) * 10 }), true); + const result = resolvePositionAtGoalX(editor, 10, 14, 0); + expect(result).toEqual({ pos: 14 }); + }); + + it('does not invert search for LTR paragraphs', () => { + // LTR: X increases with position (same as existing tests) + const editor = makeRtlEditor((pos) => ({ x: (pos - 10) * 10 }), false); + const result = resolvePositionAtGoalX(editor, 10, 14, 25); + expect(result).toEqual({ pos: 12 }); + }); + }); }); From ff7af6ada344caa04aaabeb865aa14fa3e6ec3cf Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Fri, 27 Mar 2026 16:11:02 -0300 Subject: [PATCH 2/3] refactor(super-editor): pass isRtl to resolvePositionAtGoalX from DOM Read RTL direction from the visual DOM element (set by DomPainter using resolved style properties) instead of calling doc.resolve(pmStart) inside the binary search helper. This fixes two review findings: 1. Eliminates RangeError crash when pmStart is stale (layout-derived positions can lag the current document after edits) 2. Correctly detects style-inherited RTL direction (not just inline paragraphProperties.rightToLeft) Also consolidates the duplicated makeRtlEditor test helper into the existing makeEditor, since isRtl is now a simple boolean parameter. --- .../vertical-navigation.js | 17 +++---- .../vertical-navigation.test.js | 48 ++++++------------- 2 files changed, 23 insertions(+), 42 deletions(-) diff --git a/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.js b/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.js index 17435a98e5..d50d444830 100644 --- a/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.js +++ b/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.js @@ -145,7 +145,7 @@ export const VerticalNavigation = Extension.create({ ) { // Hit test produced a position outside the adjacent line's range. // Resolve position directly from layout data using binary search at goalX. - hit = resolvePositionAtGoalX(editor, adjacent.pmStart, adjacent.pmEnd, goalX); + hit = resolvePositionAtGoalX(editor, adjacent.pmStart, adjacent.pmEnd, goalX, adjacent.isRtl); } } @@ -265,11 +265,16 @@ function getAdjacentLineClientTarget(editor, coords, direction) { const pmStart = Number(adjacentLine.dataset?.pmStart); const pmEnd = Number(adjacentLine.dataset?.pmEnd); + // Read direction from the visual DOM — DomPainter sets dir="rtl" on RTL lines + // using fully resolved properties (style cascade, not just inline attrs). + const isRtl = adjacentLine.closest?.('[dir="rtl"]') != null; + return { clientY, pageIndex: Number.isFinite(pageIndex) ? pageIndex : undefined, pmStart: Number.isFinite(pmStart) ? pmStart : undefined, pmEnd: Number.isFinite(pmEnd) ? pmEnd : undefined, + isRtl, }; } @@ -372,19 +377,15 @@ function findAdjacentLineElement(currentLine, direction, caretX) { * @param {number} pmStart - Start PM position of the target line. * @param {number} pmEnd - End PM position of the target line. * @param {number} goalX - Target X coordinate in layout space. + * @param {boolean} [isRtl=false] - Whether the target line is RTL. In RTL lines, + * X decreases as PM position increases, so the binary search must be inverted. * @returns {{ pos: number } | null} */ -export function resolvePositionAtGoalX(editor, pmStart, pmEnd, goalX) { +export function resolvePositionAtGoalX(editor, pmStart, pmEnd, goalX, isRtl = false) { const presentationEditor = editor.presentationEditor; let bestPos = pmStart; let bestDist = Infinity; - // Detect RTL: in RTL lines, X decreases as PM position increases, so the - // binary search comparison must be inverted. - const $start = editor.state.doc.resolve(pmStart); - const paraNode = $start.parent.type.name === 'paragraph' ? $start.parent : $start.node(-1); - const isRtl = paraNode?.attrs?.paragraphProperties?.rightToLeft === true; - let lo = pmStart; let hi = pmEnd; diff --git a/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.test.js b/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.test.js index 865fe908c6..a0e821ffde 100644 --- a/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.test.js +++ b/packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.test.js @@ -366,16 +366,6 @@ describe('VerticalNavigation', () => { describe('resolvePositionAtGoalX', () => { const makeEditor = (rectFn) => ({ presentationEditor: { computeCaretLayoutRect: rectFn }, - state: { - doc: { - resolve: () => ({ - parent: { - type: { name: 'paragraph' }, - attrs: { paragraphProperties: {} }, - }, - }), - }, - }, }); it('returns the position whose X is closest to goalX', () => { @@ -427,26 +417,10 @@ describe('resolvePositionAtGoalX', () => { }); describe('RTL support', () => { - const makeRtlEditor = (rectFn, isRtl) => ({ - presentationEditor: { computeCaretLayoutRect: rectFn }, - state: { - doc: { - resolve: () => ({ - parent: { - type: { name: 'paragraph' }, - attrs: { - paragraphProperties: isRtl ? { rightToLeft: true } : {}, - }, - }, - }), - }, - }, - }); - it('finds correct position in RTL line (X decreases with position)', () => { // RTL: position 10 → x=40, position 14 → x=0 (X decreases with PM position) - const editor = makeRtlEditor((pos) => ({ x: (14 - pos) * 10 }), true); - const result = resolvePositionAtGoalX(editor, 10, 14, 25); + const editor = makeEditor((pos) => ({ x: (14 - pos) * 10 })); + const result = resolvePositionAtGoalX(editor, 10, 14, 25, true); // goalX=25: pos 11 has x=30 (dist=5), pos 12 has x=20 (dist=5) // Binary search with inverted direction should find pos 11 or 12 expect(result.pos).toBeGreaterThanOrEqual(11); @@ -455,21 +429,27 @@ describe('resolvePositionAtGoalX', () => { it('returns pmStart for RTL when goalX matches the rightmost position', () => { // RTL: pmStart has highest X - const editor = makeRtlEditor((pos) => ({ x: (14 - pos) * 10 }), true); - const result = resolvePositionAtGoalX(editor, 10, 14, 40); + const editor = makeEditor((pos) => ({ x: (14 - pos) * 10 })); + const result = resolvePositionAtGoalX(editor, 10, 14, 40, true); expect(result).toEqual({ pos: 10 }); }); it('returns pmEnd for RTL when goalX matches the leftmost position', () => { // RTL: pmEnd has lowest X - const editor = makeRtlEditor((pos) => ({ x: (14 - pos) * 10 }), true); - const result = resolvePositionAtGoalX(editor, 10, 14, 0); + const editor = makeEditor((pos) => ({ x: (14 - pos) * 10 })); + const result = resolvePositionAtGoalX(editor, 10, 14, 0, true); expect(result).toEqual({ pos: 14 }); }); - it('does not invert search for LTR paragraphs', () => { + it('does not invert search when isRtl is false', () => { // LTR: X increases with position (same as existing tests) - const editor = makeRtlEditor((pos) => ({ x: (pos - 10) * 10 }), false); + const editor = makeEditor((pos) => ({ x: (pos - 10) * 10 })); + const result = resolvePositionAtGoalX(editor, 10, 14, 25, false); + expect(result).toEqual({ pos: 12 }); + }); + + it('defaults to LTR when isRtl is not provided', () => { + const editor = makeEditor((pos) => ({ x: (pos - 10) * 10 })); const result = resolvePositionAtGoalX(editor, 10, 14, 25); expect(result).toEqual({ pos: 12 }); }); From 3e27493ea01227ed8ded4d3f339fc02dba014e53 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Sat, 28 Mar 2026 06:43:50 -0300 Subject: [PATCH 3/3] test(selection): add behavior and visual tests for RTL arrow key movement Add Playwright behavior test that verifies ArrowLeft/ArrowRight move the cursor in the correct visual direction in RTL paragraphs (and regression test for LTR). Add visual regression test that screenshots cursor position after arrow navigation in an RTL document. Both use the existing rtl-mixed-bidi.docx fixture. SD-2390 --- .../selection/rtl-arrow-key-movement.spec.ts | 118 ++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 tests/behavior/tests/selection/rtl-arrow-key-movement.spec.ts diff --git a/tests/behavior/tests/selection/rtl-arrow-key-movement.spec.ts b/tests/behavior/tests/selection/rtl-arrow-key-movement.spec.ts new file mode 100644 index 0000000000..c52c0f4108 --- /dev/null +++ b/tests/behavior/tests/selection/rtl-arrow-key-movement.spec.ts @@ -0,0 +1,118 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +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'); + +test.skip(!fs.existsSync(DOC_PATH), 'RTL fixture not available'); + +test.use({ config: { toolbar: 'none', showCaret: true, showSelection: true } }); + +test.describe('RTL arrow key cursor movement (SD-2390)', () => { + test('ArrowLeft moves cursor visually left in RTL paragraph', async ({ superdoc }) => { + await superdoc.loadDocument(DOC_PATH); + await superdoc.waitForStable(); + + // First line is RTL Arabic: "هذه فقرة كاملة باللغة العربية" + const rtlLine = superdoc.page.locator('.superdoc-line').first(); + const box = await rtlLine.boundingBox(); + if (!box) throw new Error('RTL line not visible'); + + // Click near the right edge (logical start of RTL text) + await superdoc.page.mouse.click(box.x + box.width - 20, box.y + box.height / 2); + await superdoc.waitForStable(); + + const before = await superdoc.getSelection(); + + // Get caret X before + const xBefore = await superdoc.page.evaluate((pos) => { + const pe = (window as any).superdoc?.activeEditor?.presentationEditor; + return pe?.computeCaretLayoutRect(pos)?.x; + }, before.from); + + // Press ArrowLeft + await superdoc.page.keyboard.press('ArrowLeft'); + await superdoc.waitForStable(); + + const after = await superdoc.getSelection(); + const xAfter = await superdoc.page.evaluate((pos) => { + const pe = (window as any).superdoc?.activeEditor?.presentationEditor; + return pe?.computeCaretLayoutRect(pos)?.x; + }, after.from); + + // In RTL, ArrowLeft should move visually left (decreasing X) + expect(xAfter).toBeLessThan(xBefore); + // PM position should increase (moving toward end of line in document order) + expect(after.from).toBeGreaterThan(before.from); + }); + + test('ArrowRight moves cursor visually right in RTL paragraph', async ({ superdoc }) => { + await superdoc.loadDocument(DOC_PATH); + await superdoc.waitForStable(); + + const rtlLine = superdoc.page.locator('.superdoc-line').first(); + const box = await rtlLine.boundingBox(); + if (!box) throw new Error('RTL line not visible'); + + // Click near the middle of the line + await superdoc.page.mouse.click(box.x + box.width / 2, box.y + box.height / 2); + 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); + + // Press ArrowRight + await superdoc.page.keyboard.press('ArrowRight'); + await superdoc.waitForStable(); + + const after = await superdoc.getSelection(); + const xAfter = await superdoc.page.evaluate((pos) => { + const pe = (window as any).superdoc?.activeEditor?.presentationEditor; + return pe?.computeCaretLayoutRect(pos)?.x; + }, after.from); + + // In RTL, ArrowRight should move visually right (increasing X) + expect(xAfter).toBeGreaterThan(xBefore); + // PM position should decrease (moving toward start of line in document order) + expect(after.from).toBeLessThan(before.from); + }); + + test('ArrowLeft/Right in LTR paragraph still works correctly', async ({ superdoc }) => { + await superdoc.loadDocument(DOC_PATH); + await superdoc.waitForStable(); + + // Second line is LTR English: "This is a complete English paragraph" + const ltrLine = superdoc.page.locator('.superdoc-line').nth(1); + const box = await ltrLine.boundingBox(); + if (!box) throw new Error('LTR line not visible'); + + // Click near the left edge + await superdoc.page.mouse.click(box.x + 30, box.y + box.height / 2); + 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); + + // Press ArrowRight in LTR + await superdoc.page.keyboard.press('ArrowRight'); + await superdoc.waitForStable(); + + const after = await superdoc.getSelection(); + const xAfter = await superdoc.page.evaluate((pos) => { + const pe = (window as any).superdoc?.activeEditor?.presentationEditor; + return pe?.computeCaretLayoutRect(pos)?.x; + }, after.from); + + // In LTR, ArrowRight moves visually right (increasing X) and increases PM position + expect(xAfter).toBeGreaterThan(xBefore); + expect(after.from).toBeGreaterThan(before.from); + }); +});