From 1f4e0432ca6432efb9c2b98e3531fbe388bcaabb Mon Sep 17 00:00:00 2001 From: Hunter Bown Date: Thu, 20 Aug 2026 16:08:38 -0700 Subject: [PATCH] fix(web): highlight search matches using the source string's own indices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Defect: both search surfaces carried an identical `highlight()` that took `idx` from `text.toLowerCase()` and then sliced `text` with it: const lower = text.toLowerCase(); const idx = lower.indexOf(q); text.slice(idx, idx + q.length) That assumes lowercasing preserves length. It does not — `"İ"` (U+0130, Turkish dotted capital I) lowercases to two code units, so every index after one is off by one. Searching "stanbul" in "İstanbul kurulumu" marks "tanbul " instead of "stanbul": the highlight starts and ends one character late. Turkish is a routed locale (`tr`), and the search haystack is where localized copy is headed. Fix: `highlightSpan()` in lib/search-utils.ts — the module that exists so the search components' pure logic can be unit-tested — folds case one character at a time and keeps a position map, so the three returned pieces are cut at real character boundaries and always reassemble the input exactly. Both components now call it instead of repeating the arithmetic. Scope, honestly: no string in the current haystack contains a length-changing character, so this is a latent bug rather than an observed one. It is still wrong output for valid input, and it was wrong in two places. Evidence: lib/search-utils.test.ts. Restoring the lowercased-index implementation fails "keeps indices in the source string when lowercasing changes length" with `expected 'tanbul ' to be 'stanbul'`. The last case pins both components onto the shared rule. npm test 327 passed, npm run lint clean, npx tsc --noEmit clean. Implemented with agent assistance. Signed-off-by: Hunter Bown --- web/components/docs-search.tsx | 18 +++++------ web/components/faq-search.tsx | 17 +++++----- web/lib/search-utils.test.ts | 48 ++++++++++++++++++++++++++++ web/lib/search-utils.ts | 57 ++++++++++++++++++++++++++++++++++ 4 files changed, 123 insertions(+), 17 deletions(-) diff --git a/web/components/docs-search.tsx b/web/components/docs-search.tsx index 34fb6db55..adc666e13 100644 --- a/web/components/docs-search.tsx +++ b/web/components/docs-search.tsx @@ -8,7 +8,7 @@ import { docTopicIsExternal, type DocTopic, } from "@/lib/docs-map"; -import { docTopicHaystack } from "@/lib/search-utils"; +import { docTopicHaystack, highlightSpan } from "@/lib/search-utils"; /* ------------------------------------------------------------------ */ /* Locale-aware strings */ @@ -43,16 +43,16 @@ const topicHaystack = docTopicHaystack; /* ------------------------------------------------------------------ */ function highlight(text: string, query: string): React.ReactNode { - const q = query.trim().toLowerCase(); - if (!q) return text; - const lower = text.toLowerCase(); - const idx = lower.indexOf(q); - if (idx === -1) return text; + // Index arithmetic lives in search-utils: lowercasing can change a + // string's length, so `text` cannot be sliced with indices taken from + // its lowercased copy. + const span = highlightSpan(text, query); + if (!span) return text; return ( <> - {text.slice(0, idx)} - {text.slice(idx, idx + q.length)} - {text.slice(idx + q.length)} + {span.before} + {span.match} + {span.after} ); } diff --git a/web/components/faq-search.tsx b/web/components/faq-search.tsx index 1937378a8..e09426feb 100644 --- a/web/components/faq-search.tsx +++ b/web/components/faq-search.tsx @@ -3,6 +3,7 @@ import { useState, useMemo, useRef, useCallback, useEffect } from "react"; import { faqSourceHref } from "@/lib/faq-source"; import { extractText } from "@/lib/react-text"; +import { highlightSpan } from "@/lib/search-utils"; export interface FaqSearchItem { q: string; @@ -15,16 +16,16 @@ export interface FaqSearchItem { /* ------------------------------------------------------------------ */ function highlight(text: string, query: string): React.ReactNode { - const q = query.trim().toLowerCase(); - if (!q) return text; - const lower = text.toLowerCase(); - const idx = lower.indexOf(q); - if (idx === -1) return text; + // Index arithmetic lives in search-utils: lowercasing can change a + // string's length, so `text` cannot be sliced with indices taken from + // its lowercased copy. + const span = highlightSpan(text, query); + if (!span) return text; return ( <> - {text.slice(0, idx)} - {text.slice(idx, idx + q.length)} - {text.slice(idx + q.length)} + {span.before} + {span.match} + {span.after} ); } diff --git a/web/lib/search-utils.test.ts b/web/lib/search-utils.test.ts index f1f7dc71d..a83b28d66 100644 --- a/web/lib/search-utils.test.ts +++ b/web/lib/search-utils.test.ts @@ -1,7 +1,9 @@ import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; import { docTopicHaystack, filterDocTopics, + highlightSpan, normalizeQuery, matches, } from "./search-utils"; @@ -129,3 +131,49 @@ describe("filterDocTopics", () => { expect(ids).toContain("install"); }); }); + +describe("highlightSpan", () => { + const webRoot = new URL("../", import.meta.url); + const read = (p: string) => readFileSync(new URL(p, webRoot), "utf8"); + + it("splits a plain match into three reassembling pieces", () => { + const span = highlightSpan("Install Guide", "install")!; + expect(span).toEqual({ before: "", match: "Install", after: " Guide" }); + expect(span.before + span.match + span.after).toBe("Install Guide"); + }); + + it("returns null for an empty query or no match", () => { + expect(highlightSpan("Install", "")).toBeNull(); + expect(highlightSpan("Install", " ")).toBeNull(); + expect(highlightSpan("Install", "docker")).toBeNull(); + expect(highlightSpan("", "install")).toBeNull(); + }); + + it("keeps indices in the source string when lowercasing changes length", () => { + // "İ".toLowerCase() is two code units, so a lowercased copy of this + // string is one longer than the string itself. Index arithmetic done on + // the copy highlighted "tanbul " instead of "stanbul". + const text = "İstanbul kurulumu"; + expect(text.toLowerCase().length).toBe(text.length + 1); + const span = highlightSpan(text, "stanbul")!; + expect(span.match).toBe("stanbul"); + expect(span.before).toBe("İ"); + expect(span.after).toBe(" kurulumu"); + expect(span.before + span.match + span.after).toBe(text); + }); + + it("never claims half of a source character", () => { + const span = highlightSpan("İstanbul", "i")!; + expect(span.match).toBe("İ"); + expect(span.before + span.match + span.after).toBe("İstanbul"); + }); + + it("is the one match rule both search surfaces use", () => { + for (const file of ["components/docs-search.tsx", "components/faq-search.tsx"]) { + const source = read(file); + expect(source, file).toContain("highlightSpan(text, query)"); + expect(source, file).not.toContain("text.toLowerCase()"); + expect(source, file).not.toContain("text.slice("); + } + }); +}); diff --git a/web/lib/search-utils.ts b/web/lib/search-utils.ts index 0428e38de..f14895194 100644 --- a/web/lib/search-utils.ts +++ b/web/lib/search-utils.ts @@ -64,3 +64,60 @@ export function matches(haystack: string, query: string): boolean { if (!q) return true; return haystack.toLowerCase().includes(q); } + +/** The three pieces a highlighted match splits a string into. */ +export interface HighlightSpan { + before: string; + match: string; + after: string; +} + +/** + * Locate `query` inside `text`, case-insensitively, in `text`'s own indices. + * + * The obvious form — `text.toLowerCase().indexOf(q)`, then slicing `text` + * with that index — assumes lowercasing preserves length. It does not: + * `"İ".toLowerCase()` is two code units, so every index after a dotted + * capital I in the haystack is off by one and the highlight lands on the + * wrong characters. Turkish is a routed locale, so this is reachable the + * moment localized copy enters the search haystack. + * + * Lowercasing character by character and keeping a position map costs one + * pass and keeps the three returned pieces exactly reassembling `text`. + * Returns null when there is no match (including an empty query). + */ +export function highlightSpan(text: string, query: string): HighlightSpan | null { + const q = normalizeQuery(query); + if (!q) return null; + + let lower = ""; + // For each code unit of `lower`: where its source character starts and ends. + const sourceStart: number[] = []; + const sourceEnd: number[] = []; + for (let i = 0; i < text.length; ) { + const char = String.fromCodePoint(text.codePointAt(i)!); + const next = i + char.length; + const folded = char.toLowerCase(); + for (let k = 0; k < folded.length; k++) { + sourceStart.push(i); + sourceEnd.push(next); + } + lower += folded; + i = next; + } + + const idx = lower.indexOf(q); + if (idx === -1) return null; + + const start = sourceStart[idx]; + const stop = idx + q.length; + // A match ending inside one source character's expansion cannot claim half + // of that character; take the whole character rather than nothing. + const end = stop < lower.length ? Math.max(sourceStart[stop], sourceEnd[idx]) : text.length; + + return { + before: text.slice(0, start), + match: text.slice(start, end), + after: text.slice(end), + }; +}