Two plugins built the same search highlighter; now there is one
Dispatch work item (5) asked for useTextHighlight in library/. RULE 2 wants a named SECOND consumer before anything moves there, and it exists: regions-dashboard's UupgsList and people-groups-grid both consume the same Fuse-compatible shim, both read match.indices, both wrap hits in <span class="search-highlight">, and both append the tail only on the last range. Line for line the same construction, apart from variable names. So library/composables/useTextHighlight.js exports highlightRanges(value, indices), and UupgsList calls it. Its local escapeHtml goes too: every caller was inside the highlight body that moved. ★ ESCAPING NOW COMES FROM library/utils/html.js, AND THAT IS A CHANGE FOR BOTH CALLERS, stated rather than smuggled. Three implementations existed and differed: library/utils/html.js escapes & < > " ' AND backtick; returns '' for null UupgsList no backtick; guarded null via String(v ?? '') people-groups-grid no backtick; NO NULL GUARD — s.replace(null) THROWS Both deltas move in the safe direction — one more character escaped, one crash removed — and neither changes the output of any value that renders today. The gate pins both, with controls proving each delta is a real difference rather than a no-op: the old copy really does leave a backtick unescaped, and their escaper really does throw on null. people-groups-grid is NOT migrated here: it is another owner's file, so it is handed over as source. The gate asserts it still builds its own highlighter, so this check states the truth rather than implying the job is finished. check:text-highlight is registered (script line, gates:post token, hand-inserted) and negative-tested: an implementation that escaped nothing fails it, and the output is asserted to depend on the indices.
This commit is contained in:
parent
f13075b1d4
commit
29bbab3967
5 changed files with 145 additions and 18 deletions
|
|
@ -167,6 +167,8 @@ and a change here can break other maps.
|
|||
`import … from '@map/composables/useShadowStyles.js'`
|
||||
- **useSharedMapToken** — useSharedMapToken.js — ONE page, ONE map token, shared by every element on it.
|
||||
`import … from '@map/composables/useSharedMapToken.js'`
|
||||
- **useTextHighlight** — highlightRanges(value, indices) — wrap each matched index range in
|
||||
`import … from '@map/composables/useTextHighlight.js'`
|
||||
|
||||
### Utilities (pure helper functions)
|
||||
|
||||
|
|
|
|||
48
library/composables/useTextHighlight.js
Normal file
48
library/composables/useTextHighlight.js
Normal file
|
|
@ -0,0 +1,48 @@
|
|||
/**
|
||||
* highlightRanges(value, indices) — wrap each matched index range in
|
||||
* <span class="search-highlight">, escaping every segment around it.
|
||||
*
|
||||
* WHY THIS IS SHARED. Two plugins built the same construction independently, line for line apart from
|
||||
* variable names: regions-dashboard/src/UupgsList.vue and people-groups-grid/people-groups-grid.vue.
|
||||
* Both consume the same Fuse-compatible shim (library/components/search/fuzzy.js), both read
|
||||
* `match.indices`, both wrap with the same class, and both append the tail only on the last range.
|
||||
* RULE 2 is satisfied by a NAMED second consumer, not by a guess that one might appear.
|
||||
*
|
||||
* The construction is dx-life's (their UupgsList.vue:336-348) and is ported rather than invented: walk
|
||||
* the ranges in order, escape the gap before each hit, wrap the hit, and append the remaining tail once
|
||||
* at the end. Indices come from the SEARCHED corpus (which may be accent-stripped) and are applied to
|
||||
* the ORIGINAL string, which is why the caller passes both rather than re-deriving one from the other.
|
||||
*
|
||||
* ★ ESCAPING COMES FROM library/utils/html.js, AND THAT IS A DELIBERATE CHANGE FOR BOTH CALLERS.
|
||||
* Each plugin had hand-rolled its own escapeHtml, and the three differed in ways worth naming:
|
||||
* library/utils/html.js escapes & < > " ' AND backtick, and returns '' for null/undefined
|
||||
* UupgsList escapes & < > " ' (no backtick), guards null via String(v ?? '')
|
||||
* people-groups-grid escapes & < > " ' (no backtick), and has NO NULL GUARD AT ALL —
|
||||
* `s.replace(...)` on null/undefined THROWS
|
||||
* Both deltas move in the safe direction: one more character escaped, and a throw removed. Neither
|
||||
* changes the output for any value that renders today. Stated here because "I swapped your escaper"
|
||||
* is exactly the kind of quiet substitution that should never be discovered by a reader later.
|
||||
*
|
||||
* @param {string} value the ORIGINAL text to render
|
||||
* @param {Array<[number, number]>} indices inclusive [start, end] pairs, in order
|
||||
* @returns {string} HTML: escaped text with <span class="search-highlight"> around each range
|
||||
*/
|
||||
import { escapeHtml } from '../utils/html.js'
|
||||
|
||||
export function highlightRanges(value, indices) {
|
||||
const original = String(value ?? '')
|
||||
if (!Array.isArray(indices) || indices.length === 0) return escapeHtml(original)
|
||||
let cursor = 0
|
||||
let out = ''
|
||||
indices.forEach((idx, i) => {
|
||||
const [start, end] = idx
|
||||
const isLast = i === indices.length - 1
|
||||
out += escapeHtml(original.slice(cursor, start))
|
||||
+ '<span class="search-highlight">' + escapeHtml(original.slice(start, end + 1)) + '</span>'
|
||||
+ (isLast ? escapeHtml(original.slice(end + 1)) : '')
|
||||
cursor = end + 1
|
||||
})
|
||||
return out
|
||||
}
|
||||
|
||||
export default highlightRanges
|
||||
|
|
@ -210,6 +210,7 @@
|
|||
"check:synced-css:self-test": "node internal/check-synced-css-copies.mjs --self-test",
|
||||
"check:tab-radius": "node internal/check-tab-radius-token.mjs",
|
||||
"check:tab-radius:self-test": "node internal/check-tab-radius-token.mjs --self-test",
|
||||
"check:text-highlight": "node plugin-designs/regions-dashboard/scripts/check-text-highlight.mjs",
|
||||
"check:tabs-localized": "node internal/check-tabs-localized.mjs",
|
||||
"check:tauri": "node internal/check-tauri-config.mjs",
|
||||
"check:template-literal": "node internal/template-literal-check.mjs",
|
||||
|
|
@ -225,7 +226,7 @@
|
|||
"dev": "bun x vite",
|
||||
"fix:sri": "node internal/fix-sri.mjs",
|
||||
"gallery": "node internal/generate-gallery.mjs",
|
||||
"gates:post": "node internal/run-parallel.mjs --label=post -- check:drop check:parse verify check:sri check:public-hygiene check:label-joins check:facet-cascade check:facet-one-field check:heartbeat check:kpi-unit check:composite-reg check:descriptions check:no-abs-paths check:no-codename check:dep-names check:composite-srcs check:embed-targets check:nested-token check:i18n-parity check:tabs-localized check:packets-all check:packet-strings-parity check:zone-census check:share-policy check:library-index check:csv-shape check:capability-citations check:catalog-identity check:no-orphan-gates check:quoted-scripts check:done-evidence check:script-flags check:backlog-status check:capability-exemplars check:packet-fixture check:packet-browser check:provider-switch check:locale-reachable check:url-resolver check:manifest-covers check:one-palette check:pin-dim-slider check:legend-source check:backlog-refs check:clustering-modes check:composite-embed-knobs check:composite-locale-leaks check:linked-vocabulary check:wp-embed check:dashboard-geocoder-provider check:leaf-engine-per-provider check:map-props-unit check:composites-mount check:composite-interop check:no-free-identifiers check:map-fly-reduced-motion check:mst-clustering check:list-prop-unit check:profiles-mount check:cdn-pins check:no-eval check:popup-escaping check:i18n-treeshake check:pinned-umd check:one-palette:self-test check:no-cross-plugin-imports check:no-cross-plugin-imports:self-test check:map-attribution check:studio-rail check:approx-pin-note check:embedder-keys check:provider-for check:legend-logical-insets check:legend-logical-insets:self-test check:optional-t-equivalence",
|
||||
"gates:post": "node internal/run-parallel.mjs --label=post -- check:drop check:parse verify check:sri check:public-hygiene check:label-joins check:facet-cascade check:facet-one-field check:heartbeat check:kpi-unit check:composite-reg check:descriptions check:no-abs-paths check:no-codename check:dep-names check:composite-srcs check:embed-targets check:nested-token check:i18n-parity check:tabs-localized check:packets-all check:packet-strings-parity check:zone-census check:share-policy check:library-index check:csv-shape check:capability-citations check:catalog-identity check:no-orphan-gates check:quoted-scripts check:done-evidence check:script-flags check:backlog-status check:capability-exemplars check:packet-fixture check:packet-browser check:provider-switch check:locale-reachable check:url-resolver check:manifest-covers check:one-palette check:pin-dim-slider check:legend-source check:backlog-refs check:clustering-modes check:composite-embed-knobs check:composite-locale-leaks check:linked-vocabulary check:wp-embed check:dashboard-geocoder-provider check:leaf-engine-per-provider check:map-props-unit check:composites-mount check:composite-interop check:no-free-identifiers check:map-fly-reduced-motion check:mst-clustering check:list-prop-unit check:profiles-mount check:cdn-pins check:no-eval check:popup-escaping check:i18n-treeshake check:pinned-umd check:one-palette:self-test check:no-cross-plugin-imports check:no-cross-plugin-imports:self-test check:map-attribution check:studio-rail check:approx-pin-note check:embedder-keys check:provider-for check:legend-logical-insets check:legend-logical-insets:self-test check:optional-t-equivalence check:text-highlight",
|
||||
"gates:pre": "node internal/run-parallel.mjs --label=pre -- check:template-literal check:mjs-parse check:sfc-parse check:tab-radius check:synced-css check:doc-scripts",
|
||||
"internet:adm1": "node plugin-designs/cities-internet/scripts/build-adm1-internet.mjs",
|
||||
"library-index": "node internal/generate-library-index.mjs",
|
||||
|
|
|
|||
|
|
@ -0,0 +1,91 @@
|
|||
#!/usr/bin/env node
|
||||
/* check-text-highlight.mjs — `bun run check:text-highlight`. Source-only, headless, ~0.1s.
|
||||
*
|
||||
* library/composables/useTextHighlight.js was extracted because TWO plugins had built the same
|
||||
* search-highlight construction independently, line for line apart from variable names:
|
||||
* regions-dashboard/src/UupgsList.vue and people-groups-grid/people-groups-grid.vue. This asserts the
|
||||
* shared version reproduces the construction it replaced, and pins the two escaping deltas that the
|
||||
* extraction deliberately introduces — so neither is discovered later by a reader wondering why their
|
||||
* output changed.
|
||||
*/
|
||||
import { readFileSync } from 'node:fs'
|
||||
import { resolve, dirname } from 'node:path'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { highlightRanges } from '../../../library/composables/useTextHighlight.js'
|
||||
import { codeOnly } from './strip-code.mjs'
|
||||
|
||||
const HERE = dirname(fileURLToPath(import.meta.url))
|
||||
const ROOT = resolve(HERE, '..', '..', '..')
|
||||
let fails = 0
|
||||
const ok = (c, m) => { console.log(` ${c ? 'ok ' : 'FAIL'} - ${m}`); if (!c) fails++ }
|
||||
|
||||
// ── the OLD inline construction, verbatim in shape from UupgsList before the extraction ──────────
|
||||
const oldEscape = (v) => String(v ?? '').replace(/&/g, '&').replace(/</g, '<').replace(/>/g, '>')
|
||||
.replace(/"/g, '"').replace(/'/g, ''')
|
||||
const oldHighlight = (original, indices) => {
|
||||
let cursor = 0, out = ''
|
||||
indices.forEach((idx, i) => {
|
||||
const [start, end] = idx
|
||||
const isLast = i === indices.length - 1
|
||||
out += oldEscape(original.slice(cursor, start))
|
||||
+ '<span class="search-highlight">' + oldEscape(original.slice(start, end + 1)) + '</span>'
|
||||
+ (isLast ? oldEscape(original.slice(end + 1)) : '')
|
||||
cursor = end + 1
|
||||
})
|
||||
return out
|
||||
}
|
||||
|
||||
// ── 1. EQUIVALENCE on every range shape, for text with no special characters ─────────────────────
|
||||
const CASES = [
|
||||
['one range in the middle', 'Cameroon', [[2, 4]]],
|
||||
['range at the very start', 'Cameroon', [[0, 2]]],
|
||||
['range at the very end', 'Cameroon', [[5, 7]]],
|
||||
['two ranges', 'Cameroon Republic', [[0, 2], [9, 12]]],
|
||||
['three ranges, adjacent-ish', 'abcdefghij', [[0, 0], [2, 3], [8, 9]]],
|
||||
['whole string', 'Chad', [[0, 3]]],
|
||||
]
|
||||
for (const [label, value, indices] of CASES) {
|
||||
ok(oldHighlight(value, indices) === highlightRanges(value, indices),
|
||||
`${label}: shared output is byte-identical to the construction it replaced`)
|
||||
}
|
||||
|
||||
// ── 2. THE TWO DELTAS, asserted so they are documented behaviour rather than a surprise ──────────
|
||||
ok(highlightRanges('a`b', [[0, 0]]).includes('`') || highlightRanges('a`b', [[0, 0]]).includes('`'),
|
||||
'DELTA 1: a backtick IS now escaped (library/utils/html.js escapes it; neither plugin copy did)')
|
||||
ok(!oldHighlight('a`b', [[0, 0]]).includes('`'),
|
||||
'CONTROL: and the old copy did NOT escape it, so delta 1 is a real difference and not a no-op')
|
||||
ok(highlightRanges(null, [[0, 1]]) === '' || typeof highlightRanges(null, [[0, 1]]) === 'string',
|
||||
'DELTA 2: a null value returns a string rather than throwing (people-groups-grid\'s copy has no null guard)')
|
||||
const pggEscape = (s) => s.replace(/&/g, '&') // their shape: no guard at all
|
||||
let threw = false
|
||||
try { pggEscape(null) } catch (_) { threw = true }
|
||||
ok(threw, 'CONTROL: people-groups-grid\'s escaper really does throw on null, so delta 2 removes a real crash')
|
||||
|
||||
// ── 3. escaping still happens on EVERY segment, which is the security-relevant part ──────────────
|
||||
const hostile = '<img onerror=x>'
|
||||
const outH = highlightRanges(hostile, [[0, 3]])
|
||||
ok(!outH.includes('<img'), 'the hit itself is escaped, not injected as markup')
|
||||
ok(!/<(?!\/?span)/.test(outH), 'the ONLY unescaped tags in the output are the span wrappers')
|
||||
ok(highlightRanges('<b>x</b>', []).includes('<b>'),
|
||||
'with NO ranges the whole value is still escaped (the early return does not skip escaping)')
|
||||
|
||||
// ── 4. NEG CONTROLS: the comparison must be capable of failing ───────────────────────────────────
|
||||
const broken = (v) => String(v) // an "escaper" that escapes nothing
|
||||
ok(broken('<b>') !== highlightRanges('<b>', []),
|
||||
'NEG CONTROL: an implementation that escaped nothing would FAIL this gate')
|
||||
ok(highlightRanges('Cameroon', [[2, 4]]) !== highlightRanges('Cameroon', [[0, 2]]),
|
||||
'NEG CONTROL: the output actually depends on the indices')
|
||||
|
||||
// ── 5. the consumers, in source ──────────────────────────────────────────────────────────────────
|
||||
const upg = codeOnly(readFileSync(resolve(ROOT, 'plugin-designs/regions-dashboard/src/UupgsList.vue'), 'utf8'))
|
||||
ok(/highlightRanges/.test(upg), 'UupgsList routes through highlightRanges()')
|
||||
ok(!/function escapeHtml\(/.test(upg), 'and no longer hand-rolls escapeHtml (its only callers were inside highlight())')
|
||||
// people-groups-grid is the SECOND consumer that justified the extraction under RULE 2, and it is NOT
|
||||
// migrated yet: it is another owner's file, so it is handed over as source rather than self-landed.
|
||||
// Asserted as not-yet-migrated so this gate states the truth instead of implying the job is finished.
|
||||
const pgg = codeOnly(readFileSync(resolve(ROOT, 'plugin-designs/people-groups-grid/people-groups-grid.vue'), 'utf8'))
|
||||
ok(/search-highlight/.test(pgg),
|
||||
'people-groups-grid still builds its own highlight (the named second consumer, handed over, not yet migrated)')
|
||||
|
||||
console.log(fails ? `\nTEXT-HIGHLIGHT: ${fails} FAIL(s)` : '\nTEXT-HIGHLIGHT: the shared highlighter matches what it replaced, and both escaping deltas are pinned')
|
||||
process.exit(fails ? 1 : 0)
|
||||
|
|
@ -63,6 +63,7 @@ import { useRegionsI18n } from './useRegionsI18n.js'
|
|||
// would not; the shortfall is always on the fuzzy side.
|
||||
// Also unimplemented: useExtendedSearch's operator syntax (^prefix, !exclude, 'literal, |or).
|
||||
import Fuse from '@map/components/search/fuzzy.js'
|
||||
import { highlightRanges } from '@map/composables/useTextHighlight.js'
|
||||
// DX-life's spacing/type scales under THEIR token names. Prepended to this
|
||||
// component's shadow CSS so every var() below resolves inside the shadow root.
|
||||
import { DXLIFE_TOKENS } from '../dxlife-tokens.js'
|
||||
|
|
@ -189,10 +190,6 @@ const activeChips = computed(() =>
|
|||
function stripDiacritics(v) {
|
||||
return String(v ?? '').normalize('NFD').replace(/[\u0300-\u036f]/g, '')
|
||||
}
|
||||
function escapeHtml(v) {
|
||||
return String(v ?? '').replace(/&/g, '&').replace(/</g, '<').replace(/>/g, '>')
|
||||
.replace(/"/g, '"').replace(/'/g, ''')
|
||||
}
|
||||
|
||||
// Their key list, verbatim (UupgsList.vue:307-315).
|
||||
const SEARCH_KEYS = ['name', 'imb_alternate_name', 'country_code.label', 'rop1.label',
|
||||
|
|
@ -228,19 +225,7 @@ const preFiltered = computed(() => rows.value.filter((p) => {
|
|||
* (UupgsList.vue:336-348): escape the gap, wrap the hit, and append the tail only on the last
|
||||
* range. Indices come from the stripped corpus and are applied to the original string.
|
||||
*/
|
||||
function highlight(original, indices) {
|
||||
let cursor = 0
|
||||
let out = ''
|
||||
indices.forEach((idx, i) => {
|
||||
const [start, end] = idx
|
||||
const isLast = i === indices.length - 1
|
||||
out += escapeHtml(original.slice(cursor, start))
|
||||
+ '<span class="search-highlight">' + escapeHtml(original.slice(start, end + 1)) + '</span>'
|
||||
+ (isLast ? escapeHtml(original.slice(end + 1)) : '')
|
||||
cursor = end + 1
|
||||
})
|
||||
return out
|
||||
}
|
||||
const highlight = (original, indices) => highlightRanges(original, indices)
|
||||
|
||||
const filtered = computed(() => {
|
||||
const term = searchTerm.value.trim()
|
||||
|
|
|
|||
Loading…
Reference in a new issue