diff --git a/.sync/PORTING.md b/.sync/PORTING.md index 8e2b5958..c468fce7 100644 --- a/.sync/PORTING.md +++ b/.sync/PORTING.md @@ -296,6 +296,20 @@ material. Reproduce its *intent* in b24ui by editing files under `src/` only. cases fail against the upstream shape. Not reported upstream, so expect their version to keep the defect and expect the conflict on every port that touches it. +- **`useContentSearch` hands `suffix` and `description` over raw.** Upstream + escapes `<` and `>` on both by hand — verified still present on their `v4` at + `6add5fb7`, and our own port log for `a1bef8ba` records the lines arriving + verbatim. It is a *second* escape: `CommandPalette` renders both through + `{{ }}`, a text node, which never decodes entities, so a pre-escaped `<` + reaches the reader as those four characters; and where a match lands, + `highlight()` escapes the escaped value again into `&lt;`, which `v-html` + decodes one level back to the same wrong output (#406). Restoring it during a + port reads as tidy-up in a diff, which is how it survived from the original + port. `label` in the same functions was always raw, and that is the shape to + match: escaping belongs to the sink, never to the mapper. Guarded by + `test/composables/useContentSearch.spec.ts`, whose rendering cases mount what + `mapFile` returns rather than a hand-built item — built the other way round + first, and a revert passed them. - **`skills/` is b24ui-authored — never replay upstream skill or doc prose into it.** The package was seeded from nuxt/ui's skill, and every defect the #93 audit found was an inherited upstream idiom rather than an ordinary typo: @@ -499,3 +513,4 @@ forward, since every commit between the two would then never be judged. - 2026-08-14 — closed #380 (PR #399): decided the icon-dictionary question and replaced the §2 invariant that had been holding it open. The old rule said the five size-variant sites were deliberate and must not be "fixed"; the decision went the other way — `Badge`, `SidebarLayout`, `Checkbox` and `Button` now read `close`/`check`/`minus`/`chevronDown` from the dictionary, and the glyphs visibly change (60 Checkbox/CheckboxGroup snapshots, plus two `renderEach` cases added because `useClose` and `useDropdown` had none, so those two swaps were previously unpinned by any test). The quieter half of #380 — components importing the *same* glyph directly, where nothing looks wrong but the override still does not reach them — was fixed in `FormField`, `SidebarLayout`, `prose/Card` and `prose/CodeIcon`, and left in three places with reasons recorded in the guard's `ALLOWED` table. Worth correcting the issue's framing while closing it: it reported "49 hardcoded paths, half the library", which is arithmetically right and misleading — **41 of the 49 are file-type glyphs in `prose/CodeIcon.vue`** for syntax highlighting, which no dictionary should own. Outside that file the real surface was 8 paths in 5 components. `test/utils/icon-dictionary.spec.ts` now enforces the decision in both directions and fails on a stale exception as well as a new bypass; three mutations verified. One recurrence to note: writing `icons.loading` inside a comment explaining why `Button` does *not* use it tripped `icon-claims.spec.ts`, which cannot tell a comment from a promise — the same mistake made earlier in this file's history, and the reason the §2 bullet now says so explicitly. Last reviewed: 2026-08-14. - 2026-08-15 — ports of `731ff26`…`a4ab81a` (PRs #403, #404, #407): added §6 step **4b**, that a PR may batch commits only when they are **contiguous**. Learned by breaking it in this run. The four upstream commits interleave two subjects — `731ff26` (Theme `class` merge), `4a3168f` (calendar template), `0fabbe5` (slot-class replacer), `a4ab81a` (calendar lists) — and batching the two calendar no-ops into one PR skipped over `0fabbe5`, which sits between them. The cursor went to `a4ab81a`, then processing `0fabbe5` moved it **backwards**, leaving the ledger one commit behind upstream HEAD with all four entries present and correct. Nothing failed and nothing was lost; the next sync check simply reported a commit that was already done. Fixed in the closing bookkeeping PR by restoring the topological key order and setting `cursor` to `a4ab81a`. Worth distinguishing from the batch in #395, which was fine: `9b08a84` and `edf73d3` are adjacent. Also from this run, without needing a rule: `0fabbe5`'s patch would not apply as a patch — this fork's `twMergeConfig` shifts `tv.ts` by 54 lines and the context carries `b24ui` — but diffing our file against upstream's **pre-image** with `b24ui` rewritten to `ui` showed the replacer machinery is line-for-line identical, which is what made hand-applying the hunks safe rather than hopeful. That diff-against-the-pre-image check is the cheap way to tell a real divergence from a cosmetic one before touching anything. Last reviewed: 2026-08-15. - 2026-08-15 — corrections after #390 and #405, plus one new §2 invariant. Four claims in the `utils/search.ts` bullets were false, and each was falsifiable, which is how each was caught: that every constant and branch had been mutation-verified (#390 found six survivors), that the unpaired-surrogate fixtures catch a *widened* bound (only where the probe sits one code point outside it — two of four sat `0x100` away), that a pickaxe on `useTokenSearch` returns exactly one commit (it counts occurrences, so `54b93e33`'s jsDoc line joined the list), and that only 16 of ~3200 commits carry an `Upstream:` trailer — 52 do, and 52 of 3179 did when the sentence was written, so it was never right. That last one is load-bearing: it is the stated reason the trailer cannot support a provenance inference, and the conclusion survives the correction while the number does not. The first claim is now stated as intent with the procedure spelled out, because a prose claim about test coverage decays silently — nothing fails when it stops being true. Also softened the unreproducible `8 of 66` figure, updated `createClusterSnapper`'s signature after #388, added the missing `describe('truncation from the start')` guard, and named `getGraphemeSegmenter()`'s module-level memo — a second cache, distinct from the per-value view, documented nowhere and covered by no test. New invariant: **`sanitizeSnippet` splits on the tag** (#391, PR #405) — upstream's placeholder round-trip lets a snippet forge `` from its own input, so a port that replays upstream reverts the fix. Not reported upstream. Last reviewed: 2026-08-15. +- 2026-08-16 — fix of #406 (PR #414): added the §2 **`useContentSearch` hands `suffix` and `description` over raw** invariant. Upstream escapes `<` and `>` on both by hand and still does — checked their `v4` at `6add5fb7` — and our port log for `a1bef8ba` shows the lines arriving verbatim, so a faithful replay reverts the fix. It is a second escape on top of the one that renders: `{{ }}` builds a text node, which never decodes entities, so `<` reached the reader as four characters; the `v-html` sibling doubles it into `&lt;` and decodes one level back to the same place. Guarded by `test/composables/useContentSearch.spec.ts`, the composable's first tests. Worth recording how the guard nearly failed to guard: its rendering cases first built palette items by hand, never touching `mapFile`, so re-adding the escaping passed them — they now mount what the mapper actually returns. Last reviewed: 2026-08-16. diff --git a/src/runtime/composables/useContentSearch.ts b/src/runtime/composables/useContentSearch.ts index 7b3717e2..96df47f5 100644 --- a/src/runtime/composables/useContentSearch.ts +++ b/src/runtime/composables/useContentSearch.ts @@ -26,7 +26,13 @@ function _useContentSearch() { return { prefix: prefix?.length ? (prefix.join(' > ') + ' >') : undefined, label: file.id === link.path ? link.title : file.title, - suffix: file.content.replaceAll('<', '<').replaceAll('>', '>'), + // Raw, like `label` beside it. Escaping here is not belt-and-braces, it is + // a second escape on top of the one that renders: `CommandPalette` puts + // this through `{{ }}`, a text node, which never decodes entities — so a + // pre-escaped `<` reaches the reader as those four characters. And when + // a match does land here, `highlight()` escapes it again into `&lt;`, + // which `v-html` decodes one level back to the same wrong thing (#406). + suffix: file.content, to: file.id, icon: (link.icon || ancestorIcon || (file.level > 1 ? icons.hash : icons.file)) as IconComponent, level: file.level @@ -140,7 +146,14 @@ function _useContentSearch() { label: result.title, labelHtml: result.snippets?.title ? sanitizeSnippet(result.snippets.title) : undefined, prefix, - description: result.content.replaceAll('<', '<').replaceAll('>', '>'), + // Raw — see `mapFile`. This is the fallback of the two: the line below + // populates `descriptionHtml` whenever the backend honours the snippet + // request `ContentSearch` sends, and the template prefers it. So this + // field renders when there are no snippets, through `{{ }}`, which + // escapes on its own — while `descriptionHtml` is escaped by + // `sanitizeSnippet`. Two producers, two sinks, neither needing help + // here. + description: result.content, descriptionHtml: result.snippets?.content ? sanitizeSnippet(result.snippets.content) : undefined, to: result.id, icon: (link?.icon || ancestorIcon || (result.level > 1 ? icons.hash : icons.file)) as IconComponent, diff --git a/test/composables/useContentSearch.spec.ts b/test/composables/useContentSearch.spec.ts new file mode 100644 index 00000000..4a1284c8 --- /dev/null +++ b/test/composables/useContentSearch.spec.ts @@ -0,0 +1,115 @@ +import { describe, it, expect } from 'vitest' +import { mountSuspended } from '@nuxt/test-utils/runtime' +import { useContentSearch } from '../../src/runtime/composables/useContentSearch' +import CommandPalette from '../../src/runtime/components/CommandPalette.vue' +import type { ContentSearchFile, ContentSearchResult } from '../../src/runtime/components/content/ContentSearch.vue' + +describe('useContentSearch', () => { + // Angle brackets are ordinary in indexed documentation — generics, JSX, XML, + // shell redirection, comparison operators. This composable used to escape + // them by hand before handing the text to `CommandPalette` (#406). + const GENERICS = 'Array and Map' + const ESCAPED = 'Array<string> and Map<K, V>' + + describe('escaping is left to the sink', () => { + it('maps a file without escaping its content', () => { + const { mapFile } = useContentSearch() + const file: ContentSearchFile = { id: '/docs#x', title: 'X', titles: [], level: 2, content: GENERICS } + + const item = mapFile(file, { path: '/docs', title: 'Docs' } as any) + + expect(item.suffix).toBe(GENERICS) + expect(item.suffix).not.toBe(ESCAPED) + }) + + it('maps a search result without escaping its content', () => { + const { mapSearchResults } = useContentSearch() + const result: ContentSearchResult = { id: '/docs#y', title: 'Y', titles: [], level: 2, content: GENERICS } + + const [item] = mapSearchResults([result], []) + + expect(item!.description).toBe(GENERICS) + expect(item!.description).not.toBe(ESCAPED) + }) + }) + + describe('what the reader ends up seeing', () => { + // The reason the manual escaping was wrong rather than merely redundant: + // `CommandPalette` renders these two fields through `{{ }}`, which builds a + // text node, and a text node never decodes entities. A pre-escaped `<` + // arrived as those four characters on screen. The `v-html` sibling is no + // better — `highlight()` escapes again, and `v-html` decodes one level back + // to the same wrong thing. + // Rendered from what the mappers actually produce, not from a hand-built + // item. Constructing the item here instead would leave the composable + // untouched, so re-adding the escaping would sail past these. + async function palette(item: unknown, fuse?: Record) { + return mountSuspended(CommandPalette, { + props: { groups: [{ id: 'g', items: [item] }], ...fuse } as any + }) + } + + function fileItem(content: string) { + const { mapFile } = useContentSearch() + const file: ContentSearchFile = { id: '/docs#x', title: 'X', titles: [], level: 2, content } + + return mapFile(file, { path: '/docs', title: 'Docs' }) + } + + function resultItem(content: string) { + const { mapSearchResults } = useContentSearch() + const result: ContentSearchResult = { id: '/docs#y', title: 'Y', titles: [], level: 2, content } + + return mapSearchResults([result], [])[0] + } + + const paletteFor = (content: string) => palette(fileItem(content)) + + it('shows the angle brackets the author wrote', async () => { + const wrapper = await paletteFor(GENERICS) + + expect(wrapper.text()).toContain(GENERICS) + expect(wrapper.text()).not.toContain('<') + }) + + it('still renders an injected tag as text, since the sink is what escapes', async () => { + // Removing the manual escaping removes a *second* escape, not the only + // one. This is the assertion that says so. + const wrapper = await paletteFor('') + + expect(wrapper.find('img').exists()).toBe(false) + expect(wrapper.html()).toContain('<img') + expect(wrapper.html()).not.toContain(' { + // The second half of the defect, and the half the cases above miss. When + // a query matches, `CommandPalette` renders `suffixHtml` from + // `highlight()` instead of the raw field — and `highlight()` escaped the + // already-escaped value again into `&lt;`, which `v-html` decoded one + // level back to the same wrong output. + // + // Not an edge case: `ContentSearch` sets `includeMatches: true`, so this + // is what a reader sees the moment their query matches page content. + const wrapper = await palette(fileItem(GENERICS), { + searchTerm: 'Array', + fuse: { fuseOptions: { includeMatches: true, threshold: 0.1, ignoreLocation: true } } + }) + + expect(wrapper.find('mark').exists()).toBe(true) + expect(wrapper.text()).toContain(GENERICS) + expect(wrapper.text()).not.toContain('<') + }) + + it('does the same for a search result, which takes the other mapper', async () => { + // `mapSearchResults` fills `description`, and every case above goes + // through `mapFile` and `suffix`. Without this, the wiring between the + // second mapper and its sink has no end-to-end cover — a revert there + // fails only the unit assertion. + const wrapper = await palette(resultItem(GENERICS)) + + expect(wrapper.text()).toContain(GENERICS) + expect(wrapper.text()).not.toContain('<') + }) + }) +})