From 8dfc0e945e1f524ebdc05aff5ebb3200172f3174 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Sun, 17 May 2026 02:52:48 +0800 Subject: [PATCH] fix(dictionary): normalize lookup query with trim + case fallback (#4192) A double-click selection can carry trailing whitespace and most imported dictionaries store headwords lowercased, so an exact match on the raw selection often misses (e.g. `Hello` or `world ` fail to resolve `hello`/`world`). Case-sensitive formats like mdict are hit hardest since their reader compares the raw word. Seed the lookup history with a trimmed word and try ordered query variants (trimmed, lowercase, title-case, uppercase) per provider, keeping the first hit. Closes #4176. Co-authored-by: Claude Opus 4.7 (1M context) --- .../reader/annotator/DictionarySheet.test.tsx | 56 +++++++++++++++++++ .../dictionaries/lookupCandidates.test.ts | 32 +++++++++++ .../annotator/DictionaryResultsView.tsx | 39 ++++++++----- .../services/dictionaries/lookupCandidates.ts | 22 ++++++++ 4 files changed, 135 insertions(+), 14 deletions(-) create mode 100644 apps/readest-app/src/__tests__/services/dictionaries/lookupCandidates.test.ts create mode 100644 apps/readest-app/src/services/dictionaries/lookupCandidates.ts diff --git a/apps/readest-app/src/__tests__/app/reader/annotator/DictionarySheet.test.tsx b/apps/readest-app/src/__tests__/app/reader/annotator/DictionarySheet.test.tsx index f3c23df0..de2a087d 100644 --- a/apps/readest-app/src/__tests__/app/reader/annotator/DictionarySheet.test.tsx +++ b/apps/readest-app/src/__tests__/app/reader/annotator/DictionarySheet.test.tsx @@ -195,6 +195,23 @@ const buildEmptyProvider = (id: string, label: string): DictionaryProvider => ({ }, }); +// A case-sensitive, whitespace-sensitive provider that only resolves the +// exact stored headword — mimics mdict's lookup path. Used to assert the +// query-normalization fallback (trim + case folding). +const buildExactProvider = (storedHeadword: string): DictionaryProvider => { + const provider: DictionaryProvider = { + id: 'exact', + kind: 'mdict', + label: 'Exact Match', + async lookup(word, ctx) { + if (word !== storedHeadword) return { ok: false, reason: 'empty' }; + ctx.container.append(document.createTextNode(`def for ${word}`)); + return { ok: true, headword: word, sourceLabel: 'Exact Match' }; + }, + }; + return provider; +}; + // --------------------------------------------------------------------------- // Sheet harness // --------------------------------------------------------------------------- @@ -293,6 +310,45 @@ describe('DictionarySheet — concurrent lookup', () => { }); }); +describe('DictionarySheet — query normalization', () => { + it('resolves a lowercase-stored entry from a capitalized selection', async () => { + const exact = buildExactProvider('hello'); + const spy = vi.spyOn(exact, 'lookup'); + providersForNextRender.push(exact); + renderSheet({ word: 'Hello' }); + + await screen.findByText('Exact Match'); + // First probe is the selection as-is; the lowercase fallback hits. + expect(spy).toHaveBeenCalledWith('Hello', expect.any(Object)); + expect(spy).toHaveBeenCalledWith('hello', expect.any(Object)); + }); + + it('resolves an entry from a selection with trailing whitespace', async () => { + providersForNextRender.push(buildExactProvider('world')); + renderSheet({ word: 'world ' }); + + await screen.findByText('Exact Match'); + }); + + it('trims trailing whitespace from the displayed title', async () => { + providersForNextRender.push(buildExactProvider('world')); + renderSheet({ word: 'world ' }); + + expect((await screen.findByTestId('dict-title')).textContent).toBe('world'); + }); + + it('stops at the first matching variant without probing the rest', async () => { + const exact = buildExactProvider('hello'); + const spy = vi.spyOn(exact, 'lookup'); + providersForNextRender.push(exact); + renderSheet({ word: 'hello' }); + + await screen.findByText('Exact Match'); + // 'hello' hits immediately — no title-case / upper-case probes follow. + expect(spy).toHaveBeenCalledTimes(1); + }); +}); + describe('DictionarySheet — expand / collapse', () => { it('toggles aria-expanded when a card is tapped', async () => { providersForNextRender.push(buildRealStarDictProvider()); diff --git a/apps/readest-app/src/__tests__/services/dictionaries/lookupCandidates.test.ts b/apps/readest-app/src/__tests__/services/dictionaries/lookupCandidates.test.ts new file mode 100644 index 00000000..feaea3e5 --- /dev/null +++ b/apps/readest-app/src/__tests__/services/dictionaries/lookupCandidates.test.ts @@ -0,0 +1,32 @@ +import { describe, it, expect } from 'vitest'; + +import { buildLookupCandidates } from '@/services/dictionaries/lookupCandidates'; + +describe('buildLookupCandidates', () => { + it('returns the lowercase word for an already-lowercase selection', () => { + expect(buildLookupCandidates('hello')).toEqual(['hello', 'Hello', 'HELLO']); + }); + + it('trims leading/trailing whitespace from a double-click selection', () => { + expect(buildLookupCandidates('world ')).toEqual(['world', 'World', 'WORLD']); + expect(buildLookupCandidates(' spaced ')).toEqual(['spaced', 'Spaced', 'SPACED']); + }); + + it('offers a lowercase variant for a sentence-initial capitalized word', () => { + expect(buildLookupCandidates('Hello')).toEqual(['Hello', 'hello', 'HELLO']); + }); + + it('offers lowercase and title-case variants for an all-caps selection', () => { + expect(buildLookupCandidates('HELLO')).toEqual(['HELLO', 'hello', 'Hello']); + }); + + it('de-duplicates collapsed variants', () => { + // A single lowercase letter collapses to one unique candidate. + expect(buildLookupCandidates('a')).toEqual(['a', 'A']); + }); + + it('returns an empty list for a blank selection', () => { + expect(buildLookupCandidates('')).toEqual([]); + expect(buildLookupCandidates(' ')).toEqual([]); + }); +}); diff --git a/apps/readest-app/src/app/reader/components/annotator/DictionaryResultsView.tsx b/apps/readest-app/src/app/reader/components/annotator/DictionaryResultsView.tsx index 1fdfadbf..02f670e3 100644 --- a/apps/readest-app/src/app/reader/components/annotator/DictionaryResultsView.tsx +++ b/apps/readest-app/src/app/reader/components/annotator/DictionaryResultsView.tsx @@ -10,6 +10,7 @@ import { useEnv } from '@/context/EnvContext'; import { useThemeStore } from '@/store/themeStore'; import { useCustomDictionaryStore } from '@/store/customDictionaryStore'; import { getEnabledProviders } from '@/services/dictionaries/registry'; +import { buildLookupCandidates } from '@/services/dictionaries/lookupCandidates'; import { isTauriAppPlatform } from '@/services/environment'; import { getBuiltinWebSearch, @@ -84,13 +85,14 @@ export function useDictionaryResults({ const definitionProviders = useMemo(() => providers.filter((p) => p.kind !== 'web'), [providers]); const webSearchProviders = useMemo(() => providers.filter((p) => p.kind === 'web'), [providers]); - const [historyStack, setHistoryStack] = useState([word]); - const currentWord = historyStack[historyStack.length - 1] ?? word; + const [historyStack, setHistoryStack] = useState([word.trim()]); + const currentWord = historyStack[historyStack.length - 1] ?? word.trim(); // Reset the history when the host reopens with a new word from outside - // (selection change in the reader). + // (selection change in the reader). A double-click selection can carry + // trailing whitespace, so trim before seeding. useEffect(() => { - setHistoryStack([word]); + setHistoryStack([word.trim()]); }, [word]); const [cards, setCards] = useState>({}); @@ -209,16 +211,25 @@ export function useDictionaryResults({ if (!container) { outcome = { ok: false, reason: 'error', message: 'no container' }; } else { - container.replaceChildren(); - outcome = await provider.lookup(currentWord, { - lang: langCode, - signal: controller.signal, - container, - onNavigate: pushWord, - isDarkMode, - bg: themeCode.bg, - fg: themeCode.fg, - }); + // Try normalized query variants (trimmed, case-folded) in + // priority order and keep the first hit. Case-sensitive + // formats (mdict) otherwise miss `Hello` / `world ` style + // selections whose headword is stored lowercased. + outcome = { ok: false, reason: 'empty' }; + for (const candidate of buildLookupCandidates(currentWord)) { + container.replaceChildren(); + outcome = await provider.lookup(candidate, { + lang: langCode, + signal: controller.signal, + container, + onNavigate: pushWord, + isDarkMode, + bg: themeCode.bg, + fg: themeCode.fg, + }); + if (controller.signal.aborted) return; + if (outcome.ok || outcome.reason !== 'empty') break; + } } } catch (err) { outcome = { diff --git a/apps/readest-app/src/services/dictionaries/lookupCandidates.ts b/apps/readest-app/src/services/dictionaries/lookupCandidates.ts new file mode 100644 index 00000000..72278951 --- /dev/null +++ b/apps/readest-app/src/services/dictionaries/lookupCandidates.ts @@ -0,0 +1,22 @@ +/** + * Ordered, de-duplicated query variants for a dictionary lookup. + * + * A double-click selection in the reader can carry leading/trailing + * whitespace, and most imported dictionaries store headwords lowercased, so + * an exact match on the raw selection often misses — e.g. `Hello` or + * `world ` fail to resolve `hello`/`world`. The DICT/StarDict/slob readers + * already compare case-insensitively, but case-sensitive formats (mdict) do + * not. Callers try each candidate in order and keep the first hit. + * + * Variants, in priority order: the trimmed selection as-is, all-lowercase, + * title-case, all-uppercase (for acronym headwords). Returns `[]` for a + * blank input. + */ +export const buildLookupCandidates = (word: string): string[] => { + const trimmed = word.trim(); + if (!trimmed) return []; + const lower = trimmed.toLowerCase(); + const title = trimmed.charAt(0).toUpperCase() + lower.slice(1); + const upper = trimmed.toUpperCase(); + return [...new Set([trimmed, lower, title, upper])]; +};