From cfe2bb9116f07620a0509da31108e58186d1a002 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Fri, 12 Jun 2026 12:30:56 +0800 Subject: [PATCH] fix(reader): Android text selection breaks on the first word of hyphenated paragraphs (#4545) --- .github/workflows/android-e2e.yml | 131 +++++ apps/readest-app/docs/testing.md | 52 ++ apps/readest-app/package.json | 1 + apps/readest-app/scripts/test-android.sh | 29 ++ .../src/__tests__/android/helpers/adb.ts | 91 ++++ .../src/__tests__/android/helpers/cdp.ts | 122 +++++ .../src/__tests__/android/helpers/reader.ts | 485 ++++++++++++++++++ .../android/selection.android.test.ts | 218 ++++++++ .../__tests__/utils/sel-hyphen-bounds.test.ts | 298 +++++++++++ .../annotator/AnnotationRangeEditor.tsx | 2 +- .../reader/components/annotator/Annotator.tsx | 12 + .../annotator/SelectionRangeEditor.tsx | 181 +++++++ .../app/reader/hooks/useAnnotationEditor.ts | 109 +--- .../src/app/reader/hooks/useTextSelector.ts | 147 +++++- .../src/app/reader/utils/annotatorUtil.ts | 109 +++- apps/readest-app/src/utils/sel.ts | 215 ++++++++ apps/readest-app/vitest.android.config.mts | 22 + apps/readest-app/vitest.config.mts | 2 + 18 files changed, 2124 insertions(+), 102 deletions(-) create mode 100644 .github/workflows/android-e2e.yml create mode 100755 apps/readest-app/scripts/test-android.sh create mode 100644 apps/readest-app/src/__tests__/android/helpers/adb.ts create mode 100644 apps/readest-app/src/__tests__/android/helpers/cdp.ts create mode 100644 apps/readest-app/src/__tests__/android/helpers/reader.ts create mode 100644 apps/readest-app/src/__tests__/android/selection.android.test.ts create mode 100644 apps/readest-app/src/__tests__/utils/sel-hyphen-bounds.test.ts create mode 100644 apps/readest-app/src/app/reader/components/annotator/SelectionRangeEditor.tsx create mode 100644 apps/readest-app/vitest.android.config.mts diff --git a/.github/workflows/android-e2e.yml b/.github/workflows/android-e2e.yml new file mode 100644 index 00000000..32725339 --- /dev/null +++ b/.github/workflows/android-e2e.yml @@ -0,0 +1,131 @@ +name: Android E2E (CDP) + +# On-device end-to-end tests: boots an x86_64 Android emulator (KVM), installs +# a debug APK, and runs the CDP-driven selection lane (pnpm test:android). +# Not PR-blocking: runs nightly, on demand, or when a PR is labeled +# `e2e-android`. + +on: + workflow_dispatch: + schedule: + - cron: '30 19 * * *' + pull_request: + types: [labeled, synchronize] + +concurrency: + group: android-e2e-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + android-e2e: + if: github.event_name != 'pull_request' || contains(github.event.pull_request.labels.*.name, 'e2e-android') + runs-on: ubuntu-latest + timeout-minutes: 90 + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + + - name: initialize git submodules + run: git submodule update --init --recursive + + - name: enable KVM for the emulator + run: | + echo 'KERNEL=="kvm", GROUP="kvm", MODE="0666", OPTIONS+="static_node=kvm"' | sudo tee /etc/udev/rules.d/99-kvm4all.rules + sudo udevadm control --reload-rules + sudo udevadm trigger --name-match=kvm + + - name: setup pnpm + uses: pnpm/action-setup@0e279bb959325dab635dd2c09392533439d90093 # v6 + + - name: setup node + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6 + with: + node-version: 24 + cache: pnpm + + - name: setup Java + uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5 + with: + distribution: 'zulu' + java-version: '17' + + - name: setup Android SDK + uses: android-actions/setup-android@40fd30fb8d7440372e1316f5d1809ec01dcd3699 # v4 + + - name: install NDK + run: sdkmanager "ndk;28.2.13676358" + + - name: install dependencies + run: pnpm install --frozen-lockfile --prefer-offline + + - name: copy pdfjs-dist and simplecc-dist to public directory + run: pnpm --filter @readest/readest-app setup-vendors + + - name: install Rust stable + uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + with: + targets: x86_64-linux-android + + - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2 + with: + workspaces: apps/readest-app/src-tauri + + - name: create .env.local file for Next.js + run: | + echo "NEXT_PUBLIC_APP_PLATFORM=tauri" >> .env.local + cp .env.local apps/readest-app/.env.local + + - name: build debug APK (x86_64) + env: + NDK_HOME: ${{ env.ANDROID_HOME }}/ndk/28.2.13676358 + run: | + cd apps/readest-app + # Only the customized files of gen/android are tracked — regenerate + # the gradle scaffolding, then restore the tracked customizations + # (same flow as the release workflow). + rm -rf src-tauri/gen/android + pnpm tauri android init + pnpm tauri icon ../../data/icons/readest-book.png + git checkout . + # Debug build: signed with the debug keystore, no release secrets + # needed (gradle only loads keystore.properties when it exists). + pnpm tauri android build --debug --target x86_64 + APK=$(find src-tauri/gen/android/app/build/outputs/apk -name '*-debug.apk' | head -n 1) + echo "APK=$PWD/$APK" >> "$GITHUB_ENV" + test -n "$APK" + + - name: cache AVD snapshot + uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 + id: avd-cache + with: + path: | + ~/.android/avd/* + ~/.android/adb* + key: avd-api-34 + + - name: create AVD snapshot for caching + if: steps.avd-cache.outputs.cache-hit != 'true' + uses: reactivecircus/android-emulator-runner@v2 + with: + api-level: 34 + arch: x86_64 + target: google_apis + force-avd-creation: false + emulator-options: -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none + disable-animations: false + script: echo "AVD snapshot created" + + - name: run Android e2e lane + uses: reactivecircus/android-emulator-runner@v2 + with: + api-level: 34 + arch: x86_64 + target: google_apis + force-avd-creation: false + emulator-options: -no-snapshot-save -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none + disable-animations: true + script: | + adb install -r "$APK" + cd apps/readest-app && pnpm test:android diff --git a/apps/readest-app/docs/testing.md b/apps/readest-app/docs/testing.md index d5275089..7e1fc8f9 100644 --- a/apps/readest-app/docs/testing.md +++ b/apps/readest-app/docs/testing.md @@ -82,6 +82,57 @@ The `invoke()` helper accesses `window.top.__TAURI_INTERNALS__` (Vitest runs in **Limitations:** Only custom invoke commands and plugin commands listed in the webdriver capability work. Standard Tauri JS APIs (e.g. `@tauri-apps/api`) that rely on `URL: local` may not work from the Vitest iframe. +## Android Device E2E (`pnpm test:android`) + +Drives the **installed Readest app** on an adb-connected Android device or +emulator: gestures are injected with `adb shell input`, and the app's state is +probed through the WebView's **Chrome DevTools Protocol** (forwarded from the +`webview_devtools_remote_` abstract socket). This is the only lane that +exercises real Android touch selection, native handle behavior, and page-turn +gestures (e.g. the issue #1553 hyphen-selection fixes). + +```bash +# One-time: install a dev build on the device/emulator +pnpm dev-android + +# Start an emulator if no device is attached (see `emulator -list-avds`) +emulator -avd Pixel_9_Pro & + +# Run the lane (soft-skips when no adb/device/app is available) +pnpm test:android + +# With several devices attached, pick one: +ANDROID_SERIAL=emulator-5554 pnpm test:android +``` + +- **Config:** `vitest.android.config.mts` (node environment, serial execution, `retry: 1`) +- **Pattern:** `src/**/*.android.test.ts` +- **Helpers:** `src/__tests__/android/helpers/` — `adb.ts` (gestures), `cdp.ts` (DevTools client), `reader.ts` (app-level probes) +- **Fixtures:** plain EPUBs from `src/__tests__/fixtures/data/` (e.g. `sample-alice.epub`), opened transiently via a `VIEW` intent so the device library is never modified +- **Use for:** native text selection, touch gestures, selection handles, anything that only reproduces in the Android WebView compositor. + +### Conventions + +- **Probe, don't hardcode:** locate words/handles at runtime via CDP and derive + device pixels from `devicePixelRatio` — never bake in coordinates. +- **Poll, don't sleep:** use `waitFor()` on an observable condition (selection + state, handle count, frame position); reserve fixed pauses for gesture + pacing (long-press hold, corner dwell). +- **Discover, don't assume:** the harness finds a hyphenated on-screen + paragraph at runtime and derives every gesture target from live layout, so + any English fixture works regardless of fonts or screen size (hyphenation + is on by default in the app). +- **Serial only:** one device, one app — the config disables parallelism. + +### CI + +`.github/workflows/android-e2e.yml` runs the lane on an x86_64 emulator +(ubuntu runner with KVM): it builds a **debug** APK for `x86_64` (no signing +secrets needed), boots a cached AVD via `reactivecircus/android-emulator-runner`, +installs the APK, and runs `pnpm test:android`. It is intentionally not +PR-blocking — it runs nightly, on `workflow_dispatch`, or when a PR gets the +`e2e-android` label. + ## E2E Tests (WDIO) Full end-to-end tests using WebDriverIO, for UI-level testing against the running Tauri app. Same two-step workflow as Tauri integration tests. @@ -108,3 +159,4 @@ pnpm test:e2e | `*.browser.test.ts` | `pnpm test:browser` | Chromium (Playwright) | | `*.tauri.test.ts` | `pnpm test:tauri` | Tauri WebView | | `*.e2e.ts` | `pnpm test:e2e` | Tauri app (WDIO) | +| `*.android.test.ts` | `pnpm test:android` | Android device (CDP) | diff --git a/apps/readest-app/package.json b/apps/readest-app/package.json index 737964fe..db38425d 100644 --- a/apps/readest-app/package.json +++ b/apps/readest-app/package.json @@ -27,6 +27,7 @@ "test:coverage": "dotenv -e .env -e .env.test.local -- vitest run --coverage", "test:browser": "dotenv -e .env -e .env.test.local -- vitest run --config vitest.browser.config.mts", "test:tauri": "bash scripts/test-tauri.sh", + "test:android": "bash scripts/test-android.sh", "test:pr:web": "dotenv -e .env -e .env.test.local -- vitest run --maxWorkers=4 && pnpm test:browser && pnpm test:extension && pnpm build-browser-ext", "test:pr:tauri": "bash scripts/test-tauri.sh", "test:all": "pnpm test -- --watch=false && pnpm test:browser && bash scripts/test-tauri.sh", diff --git a/apps/readest-app/scripts/test-android.sh b/apps/readest-app/scripts/test-android.sh new file mode 100755 index 00000000..aa8a170d --- /dev/null +++ b/apps/readest-app/scripts/test-android.sh @@ -0,0 +1,29 @@ +#!/usr/bin/env bash +# Android CDP e2e lane: runs vitest against the Readest app installed on an +# adb-connected device or emulator. Soft-skips (exit 0) when no adb, no +# device, or no installed app is found, so it is safe in any environment. +# Select a device with ANDROID_SERIAL when several are attached. +set -uo pipefail +cd "$(dirname "$0")/.." + +PKG="com.bilingify.readest" + +if ! command -v adb >/dev/null 2>&1; then + echo "[test:android] adb not found — skipping Android e2e lane" + exit 0 +fi + +DEVICES=$(adb devices | tail -n +2 | awk '$2 == "device" { print $1 }') +if [ -z "$DEVICES" ]; then + echo "[test:android] no adb device/emulator connected — skipping Android e2e lane" + echo "[test:android] hint: start one with: emulator -avd (see 'emulator -list-avds')" + exit 0 +fi + +if ! adb shell pm list packages "$PKG" 2>/dev/null | grep -q "package:$PKG"; then + echo "[test:android] $PKG is not installed on the device — skipping Android e2e lane" + echo "[test:android] hint: install a dev build with: pnpm dev-android" + exit 0 +fi + +exec pnpm exec vitest run --config vitest.android.config.mts "$@" diff --git a/apps/readest-app/src/__tests__/android/helpers/adb.ts b/apps/readest-app/src/__tests__/android/helpers/adb.ts new file mode 100644 index 00000000..258a577a --- /dev/null +++ b/apps/readest-app/src/__tests__/android/helpers/adb.ts @@ -0,0 +1,91 @@ +import { execFile } from 'node:child_process'; +import { promisify } from 'node:util'; + +const execFileAsync = promisify(execFile); + +// All commands honor ANDROID_SERIAL, so the lane works unchanged against a +// physical device or an emulator — set the env var to pick one when several +// devices are attached. +export const adb = async (args: string[], timeoutMs = 30_000): Promise => { + const { stdout } = await execFileAsync('adb', args, { timeout: timeoutMs }); + return stdout.toString(); +}; + +export const adbShell = (cmd: string, timeoutMs = 30_000): Promise => + adb(['shell', cmd], timeoutMs); + +export const hasAdb = async (): Promise => { + try { + await adb(['version']); + return true; + } catch { + return false; + } +}; + +export const listDeviceSerials = async (): Promise => { + const out = await adb(['devices']); + return out + .split('\n') + .slice(1) + .map((l) => l.trim()) + .filter((l) => l.endsWith('device')) + .map((l) => l.split(/\s+/)[0]!) + .filter(Boolean); +}; + +export const isPackageInstalled = async (pkg: string): Promise => { + const out = await adbShell(`pm list packages ${pkg}`); + return out.includes(`package:${pkg}`); +}; + +export const screenSize = async (): Promise<{ width: number; height: number }> => { + const out = await adbShell('wm size'); + const m = out.match(/(\d+)x(\d+)/); + if (!m) throw new Error(`cannot parse screen size from: ${out}`); + return { width: Number(m[1]), height: Number(m[2]) }; +}; + +export const tap = (x: number, y: number): Promise => + adbShell(`input tap ${Math.round(x)} ${Math.round(y)}`); + +// `input swipe` with identical endpoints and a long duration is a long-press. +export const longPress = (x: number, y: number, ms = 700): Promise => + adbShell(`input swipe ${Math.round(x)} ${Math.round(y)} ${Math.round(x)} ${Math.round(y)} ${ms}`); + +export interface MotionStep { + x: number; + y: number; + /** Seconds to wait AFTER this step before the next one. */ + pauseSec?: number; +} + +// A raw touch gesture: DOWN at the first step, MOVE through the rest, UP at +// the last position. Unlike `input swipe` this supports long-press-then-drag +// and mid-gesture dwells (corner auto-turn). +export const motionGesture = async (steps: MotionStep[]): Promise => { + if (steps.length === 0) throw new Error('motionGesture needs at least one step'); + const parts: string[] = []; + steps.forEach((s, i) => { + const kind = i === 0 ? 'DOWN' : 'MOVE'; + parts.push(`input motionevent ${kind} ${Math.round(s.x)} ${Math.round(s.y)}`); + if (s.pauseSec) parts.push(`sleep ${s.pauseSec}`); + }); + const last = steps[steps.length - 1]!; + parts.push(`input motionevent UP ${Math.round(last.x)} ${Math.round(last.y)}`); + // One shell invocation so inter-event timing isn't at the mercy of adb + // round-trips. + return adbShell(parts.join(' && '), 120_000); +}; + +export const pushFile = (local: string, remote: string): Promise => + adb(['push', local, remote]); + +export const forwardTcpToLocalAbstract = async (port: number, socket: string): Promise => { + try { + await adb(['forward', '--remove', `tcp:${port}`]); + } catch { + // not forwarded yet + } + await adb(['forward', `tcp:${port}`, `localabstract:${socket}`]); +}; diff --git a/apps/readest-app/src/__tests__/android/helpers/cdp.ts b/apps/readest-app/src/__tests__/android/helpers/cdp.ts new file mode 100644 index 00000000..6ee4aaa5 --- /dev/null +++ b/apps/readest-app/src/__tests__/android/helpers/cdp.ts @@ -0,0 +1,122 @@ +import http from 'node:http'; +import { adbShell, forwardTcpToLocalAbstract } from './adb'; + +// The Android WebView exposes a Chrome DevTools endpoint on the abstract unix +// socket `webview_devtools_remote_` whenever remote debugging is enabled. +// We adb-forward it to a host TCP port and drive the page with the CDP +// Runtime domain. NOTE: the WebView's HTTP framing confuses curl — use +// node:http with an explicit `Host: localhost` header. + +export interface CdpTarget { + id: string; + url: string; + title: string; + type: string; +} + +const httpGetJson = (port: number, path: string): Promise => + new Promise((resolve, reject) => { + const req = http.get( + { host: '127.0.0.1', port, path, headers: { Host: 'localhost' } }, + (res) => { + let data = ''; + res.on('data', (c) => (data += c)); + res.on('end', () => { + try { + resolve(JSON.parse(data)); + } catch (e) { + reject(e); + } + }); + }, + ); + req.on('error', reject); + req.setTimeout(5000, () => req.destroy(new Error(`timeout GET ${path}`))); + }); + +export const forwardWebViewDevtools = async (pkg: string, port: number): Promise => { + const pid = (await adbShell(`pidof ${pkg}`)).trim().split(/\s+/)[0]; + if (!pid) throw new Error(`${pkg} is not running`); + const sockets = await adbShell('cat /proc/net/unix'); + const name = `webview_devtools_remote_${pid}`; + if (!sockets.includes(`@${name}`)) { + throw new Error(`no devtools socket ${name}; is WebView debugging enabled?`); + } + await forwardTcpToLocalAbstract(port, name); +}; + +export const listPages = (port: number): Promise => httpGetJson(port, '/json/list'); + +interface CdpResponse { + id?: number; + error?: { message?: string }; + result?: { + result?: { value?: unknown }; + exceptionDetails?: { exception?: { description?: string }; text?: string }; + }; +} + +export class CdpPage { + private ws: WebSocket; + private nextId = 1; + private pending = new Map< + number, + { resolve: (v: CdpResponse) => void; reject: (e: Error) => void } + >(); + + private constructor(ws: WebSocket) { + this.ws = ws; + this.ws.addEventListener('message', (ev) => { + const msg = JSON.parse(String(ev.data)) as CdpResponse; + if (msg.id && this.pending.has(msg.id)) { + const { resolve, reject } = this.pending.get(msg.id)!; + this.pending.delete(msg.id); + if (msg.error) reject(new Error(msg.error.message ?? 'CDP error')); + else resolve(msg); + } + }); + } + + static async connect(port: number, pageId: string): Promise { + const ws = new WebSocket(`ws://127.0.0.1:${port}/devtools/page/${pageId}`); + await new Promise((resolve, reject) => { + ws.addEventListener('open', () => resolve()); + ws.addEventListener('error', () => reject(new Error('CDP websocket failed to connect'))); + }); + const page = new CdpPage(ws); + await page.send('Runtime.enable'); + return page; + } + + private send(method: string, params: Record = {}): Promise { + return new Promise((resolve, reject) => { + const id = this.nextId++; + this.pending.set(id, { resolve, reject }); + this.ws.send(JSON.stringify({ id, method, params })); + }); + } + + /** + * Run `body` as the body of an async IIFE in the page and return its + * (JSON-serializable) return value. + */ + async evaluate(body: string): Promise { + const res = await this.send('Runtime.evaluate', { + expression: `(async () => { ${body} })()`, + awaitPromise: true, + returnByValue: true, + timeout: 30_000, + }); + const details = res.result?.exceptionDetails; + if (details) { + throw new Error( + `page evaluate threw: ${details.exception?.description ?? details.text ?? 'unknown'}`, + ); + } + return res.result?.result?.value as T; + } + + close(): void { + this.ws.close(); + } +} diff --git a/apps/readest-app/src/__tests__/android/helpers/reader.ts b/apps/readest-app/src/__tests__/android/helpers/reader.ts new file mode 100644 index 00000000..31149fc6 --- /dev/null +++ b/apps/readest-app/src/__tests__/android/helpers/reader.ts @@ -0,0 +1,485 @@ +import path from 'node:path'; +import { + adbShell, + hasAdb, + isPackageInstalled, + listDeviceSerials, + longPress, + pushFile, + tap, +} from './adb'; +import { CdpPage, forwardWebViewDevtools, listPages } from './cdp'; + +export const APP_PKG = 'com.bilingify.readest'; +const CDP_PORT = Number(process.env['READEST_CDP_PORT'] ?? 9333); +const REMOTE_FIXTURE_DIR = '/sdcard/Download'; + +export interface AndroidEnv { + serial: string; +} + +// The lane soft-skips unless adb, a device, and an installed Readest app are +// all present, so it is safe to run `pnpm test:android` anywhere. +export const detectAndroidEnv = async (): Promise => { + if (!(await hasAdb())) return null; + const serials = await listDeviceSerials(); + if (serials.length === 0) return null; + if (!(await isPackageInstalled(APP_PKG))) return null; + return { serial: process.env['ANDROID_SERIAL'] ?? serials[0]! }; +}; + +export const waitFor = async ( + probe: () => Promise, + { timeoutMs = 15_000, intervalMs = 250, label = 'condition' } = {}, +): Promise => { + const deadline = Date.now() + timeoutMs; + let last: unknown; + while (Date.now() < deadline) { + try { + const value = await probe(); + if (value) return value; + last = value; + } catch (e) { + last = e; + } + await new Promise((r) => setTimeout(r, intervalMs)); + } + throw new Error(`timed out waiting for ${label} (last: ${String(last)})`); +}; + +const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +/** + * Open a fixture EPUB in the reader via a VIEW intent (the "Open with" flow, + * which opens the book transiently without touching the user's library) and + * return a CDP session attached to the reader page. + */ +export const openFixtureBook = async (fixtureFile: string): Promise => { + const remote = `${REMOTE_FIXTURE_DIR}/${path.basename(fixtureFile)}`; + await pushFile(fixtureFile, remote); + + // Register the file with MediaStore so a content:// URI exists — receivers + // get a read grant with the intent, so this works without the app holding + // any storage permission (fresh emulator installs). + await adbShell( + `am broadcast -a android.intent.action.MEDIA_SCANNER_SCAN_FILE -d file://${remote}`, + ); + let uri = `file://${remote}`; + try { + // Match by basename: MediaStore stores the canonical + // /storage/emulated/0/... path, not the /sdcard symlink we pushed to. + const basename = path.basename(remote); + const row = await waitFor( + async () => { + const out = await adbShell( + `content query --uri content://media/external/file --projection _id --where "_data LIKE '%/${basename}'"`, + ); + const m = out.match(/_id=(\d+)/); + return m ? m[1] : null; + }, + { timeoutMs: 5_000, label: 'MediaStore row' }, + ); + uri = `content://media/external/file/${row}`; + } catch { + // fall back to the file:// URI (works when the app has storage access) + } + + await adbShell( + `am start -a android.intent.action.VIEW -d "${uri}" -t application/epub+zip ` + + `--grant-read-uri-permission ${APP_PKG}`, + ); + + const target = await waitFor( + async () => { + await forwardWebViewDevtools(APP_PKG, CDP_PORT); + const pages = await listPages(CDP_PORT); + return pages.find((p) => p.type === 'page' && p.url.includes('/reader')); + }, + { timeoutMs: 30_000, intervalMs: 500, label: 'reader page' }, + ); + const page = await CdpPage.connect(CDP_PORT, target.id); + + // Wait until the book is rendered with real text content. + await waitFor( + () => + page.evaluate(` + const view = document.querySelector('foliate-view'); + if (!view || !view.renderer) return false; + const contents = view.renderer.getContents(); + return contents.some((c) => c.doc && (c.doc.body?.textContent ?? '').trim().length > 200); + `), + { timeoutMs: 30_000, intervalMs: 500, label: 'book content rendered' }, + ); + return page; +}; + +export interface WordTarget { + /** Device pixels — pass straight to adb input. */ + deviceX: number; + deviceY: number; + cssX: number; + cssY: number; + onScreen: boolean; +} + +export interface WordRef { + word: string; + /** Character index within the paragraph's first text node. */ + index: number; +} + +export interface SelectionTargets { + /** Identifies the paragraph: leading characters of its first text node. */ + prefix: string; + /** Spine section the paragraph lives in. */ + sectionIndex: number; + firstWord: WordRef; + midWord: WordRef; + /** A word at least two lines below the first one (drag destination). */ + dragWord: WordRef; + /** Pagination position (frame x) where the paragraph is on screen. */ + frameX: number; +} + +/** + * Force hyphenation in every rendered section document. The bug under test + * needs generated hyphens, and the app's hyphenation setting may be off on + * the test device — the injected style makes the lane independent of it. + */ +export const ensureHyphenation = (page: CdpPage) => + page.evaluate(` + const view = document.querySelector('foliate-view'); + let injected = 0; + for (const c of view.renderer.getContents()) { + if (!c.doc || c.doc.getElementById('e2e-hyphens')) continue; + const style = c.doc.createElement('style'); + style.id = 'e2e-hyphens'; + style.textContent = + 'p { -webkit-hyphens: auto !important; hyphens: auto !important; ' + + 'text-align: justify !important; }'; + (c.doc.head ?? c.doc.documentElement).appendChild(style); + injected++; + } + if (injected) await new Promise((r) => setTimeout(r, 400)); + return injected; + `); + +/** + * Navigate to the chapter whose TOC label matches `labelPattern` (case + * insensitive). Returns false when the TOC has no such entry. + */ +export const gotoChapter = async (page: CdpPage, labelPattern: string): Promise => { + const ok = await page.evaluate(` + const view = document.querySelector('foliate-view'); + const flat = []; + const walk = (items) => { + for (const item of items ?? []) { + flat.push(item); + walk(item.subitems); + } + }; + walk(view.book.toc); + const re = new RegExp(${JSON.stringify(labelPattern)}, 'i'); + const match = flat.find((item) => re.test(item.label ?? '')); + if (!match) return false; + await view.goTo(match.href); + return true; + `); + if (ok) { + // Let the section render and settle. + await new Promise((r) => setTimeout(r, 1000)); + } + return ok; +}; + +/** + * Find a paragraph suitable for the hyphen-selection cases on the CURRENT + * page: it must render at least one generated hyphen (auto-hyphenation) and + * start on screen. Returns null when this page has none — callers page + * forward and retry, which makes the lane fixture-agnostic (any English book + * works, e.g. sample-alice.epub). + */ +const findTargetsOnCurrentPage = (page: CdpPage) => + page.evaluate(` + const view = document.querySelector('foliate-view'); + const vw = window.innerWidth; + const vh = window.innerHeight; + const WORD_RE = () => /[A-Za-z][A-Za-z'\\u2019-]*/g; + for (const c of view.renderer.getContents()) { + if (!c.doc) continue; + const win = c.doc.defaultView; + const fe = win.frameElement.getBoundingClientRect(); + for (const p of c.doc.querySelectorAll('p')) { + const tw = c.doc.createTreeWalker(p, NodeFilter.SHOW_TEXT); + let node = null; + let t; + while ((t = tw.nextNode())) { + if (t.data.trim().length > 40) { + node = t; + break; + } + } + if (!node) continue; + + // Generated auto-hyphens show up as an extra sub-glyph-width rect + // appended to a line of a single text node. + const em = parseFloat(win.getComputedStyle(p).fontSize) || 16; + let hasHyphen = false; + const hw = c.doc.createTreeWalker(p, NodeFilter.SHOW_TEXT); + const probe = c.doc.createRange(); + let h; + while (!hasHyphen && (h = hw.nextNode())) { + if (!h.data.trim()) continue; + probe.selectNodeContents(h); + const rects = [...probe.getClientRects()]; + for (let i = 1; i < rects.length; i++) { + const a = rects[i - 1]; + const b = rects[i]; + const sameLine = Math.abs(a.top - b.top) < Math.max(a.height, b.height) / 2; + const adjacent = Math.abs(b.left - (a.left + a.width)) <= 2; + if (sameLine && adjacent && b.width > 0 && b.width <= em * 0.6) { + hasHyphen = true; + break; + } + } + } + if (!hasHyphen) continue; + + const data = node.data; + const re = WORD_RE(); + const first = re.exec(data); + if (!first) continue; + + // The selection must start at the paragraph's first character — + // bail if any non-whitespace content precedes the word. + const before = c.doc.createRange(); + before.selectNodeContents(p); + before.setEnd(node, first.index); + if (before.toString().trim().length > 0) continue; + + const rectOf = (index, length) => { + const r = c.doc.createRange(); + r.setStart(node, index); + r.setEnd(node, index + length); + return r.getBoundingClientRect(); + }; + const firstRect = rectOf(first.index, first[0].length); + const fx = fe.left + firstRect.left + firstRect.width / 2; + const fy = fe.top + firstRect.top + firstRect.height / 2; + if (fx < 0 || fx > vw || fy < 0 || fy > vh) continue; + + const second = re.exec(data); + const third = second ? re.exec(data) : null; + const mid = third ?? second; + if (!mid) continue; + + // Drag destination: a word at least two line-heights below the + // first word, still on screen. + const lineH = firstRect.height || em * 1.5; + let drag = null; + const dre = WORD_RE(); + let m; + while ((m = dre.exec(data))) { + const r = rectOf(m.index, m[0].length); + const cx = fe.left + r.left + r.width / 2; + const cy = fe.top + r.top + r.height / 2; + if (r.top >= firstRect.top + 2 * lineH && cx >= 0 && cx <= vw && cy >= 0 && cy <= vh) { + drag = { word: m[0], index: m.index }; + break; + } + } + if (!drag) continue; + + return { + prefix: data.trim().slice(0, 24), + sectionIndex: c.index ?? 0, + firstWord: { word: first[0], index: first.index }, + midWord: { word: mid[0], index: mid.index }, + dragWord: drag, + frameX: fe.left, + }; + } + } + return null; + `); + +const turnPage = (page: CdpPage, dir: 'next' | 'prev') => + page.evaluate(` + const view = document.querySelector('foliate-view'); + await view.renderer.${dir}(); + await new Promise((r) => setTimeout(r, 200)); + return true; + `); + +/** Page forward from the current position until selection targets are found. */ +export const findSelectionTargets = async (page: CdpPage): Promise => { + for (let i = 0; i < 40; i++) { + // Re-ensure on every step: paging forward can render new sections. + await ensureHyphenation(page); + const targets = await findTargetsOnCurrentPage(page); + if (targets) return targets; + await turnPage(page, 'next'); + } + throw new Error('no hyphenated on-screen paragraph found in the fixture book'); +}; + +/** Locate a word (by its exact character index in the paragraph's first text node). */ +export const locateWord = (page: CdpPage, prefix: string, ref: WordRef) => + page.evaluate(` + const view = document.querySelector('foliate-view'); + for (const c of view.renderer.getContents()) { + if (!c.doc) continue; + const walker = c.doc.createTreeWalker(c.doc.body, NodeFilter.SHOW_TEXT); + let node; + while ((node = walker.nextNode())) { + if (!node.data.trim().startsWith(${JSON.stringify(prefix)})) continue; + if (node.data.slice(${JSON.stringify(ref.index)}, ${ref.index + ref.word.length}) !== + ${JSON.stringify(ref.word)}) continue; + const range = c.doc.createRange(); + range.setStart(node, ${ref.index}); + range.setEnd(node, ${ref.index + ref.word.length}); + const rect = range.getBoundingClientRect(); + const frame = c.doc.defaultView.frameElement.getBoundingClientRect(); + const dpr = window.devicePixelRatio || 1; + const cssX = frame.left + rect.left + rect.width / 2; + const cssY = frame.top + rect.top + rect.height / 2; + return { + deviceX: Math.round(cssX * dpr), + deviceY: Math.round(cssY * dpr), + cssX, + cssY, + onScreen: cssX >= 0 && cssX <= window.innerWidth && cssY >= 0 && cssY <= window.innerHeight, + }; + } + } + return null; + `); + +export interface SelectionState { + exists: boolean; + collapsed: boolean; + text: string; + startOffset: number; + startText: string; +} + +export const getSelectionState = (page: CdpPage) => + page.evaluate(` + const view = document.querySelector('foliate-view'); + for (const c of view.renderer.getContents()) { + const sel = c.doc?.getSelection(); + if (sel && sel.rangeCount > 0) { + const range = sel.getRangeAt(0); + return { + exists: true, + collapsed: sel.isCollapsed, + text: sel.toString(), + startOffset: range.startOffset, + startText: (range.startContainer.data ?? '').slice(0, 40), + }; + } + } + return { exists: false, collapsed: true, text: '', startOffset: -1, startText: '' }; + `); + +/** The app's own selection drag handles (SelectionRangeEditor) in the top document. */ +export const getCustomHandles = (page: CdpPage) => + page.evaluate<{ x: number; y: number }[]>(` + return [...document.querySelectorAll('div.cursor-grab')].map((el) => { + const r = el.getBoundingClientRect(); + return { x: r.x, y: r.y, w: r.width, h: r.height }; + }); + `); + +export interface ReaderMetrics { + dpr: number; + viewWidth: number; + viewHeight: number; + frameX: number; + marginRight: number; + marginBottom: number; +} + +export const getReaderMetrics = (page: CdpPage, sectionIndex?: number) => + page.evaluate(` + const view = document.querySelector('foliate-view'); + const renderer = view.renderer; + const contents = renderer.getContents(); + const content = + contents.find((c) => c.doc && c.index === ${sectionIndex ?? -1}) ?? + contents.find((c) => c.doc); + return { + dpr: window.devicePixelRatio || 1, + viewWidth: window.innerWidth, + viewHeight: window.innerHeight, + frameX: content.doc.defaultView.frameElement.getBoundingClientRect().x, + marginRight: parseFloat(renderer.getAttribute('margin-right')) || 0, + marginBottom: parseFloat(renderer.getAttribute('margin-bottom')) || 0, + }; + `); + +/** Navigate (next/prev) until the rendered section's frame x matches. */ +export const gotoFrameX = (page: CdpPage, frameX: number, sectionIndex?: number) => + page.evaluate(` + const view = document.querySelector('foliate-view'); + const contents = view.renderer.getContents(); + const content = + contents.find((c) => c.doc && c.index === ${sectionIndex ?? -1}) ?? + contents.find((c) => c.doc); + const fx = () => content.doc.defaultView.frameElement.getBoundingClientRect().x; + for (let i = 0; i < 60; i++) { + const x = fx(); + if (Math.abs(x - ${frameX}) < 2) break; + if (x < ${frameX}) await view.renderer.prev(); + else await view.renderer.next(); + await new Promise((r) => setTimeout(r, 150)); + } + return fx(); + `); + +export const clearDomSelection = (page: CdpPage) => + page.evaluate(` + const view = document.querySelector('foliate-view'); + for (const c of view.renderer.getContents()) { + c.doc?.getSelection()?.removeAllRanges(); + } + window.getSelection()?.removeAllRanges(); + return 'cleared'; + `); + +/** Long-press a word and wait until it is selected. */ +export const longPressWord = async ( + page: CdpPage, + prefix: string, + ref: WordRef, +): Promise => { + const target = await waitFor(() => locateWord(page, prefix, ref), { + label: `word "${ref.word}"`, + }); + if (!target.onScreen) throw new Error(`word "${ref.word}" is off-screen`); + await longPress(target.deviceX, target.deviceY); + await waitFor( + async () => { + const sel = await getSelectionState(page); + return sel.exists && !sel.collapsed && sel.text.includes(ref.word) ? sel : null; + }, + { label: `selection of "${ref.word}"` }, + ); + return target; +}; + +/** Tap an empty-ish text spot away from the popup to dismiss the selection. */ +export const dismissSelection = async (page: CdpPage): Promise => { + const { dpr, viewWidth, viewHeight } = await getReaderMetrics(page); + await tap(viewWidth * 0.5 * dpr, viewHeight * 0.78 * dpr); + await waitFor( + async () => { + const handles = await getCustomHandles(page); + const sel = await getSelectionState(page); + return handles.length === 0 && (!sel.exists || sel.collapsed); + }, + { label: 'selection dismissed' }, + ); + await clearDomSelection(page); + await sleep(300); +}; diff --git a/apps/readest-app/src/__tests__/android/selection.android.test.ts b/apps/readest-app/src/__tests__/android/selection.android.test.ts new file mode 100644 index 00000000..f98aa935 --- /dev/null +++ b/apps/readest-app/src/__tests__/android/selection.android.test.ts @@ -0,0 +1,218 @@ +import path from 'node:path'; +import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'vitest'; +import { motionGesture } from './helpers/adb'; +import { CdpPage } from './helpers/cdp'; +import { + clearDomSelection, + detectAndroidEnv, + dismissSelection, + findSelectionTargets, + getCustomHandles, + getReaderMetrics, + getSelectionState, + gotoChapter, + gotoFrameX, + locateWord, + longPressWord, + openFixtureBook, + SelectionTargets, + waitFor, +} from './helpers/reader'; + +// End-to-end coverage for issue #1553 (Blink paints touch-selection bounds on +// generated hyphen fragments): the app must repair corrupted long-press drags, +// replace the native handles with its own for prone selections, and keep +// everything else native. +// +// The fixture is a plain English book; the harness discovers a hyphenated, +// on-screen paragraph at runtime and derives all gesture targets from live +// layout, so the suite is independent of fonts, screen sizes, and the +// fixture's exact text. Runs against any adb device/emulator with Readest +// installed; soft-skips otherwise. + +const FIXTURE = path.resolve(__dirname, '../fixtures/data/sample-alice.epub'); + +const env = await detectAndroidEnv(); +if (!env) { + console.warn('[test:android] no adb device with Readest installed — skipping the Android lane'); +} + +describe.runIf(env)('Android text selection over CDP (#1553)', () => { + let page: CdpPage; + let targets: SelectionTargets; + + beforeAll(async () => { + page = await openFixtureBook(FIXTURE); + // Start the search in the book's main text — front matter rarely has + // full hyphenated paragraphs. Hyphenation itself is forced by the + // harness (ensureHyphenation), so app settings don't matter. + await gotoChapter(page, 'chapter\\s*4'); + targets = await findSelectionTargets(page); + }, 120_000); + + afterAll(() => { + page?.close(); + }); + + beforeEach(async () => { + await gotoFrameX(page, targets.frameX, targets.sectionIndex); + const handles = await getCustomHandles(page); + const sel = await getSelectionState(page); + if (handles.length > 0 || (sel.exists && !sel.collapsed)) { + await dismissSelection(page); + } else { + await clearDomSelection(page); + } + }, 60_000); + + it('selects the first word of a hyphenated paragraph and shows the app handles', async () => { + await longPressWord(page, targets.prefix, targets.firstWord); + + const sel = await getSelectionState(page); + expect(sel.text).toBe(targets.firstWord.word); + + // Native handles are suppressed for this prone selection; the app's own + // drag handles take over (rendered in the top document). + const handles = await waitFor( + async () => { + const h = await getCustomHandles(page); + return h.length === 2 ? h : null; + }, + { label: 'app selection handles' }, + ); + expect(handles).toHaveLength(2); + }); + + it('repairs a long-press drag from the first word to end at the finger', async () => { + const from = await waitFor(() => locateWord(page, targets.prefix, targets.firstWord), { + label: targets.firstWord.word, + }); + const to = await waitFor(() => locateWord(page, targets.prefix, targets.dragWord), { + label: targets.dragWord.word, + }); + + // Long-press, then drag without lifting — the gesture the Blink bug + // corrupts by re-anchoring one end at the paragraph's last hyphen. + await motionGesture([ + { x: from.deviceX, y: from.deviceY, pauseSec: 0.9 }, + { x: (from.deviceX + to.deviceX) / 2, y: (from.deviceY + to.deviceY) / 2, pauseSec: 0.15 }, + { x: to.deviceX, y: to.deviceY, pauseSec: 0.3 }, + ]); + + const sel = await waitFor( + async () => { + const s = await getSelectionState(page); + return s.exists && !s.collapsed ? s : null; + }, + { label: 'drag selection' }, + ); + expect(sel.text.startsWith(targets.firstWord.word)).toBe(true); + // Clamped to the finger: ends at the dragged-to word, not at the + // paragraph's last hyphen (which would overshoot by hundreds of chars). + expect(sel.text.endsWith(targets.dragWord.word)).toBe(true); + }); + + it('dismisses the selection and app handles on a tap outside', async () => { + await longPressWord(page, targets.prefix, targets.firstWord); + await waitFor(async () => (await getCustomHandles(page)).length === 2, { + label: 'app handles', + }); + + // dismissSelection taps away and asserts handles + selection are gone — + // the regression here was a pair of empty handles left at the tap point. + await dismissSelection(page); + expect((await getCustomHandles(page)).length).toBe(0); + }); + + it('extends the selection by dragging the app end handle', async () => { + await longPressWord(page, targets.prefix, targets.firstWord); + const handles = await waitFor( + async () => { + const h = await getCustomHandles(page); + return h.length === 2 ? h : null; + }, + { label: 'app handles' }, + ); + const { dpr } = await getReaderMetrics(page); + const endHandle = handles.reduce((a, b) => (a.y > b.y || (a.y === b.y && a.x > b.x) ? a : b)); + const to = await waitFor(() => locateWord(page, targets.prefix, targets.dragWord), { + label: targets.dragWord.word, + }); + + await motionGesture([ + { x: (endHandle.x + 15) * dpr, y: (endHandle.y + 33) * dpr, pauseSec: 0.3 }, + { x: to.deviceX, y: to.deviceY + 30, pauseSec: 0.2 }, + { x: to.deviceX, y: to.deviceY, pauseSec: 0.3 }, + ]); + + const sel = await waitFor( + async () => { + const s = await getSelectionState(page); + return s.text.length > targets.firstWord.word.length ? s : null; + }, + { label: 'extended selection' }, + ); + expect(sel.text.startsWith(targets.firstWord.word)).toBe(true); + expect(sel.text.endsWith(targets.dragWord.word)).toBe(true); + }); + + it('keeps native selection handles for mid-paragraph selections', async () => { + await longPressWord(page, targets.prefix, targets.midWord); + const sel = await getSelectionState(page); + expect(sel.text).toBe(targets.midWord.word); + + // Give the app a moment to (wrongly) mount its handles, then assert the + // native flow was left alone. + await new Promise((r) => setTimeout(r, 1200)); + expect((await getCustomHandles(page)).length).toBe(0); + }); + + it('keeps the previous page selected across a corner-dwell auto page turn', async () => { + await longPressWord(page, targets.prefix, targets.firstWord); + const handles = await waitFor( + async () => { + const h = await getCustomHandles(page); + return h.length === 2 ? h : null; + }, + { label: 'app handles' }, + ); + const metrics = await getReaderMetrics(page, targets.sectionIndex); + const { dpr, viewWidth, viewHeight, marginRight, marginBottom, frameX } = metrics; + const endHandle = handles.reduce((a, b) => (a.y > b.y || (a.y === b.y && a.x > b.x) ? a : b)); + // A point inside the text area near the bottom-right corner — inside the + // auto-turn corner zone but not in the page margins (which the corner + // detector ignores). + const cornerX = (viewWidth - marginRight - viewWidth * 0.04) * dpr; + const cornerY = (viewHeight - marginBottom - viewHeight * 0.03) * dpr; + + await motionGesture([ + { x: (endHandle.x + 15) * dpr, y: (endHandle.y + 33) * dpr, pauseSec: 0.3 }, + { x: cornerX * 0.7, y: cornerY * 0.7, pauseSec: 0.15 }, + // Dwell in the corner long enough for the auto page turn (500ms). + { x: cornerX, y: cornerY, pauseSec: 1.6 }, + // Keep dragging on the new page so the selection rebuilds after the turn. + { x: viewWidth * 0.7 * dpr, y: viewHeight * 0.55 * dpr, pauseSec: 0.2 }, + { x: viewWidth * 0.55 * dpr, y: viewHeight * 0.45 * dpr, pauseSec: 0.3 }, + ]); + + const turned = await waitFor( + async () => { + const m = await getReaderMetrics(page, targets.sectionIndex); + return m.frameX <= frameX - viewWidth ? m : null; + }, + { label: 'auto page turn' }, + ); + expect(turned.frameX).toBeLessThanOrEqual(frameX - viewWidth); + + // The regression lost the previous page's part of the selection: after + // the turn only current-page text stayed selected. The selection must + // still start at the word the gesture began on. + const sel = await getSelectionState(page); + expect(sel.text.startsWith(targets.firstWord.word)).toBe(true); + expect(sel.text.length).toBeGreaterThan(targets.firstWord.word.length); + + // Restore for any test added after this one. + await dismissSelection(page); + await gotoFrameX(page, targets.frameX, targets.sectionIndex); + }); +}); diff --git a/apps/readest-app/src/__tests__/utils/sel-hyphen-bounds.test.ts b/apps/readest-app/src/__tests__/utils/sel-hyphen-bounds.test.ts new file mode 100644 index 00000000..ae4e4bd1 --- /dev/null +++ b/apps/readest-app/src/__tests__/utils/sel-hyphen-bounds.test.ts @@ -0,0 +1,298 @@ +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { + isRangeStartAtBlockStart, + hasTrailingHyphenRectPattern, + isHyphenHandleBugProneRange, + rangeFromAnchorToPoint, + repairJumpedSelectionRange, +} from '@/utils/sel'; + +// Issue #1553: Android WebView (Blink) records a bogus selection start bound on +// auto-hyphen fragments whenever a touch selection starts at the first character +// of a paragraph. These utilities detect that condition and repair the corrupted +// anchor so the app can suppress the broken native handles. + +const makeParagraph = (html: string) => { + document.body.innerHTML = html; + return document.body.querySelector('p')!; +}; + +const originalGetClientRects = Range.prototype.getClientRects; + +afterEach(() => { + document.body.innerHTML = ''; + vi.restoreAllMocks(); + Range.prototype.getClientRects = originalGetClientRects; +}); + +describe('isRangeStartAtBlockStart', () => { + it('returns true for a range starting at offset 0 of the first text node', () => { + const p = makeParagraph('

Argentina has suffered

'); + const range = document.createRange(); + range.setStart(p.firstChild!, 0); + range.setEnd(p.firstChild!, 9); + expect(isRangeStartAtBlockStart(range)).toBe(true); + }); + + it('returns true when only collapsed whitespace precedes the start', () => { + const p = makeParagraph('

\n Argentina has suffered

'); + const range = document.createRange(); + range.setStart(p.firstChild!, 3); + range.setEnd(p.firstChild!, 12); + expect(isRangeStartAtBlockStart(range)).toBe(true); + }); + + it('returns true when the start is inside a leading inline element', () => { + const p = makeParagraph('

Argentina has suffered

'); + const em = p.querySelector('em')!; + const range = document.createRange(); + range.setStart(em.firstChild!, 0); + range.setEnd(p.lastChild!, 4); + expect(isRangeStartAtBlockStart(range)).toBe(true); + }); + + it('returns false for a mid-paragraph selection', () => { + const p = makeParagraph('

Argentina has suffered

'); + const range = document.createRange(); + range.setStart(p.firstChild!, 10); + range.setEnd(p.firstChild!, 13); + expect(isRangeStartAtBlockStart(range)).toBe(false); + }); + + it('returns false when non-whitespace text precedes in an earlier node', () => { + const p = makeParagraph('

Since the 1970s

'); + const textAfterEm = p.lastChild!; + const range = document.createRange(); + range.setStart(textAfterEm, 1); + range.setEnd(textAfterEm, 4); + expect(isRangeStartAtBlockStart(range)).toBe(false); + }); +}); + +describe('hasTrailingHyphenRectPattern', () => { + const em = 17.5; + const line = (x: number, y: number, w: number, h = 24) => ({ + left: x, + top: y, + width: w, + height: h, + }); + + it('detects a narrow rect appended to the end of a line (auto-hyphen)', () => { + // Mirrors the measured layout of issue #1553: justified lines of ~316px + // with a 9.2px generated hyphen box at the line end. + const rects = [line(737.5, 154, 315.9), line(1053.4, 154, 9.2), line(737.5, 182, 325.1)]; + expect(hasTrailingHyphenRectPattern(rects, em, false)).toBe(true); + }); + + it('returns false when each line is a single rect', () => { + const rects = [line(737.5, 154, 315.9), line(737.5, 182, 325.1), line(737.5, 210, 216.9)]; + expect(hasTrailingHyphenRectPattern(rects, em, false)).toBe(false); + }); + + it('returns false when the narrow rect is on a different line', () => { + const rects = [line(737.5, 154, 315.9), line(737.5, 182, 9.2)]; + expect(hasTrailingHyphenRectPattern(rects, em, false)).toBe(false); + }); + + it('returns false when the same-line rect is too wide to be a hyphen', () => { + // BiDi runs split a line into multiple wide rects. + const rects = [line(737.5, 154, 200), line(937.5, 154, 100)]; + expect(hasTrailingHyphenRectPattern(rects, em, false)).toBe(false); + }); + + it('returns false when the narrow rect is not adjacent to the line end', () => { + const rects = [line(737.5, 154, 200), line(1053.4, 154, 9.2)]; + expect(hasTrailingHyphenRectPattern(rects, em, false)).toBe(false); + }); + + it('detects the pattern in vertical writing mode (axes swapped)', () => { + const rects = [ + { left: 154, top: 737.5, width: 24, height: 315.9 }, + { left: 154, top: 1053.4, width: 24, height: 9.2 }, + ]; + expect(hasTrailingHyphenRectPattern(rects, em, true)).toBe(true); + }); + + it('returns false for empty or single-rect input', () => { + expect(hasTrailingHyphenRectPattern([], em, false)).toBe(false); + expect(hasTrailingHyphenRectPattern([line(0, 0, 300)], em, false)).toBe(false); + }); +}); + +describe('repairJumpedSelectionRange', () => { + const longText = + 'Argentina has suffered from repeated bouts of high inflation throughout its history. ' + + 'Every once in a while, the central bank replaces the existing currency with a new one ' + + 'with zeros removed, which makes it more readable. Think of replacing dollars with a ' + + 'new currency called bollars and declaring one bollar worth one billion of the old dollars.'; + + const setup = () => { + const p = makeParagraph(`

${longText}

`); + const node = p.firstChild as Text; + const sel = window.getSelection()!; + sel.removeAllRanges(); + return { node, sel }; + }; + + it('returns null when the gesture-initial anchor is still inside the range', () => { + const { node, sel } = setup(); + sel.setBaseAndExtent(node, 0, node, 9); + expect(repairJumpedSelectionRange(sel, node, 0)).toBeNull(); + }); + + it('rebuilds [initial anchor → focus] when the anchor jumped forward (issue #1553)', () => { + const { node, sel } = setup(); + // The corrupted state observed on device: the renderer re-anchored the + // base at the last hyphen (offset 325) while the finger/focus is at 53. + sel.setBaseAndExtent(node, 325, node, 53); + const repaired = repairJumpedSelectionRange(sel, node, 0); + expect(repaired).not.toBeNull(); + expect(repaired!.startContainer).toBe(node); + expect(repaired!.startOffset).toBe(0); + expect(repaired!.endContainer).toBe(node); + // Snapped forward to the containing word boundary ("inflation"). + expect(repaired!.endOffset).toBeGreaterThanOrEqual(53); + expect(repaired!.toString().startsWith('Argentina')).toBe(true); + }); + + it('orders the range correctly when the focus precedes the initial anchor', () => { + const { node, sel } = setup(); + // Anchor jumped forward past the initial anchor; the focus stayed before it. + sel.setBaseAndExtent(node, 325, node, 250); + const repaired = repairJumpedSelectionRange(sel, node, 340); + expect(repaired).not.toBeNull(); + expect(repaired!.startOffset).toBeLessThan(repaired!.endOffset); + expect(repaired!.startOffset).toBeLessThanOrEqual(250); + expect(repaired!.endOffset).toBeGreaterThanOrEqual(340); + }); + + it('returns null for a collapsed result or empty selection', () => { + const { node, sel } = setup(); + expect(repairJumpedSelectionRange(sel, node, 0)).toBeNull(); + sel.setBaseAndExtent(node, 325, node, 53); + // focus == initial anchor would collapse; jsdom snaps may expand, so just + // assert it does not throw and returns a range or null. + const result = repairJumpedSelectionRange(sel, node, 53); + if (result) expect(result.collapsed).toBe(false); + }); +}); + +describe('rangeFromAnchorToPoint', () => { + const text = 'Since the mid-1990s, a group of economists have been saying just that.'; + + const setupCaretStub = (node: Text, offset: number) => { + const stub = () => ({ offsetNode: node, offset }); + (document as unknown as Record)['caretPositionFromPoint'] = stub; + }; + + afterEach(() => { + Reflect.deleteProperty(document, 'caretPositionFromPoint'); + }); + + it('builds a forward range from the anchor to the caret at the point', () => { + const p = makeParagraph(`

${text}

`); + const node = p.firstChild as Text; + setupCaretStub(node, 43); + const range = rangeFromAnchorToPoint(document, node, 0, 100, 50); + expect(range).not.toBeNull(); + expect(range!.startOffset).toBe(0); + // Snapped to the word boundary containing offset 43 ("economists"). + expect(range!.endOffset).toBeGreaterThanOrEqual(42); + expect(range!.toString().startsWith('Since')).toBe(true); + }); + + it('orders the range when the point precedes the anchor', () => { + const p = makeParagraph(`

${text}

`); + const node = p.firstChild as Text; + setupCaretStub(node, 10); + const range = rangeFromAnchorToPoint(document, node, 32, 5, 5); + expect(range).not.toBeNull(); + expect(range!.startOffset).toBeLessThan(range!.endOffset); + expect(range!.endOffset).toBe(32); + }); + + it('returns null when the point resolves nowhere or collapses', () => { + const p = makeParagraph(`

${text}

`); + const node = p.firstChild as Text; + // jsdom has no caretPositionFromPoint/caretRangeFromPoint by default. + expect(rangeFromAnchorToPoint(document, node, 0, 10, 10)).toBeNull(); + setupCaretStub(node, 0); + expect(rangeFromAnchorToPoint(document, node, 0, 10, 10)).toBeNull(); + }); +}); + +describe('isHyphenHandleBugProneRange', () => { + const stubLayout = (p: HTMLElement, rects: DOMRect[], hyphens = 'auto') => { + const origGetComputedStyle = window.getComputedStyle.bind(window); + vi.spyOn(window, 'getComputedStyle').mockImplementation((el: Element) => { + const style = origGetComputedStyle(el); + if (el === p) { + const proxy = Object.create(style); + proxy.getPropertyValue = (prop: string) => + prop === 'hyphens' || prop === '-webkit-hyphens' ? hyphens : style.getPropertyValue(prop); + proxy.fontSize = '17.5px'; + return proxy; + } + return style; + }); + // jsdom does not implement Range.getClientRects at all; install one. + Range.prototype.getClientRects = function (this: Range) { + const list = this.startContainer.parentElement === p ? rects : []; + return Object.assign(list.slice(), { + item: (i: number) => list[i] ?? null, + }) as unknown as DOMRectList; + }; + }; + + const hyphenRects = [ + new DOMRect(737.5, 154, 315.9, 24), + new DOMRect(1053.4, 154, 9.2, 24), + new DOMRect(737.5, 182, 325.1, 24), + ]; + + it('detects a first-word selection in a hyphenated paragraph', () => { + const p = makeParagraph('

Argentina has suffered from repeated bouts

'); + stubLayout(p, hyphenRects); + const range = document.createRange(); + range.setStart(p.firstChild!, 0); + range.setEnd(p.firstChild!, 9); + expect(isHyphenHandleBugProneRange(range)).toBe(true); + }); + + it('returns false for a mid-paragraph selection in the same paragraph', () => { + const p = makeParagraph('

Argentina has suffered from repeated bouts

'); + stubLayout(p, hyphenRects); + const range = document.createRange(); + range.setStart(p.firstChild!, 10); + range.setEnd(p.firstChild!, 13); + expect(isHyphenHandleBugProneRange(range)).toBe(false); + }); + + it('returns false when hyphenation is not enabled and no soft hyphens exist', () => { + const p = makeParagraph('

Argentina has suffered from repeated bouts

'); + stubLayout(p, hyphenRects, 'manual'); + const range = document.createRange(); + range.setStart(p.firstChild!, 0); + range.setEnd(p.firstChild!, 9); + expect(isHyphenHandleBugProneRange(range)).toBe(false); + }); + + it('still applies with hyphens:manual when the text carries soft hyphens', () => { + const p = makeParagraph('

Argentina has suf­fered from repeated bouts

'); + stubLayout(p, hyphenRects, 'manual'); + const range = document.createRange(); + range.setStart(p.firstChild!, 0); + range.setEnd(p.firstChild!, 9); + expect(isHyphenHandleBugProneRange(range)).toBe(true); + }); + + it('returns false when the paragraph has no generated hyphen boxes', () => { + const p = makeParagraph('

The traditional explanation for this

'); + stubLayout(p, [new DOMRect(737.5, 530, 325.1, 24), new DOMRect(737.5, 558, 325.1, 24)]); + const range = document.createRange(); + range.setStart(p.firstChild!, 0); + range.setEnd(p.firstChild!, 3); + expect(isHyphenHandleBugProneRange(range)).toBe(false); + }); +}); diff --git a/apps/readest-app/src/app/reader/components/annotator/AnnotationRangeEditor.tsx b/apps/readest-app/src/app/reader/components/annotator/AnnotationRangeEditor.tsx index 434a1556..ce904353 100644 --- a/apps/readest-app/src/app/reader/components/annotator/AnnotationRangeEditor.tsx +++ b/apps/readest-app/src/app/reader/components/annotator/AnnotationRangeEditor.tsx @@ -23,7 +23,7 @@ interface HandleProps { onDragEnd: () => void; } -const Handle: React.FC = ({ +export const Handle: React.FC = ({ hidden, position, isVertical, diff --git a/apps/readest-app/src/app/reader/components/annotator/Annotator.tsx b/apps/readest-app/src/app/reader/components/annotator/Annotator.tsx index 43d162e2..fde714b9 100644 --- a/apps/readest-app/src/app/reader/components/annotator/Annotator.tsx +++ b/apps/readest-app/src/app/reader/components/annotator/Annotator.tsx @@ -63,6 +63,7 @@ import { } from '../../utils/globalAnnotations'; import { annotationToolButtons } from './AnnotationTools'; import AnnotationRangeEditor from './AnnotationRangeEditor'; +import SelectionRangeEditor from './SelectionRangeEditor'; import AnnotationPopup from './AnnotationPopup'; import DictionaryPopup from './DictionaryPopup'; import DictionarySheet from './DictionarySheet'; @@ -284,6 +285,7 @@ const Annotator: React.FC<{ bookKey: string; contentInsets: Insets }> = ({ handleShowPopup, handleUpToPopup, handleContextmenu, + applyProgrammaticSelection, } = useTextSelector( bookKey, contentInsets, @@ -1442,6 +1444,16 @@ const Annotator: React.FC<{ bookKey: string; contentInsets: Insets }> = ({ }} /> )} + {!editingAnnotation && selection?.handlesSuppressed && selection.range && ( + + )} {editingAnnotation && editingAnnotation.color && selection && ( void; + onStartDrag: () => void; +} + +// Drag handles for a plain (not yet annotated) text selection. Used on +// Android when the native selection handles had to be suppressed because of +// the Blink hyphen selection-bounds bug (issue #1553): the DOM selection +// stays as the visible highlight while these handles replace the native +// ones for adjusting the range. +const SelectionRangeEditor: React.FC = ({ + bookKey, + isVertical, + selection, + handleColor, + onRangeChange, + onStartDrag, +}) => { + const { appService } = useEnv(); + const { settings } = useSettingsStore(); + const { isDarkMode } = useThemeStore(); + const { getViewSettings } = useReaderStore(); + const viewSettings = getViewSettings(bookKey); + const isEink = settings.globalViewSettings.isEink; + const einkFgColor = isDarkMode ? '#ffffff' : '#000000'; + const handleColorHex = getHighlightColorHex(settings, handleColor) ?? '#FFFF00'; + + const draggingRef = useRef<'start' | 'end' | null>(null); + const startRef = useRef({ x: 0, y: 0 }); + const endRef = useRef({ x: 0, y: 0 }); + const lastBuiltRef = useRef<{ range: Range; index: number } | null>(null); + const [draggingHandle, setDraggingHandle] = useState<'start' | 'end' | null>(null); + const [currentStart, setCurrentStart] = useState({ x: 0, y: 0 }); + const [currentEnd, setCurrentEnd] = useState({ x: 0, y: 0 }); + const [loupePoint, setLoupePoint] = useState(null); + + useEffect(() => { + if (draggingRef.current) return; + const positions = getHandlePositionsFromRange(bookKey, selection.range, isVertical); + if (positions) { + setCurrentStart(positions.start); + setCurrentEnd(positions.end); + startRef.current = positions.start; + endRef.current = positions.end; + lastBuiltRef.current = { range: selection.range, index: selection.index }; + } + }, [bookKey, selection, isVertical]); + + // The non-dragged end is anchored as a DOM position captured at drag start. + // Anchoring it to its window coordinate instead would silently re-target it + // whenever the content shifts underneath — e.g. the corner-dwell auto page + // turn (#1354) mid-drag — losing the previous page's part of the selection. + const fixedAnchorRef = useRef<{ node: Node; offset: number } | null>(null); + + const updateFromDraggedPoint = useCallback( + (point: Point) => { + const anchor = fixedAnchorRef.current; + if (!anchor) return; + const doc = anchor.node.ownerDocument; + const win = doc?.defaultView; + if (!doc || !win) return; + const feRect = win.frameElement?.getBoundingClientRect(); + const built = rangeFromAnchorToPoint( + doc, + anchor.node, + anchor.offset, + point.x - (feRect?.left ?? 0), + point.y - (feRect?.top ?? 0), + ); + if (!built) return; + lastBuiltRef.current = { range: built, index: selection.index }; + onRangeChange(built, selection.index, false); + }, + [selection.index, onRangeChange], + ); + + const handleStartDragStart = useCallback(() => { + const base = lastBuiltRef.current?.range ?? selection.range; + fixedAnchorRef.current = { node: base.endContainer, offset: base.endOffset }; + draggingRef.current = 'start'; + setDraggingHandle('start'); + setLoupePoint({ ...startRef.current }); + onStartDrag(); + }, [selection, onStartDrag]); + + const handleEndDragStart = useCallback(() => { + const base = lastBuiltRef.current?.range ?? selection.range; + fixedAnchorRef.current = { node: base.startContainer, offset: base.startOffset }; + draggingRef.current = 'end'; + setDraggingHandle('end'); + setLoupePoint({ ...endRef.current }); + onStartDrag(); + }, [selection, onStartDrag]); + + const handleStartDrag = useCallback( + (point: Point) => { + setCurrentStart(point); + setLoupePoint(point); + startRef.current = point; + updateFromDraggedPoint(point); + }, + [updateFromDraggedPoint], + ); + + const handleEndDrag = useCallback( + (point: Point) => { + setCurrentEnd(point); + setLoupePoint(point); + endRef.current = point; + updateFromDraggedPoint(point); + }, + [updateFromDraggedPoint], + ); + + const handleDragEnd = useCallback(() => { + draggingRef.current = null; + setDraggingHandle(null); + setLoupePoint(null); + const last = lastBuiltRef.current; + if (last) { + onRangeChange(last.range, last.index, true); + } + }, [onRangeChange]); + + if (currentStart.x === 0 && currentStart.y === 0) { + return null; + } + + const showLoupe = appService?.isMobile && !viewSettings?.isEink && !viewSettings?.vertical; + + return ( +
+
+ ); +}; + +export default SelectionRangeEditor; diff --git a/apps/readest-app/src/app/reader/hooks/useAnnotationEditor.ts b/apps/readest-app/src/app/reader/hooks/useAnnotationEditor.ts index be50829d..810259a8 100644 --- a/apps/readest-app/src/app/reader/hooks/useAnnotationEditor.ts +++ b/apps/readest-app/src/app/reader/hooks/useAnnotationEditor.ts @@ -1,15 +1,15 @@ import { useCallback, useRef, useState } from 'react'; import { BookNote } from '@/types/book'; -import { Point, TextSelection, snapRangeToWords } from '@/utils/sel'; +import { Point, TextSelection } from '@/utils/sel'; import { useEnv } from '@/context/EnvContext'; import { useReaderStore } from '@/store/readerStore'; import { useSettingsStore } from '@/store/settingsStore'; import { useBookDataStore } from '@/store/bookDataStore'; - -interface HandlePositions { - start: Point; - end: Point; -} +import { + buildRangeFromPoints, + getHandlePositionsFromRange as getHandlePositionsForBook, + HandlePositions, +} from '../utils/annotatorUtil'; interface UseAnnotationEditorProps { bookKey: string; @@ -34,29 +34,8 @@ export const useAnnotationEditor = ({ const [handlePositions, setHandlePositions] = useState(null); const getHandlePositionsFromRange = useCallback( - (range: Range, isVertical: boolean): HandlePositions | null => { - const gridFrame = document.querySelector(`#gridcell-${bookKey}`); - if (!gridFrame) return null; - - const rects = Array.from(range.getClientRects()); - if (rects.length === 0) return null; - - const firstRect = rects[0]!; - const lastRect = rects[rects.length - 1]!; - const frameElement = range.commonAncestorContainer.ownerDocument?.defaultView?.frameElement; - const frameRect = frameElement?.getBoundingClientRect() ?? { top: 0, left: 0 }; - - return { - start: { - x: frameRect.left + (isVertical ? firstRect.right : firstRect.left), - y: frameRect.top + firstRect.top, - }, - end: { - x: frameRect.left + (isVertical ? lastRect.left : lastRect.right), - y: frameRect.top + lastRect.bottom, - }, - }; - }, + (range: Range, isVertical: boolean): HandlePositions | null => + getHandlePositionsForBook(bookKey, range, isVertical), [bookKey], ); @@ -64,75 +43,9 @@ export const useAnnotationEditor = ({ async (startPoint: Point, endPoint: Point, isVertical: boolean, isDragging: boolean) => { if (!editingAnnotationRef.current || !view) return; - const contents = view.renderer.getContents(); - if (!contents || contents.length === 0) return; - - // the point is from viewport, need to adjust to each content's coordinate - const findPositionAtPoint = (doc: Document, x: number, y: number) => { - const frameElement = doc.defaultView?.frameElement; - const frameRect = frameElement?.getBoundingClientRect() ?? { top: 0, left: 0 }; - const adjustedX = x - frameRect.left; - const adjustedY = y - frameRect.top; - - if (doc.caretPositionFromPoint) { - const pos = doc.caretPositionFromPoint(adjustedX, adjustedY); - if (pos) return { node: pos.offsetNode, offset: pos.offset }; - } - if (doc.caretRangeFromPoint) { - const range = doc.caretRangeFromPoint(adjustedX, adjustedY); - if (range) return { node: range.startContainer, offset: range.startOffset }; - } - return null; - }; - - let startPos = null; - let endPos = null; - let targetDoc: Document | null = null; - let targetIndex = 0; - - for (const content of contents) { - const { doc, index } = content; - if (!doc) continue; - - const sp = findPositionAtPoint(doc, startPoint.x, startPoint.y); - const ep = findPositionAtPoint(doc, endPoint.x, endPoint.y); - - if (sp && ep) { - startPos = sp; - endPos = ep; - targetDoc = doc; - targetIndex = index ?? 0; - break; - } - } - - if (!startPos || !endPos || !targetDoc) return; - - const newRange = targetDoc.createRange(); - try { - const positionComparison = startPos.node.compareDocumentPosition(endPos.node); - const needsSwap = - positionComparison & Node.DOCUMENT_POSITION_PRECEDING || - (startPos.node === endPos.node && startPos.offset > endPos.offset); - - if (needsSwap) { - newRange.setStart(endPos.node, endPos.offset); - newRange.setEnd(startPos.node, startPos.offset); - } else { - newRange.setStart(startPos.node, startPos.offset); - newRange.setEnd(endPos.node, endPos.offset); - } - - if (newRange.collapsed) { - console.warn('Range is collapsed'); - return; - } - - snapRangeToWords(newRange); - } catch (e) { - console.warn('Failed to create range:', e); - return; - } + const built = buildRangeFromPoints(view, startPoint, endPoint); + if (!built) return; + const { range: newRange, index: targetIndex } = built; const newPositions = getHandlePositionsFromRange(newRange, isVertical); if (newPositions) { diff --git a/apps/readest-app/src/app/reader/hooks/useTextSelector.ts b/apps/readest-app/src/app/reader/hooks/useTextSelector.ts index a06a07f8..b2b4290e 100644 --- a/apps/readest-app/src/app/reader/hooks/useTextSelector.ts +++ b/apps/readest-app/src/app/reader/hooks/useTextSelector.ts @@ -6,7 +6,14 @@ import { useReaderStore } from '@/store/readerStore'; import { useBookDataStore } from '@/store/bookDataStore'; import { getOSPlatform } from '@/utils/misc'; import { eventDispatcher } from '@/utils/event'; -import { isPointerInsideSelection, Point, TextSelection } from '@/utils/sel'; +import { + isHyphenHandleBugProneRange, + isPointerInsideSelection, + Point, + rangeFromAnchorToPoint, + repairJumpedSelectionRange, + TextSelection, +} from '@/utils/sel'; import { useInstantAnnotation } from './useInstantAnnotation'; const ZERO_INSETS: Insets = { top: 0, right: 0, bottom: 0, left: 0 }; @@ -136,6 +143,30 @@ export const useTextSelector = ( // native touchmove). One of the engagement signals alongside the caret. const pointerPos = useRef<{ x: number; y: number } | null>(null); + // Android hyphen selection-bounds bug (#1553): the selection anchor captured + // at the first selectionchange of a touch gesture, plus whether that initial + // range is prone to the bug (starts at the first word of a hyphenated + // paragraph). Evaluated once per gesture. + const gestureInitialRef = useRef<{ node: Node; offset: number; prone: boolean } | null>(null); + // While we mutate the DOM selection ourselves (handle suppression, custom + // handle drags), selectionchange events are echoes of our own writes — + // handleSelectionchange must ignore them. Cleared on a delay because + // selectionchange dispatches a task after the mutation. + const programmaticSelectionRef = useRef(false); + const programmaticClearTimer = useRef | null>(null); + + const guardProgrammaticSelection = () => { + if (programmaticClearTimer.current) clearTimeout(programmaticClearTimer.current); + programmaticSelectionRef.current = true; + }; + + const releaseProgrammaticSelection = () => { + if (programmaticClearTimer.current) clearTimeout(programmaticClearTimer.current); + programmaticClearTimer.current = setTimeout(() => { + programmaticSelectionRef.current = false; + }, 150); + }; + const { isInstantAnnotationEnabled, handleInstantAnnotationPointerDown, @@ -154,7 +185,12 @@ export const useTextSelector = ( return sel && sel.toString().trim().length > 0 && sel.rangeCount > 0; }; - const makeSelection = async (sel: Selection, index: number, rebuildRange = false) => { + const makeSelection = async ( + sel: Selection, + index: number, + rebuildRange = false, + handlesSuppressed = false, + ) => { isTextSelected.current = true; const range = sel.getRangeAt(0); if (rebuildRange) { @@ -169,6 +205,7 @@ export const useTextSelector = ( page: bookData?.isFixedLayout ? index + 1 : progress?.page || 0, range, index, + handlesSuppressed, }); }; @@ -277,6 +314,87 @@ export const useTextSelector = ( } }; + // Android (#1553): when a touch selection starts on the first word of a + // hyphenated paragraph, Blink paints the start handle on the paragraph's + // last hyphen and drag gestures re-anchor the selection base there. At + // gesture end: restore the intended range if the anchor jumped, then clear + // and re-add the selection through the Selection API — a selection that goes + // empty for one painted frame loses its touch-handle visibility, so the + // broken native handles disappear (the app's own handles take over) while + // the highlight stays. + const sanitizedGestureRef = useRef(false); + const sanitizeAndroidHyphenSelection = async (doc: Document, index: number) => { + if (sanitizedGestureRef.current) return; + const sel = doc.getSelection(); + const win = doc.defaultView; + if (!sel || !win || !isValidSelection(sel)) return; + // Only act when THIS gesture produced a selection — a tap that merely + // dismisses an existing selection must not re-assert it here. + const initial = gestureInitialRef.current; + if (!initial) return; + const viewSettings = getViewSettings(bookKey); + // The initial range is prone (long-press on the first word), or the + // gesture dragged the selection start back onto a hyphenated paragraph + // start (upward selection) — either way the painted start bound is bogus. + const prone = + initial.prone || isHyphenHandleBugProneRange(sel.getRangeAt(0), viewSettings?.vertical); + if (!prone) return; + sanitizedGestureRef.current = true; + let finalRange = sel.getRangeAt(0); + if (initial.prone) { + // The corrupted drag can leave EITHER selection end at the bogus bound + // (base re-anchor or extent overshoot). The trustworthy facts are the + // gesture-initial anchor and the finger position (pointerPos, reset at + // touchstart and fed by native touchmove): rebuild between those when + // the gesture moved; otherwise fall back to the anchor-jump repair. + const p = pointerPos.current; + const feRect = win.frameElement?.getBoundingClientRect(); + const clamped = p + ? rangeFromAnchorToPoint( + doc, + initial.node, + initial.offset, + p.x - (feRect?.left ?? 0), + p.y - (feRect?.top ?? 0), + ) + : null; + const repaired = clamped ?? repairJumpedSelectionRange(sel, initial.node, initial.offset); + if (repaired) finalRange = repaired; + } + guardProgrammaticSelection(); + sel.removeAllRanges(); + await new Promise((resolve) => + win.requestAnimationFrame(() => win.requestAnimationFrame(() => resolve())), + ); + // A competing gesture may have touched the selection while we waited — + // e.g. a tap collapses the old selection to a caret at the tapped point, + // which can also mutate `finalRange` in place. Re-asserting then would + // resurrect a stale (or collapsed) selection, so bail out instead. + if (sel.rangeCount > 0 || finalRange.collapsed) { + releaseProgrammaticSelection(); + return; + } + sel.addRange(finalRange); + releaseProgrammaticSelection(); + await makeSelection(sel, index, false, true); + }; + + // Replace the live DOM selection from the custom selection handles. Guarded + // so the resulting selectionchange echoes are ignored; a commit refreshes + // the selection state (and thus the popup) once the drag ends. + const applyProgrammaticSelection = async (range: Range, index: number, commit: boolean) => { + const doc = range.startContainer.ownerDocument; + const sel = doc?.getSelection(); + if (!doc || !sel) return; + guardProgrammaticSelection(); + sel.removeAllRanges(); + sel.addRange(range); + if (commit) { + releaseProgrammaticSelection(); + await makeSelection(sel, index, false, true); + } + }; + const handlePointerUp = async (doc: Document, index: number, ev?: PointerEvent) => { if (isInstantAnnotating.current && ev) { stopInstantAnnotating(ev); @@ -316,9 +434,18 @@ export const useTextSelector = ( isUpToPopup.current = false; } } + + if (osPlatform === 'android' && appService?.isAndroidApp) { + await sanitizeAndroidHyphenSelection(doc, index); + } }; const handleTouchStart = () => { isTouchStarted.current = true; + gestureInitialRef.current = null; + sanitizedGestureRef.current = false; + // Pointer positions are per-gesture: a stale point from a previous touch + // must not steer this gesture's selection repair. Touch moves re-feed it. + pointerPos.current = null; }; const handleTouchMove = (ev: TouchEvent) => { if (isInstantAnnotating.current) { @@ -391,6 +518,10 @@ export const useTextSelector = ( }; const handleSelectionchange = (doc: Document, index: number) => { + // Echo of our own programmatic selection writes (handle suppression or a + // custom-handle drag) — not user input. + if (programmaticSelectionRef.current) return; + // Available on iOS, Android and Desktop, fired when the selection is changed. // On Android native app, this is the primary way to detect text selection. // On web with touch/pen in scroll mode, pointerup never fires (pointercancel @@ -401,6 +532,17 @@ export const useTextSelector = ( const sel = doc.getSelection() as Selection; const viewSettings = getViewSettings(bookKey); + // First selection of an Android touch gesture: remember the anchor and + // whether it is prone to the hyphen bounds bug (#1553), before any + // drag-extension can corrupt it. + if (isAndroid && !gestureInitialRef.current && isValidSelection(sel) && sel.anchorNode) { + gestureInitialRef.current = { + node: sel.anchorNode, + offset: sel.anchorOffset, + prone: isHyphenHandleBugProneRange(sel.getRangeAt(0), viewSettings?.vertical), + }; + } + // Auto page-turn (#1354): the selection caret is one of the engagement // signals on every platform (and the only one on Android during a native // selection drag, where pointer/touch-move don't fire). Feed it into the same @@ -520,5 +662,6 @@ export const useTextSelector = ( handleShowPopup, handleUpToPopup, handleContextmenu, + applyProgrammaticSelection, }; }; diff --git a/apps/readest-app/src/app/reader/utils/annotatorUtil.ts b/apps/readest-app/src/app/reader/utils/annotatorUtil.ts index a24175e7..e5e2a4ca 100644 --- a/apps/readest-app/src/app/reader/utils/annotatorUtil.ts +++ b/apps/readest-app/src/app/reader/utils/annotatorUtil.ts @@ -3,7 +3,7 @@ import { BookNote, DEFAULT_HIGHLIGHT_COLORS, HighlightColor, HighlightStyle } fr import { uniqueId } from '@/utils/misc'; import { SystemSettings } from '@/types/settings'; import { FoliateView, NOTE_PREFIX } from '@/types/view'; -import { Point } from '@/utils/sel'; +import { Point, snapRangeToWords } from '@/utils/sel'; export const isDefaultHighlightColor = ( color: HighlightColor, @@ -65,6 +65,113 @@ export function toParentViewportPoint(doc: Document, x: number, y: number): Poin return { x: x + frameRect.left, y: y + frameRect.top }; } +export interface HandlePositions { + start: Point; + end: Point; +} + +/** + * Window-coordinate positions for a pair of range-edit drag handles: the + * start handle anchors at the leading edge of the range's first line rect, + * the end handle at the trailing edge of its last one. + */ +export function getHandlePositionsFromRange( + bookKey: string, + range: Range, + isVertical: boolean, +): HandlePositions | null { + const gridFrame = document.querySelector(`#gridcell-${bookKey}`); + if (!gridFrame) return null; + + const rects = Array.from(range.getClientRects()); + if (rects.length === 0) return null; + + const firstRect = rects[0]!; + const lastRect = rects[rects.length - 1]!; + const frameElement = range.commonAncestorContainer.ownerDocument?.defaultView?.frameElement; + const frameRect = frameElement?.getBoundingClientRect() ?? { top: 0, left: 0 }; + + return { + start: { + x: frameRect.left + (isVertical ? firstRect.right : firstRect.left), + y: frameRect.top + firstRect.top, + }, + end: { + x: frameRect.left + (isVertical ? lastRect.left : lastRect.right), + y: frameRect.top + lastRect.bottom, + }, + }; +} + +/** + * Build a word-snapped Range between two window-coordinate points by + * hit-testing every rendered section document. Returns the document and + * section index the range landed in, or `null` when neither point maps + * into the same document. + */ +export function buildRangeFromPoints( + view: FoliateView | null, + startPoint: Point, + endPoint: Point, +): { range: Range; index: number; doc: Document } | null { + const contents = view?.renderer.getContents(); + if (!contents || contents.length === 0) return null; + + // the point is from viewport, need to adjust to each content's coordinate + const findPositionAtPoint = (doc: Document, x: number, y: number) => { + const frameElement = doc.defaultView?.frameElement; + const frameRect = frameElement?.getBoundingClientRect() ?? { top: 0, left: 0 }; + const adjustedX = x - frameRect.left; + const adjustedY = y - frameRect.top; + + if (doc.caretPositionFromPoint) { + const pos = doc.caretPositionFromPoint(adjustedX, adjustedY); + if (pos) return { node: pos.offsetNode, offset: pos.offset }; + } + if (doc.caretRangeFromPoint) { + const range = doc.caretRangeFromPoint(adjustedX, adjustedY); + if (range) return { node: range.startContainer, offset: range.startOffset }; + } + return null; + }; + + for (const content of contents) { + const { doc, index } = content; + if (!doc) continue; + + const startPos = findPositionAtPoint(doc, startPoint.x, startPoint.y); + const endPos = findPositionAtPoint(doc, endPoint.x, endPoint.y); + if (!startPos || !endPos) continue; + + const range = doc.createRange(); + try { + const positionComparison = startPos.node.compareDocumentPosition(endPos.node); + const needsSwap = + positionComparison & Node.DOCUMENT_POSITION_PRECEDING || + (startPos.node === endPos.node && startPos.offset > endPos.offset); + + if (needsSwap) { + range.setStart(endPos.node, endPos.offset); + range.setEnd(startPos.node, startPos.offset); + } else { + range.setStart(startPos.node, startPos.offset); + range.setEnd(endPos.node, endPos.offset); + } + + if (range.collapsed) { + return null; + } + + snapRangeToWords(range); + } catch (e) { + console.warn('Failed to create range:', e); + return null; + } + return { range, index: index ?? 0, doc }; + } + return null; +} + /** * Remove any overlays drawn for a BookNote from the given view. * diff --git a/apps/readest-app/src/utils/sel.ts b/apps/readest-app/src/utils/sel.ts index 0e2177b3..82d99e0f 100644 --- a/apps/readest-app/src/utils/sel.ts +++ b/apps/readest-app/src/utils/sel.ts @@ -32,6 +32,9 @@ export interface TextSelection { href?: string; annotated?: boolean; rect?: Rect; + // Native Android selection handles were suppressed for this selection + // (Blink hyphen bounds bug, issue #1553) — the app draws its own handles. + handlesSuppressed?: boolean; } const frameRect = (frame: Frame, rect?: Rect, sx = 1, sy = 1) => { @@ -419,6 +422,218 @@ export const snapRangeToWords = (range: Range): void => { snapEndToWordBoundary(); }; +// --- Android hyphenation selection-bounds bug (issue #1553) ----------------- +// +// Blink's `LayoutSelection::ComputePaintingSelectionStateForCursor` compares +// the selection's paragraph text-content offsets against each fragment's +// `TextOffset()`. Auto/soft-hyphen fragments are layout-generated text whose +// offsets are self-relative ({0,1}), so a touch selection starting at the +// first character of a paragraph marks EVERY hyphen fragment in it as a +// selection start: the native start handle is painted on the paragraph's last +// hyphen and drag gestures re-anchor the selection base there. The helpers +// below detect that condition so the app can repair the range and suppress +// the broken native handles (touch handles are the only consumer of the bogus +// bounds, so mouse/desktop selections are unaffected). + +const isInlineDisplay = (el: Element): boolean => { + const display = el.ownerDocument.defaultView?.getComputedStyle(el).display ?? ''; + return display === 'inline' || display.startsWith('ruby'); +}; + +// The element establishing the inline formatting context the range starts in. +const getBlockAncestor = (node: Node): Element | null => { + let el: Element | null = + node.nodeType === Node.ELEMENT_NODE ? (node as Element) : node.parentElement; + while (el && isInlineDisplay(el)) { + el = el.parentElement; + } + return el; +}; + +// Whether the range starts at the first character of its paragraph's inline +// content (leading collapsed whitespace does not count: it never reaches the +// paragraph's laid-out text, so the selection still maps to offset 0). +export const isRangeStartAtBlockStart = (range: Range): boolean => { + const block = getBlockAncestor(range.startContainer); + if (!block) return false; + const probe = (block.ownerDocument ?? document).createRange(); + try { + probe.selectNodeContents(block); + probe.setEnd(range.startContainer, range.startOffset); + } catch { + return false; + } + return probe.toString().trim().length === 0; +}; + +// A generated hyphen is the only way a single text node produces two adjacent +// boxes on the same line where the trailing one is sub-glyph narrow. +const HYPHEN_MAX_EM = 0.6; +const HYPHEN_ADJACENCY_PX = 2; + +interface RectLike { + left: number; + top: number; + width: number; + height: number; +} + +export const hasTrailingHyphenRectPattern = ( + rects: RectLike[], + emPx: number, + vertical: boolean, +): boolean => { + for (let i = 1; i < rects.length; i++) { + const a = rects[i - 1]!; + const b = rects[i]!; + const sameLine = vertical + ? Math.abs(a.left - b.left) < Math.max(a.width, b.width) / 2 + : Math.abs(a.top - b.top) < Math.max(a.height, b.height) / 2; + if (!sameLine) continue; + const adjacent = vertical + ? Math.abs(b.top - (a.top + a.height)) <= HYPHEN_ADJACENCY_PX + : Math.abs(b.left - (a.left + a.width)) <= HYPHEN_ADJACENCY_PX || + Math.abs(a.left - (b.left + b.width)) <= HYPHEN_ADJACENCY_PX; + if (!adjacent) continue; + const size = vertical ? b.height : b.width; + if (size > 0 && size <= emPx * HYPHEN_MAX_EM) return true; + } + return false; +}; + +const blockHasGeneratedHyphens = (block: Element, vertical: boolean): boolean => { + const doc = block.ownerDocument; + const win = doc.defaultView; + if (!win) return false; + const emPx = parseFloat(win.getComputedStyle(block).fontSize) || 16; + const walker = doc.createTreeWalker(block, NodeFilter.SHOW_TEXT); + const probe = doc.createRange(); + let node: Node | null; + while ((node = walker.nextNode())) { + if (!(node as Text).data.trim()) continue; + let rects: RectLike[]; + try { + probe.selectNodeContents(node); + rects = Array.from(probe.getClientRects()); + } catch { + return false; + } + if (hasTrailingHyphenRectPattern(rects, emPx, vertical)) return true; + } + return false; +}; + +// Whether painting this selection hits the Blink generated-hyphen bounds bug: +// it starts at the first character of a paragraph that renders hyphens. +export const isHyphenHandleBugProneRange = (range: Range, vertical = false): boolean => { + const block = getBlockAncestor(range.startContainer); + if (!block) return false; + const win = block.ownerDocument.defaultView; + if (!win) return false; + const style = win.getComputedStyle(block); + const mayHyphenate = + style.getPropertyValue('hyphens') === 'auto' || + style.getPropertyValue('-webkit-hyphens') === 'auto' || + (block.textContent ?? '').includes('­'); + if (!mayHyphenate) return false; + if (!isRangeStartAtBlockStart(range)) return false; + return blockHasGeneratedHyphens(block, vertical); +}; + +// Rebuild a selection range between a known-good anchor and the caret at a +// point (in `doc` viewport coordinates) — used to restore the range a +// corrupted long-press drag was meant to produce: anchored at the +// gesture-initial position, ending where the finger was. Snapped to word +// boundaries like the native word-granularity drag. +export const rangeFromAnchorToPoint = ( + doc: Document, + anchorNode: Node, + anchorOffset: number, + x: number, + y: number, +): Range | null => { + let pointNode: Node | null = null; + let pointOffset = 0; + if (doc.caretPositionFromPoint) { + const pos = doc.caretPositionFromPoint(x, y); + if (pos) { + pointNode = pos.offsetNode; + pointOffset = pos.offset; + } + } else if (doc.caretRangeFromPoint) { + const range = doc.caretRangeFromPoint(x, y); + if (range) { + pointNode = range.startContainer; + pointOffset = range.startOffset; + } + } + if (!pointNode) return null; + const range = doc.createRange(); + try { + const anchorPoint = doc.createRange(); + anchorPoint.setStart(anchorNode, anchorOffset); + anchorPoint.collapse(true); + const caretPoint = doc.createRange(); + caretPoint.setStart(pointNode, pointOffset); + caretPoint.collapse(true); + if (anchorPoint.compareBoundaryPoints(Range.START_TO_START, caretPoint) <= 0) { + range.setStart(anchorNode, anchorOffset); + range.setEnd(pointNode, pointOffset); + } else { + range.setStart(pointNode, pointOffset); + range.setEnd(anchorNode, anchorOffset); + } + } catch { + return null; + } + if (range.collapsed) return null; + snapRangeToWords(range); + return range; +}; + +// During a long-press drag the bug re-anchors the selection base at the last +// hyphen, dropping the word the gesture started on. If the gesture-initial +// anchor fell out of the final range, rebuild [initial anchor → focus] (the +// focus is where the finger actually went) and snap it to word boundaries +// like the native word-granularity drag would. +export const repairJumpedSelectionRange = ( + sel: Selection, + initialNode: Node, + initialOffset: number, +): Range | null => { + if (sel.rangeCount === 0) return null; + const range = sel.getRangeAt(0); + const { focusNode, focusOffset } = sel; + const doc = range.startContainer.ownerDocument; + if (!focusNode || !doc) return null; + try { + if (range.comparePoint(initialNode, initialOffset) === 0) return null; + } catch { + return null; + } + const repaired = doc.createRange(); + try { + const initialPoint = doc.createRange(); + initialPoint.setStart(initialNode, initialOffset); + initialPoint.collapse(true); + const focusPoint = doc.createRange(); + focusPoint.setStart(focusNode, focusOffset); + focusPoint.collapse(true); + if (initialPoint.compareBoundaryPoints(Range.START_TO_START, focusPoint) <= 0) { + repaired.setStart(initialNode, initialOffset); + repaired.setEnd(focusNode, focusOffset); + } else { + repaired.setStart(focusNode, focusOffset); + repaired.setEnd(initialNode, initialOffset); + } + } catch { + return null; + } + if (repaired.collapsed) return null; + snapRangeToWords(repaired); + return repaired; +}; + export const getTextFromRange = (range: Range, rejectTags: string[] = []): string => { const clonedRange = range.cloneRange(); const fragment = clonedRange.cloneContents(); diff --git a/apps/readest-app/vitest.android.config.mts b/apps/readest-app/vitest.android.config.mts new file mode 100644 index 00000000..04c2726f --- /dev/null +++ b/apps/readest-app/vitest.android.config.mts @@ -0,0 +1,22 @@ +import { defineConfig } from 'vitest/config'; +import tsconfigPaths from 'vite-tsconfig-paths'; + +// Android device lane: drives the installed Readest app on an adb-connected +// device or emulator through the WebView's Chrome DevTools Protocol plus adb +// input gestures. Tests run on the host in a plain node environment — the +// "browser" under test is the app itself. +export default defineConfig({ + plugins: [tsconfigPaths()], + test: { + environment: 'node', + include: ['src/**/*.android.test.ts'], + // One device, one app instance: never parallelize. + fileParallelism: false, + maxConcurrency: 1, + sequence: { concurrent: false }, + // Real gestures + page turns are slow; native gestures can be flaky once. + testTimeout: 120_000, + hookTimeout: 120_000, + retry: 1, + }, +}); diff --git a/apps/readest-app/vitest.config.mts b/apps/readest-app/vitest.config.mts index dbf5d251..f84c0ed4 100644 --- a/apps/readest-app/vitest.config.mts +++ b/apps/readest-app/vitest.config.mts @@ -32,6 +32,8 @@ export default defineConfig({ '**/*.browser.test.ts', '**/*.browser.test.tsx', '**/*.tauri.test.ts', + // Android device e2e — run via `pnpm test:android`, not the unit lane. + '**/*.android.test.ts', ], coverage: { provider: 'v8',