From be17654fc0179f9b2f0289ccc63c3258e53aacf7 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Thu, 18 Jun 2026 22:03:43 +0800 Subject: [PATCH] fix(rsvp): render RTL words whole so Arabic shapes correctly (#4630) (#4648) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The RSVP word window split each word into before/orp/after spans at the ORP index and laid them out left-to-right. Slicing an Arabic/Hebrew word by character index breaks letter shaping (letters stop connecting, some slices render as notdef boxes) and the LTR layout reverses the visual order, so e.g. علم showed as disconnected, out-of-order letters. Detect RTL text and render the word as a single centered span — reusing the existing CJK Highlight Word path — with dir="rtl" so the browser shapes and orders it correctly, matching the context panel. ORP anchoring is meaningless for unsplittable shaped scripts, so RTL always renders whole; no new toggle. Co-authored-by: Claude Opus 4.8 (1M context) --- .../components/rsvp-overlay-context.test.tsx | 49 +++++++++++++++++++ .../src/__tests__/services/rsvp-utils.test.ts | 36 ++++++++++++++ .../reader/components/rsvp/RSVPOverlay.tsx | 16 ++++-- apps/readest-app/src/services/rsvp/utils.ts | 14 ++++++ 4 files changed, 111 insertions(+), 4 deletions(-) diff --git a/apps/readest-app/src/__tests__/components/rsvp-overlay-context.test.tsx b/apps/readest-app/src/__tests__/components/rsvp-overlay-context.test.tsx index e4eac9ae..0c6485e5 100644 --- a/apps/readest-app/src/__tests__/components/rsvp-overlay-context.test.tsx +++ b/apps/readest-app/src/__tests__/components/rsvp-overlay-context.test.tsx @@ -343,6 +343,55 @@ describe('RSVPOverlay — CJK reading options', () => { }); }); +describe('RSVPOverlay — RTL word display (#4630)', () => { + afterEach(() => { + cleanup(); + localStorage.clear(); + }); + + test('renders an Arabic word as a single RTL whole-word span, never split', () => { + const state = buildState({ + words: [{ text: 'علم', orpIndex: 0, pauseMultiplier: 1 }], + currentIndex: 0, + }); + const { container } = renderOverlay(state); + + // Splitting the word into before/orp/after spans breaks Arabic shaping and + // reverses the visual order — RTL words must render whole instead. + const whole = container.querySelector('.rsvp-word-whole'); + expect(whole).not.toBeNull(); + expect(whole!.textContent).toBe('علم'); + expect(whole!.getAttribute('dir')).toBe('rtl'); + expect(container.querySelector('.rsvp-word-orp')).toBeNull(); + expect(container.querySelector('.rsvp-word-before')).toBeNull(); + expect(container.querySelector('.rsvp-word-after')).toBeNull(); + }); + + test('renders a Hebrew word whole as well', () => { + const state = buildState({ + words: [{ text: 'שלום', orpIndex: 0, pauseMultiplier: 1 }], + currentIndex: 0, + }); + const { container } = renderOverlay(state); + + const whole = container.querySelector('.rsvp-word-whole'); + expect(whole).not.toBeNull(); + expect(whole!.textContent).toBe('שלום'); + expect(container.querySelector('.rsvp-word-orp')).toBeNull(); + }); + + test('keeps the focus-letter split for Latin words (no spurious dir)', () => { + const state = buildState({ + words: [{ text: 'hello', orpIndex: 1, pauseMultiplier: 1 }], + currentIndex: 0, + }); + const { container } = renderOverlay(state); + + expect(container.querySelector('.rsvp-word-orp')).not.toBeNull(); + expect(container.querySelector('.rsvp-word-whole')).toBeNull(); + }); +}); + describe('RSVPOverlay — manual word stepping (#4476)', () => { afterEach(() => cleanup()); diff --git a/apps/readest-app/src/__tests__/services/rsvp-utils.test.ts b/apps/readest-app/src/__tests__/services/rsvp-utils.test.ts index 64d14fcd..f3af587d 100644 --- a/apps/readest-app/src/__tests__/services/rsvp-utils.test.ts +++ b/apps/readest-app/src/__tests__/services/rsvp-utils.test.ts @@ -3,6 +3,7 @@ import { isCJK, containsCJK, isCJKPunctuation, + isRTLText, getSegmenterLocale, segmentCJKText, splitTextIntoWords, @@ -118,6 +119,41 @@ describe('rsvp/utils', () => { }); }); + describe('isRTLText', () => { + test('returns true for Arabic text', () => { + expect(isRTLText('علم')).toBe(true); + expect(isRTLText('مرحبا')).toBe(true); + }); + + test('returns true for Hebrew text', () => { + expect(isRTLText('שלום')).toBe(true); + }); + + test('returns true for Arabic presentation forms', () => { + expect(isRTLText('ﺍ')).toBe(true); // Arabic letter alef isolated form + }); + + test('returns true for text mixing Arabic and Latin', () => { + expect(isRTLText('Hello علم')).toBe(true); + }); + + test('returns false for pure Latin text', () => { + expect(isRTLText('hello')).toBe(false); + }); + + test('returns false for CJK text', () => { + expect(isRTLText('你好世界')).toBe(false); + }); + + test('returns false for digits and Latin punctuation', () => { + expect(isRTLText('123, 456.')).toBe(false); + }); + + test('returns false for empty string', () => { + expect(isRTLText('')).toBe(false); + }); + }); + describe('getSegmenterLocale', () => { test('returns ja for Japanese hiragana text', () => { expect(getSegmenterLocale('こんにちは')).toBe('ja'); diff --git a/apps/readest-app/src/app/reader/components/rsvp/RSVPOverlay.tsx b/apps/readest-app/src/app/reader/components/rsvp/RSVPOverlay.tsx index 328276ed..24970260 100644 --- a/apps/readest-app/src/app/reader/components/rsvp/RSVPOverlay.tsx +++ b/apps/readest-app/src/app/reader/components/rsvp/RSVPOverlay.tsx @@ -4,7 +4,7 @@ import React, { useEffect, useState, useCallback, useRef, useMemo } from 'react' import clsx from 'clsx'; import { Insets } from '@/types/misc'; import { RsvpState, RSVPController } from '@/services/rsvp'; -import { containsCJK } from '@/services/rsvp/utils'; +import { containsCJK, isRTLText } from '@/services/rsvp/utils'; import { useThemeStore } from '@/store/themeStore'; import { useSettingsStore } from '@/store/settingsStore'; import { TOCItem } from '@/libs/document'; @@ -351,6 +351,10 @@ const RSVPOverlay: React.FC = ({ const orpChar = currentWord ? currentWord.text.charAt(currentWord.orpIndex) : ''; const wordAfter = currentWord ? currentWord.text.substring(currentWord.orpIndex + 1) : ''; const isCJKWord = currentWord ? containsCJK(currentWord.text) : false; + // RTL words (Arabic, Hebrew, …) must never be split into before/orp/after + // spans: slicing by character index breaks letter shaping and reverses the + // visual order. Render them whole instead, like CJK Highlight Word (#4630). + const isRTLWord = currentWord ? isRTLText(currentWord.text) : false; const wordLetterSpacing = undefined; const wordSideOffset = isCJKWord ? '0.45em' : '0.3em'; @@ -973,12 +977,16 @@ const RSVPOverlay: React.FC = ({ }} > {currentWord ? ( - isCJKWord && highlightWholeWord ? ( - // Whole-word mode: center the full CJK word and color every - // character, instead of anchoring a single focus character. + isRTLWord || (isCJKWord && highlightWholeWord) ? ( + // Whole-word mode: center the full word and color it, instead + // of anchoring a single focus character. Used for CJK Highlight + // Word and always for RTL words, whose shaping/order would + // break if sliced into before/orp/after spans (#4630). dir=rtl + // restores correct letter order and connection for RTL. {currentWord.text} diff --git a/apps/readest-app/src/services/rsvp/utils.ts b/apps/readest-app/src/services/rsvp/utils.ts index 43ed7404..a08256bf 100644 --- a/apps/readest-app/src/services/rsvp/utils.ts +++ b/apps/readest-app/src/services/rsvp/utils.ts @@ -34,6 +34,20 @@ export function containsCJK(text: string): boolean { return false; } +// Strong right-to-left scripts: Hebrew, Arabic, Syriac, Thaana, N'Ko, +// Samaritan, Mandaic and the Arabic Extended blocks (U+0590–U+08FF), plus the +// Hebrew/Arabic presentation forms (U+FB1D–U+FDFF and U+FE70–U+FEFF). +const RTL_PATTERN = /[\u0590-\u08FF\uFB1D-\uFDFF\uFE70-\uFEFF]/; + +/** + * Check if text contains any right-to-left characters. Splitting such text by + * character index (as the RSVP focus-letter layout does) breaks letter shaping + * and reverses the visual order, so RTL words are rendered whole instead (#4630). + */ +export function isRTLText(text: string): boolean { + return RTL_PATTERN.test(text); +} + /** * Check if text is CJK punctuation */