forked from akai/readest
fix(dict): stop iOS instant system dictionary popping multiple times (#4644)
On iOS a single long-press emits several selectionchange events, so the instant quick action fired the system dictionary 2-3 times, stacking UIReferenceLibraryViewController sheets. Add a once-per-gesture latch to deferredAction (re-armed by beginGesture on touchstart/pointerdown) so the action runs at most once per gesture, mirroring the Android defer-to-touchend coalescing. Also fix two related cases: - Tapping outside to deselect after dismissing the dictionary occasionally re-opened it (~1/3): the deselect tap re-armed the latch and a racy lingering selectionchange re-fired. Gate the instant action on a long-press hold (isLongPressHold, 300ms, touch only) so a quick tap can't trigger it. - A Word Lens gloss tap ignored the system-dictionary setting (always opened the in-app popup); route it through handleDictionary so it honors the system dictionary like the toolbar and instant-quick-action paths. Verified on Android (Xiaomi) that the instant dictionary still fires once per long-press and re-arms for the next gesture; iOS double-popup and tap-to-deselect re-fire confirmed fixed. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,8 +1,9 @@
|
||||
import { describe, test, expect, vi } from 'vitest';
|
||||
import {
|
||||
cancelDeferredAction,
|
||||
beginGesture,
|
||||
createDeferredActionState,
|
||||
flushDeferredAction,
|
||||
isLongPressHold,
|
||||
runOrDeferAction,
|
||||
} from '@/app/reader/utils/deferredAction';
|
||||
|
||||
@@ -54,20 +55,64 @@ describe('deferredAction', () => {
|
||||
expect(second).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
test('cancelDeferredAction discards a pending action without running it', () => {
|
||||
test('beginGesture discards a pending action without running it', () => {
|
||||
const state = createDeferredActionState();
|
||||
const action = vi.fn();
|
||||
|
||||
runOrDeferAction(state, true, action);
|
||||
cancelDeferredAction(state);
|
||||
beginGesture(state);
|
||||
flushDeferredAction(state);
|
||||
|
||||
expect(action).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('runs the action only once per gesture even when triggered repeatedly', () => {
|
||||
// iOS long-press emits multiple selectionchange events for the same
|
||||
// selection; each runs immediately (shouldDefer false). Only the first
|
||||
// should fire so e.g. the system-dictionary sheet is presented once.
|
||||
const state = createDeferredActionState();
|
||||
const action = vi.fn();
|
||||
|
||||
runOrDeferAction(state, false, action);
|
||||
runOrDeferAction(state, false, action);
|
||||
runOrDeferAction(state, false, action);
|
||||
|
||||
expect(action).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
test('beginGesture re-arms the action for the next gesture', () => {
|
||||
const state = createDeferredActionState();
|
||||
const action = vi.fn();
|
||||
|
||||
runOrDeferAction(state, false, action);
|
||||
runOrDeferAction(state, false, action); // same gesture: blocked
|
||||
expect(action).toHaveBeenCalledTimes(1);
|
||||
|
||||
beginGesture(state); // pointerdown/touchstart of the next gesture
|
||||
runOrDeferAction(state, false, action);
|
||||
expect(action).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
test('iOS long-press scenario: multiple selectionchange events look up the word once', () => {
|
||||
// Models the Annotator flow on iOS (no native-touch deferral):
|
||||
// 1. pointerdown -> beginGesture (re-arm)
|
||||
// 2. selectionchange -> runOrDeferAction(false) -> dictionary lookup
|
||||
// 3. selectionchange -> runOrDeferAction(false) -> blocked (same gesture)
|
||||
// 4. selectionchange -> runOrDeferAction(false) -> blocked (same gesture)
|
||||
const state = createDeferredActionState();
|
||||
const lookup = vi.fn();
|
||||
|
||||
beginGesture(state); // pointerdown
|
||||
runOrDeferAction(state, false, lookup); // selectionchange #1
|
||||
runOrDeferAction(state, false, lookup); // selectionchange #2 (echo)
|
||||
runOrDeferAction(state, false, lookup); // selectionchange #3 (echo)
|
||||
|
||||
expect(lookup).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
test('Android long-press scenario: selection-then-touchend runs the action once', () => {
|
||||
// Models the Annotator flow on Android:
|
||||
// 1. touchstart -> androidTouchEnd = false (defer state cleared)
|
||||
// 1. touchstart -> beginGesture (defer state cleared + re-armed)
|
||||
// 2. selectionchange -> handleQuickAction (deferred because !androidTouchEnd)
|
||||
// 3. touchend -> androidTouchEnd = true, flushDeferredAction
|
||||
const state = createDeferredActionState();
|
||||
@@ -75,7 +120,7 @@ describe('deferredAction', () => {
|
||||
let androidTouchEnd = false;
|
||||
|
||||
// touchstart
|
||||
cancelDeferredAction(state);
|
||||
beginGesture(state);
|
||||
androidTouchEnd = false;
|
||||
|
||||
// selectionchange triggers the quick action
|
||||
@@ -90,5 +135,33 @@ describe('deferredAction', () => {
|
||||
// A subsequent stray touchend must not re-run the action
|
||||
flushDeferredAction(state);
|
||||
expect(quickAction).toHaveBeenCalledTimes(1);
|
||||
|
||||
// An iOS-style echo (immediate trigger) after touchend within the same
|
||||
// gesture must not re-run it either.
|
||||
runOrDeferAction(state, false, quickAction);
|
||||
expect(quickAction).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isLongPressHold', () => {
|
||||
const MIN = 300;
|
||||
|
||||
test('no recorded pointerdown (e.g. mouse) always qualifies', () => {
|
||||
expect(isLongPressHold(0, 999_999, MIN)).toBe(true);
|
||||
});
|
||||
|
||||
test('a held long-press (elapsed >= threshold) qualifies', () => {
|
||||
// iOS surfaces a touch selection only after the ~500ms OS long-press.
|
||||
expect(isLongPressHold(1000, 1000 + 500, MIN)).toBe(true);
|
||||
});
|
||||
|
||||
test('exactly at the threshold qualifies', () => {
|
||||
expect(isLongPressHold(1000, 1000 + MIN, MIN)).toBe(true);
|
||||
});
|
||||
|
||||
test('a quick tap (elapsed < threshold) does NOT qualify', () => {
|
||||
// The tap-to-deselect bug: a tap re-reports the lingering selection a few
|
||||
// tens of ms after its pointerdown — must not fire the instant action.
|
||||
expect(isLongPressHold(1000, 1000 + 40, MIN)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -38,9 +38,10 @@ import { eventDispatcher } from '@/utils/event';
|
||||
import { findTocItemBS } from '@/services/nav';
|
||||
import { throttle } from '@/utils/throttle';
|
||||
import {
|
||||
cancelDeferredAction,
|
||||
beginGesture,
|
||||
createDeferredActionState,
|
||||
flushDeferredAction,
|
||||
isLongPressHold,
|
||||
runOrDeferAction,
|
||||
} from '../../utils/deferredAction';
|
||||
import { Insets } from '@/types/misc';
|
||||
@@ -174,6 +175,10 @@ const Annotator: React.FC<{ bookKey: string; contentInsets: Insets }> = ({
|
||||
// (Android long-press selects text via selectionchange before touchend). The
|
||||
// pending action runs on touchend so popups don't open under an active touch.
|
||||
const deferredQuickActionRef = useRef(createDeferredActionState());
|
||||
// Timestamp of the latest touch pointerdown (0 for mouse). Used to require a
|
||||
// long-press hold before the instant quick action fires, so a tap-to-deselect
|
||||
// can't re-open the dictionary off a racy lingering selectionchange (iOS).
|
||||
const pointerDownTimeRef = useRef(0);
|
||||
// Set when a Word Lens gloss tap synthesizes a selection so the
|
||||
// selection-change effect opens the dictionary popup instead of the
|
||||
// annotation toolbar. Cleared as soon as it's consumed.
|
||||
@@ -353,7 +358,7 @@ const Annotator: React.FC<{ bookKey: string; contentInsets: Insets }> = ({
|
||||
const ev = event.detail as NativeTouchEventType;
|
||||
if (ev.type === 'touchstart') {
|
||||
androidTouchEndRef.current = false;
|
||||
cancelDeferredAction(deferredQuickActionRef.current);
|
||||
beginGesture(deferredQuickActionRef.current);
|
||||
handleTouchStart();
|
||||
} else if (ev.type === 'touchmove') {
|
||||
// The Android pointer engagement signal (throttled in MainActivity.kt).
|
||||
@@ -382,6 +387,25 @@ const Annotator: React.FC<{ bookKey: string; contentInsets: Insets }> = ({
|
||||
detail.doc?.addEventListener('touchstart', handleTouchStart, opts);
|
||||
detail.doc?.addEventListener('touchmove', handleTouchmove, opts);
|
||||
detail.doc?.addEventListener('touchend', handleTouchEnd);
|
||||
// Re-arm the instant quick action at the start of each gesture. Android does
|
||||
// this via the native-touch touchstart above; iOS/desktop have no such path,
|
||||
// and a single iOS long-press emits multiple selectionchange events for the
|
||||
// same word — without re-arming, the system-dictionary sheet stacked twice
|
||||
// (the action fired once per event instead of once per gesture).
|
||||
if (!appService?.isAndroidApp) {
|
||||
detail.doc?.addEventListener(
|
||||
'pointerdown',
|
||||
(ev: Event) => {
|
||||
beginGesture(deferredQuickActionRef.current);
|
||||
// Remember when the gesture started so the instant quick action can
|
||||
// require a long-press hold (touch only — mouse selections fire on
|
||||
// pointerup and shouldn't be time-gated).
|
||||
pointerDownTimeRef.current =
|
||||
(ev as PointerEvent).pointerType === 'mouse' ? 0 : Date.now();
|
||||
},
|
||||
opts,
|
||||
);
|
||||
}
|
||||
detail.doc?.addEventListener('pointerdown', handlePointerDown.bind(null, doc, index), opts);
|
||||
detail.doc?.addEventListener('pointermove', handlePointerMove.bind(null, doc, index), opts);
|
||||
detail.doc?.addEventListener('pointercancel', handlePointerCancel.bind(null, doc, index));
|
||||
@@ -775,7 +799,21 @@ const Annotator: React.FC<{ bookKey: string; contentInsets: Insets }> = ({
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, []);
|
||||
|
||||
// A real touch selection only appears after the OS long-press (~500ms); a
|
||||
// quick tap that re-reports a lingering selection fires far sooner.
|
||||
const quickActionMinHoldMs = 300;
|
||||
|
||||
const handleQuickAction = () => {
|
||||
// iOS/desktop immediate path: only fire from a long-press hold. Without this
|
||||
// a tap-to-deselect after dismissing the system dictionary occasionally
|
||||
// re-opened it off a racy lingering selectionchange. Android defers to
|
||||
// touchend (a deliberate lift) and is left as-is.
|
||||
if (
|
||||
!appService?.isAndroidApp &&
|
||||
!isLongPressHold(pointerDownTimeRef.current, Date.now(), quickActionMinHoldMs)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
const action = viewSettings.annotationQuickAction;
|
||||
const runAction = () => {
|
||||
switch (action) {
|
||||
@@ -867,8 +905,10 @@ const Annotator: React.FC<{ bookKey: string; contentInsets: Insets }> = ({
|
||||
|
||||
const { enableAnnotationQuickActions, annotationQuickAction } = viewSettings;
|
||||
if (wantWordLensDict) {
|
||||
setShowAnnotPopup(false);
|
||||
setShowDictionaryPopup(true);
|
||||
// Route through handleDictionary so a Word Lens gloss tap honours the
|
||||
// dictionary settings (system dictionary vs the in-app popup) — same
|
||||
// as the selection-toolbar and instant-quick-action dictionary paths.
|
||||
handleDictionary();
|
||||
} else if (enableAnnotationQuickActions && annotationQuickAction && isTextSelected.current) {
|
||||
handleQuickAction();
|
||||
} else {
|
||||
|
||||
@@ -1,14 +1,32 @@
|
||||
// Tiny helper for actions that need to wait for an asynchronous "ready" signal
|
||||
// before running. Used by the Annotator on Android: a long-press selects text
|
||||
// while the finger is still down, but the quick action (translate/dictionary/
|
||||
// tts/etc.) must not fire until the user lifts (touchend), or the popup it
|
||||
// opens would be dismissed by the in-progress touch.
|
||||
// Tiny helper for the Annotator's instant quick action (translate/dictionary/
|
||||
// tts/etc.). It solves two related touch-gesture problems:
|
||||
//
|
||||
// * On Android a long-press selects text via selectionchange while the finger
|
||||
// is still down, so the action is deferred until touchend — otherwise the
|
||||
// popup it opens is dismissed by the in-progress touch (closes #3935).
|
||||
// * A single long-press can emit MULTIPLE selectionchange events for the same
|
||||
// selection. iOS is the worst offender: it re-confirms the native selection
|
||||
// after our deselect(), firing the action again. Running per event stacked
|
||||
// duplicate popups — e.g. two/three system-dictionary sheets on iOS. The
|
||||
// `fired` latch makes the action run at most once per gesture; `beginGesture`
|
||||
// re-arms it at the next touchstart/pointerdown.
|
||||
|
||||
export interface DeferredActionState {
|
||||
pending: (() => void) | null;
|
||||
// Whether the quick action has already run for the current gesture.
|
||||
fired: boolean;
|
||||
}
|
||||
|
||||
export const createDeferredActionState = (): DeferredActionState => ({ pending: null });
|
||||
export const createDeferredActionState = (): DeferredActionState => ({
|
||||
pending: null,
|
||||
fired: false,
|
||||
});
|
||||
|
||||
const runOnce = (state: DeferredActionState, action: () => void): void => {
|
||||
if (state.fired) return;
|
||||
state.fired = true;
|
||||
action();
|
||||
};
|
||||
|
||||
export const runOrDeferAction = (
|
||||
state: DeferredActionState,
|
||||
@@ -20,16 +38,32 @@ export const runOrDeferAction = (
|
||||
return;
|
||||
}
|
||||
state.pending = null;
|
||||
action();
|
||||
runOnce(state, action);
|
||||
};
|
||||
|
||||
export const flushDeferredAction = (state: DeferredActionState): void => {
|
||||
const fn = state.pending;
|
||||
if (!fn) return;
|
||||
state.pending = null;
|
||||
fn();
|
||||
runOnce(state, fn);
|
||||
};
|
||||
|
||||
export const cancelDeferredAction = (state: DeferredActionState): void => {
|
||||
// Start of a new touch gesture: drop any pending action and re-arm the latch so
|
||||
// the next action can run. Call on touchstart/pointerdown.
|
||||
export const beginGesture = (state: DeferredActionState): void => {
|
||||
state.pending = null;
|
||||
state.fired = false;
|
||||
};
|
||||
|
||||
// A touch instant quick action must only fire from a long-press hold, never a
|
||||
// quick tap. After the system dictionary is dismissed iOS re-selects the word;
|
||||
// tapping outside to deselect it is a WebView pointerdown that re-arms the latch,
|
||||
// and a racy selectionchange can re-report the lingering selection a few tens of
|
||||
// ms later — which used to re-open the dictionary (~1/3 of taps). A genuine iOS
|
||||
// touch selection only appears after the OS long-press (~500ms), so we require
|
||||
// at least `minHoldMs` to have elapsed since the gesture's pointerdown.
|
||||
//
|
||||
// `pointerDownTime === 0` means no touch pointerdown was recorded (e.g. mouse on
|
||||
// desktop), where this gate does not apply and the action always qualifies.
|
||||
export const isLongPressHold = (pointerDownTime: number, now: number, minHoldMs: number): boolean =>
|
||||
pointerDownTime === 0 || now - pointerDownTime >= minHoldMs;
|
||||
|
||||
Reference in New Issue
Block a user