fix(): Accessibility, security and performance fixes - #485
Conversation
Escape resolved its containment target to $dropboxWrapper for all non-popup layouts. Under the default `dropboxWrapper: 'self'` there is no $dropboxWrapper, so the guard short-circuited on `undefined` and the key did nothing: on a desktop viewport the dropdown could not be dismissed from the keyboard at all. Pick the element that actually contains the focused node - $dropboxWrapper only when the dropbox is portalled out, $wrapper otherwise. WCAG 2.1.1 Keyboard (A) and 2.1.2 No Keyboard Trap (A). Adds cypress/e2e/a11y-escape-close.cy.ts covering all four layouts (self/desktop, popup, external dropboxWrapper, keepAlwaysOpen); the first three failed before this change. Adds cypress/support/mount.ts, a shared per-test mount helper. (cherry picked from commit dad63ad)
…ation (AI-3 / A11Y-04) "Select All" rendered as a bare <span tabindex="0" aria-label>. Assistive technology saw a generic element with no role and no checked state, so every select/deselect was silent, and Space - the expected activation key for a checkbox - fell through to the page and scrolled it. Only Enter worked. - render role="checkbox" aria-checked="false" on .vscomp-toggle-all-button - sync aria-checked in toggleAllOptionsClass(), the single point all selection paths funnel through, so the exposed state cannot drift from the visual one - accept keyCode 32 alongside 13 and preventDefault so Space no longer scrolls WCAG 4.1.2 Name/Role/Value (A), 1.3.1 Info and Relationships (A), 2.1.1 Keyboard (A). Adds cypress/e2e/a11y-select-all.cy.ts (7 cases, all failing before this change), including that aria-checked follows selection driven from the options themselves. (cherry picked from commit f938209)
…n (AI-6 / A11Y-03) The stylesheet already shipped a .vscomp-live-region rule, but no code ever created the element: the component had zero live regions, so search result counts, "no results", server-search loading and selection changes were conveyed visually only. - render one visually-hidden role="status" aria-live="polite" aria-atomic="true" region per instance, inside the wrapper so it is torn down with the element - announce match counts and no-results while searching, the loading state and outcome of a server search, and every selection change - track filteredOptionsCount separately: setVisibleOptions() overwrites visibleOptionsCount with the size of the virtualisation window, which is not the number of matches - suppress announcements until construction finishes (isInitialized), so an initial value does not speak on page load, and while focus is outside the search input, so closing the dropdown does not read a stale count - identical consecutive messages are left in place, so "No results found" is not repeated on every further non-matching keystroke Announcement strings are new overridable props for localisation: searchResultsText, searchResultText, noOptionsSelectedText, selectedText, loadingText. Documented in docs/properties.md and the JSDoc typings. WCAG 4.1.3 Status Messages (AA). Adds cypress/e2e/a11y-live-region.cy.ts (14 cases, all failing before this change). (cherry picked from commit 82598ce)
… (AI-4 / A11Y-07)
`required` was never exposed to assistive technology, and a failed validate() only
toggled a `has-error` class that recoloured the toggle button border. There was no
aria-required, no aria-invalid, no message and no announcement: the failure was
conveyed by colour alone and was silent to screen readers.
- aria-required on the wrapper from setEleProps() and kept current by toggleRequired()
- aria-invalid toggled in validate()
- a text error message element per instance, linked via aria-describedby while in error
and announced through the live region added in AI-6
- the message plus a leading warning glyph give the non-colour cue
Distinguishes the two failure modes: requiredErrorText for an empty required select,
minValuesErrorText (with a {count} placeholder) when fewer than minValues are selected.
Both are overridable props, documented in docs/properties.md and the JSDoc typings.
Adds DomUtils.toggleAria()/removeAttr(): aria-required and aria-invalid are removed
rather than written as "false", since some screen readers verbalise a literal false.
WCAG 3.3.1 Error Identification (A), 1.4.1 Use of Colour (A), 4.1.2 Name/Role/Value (A).
Adds cypress/e2e/a11y-required-error.cy.ts (12 cases; 11 failed before this change).
Also completes the HTMLElement typing for the public element API the component attaches
in setEleProps(), so specs can drive it the way consumers do.
(cherry picked from commit cf9c6c0)
…trally (AI-1 / SEC-01) Option label/value/description are interpolated into innerHTML and secureText() is a no-op unless enableSecureText is on, which it is not by default. Until now a host application had no way to turn escaping on for every dropdown at once; it had to remember the flag at each call site, and missing one reintroduces DOM XSS for untrusted option text. Adds VirtualSelect.setGlobalDefaults(props) / getGlobalDefaults(), applied under per-instance options: per-instance options > page-level globals > built-in defaults Deliberately non-breaking: the per-instance default for enableSecureText stays off, so consumers who intentionally render HTML/icon labels, and large trusted lists that should not pay the per-option escaping cost, are unaffected. No version bump needed. Because these are defaults rather than overrides, a host that forwards enableSecureText on every init() call must stop forwarding it (or forward true) for the global to take effect. Called out in the method docs, since it decides whether the mechanism actually helps. setGlobalDefaults ignores `ele` and `options`, which are per-instance by nature and would otherwise alias state across instances. Defaults derived from another prop (zIndex from keepAlwaysOpen, and the hasOptionDescription sizing) now resolve through the same precedence chain, so a global drives them too. Documented in docs/methods.md, cross-referenced from the security note in docs/properties.md. Adds cypress/e2e/security-global-defaults.cy.ts (8 cases), covering that a payload still executes with the default config, is neutralised once the global is set, and that an explicit per-instance false opts back out. Existing security specs still pass. (cherry picked from commit 49971b1)
The options container carried role="listbox" but no aria-multiselectable, so assistive technology presented a multi-select dropdown with single-select semantics: users had no way to know more than one option could be chosen. Emit aria-multiselectable="true" only in multiple mode; a single select correctly omits the attribute rather than declaring "false". WCAG 4.1.2 Name, Role, Value (A). Adds cypress/e2e/a11y-listbox-multiselectable.cy.ts, including the case where `multiple` comes from the host element's attribute rather than the options object. (cherry picked from commit 4e8be99)
… minimum (AI-11 / A11Y-13) Two pointer targets were below the WCAG minimum: "Select All" collapsed to its 25x15 content box, and the per-tag clear button was 20x20 - under half the required area for users with limited dexterity. - add $min-target-size (24px) and route $value-tag-clear-width through it, so the clear button and the tag content's width calc stay consistent from one variable - give .vscomp-toggle-all-button a min-height/min-width floor rather than a fixed size, so the hit area grows without scaling the checkbox glyph inside it RTL needs no counterpart: rtl.scss only adjusts alignment and spacing, not sizes. WCAG 2.5.8 Target Size (Minimum), AA. Adds cypress/e2e/a11y-target-size.cy.ts. Alongside the three size assertions (which failed before this change) it pins two behaviours, so a future size change cannot leak into layout or break the control: the clear button still removes its own tag, and the visible tag stays compact. (cherry picked from commit 73c088c)
…-7 / A11Y-01 + A11Y-05) Opening the dropdown focuses the search input, and both arrow handlers early-returned in exactly that state so the caret could move. The consequence was that in the default flow the arrows did nothing at all: no option could ever be highlighted from the keyboard, and nothing was announced. A user had to discover an undocumented Tab into the listbox first. WCAG 2.1.1 (A). The highlight was also published as aria-activedescendant on the wrapper and on $dropboxContainer - a plain div with no role, where the attribute is meaningless - and never on the element that actually held focus, so the active option was not conveyed even once navigation did work. WCAG 4.1.2 (A). - Up/Down now drive the list while DOM focus stays in the field (WAI-ARIA APG editable-combobox), via a shared navigateOptions() - the search input becomes a combobox over the listbox: role="combobox", aria-autocomplete="list", aria-expanded kept in sync on open/close, and aria-controls pointing at the options container, which needed an id - aria-activedescendant is written to the wrapper and the search input through one setActiveDescendant() helper, and no longer to the role-less container; it is cleared when the highlight is removed and when the dropdown closes BEHAVIOUR CHANGE, user-visible: Up/Down in the search input no longer move the text caret. Home/End and Left/Right do, and are unchanged. This reverses a deliberate earlier decision, because that decision is what caused the Level A failure above. It must be called out in the 1.4.0 release notes. Adds cypress/e2e/a11y-search-arrow-navigation.cy.ts (12 cases; 9 failed before this change), opted into testIsolation so leftover focus between cases cannot make the focus assertions flaky. examples.cy.ts updates, all test-side, none loosening a real assertion: - aria-activedescendant assertions retargeted from the role-less container to the combobox, plus a new assertion that the container does not carry it. The old assertion encoded the A11Y-05 defect. - the Up/Down caret cases moved to Home/End, with added assertions that the arrows now highlight an option and that focus stays in the field. The old cases encoded A11Y-01. - press counts reduced by one where the first press used to be swallowed, and a known starting state added per keyboard case since cy.open() toggles. - one racy press replaced with a real key press: the virtualiser replaces .vscomp-option nodes on every render, so chaining .type() onto them races the rebuild. Suite: 306 tests, 304 pass, 2 fail. Both remaining failures are pre-existing and fail identically at baseline 992f6a9; a third baseline failure ("keeps focus on the last option when navigating past the end of the list") is fixed by this change. Also adds the versioned post-remediation assessment (AUDIT-REPORT-v1.4.0.md) and updates ACTION-ITEMS.md with status plus five follow-ups, notably that SEC-01 is still live for the OutSystems wrapper because it forwards enableSecureText explicitly. (cherry picked from commit 5dd2a4e)
`Add image/icon > has flag icon on selected item` failed because cy.open() is a click, i.e. a toggle: the preceding case leaves this dropdown open, so cy.open() closed it and the option click then landed on a dropbox with `display: none`. Open only when actually closed, and re-establish the scroll position so option 16 is rendered whether or not the preceding case ran. Corrects an earlier reading of this failure. It was previously attributed to the popover hiding the dropbox while the virtualised list is scrolled, and a product ticket (AI-1d) was drafted for that. The fix confirms the real cause is the toggle, so AI-1d is withdrawn - there is no product defect here. Baseline re-measured directly to keep the comparison honest: 992f6a9's own bundle and its own examples.cy.ts were checked out and run, giving 216 pass / 3 fail. Both tests reported as newly failing were in that set. Two of the three are now fixed. Suite: 306 tests, 305 pass, 1 fail (was 216/3 at baseline). The remaining failure, `Option group > activates group select/deselect with Enter when group title is focused`, fails identically at baseline and is filed as AI-1f with a lead but no diagnosis: the first Enter selects the group, the second does not deselect it, and the keyboard path routes through onGroupTitleClick(), which reads its direction from the `selected` class on an element the re-render replaces between presses. Clicking the group title toggles correctly, so it is specific to the keyboard path. (cherry picked from commit f7ae693)
… re-renders (AI-5 / PERF-01 + PERF-02) Two costs sat directly on the scroll path: - calculateAriaMetadata() walks every option and ran at the top of every renderOptions() (~3.9 ms/call at 100k options), even though aria-setsize/aria-posinset only change when the filtered set or its order changes - never when the virtualisation window moves. - onOptionsScroll was bound with no throttling, so a drag produced one full re-render per scroll event (~9.5 ms at 100k unthrottled, ~44 ms at 4x CPU) and kept the main thread blocked for the whole gesture. The scan is now guarded by an ariaMetadataDirty flag, set by everything that alters the filtered set or its order: setVisibleOptionsCount(), setSortedOptions(), setNewOption() and removeNewOption(). A flag rather than an enumeration of call sites, so a future code path that changes the set cannot silently skip the recompute by being forgotten here. Scroll re-renders are coalesced to at most one per animation frame, and the pending frame is cancelled in destroy() so it cannot run against detached DOM. No API or behaviour change: the same attributes end up on the same elements, just computed when they can actually differ. Adds cypress/e2e/perf-scroll-aria.cy.ts (6 cases): zero ARIA rescans across five scroll positions, 20 synchronous scroll events producing fewer than 20 re-renders, setsize/posinset still correct after scrolling, metadata recomputed on search and on setOptions, and the queued frame cancelled on destroy. (cherry picked from commit 72ab721)
Option labels may legitimately contain markup - a flag icon, <b>, a <br> - and those labels
are also interpolated into aria-label attributes, where markup is meaningless. It reached the
screen reader as tag soup, and a double quote in a label closed the attribute early so the
rest of the accessible name was silently lost.
Adds Utils.getAriaLabelText(): tags collapse to a single space so adjacent words do not run
together ("Paris<br>France" must not become "ParisFrance"), whitespace is collapsed, then the
remaining quotes are escaped. Applied to the group header name, the group name carried into
each child's label, the customData group_name/description, and the option label itself.
Also replaces the inline strip on the tag clear button, which removed tags but never escaped
quotes - the same attribute breakout, in the one place that already knew about the problem.
WCAG 4.1.2 Name, Role, Value (A) and 1.1.1 Non-text Content (A).
Non-breaking: only the text inside aria-label changes, and only where it was malformed. No
roles, attributes or DOM structure change.
(cherry picked from commit f9d37b1)
Deciding whether a selected tag's text overflows (and therefore needs a tooltip) used to create a div, read getComputedStyle twice, append it to <body>, read clientWidth and remove it again - once per tag. Each DOM mutation invalidates layout for the read that follows, so rendering many tags produced a burst of forced synchronous layouts. Now one reusable off-screen node, created lazily and shared, and a single getComputedStyle read for every property. Font weight and letter spacing are applied too: both change advance width, so ignoring them under-reported overflow and could drop a tooltip that was needed. The node is absolutely positioned off-screen and aria-hidden, so it cannot affect layout or be announced, and it is removed in the existing last-instance teardown alongside the global listeners and the shared observer - nothing of ours is left in the document. Non-breaking: same boolean result, same tooltip behaviour. (cherry picked from commit 10cecf3)
The dropbox slide ignored the user's OS-level "reduce motion" setting. The animation is driven from both CSS and JS, so a stylesheet rule alone is not enough: the popover would still animate for showDuration/hideDuration milliseconds. Both are handled now - a prefers-reduced-motion block zeroes the transitions, and showDuration/hideDuration resolve to 0 when the query matches. Utils.prefersReducedMotion() reads the query per call rather than caching it, so a preference changed after page load is picked up by the next instance, and it is guarded for environments without matchMedia. The loader spin is deliberately left running. It is a status indicator rather than decoration, and a frozen spinner reads as a broken UI instead of "working"; screen reader users get the same information from the live region, so nothing is lost by keeping it. Recorded in the stylesheet so the omission is not mistaken for an oversight. Non-breaking: no API change, and behaviour differs only for users who asked for reduced motion. (cherry picked from commit 600c4f7)
The specs led with internal audit identifiers ("Regression test for AI-2 / [A11Y-02]"), which
tell a developer reading the file nothing. Each header now states the behaviour being
protected and cites the WCAG criterion or standard, which is something they can look up.
Also drops the identifiers from describe() titles, so a failing test names the broken
behaviour rather than a backlog row.
WCAG criterion numbers and OWASP references are kept deliberately: those are external
standards, not internal bookkeeping.
No test logic changes. Suite unchanged at 325 tests, 324 passing.
(cherry picked from commit 08c9c81)
closeDropbox() is asynchronous for every default instance: initDropboxPopover()
runs whenever !keepAlwaysOpen && !showAsPopup, so the close goes through
dropboxPopover.hide() and afterHidePopper() - the only caller of
removeOptionFocus() - does not run until the hide transition ends. Measured
live: for ~150-200ms after closeDropbox() returns, isOpened() is still true, the
previous .focused option is still in the DOM and focusedOptionIndex still holds
its index. Reopening inside that window resumed arrow navigation from a row the
user could no longer see, one step past where they expected - which on a grouped
multi-select put Enter on the first child option instead of the group title.
Clear the highlight next to the existing setActiveDescendant('') instead of
waiting for the transition. removeOptionFocus() is a no-op when nothing is
highlighted, so the afterHidePopper() call stays for the silent path.
toggleOptionFocusedState() also moved DOM focus on both edges, so clearing a
highlight focused the element it had just un-highlighted. Harmless during
navigation (the next call immediately focuses the new option) but wrong on
close, where it pulled focus into a dropbox being hidden and fought the wrapper
refocus. It now moves focus only when taking the highlight.
This is what the red examples.cy.ts case was actually hitting. It was filed as
"Enter on a group title selects but does not deselect"; the run log shows it
failed on the first assertion, not the second, and the toggle itself works in
both directions against the unfixed bundle.
Spec: a11y-close-clears-highlight.cy.ts (5), confirmed failing before the fix.
(cherry picked from commit af297ec)
examples.cy.ts runs with testIsolation: false, so every instance survives between cases, and cy.open() is a click - therefore a toggle - preceded by a fixed wait(200) that races the hide transition. Whether a case started open or closed, with or without a carried-over highlight, silently changed how many key presses the next one needed. cy.openFresh(id) replaces the six-line reset() + closeDropbox() + cy.open() preamble in all five Option-group keyboard cases. It asserts its way into the starting state rather than waiting a fixed time for it: clear the value, close, wait for the `closed` class, click, wait for it to be gone, then assert no option carries the highlight. All three inputs a press count can depend on - open/closed, value, highlight - are pinned before the first key press. Neither approach the v1.4.0 assessment rejected was retried: this is not testIsolation: true (which would break the `go to section` pattern every describe relies on) and not API-opening (which leaves focus outside the component and trips the dropbox focus sentinels). examples.cy.ts also drops from ~13 min to ~2 min, because several cases had been reaching their assertions only after Cypress retry timeouts. (cherry picked from commit fd60b7c)
…I-1f)
Follow-up to the AI-1f fix replayed from the audit branch, from code review of it.
closeDropbox() ends with setSearchValue(''), and afterSetSearchValue()
unconditionally calls focusOption({ focusFirst: true }). So with a filter typed,
the synchronous removeOptionFocus() earlier in closeDropbox() was immediately
undone: the highlight and aria-activedescendant came back on a combobox already
marked aria-expanded="false", and focusOption() pulled DOM focus onto an option
about to be display:none - verified live, keyboard focus ended on <body>.
Guarded with an isClosing flag scoped to that one call, so everything before it
(the wrapper refocus in particular) still sees real state.
announceSearchResults() needs no guard: it already bails unless the search input
holds focus, and a11y-live-region.cy.ts pins that.
Also tightens toggleFocusedProp(), which tested focusedOptionIndex for
truthiness. That only works because DomUtils.getData() returns the string "0";
normalising it to a number would stop index 0 being cleared, and renderOptions()
re-applies .focused from the isFocused prop, so the stale highlight would come
back through the data path on the next render.
Two cases added to a11y-close-clears-highlight.cy.ts (7 total). Against the
pre-fix baseline 3 of the 7 fail; against the branch before this commit, the new
typed-search case fails on its own.
From code review of the AI-1e work replayed from the audit branch. openFresh() claimed to pin every input a press count can depend on, but missed two: - scrollTop. focusOption() resolves "the first option" through getFirstVisibleOptionIndex(), i.e. scrollTop / optionHeight, and opening does not reset it: setScrollTop() returns early with no selection (which openFresh guarantees by resetting) and scrollToTop() only runs under showSelectedOptionsFirst. The last converted case scrolls to the bottom of the list, so any keyboard case added after it would have inherited that position. - the search value. It was only being cleared as a side effect of closeDropbox(), which returns early when the dropdown is already closed - and reset() never touches it. A case that left the instance closed with a filter applied would have opened onto a filtered list with every assertion still passing. Both are now set explicitly and then asserted, so the command fails loudly rather than silently opening from the wrong state. The setSearchValue() call is guarded for instances built with search: false, which have no $searchInput.
Escape resolved its containment target to $dropboxWrapper for all non-popup layouts. Under the default `dropboxWrapper: 'self'` there is no $dropboxWrapper, so the guard short-circuited on `undefined` and the key did nothing: on a desktop viewport the dropdown could not be dismissed from the keyboard at all. Pick the element that actually contains the focused node - $dropboxWrapper only when the dropbox is portalled out, $wrapper otherwise. WCAG 2.1.1 Keyboard (A) and 2.1.2 No Keyboard Trap (A). Adds cypress/e2e/a11y-escape-close.cy.ts covering all four layouts (self/desktop, popup, external dropboxWrapper, keepAlwaysOpen); the first three failed before this change. Adds cypress/support/mount.ts, a shared per-test mount helper. (cherry picked from commit dad63ad)
…ation (AI-3 / A11Y-04) "Select All" rendered as a bare <span tabindex="0" aria-label>. Assistive technology saw a generic element with no role and no checked state, so every select/deselect was silent, and Space - the expected activation key for a checkbox - fell through to the page and scrolled it. Only Enter worked. - render role="checkbox" aria-checked="false" on .vscomp-toggle-all-button - sync aria-checked in toggleAllOptionsClass(), the single point all selection paths funnel through, so the exposed state cannot drift from the visual one - accept keyCode 32 alongside 13 and preventDefault so Space no longer scrolls WCAG 4.1.2 Name/Role/Value (A), 1.3.1 Info and Relationships (A), 2.1.1 Keyboard (A). Adds cypress/e2e/a11y-select-all.cy.ts (7 cases, all failing before this change), including that aria-checked follows selection driven from the options themselves. (cherry picked from commit f938209)
…n (AI-6 / A11Y-03) The stylesheet already shipped a .vscomp-live-region rule, but no code ever created the element: the component had zero live regions, so search result counts, "no results", server-search loading and selection changes were conveyed visually only. - render one visually-hidden role="status" aria-live="polite" aria-atomic="true" region per instance, inside the wrapper so it is torn down with the element - announce match counts and no-results while searching, the loading state and outcome of a server search, and every selection change - track filteredOptionsCount separately: setVisibleOptions() overwrites visibleOptionsCount with the size of the virtualisation window, which is not the number of matches - suppress announcements until construction finishes (isInitialized), so an initial value does not speak on page load, and while focus is outside the search input, so closing the dropdown does not read a stale count - identical consecutive messages are left in place, so "No results found" is not repeated on every further non-matching keystroke Announcement strings are new overridable props for localisation: searchResultsText, searchResultText, noOptionsSelectedText, selectedText, loadingText. Documented in docs/properties.md and the JSDoc typings. WCAG 4.1.3 Status Messages (AA). Adds cypress/e2e/a11y-live-region.cy.ts (14 cases, all failing before this change). (cherry picked from commit 82598ce)
… (AI-4 / A11Y-07)
`required` was never exposed to assistive technology, and a failed validate() only
toggled a `has-error` class that recoloured the toggle button border. There was no
aria-required, no aria-invalid, no message and no announcement: the failure was
conveyed by colour alone and was silent to screen readers.
- aria-required on the wrapper from setEleProps() and kept current by toggleRequired()
- aria-invalid toggled in validate()
- a text error message element per instance, linked via aria-describedby while in error
and announced through the live region added in AI-6
- the message plus a leading warning glyph give the non-colour cue
Distinguishes the two failure modes: requiredErrorText for an empty required select,
minValuesErrorText (with a {count} placeholder) when fewer than minValues are selected.
Both are overridable props, documented in docs/properties.md and the JSDoc typings.
Adds DomUtils.toggleAria()/removeAttr(): aria-required and aria-invalid are removed
rather than written as "false", since some screen readers verbalise a literal false.
WCAG 3.3.1 Error Identification (A), 1.4.1 Use of Colour (A), 4.1.2 Name/Role/Value (A).
Adds cypress/e2e/a11y-required-error.cy.ts (12 cases; 11 failed before this change).
Also completes the HTMLElement typing for the public element API the component attaches
in setEleProps(), so specs can drive it the way consumers do.
(cherry picked from commit cf9c6c0)
…trally (AI-1 / SEC-01) Option label/value/description are interpolated into innerHTML and secureText() is a no-op unless enableSecureText is on, which it is not by default. Until now a host application had no way to turn escaping on for every dropdown at once; it had to remember the flag at each call site, and missing one reintroduces DOM XSS for untrusted option text. Adds VirtualSelect.setGlobalDefaults(props) / getGlobalDefaults(), applied under per-instance options: per-instance options > page-level globals > built-in defaults Deliberately non-breaking: the per-instance default for enableSecureText stays off, so consumers who intentionally render HTML/icon labels, and large trusted lists that should not pay the per-option escaping cost, are unaffected. No version bump needed. Because these are defaults rather than overrides, a host that forwards enableSecureText on every init() call must stop forwarding it (or forward true) for the global to take effect. Called out in the method docs, since it decides whether the mechanism actually helps. setGlobalDefaults ignores `ele` and `options`, which are per-instance by nature and would otherwise alias state across instances. Defaults derived from another prop (zIndex from keepAlwaysOpen, and the hasOptionDescription sizing) now resolve through the same precedence chain, so a global drives them too. Documented in docs/methods.md, cross-referenced from the security note in docs/properties.md. Adds cypress/e2e/security-global-defaults.cy.ts (8 cases), covering that a payload still executes with the default config, is neutralised once the global is set, and that an explicit per-instance false opts back out. Existing security specs still pass. (cherry picked from commit 49971b1)
The options container carried role="listbox" but no aria-multiselectable, so assistive technology presented a multi-select dropdown with single-select semantics: users had no way to know more than one option could be chosen. Emit aria-multiselectable="true" only in multiple mode; a single select correctly omits the attribute rather than declaring "false". WCAG 4.1.2 Name, Role, Value (A). Adds cypress/e2e/a11y-listbox-multiselectable.cy.ts, including the case where `multiple` comes from the host element's attribute rather than the options object. (cherry picked from commit 4e8be99)
… minimum (AI-11 / A11Y-13) Two pointer targets were below the WCAG minimum: "Select All" collapsed to its 25x15 content box, and the per-tag clear button was 20x20 - under half the required area for users with limited dexterity. - add $min-target-size (24px) and route $value-tag-clear-width through it, so the clear button and the tag content's width calc stay consistent from one variable - give .vscomp-toggle-all-button a min-height/min-width floor rather than a fixed size, so the hit area grows without scaling the checkbox glyph inside it RTL needs no counterpart: rtl.scss only adjusts alignment and spacing, not sizes. WCAG 2.5.8 Target Size (Minimum), AA. Adds cypress/e2e/a11y-target-size.cy.ts. Alongside the three size assertions (which failed before this change) it pins two behaviours, so a future size change cannot leak into layout or break the control: the clear button still removes its own tag, and the visible tag stays compact. (cherry picked from commit 73c088c)
…-7 / A11Y-01 + A11Y-05) Opening the dropdown focuses the search input, and both arrow handlers early-returned in exactly that state so the caret could move. The consequence was that in the default flow the arrows did nothing at all: no option could ever be highlighted from the keyboard, and nothing was announced. A user had to discover an undocumented Tab into the listbox first. WCAG 2.1.1 (A). The highlight was also published as aria-activedescendant on the wrapper and on $dropboxContainer - a plain div with no role, where the attribute is meaningless - and never on the element that actually held focus, so the active option was not conveyed even once navigation did work. WCAG 4.1.2 (A). - Up/Down now drive the list while DOM focus stays in the field (WAI-ARIA APG editable-combobox), via a shared navigateOptions() - the search input becomes a combobox over the listbox: role="combobox", aria-autocomplete="list", aria-expanded kept in sync on open/close, and aria-controls pointing at the options container, which needed an id - aria-activedescendant is written to the wrapper and the search input through one setActiveDescendant() helper, and no longer to the role-less container; it is cleared when the highlight is removed and when the dropdown closes BEHAVIOUR CHANGE, user-visible: Up/Down in the search input no longer move the text caret. Home/End and Left/Right do, and are unchanged. This reverses a deliberate earlier decision, because that decision is what caused the Level A failure above. It must be called out in the 1.4.0 release notes. Adds cypress/e2e/a11y-search-arrow-navigation.cy.ts (12 cases; 9 failed before this change), opted into testIsolation so leftover focus between cases cannot make the focus assertions flaky. examples.cy.ts updates, all test-side, none loosening a real assertion: - aria-activedescendant assertions retargeted from the role-less container to the combobox, plus a new assertion that the container does not carry it. The old assertion encoded the A11Y-05 defect. - the Up/Down caret cases moved to Home/End, with added assertions that the arrows now highlight an option and that focus stays in the field. The old cases encoded A11Y-01. - press counts reduced by one where the first press used to be swallowed, and a known starting state added per keyboard case since cy.open() toggles. - one racy press replaced with a real key press: the virtualiser replaces .vscomp-option nodes on every render, so chaining .type() onto them races the rebuild. Suite: 306 tests, 304 pass, 2 fail. Both remaining failures are pre-existing and fail identically at baseline 992f6a9; a third baseline failure ("keeps focus on the last option when navigating past the end of the list") is fixed by this change. Also adds the versioned post-remediation assessment (AUDIT-REPORT-v1.4.0.md) and updates ACTION-ITEMS.md with status plus five follow-ups, notably that SEC-01 is still live for the OutSystems wrapper because it forwards enableSecureText explicitly. (cherry picked from commit 5dd2a4e)
`Add image/icon > has flag icon on selected item` failed because cy.open() is a click, i.e. a toggle: the preceding case leaves this dropdown open, so cy.open() closed it and the option click then landed on a dropbox with `display: none`. Open only when actually closed, and re-establish the scroll position so option 16 is rendered whether or not the preceding case ran. Corrects an earlier reading of this failure. It was previously attributed to the popover hiding the dropbox while the virtualised list is scrolled, and a product ticket (AI-1d) was drafted for that. The fix confirms the real cause is the toggle, so AI-1d is withdrawn - there is no product defect here. Baseline re-measured directly to keep the comparison honest: 992f6a9's own bundle and its own examples.cy.ts were checked out and run, giving 216 pass / 3 fail. Both tests reported as newly failing were in that set. Two of the three are now fixed. Suite: 306 tests, 305 pass, 1 fail (was 216/3 at baseline). The remaining failure, `Option group > activates group select/deselect with Enter when group title is focused`, fails identically at baseline and is filed as AI-1f with a lead but no diagnosis: the first Enter selects the group, the second does not deselect it, and the keyboard path routes through onGroupTitleClick(), which reads its direction from the `selected` class on an element the re-render replaces between presses. Clicking the group title toggles correctly, so it is specific to the keyboard path. (cherry picked from commit f7ae693)
… re-renders (AI-5 / PERF-01 + PERF-02) Two costs sat directly on the scroll path: - calculateAriaMetadata() walks every option and ran at the top of every renderOptions() (~3.9 ms/call at 100k options), even though aria-setsize/aria-posinset only change when the filtered set or its order changes - never when the virtualisation window moves. - onOptionsScroll was bound with no throttling, so a drag produced one full re-render per scroll event (~9.5 ms at 100k unthrottled, ~44 ms at 4x CPU) and kept the main thread blocked for the whole gesture. The scan is now guarded by an ariaMetadataDirty flag, set by everything that alters the filtered set or its order: setVisibleOptionsCount(), setSortedOptions(), setNewOption() and removeNewOption(). A flag rather than an enumeration of call sites, so a future code path that changes the set cannot silently skip the recompute by being forgotten here. Scroll re-renders are coalesced to at most one per animation frame, and the pending frame is cancelled in destroy() so it cannot run against detached DOM. No API or behaviour change: the same attributes end up on the same elements, just computed when they can actually differ. Adds cypress/e2e/perf-scroll-aria.cy.ts (6 cases): zero ARIA rescans across five scroll positions, 20 synchronous scroll events producing fewer than 20 re-renders, setsize/posinset still correct after scrolling, metadata recomputed on search and on setOptions, and the queued frame cancelled on destroy. (cherry picked from commit 72ab721)
Option labels may legitimately contain markup - a flag icon, <b>, a <br> - and those labels
are also interpolated into aria-label attributes, where markup is meaningless. It reached the
screen reader as tag soup, and a double quote in a label closed the attribute early so the
rest of the accessible name was silently lost.
Adds Utils.getAriaLabelText(): tags collapse to a single space so adjacent words do not run
together ("Paris<br>France" must not become "ParisFrance"), whitespace is collapsed, then the
remaining quotes are escaped. Applied to the group header name, the group name carried into
each child's label, the customData group_name/description, and the option label itself.
Also replaces the inline strip on the tag clear button, which removed tags but never escaped
quotes - the same attribute breakout, in the one place that already knew about the problem.
WCAG 4.1.2 Name, Role, Value (A) and 1.1.1 Non-text Content (A).
Non-breaking: only the text inside aria-label changes, and only where it was malformed. No
roles, attributes or DOM structure change.
(cherry picked from commit f9d37b1)
Deciding whether a selected tag's text overflows (and therefore needs a tooltip) used to create a div, read getComputedStyle twice, append it to <body>, read clientWidth and remove it again - once per tag. Each DOM mutation invalidates layout for the read that follows, so rendering many tags produced a burst of forced synchronous layouts. Now one reusable off-screen node, created lazily and shared, and a single getComputedStyle read for every property. Font weight and letter spacing are applied too: both change advance width, so ignoring them under-reported overflow and could drop a tooltip that was needed. The node is absolutely positioned off-screen and aria-hidden, so it cannot affect layout or be announced, and it is removed in the existing last-instance teardown alongside the global listeners and the shared observer - nothing of ours is left in the document. Non-breaking: same boolean result, same tooltip behaviour. (cherry picked from commit 10cecf3)
… user Four blockers from the branch code review. All four were reproduced against the built bundle before the fix and re-checked after; each new spec case was confirmed failing first. They fall into two root causes. --- the announcement was written and then thrown away --- CR-01: setValue() called validate() - which announces its message through the polite live region - and then announced the selection summary in the same tick. A polite region is read from its *final* content, so the summary silently replaced the validation message: the region said "No options selected" while aria-invalid was true and the visible message said "This field is required". That silenced every interactive path (the clear button, deselecting below minValues) while still showing the error on screen, which is precisely the 3.3.1 failure the region was added to fix. Only the direct validate() call was covered by a spec, and that path announces correctly, so it shipped green. The summary is now skipped when validation failed: the error is the more urgent message and it already implies the selection state. CR-02: reset(formReset = true), the handler for the form's own reset event, removed the has-error class but left aria-invalid="true" and aria-describedby pointing at an error element that still held its text. The control stayed announced as invalid, describing a message the user could no longer see, with no interaction able to clear it. toggleRequired(false) was updated for this; reset() was not. It now clears aria-invalid and the message too. --- escaped storage text was reused as human-readable text --- With enableSecureText on, option label and description are stored HTML-escaped because they are inserted as HTML. Two places consume that text as *text*, and both were reading the escape sequences out loud. CR-03: Utils.getAriaLabelText() strips markup with /<[^>]+>/gi, but the label arrives already escaped, so there is no `<` to match and the markup passed through verbatim. The same option produced "Group, France" with escaping off and "Group, <i class=\"flag\"></i> France" with it on - so AI-14's fix was a no-op in exactly the mode this release promotes via setGlobalDefaults() and the OutSystems wrapper default. CR-04: announce() writes with textContent, so a single select announcing its chosen label put the escape sequence into the region: "Tom & Jerry selected". Both now go through Utils.getPlainText(): undo the escaping, then strip markup, then collapse whitespace. getAriaLabelText() is that plus attribute escaping, which now covers `&` as well as `"` so a bare ampersand in a label is a valid character reference rather than raw markup. Worth recording: decoding alone was not enough, and a regression guard caught it. Undoing the escaping turns "<i class=\"flag\">" into "<i class=\"flag\">" - one unreadable announcement swapped for another. Reducing to plain text is what makes it speech. The review's CR-04 described only the ampersand half. The XSS guard is unaffected: decoding happens on the way *out* to text sinks, not on the way in to innerHTML. Verified that the img/onerror payload is still inert, that the live region contains no elements, and that HTML labels still render. Verification: 7 assertions red before, 23 green after, plus a 22-assertion re-run of the AI-20/AI-22/AI-23 security work and a sweep over the 36 demo instances - 198 data-value round-trips and 34 announcements, none containing markup or escape sequences. tsc, eslint, stylelint and the 72 scripts/ci tests pass. One commit rather than four: the fixes were verified as two shared root causes in a single red-then-green cycle, and the history should match the verification behind it.
…v/virtual-select into gm/a11y-improvements-v1
… honour globals
Three findings from the second code review. Each was reproduced against the built
bundle first and each new spec case was confirmed failing before the fix.
--- BL-01: decodeSecureText() missed the fourth entity ---
secureText() escapes by assigning to a text node and reading innerHTML back, and
that serialiser escapes FOUR characters, not three. Measured rather than assumed:
& -> & < -> < > -> > U+00A0 ->
" ' ` = é — U+202F U+200B all pass through untouched
decodeSecureText() inverted the first three, so with enableSecureText on a label
containing a no-break space was named and announced as the literal `Item A`
while rendering correctly on screen - the exact failure mode the previous commit
fixed for the other three, surviving for the fourth. My miss: I derived that list
by reasoning about the serialiser instead of measuring it.
` ` is decoded before `&`, which must stay last, or a literal
`&nbsp;` would turn into a real no-break space.
Note what the fix does and does not do. The entity no longer survives; the
character itself is normalised to an ordinary space in names and announcements,
because getPlainText() collapses whitespace and JavaScript's \s matches U+00A0.
That is the right outcome for text that will be spoken, and the visible label
still carries the real character - both are pinned by spec cases.
--- BL-02: a validation error was announced for interactions nobody made ---
setErrorMessage() announced unconditionally, so two paths spoke an error the user
had not caused:
- construction. The initial setValueMethod() runs before isInitialized is set,
so { multiple, required, minValues: 2, selectedValue: ['o1'] } loaded already
saying "Select at least 2 options".
- a programmatic refresh. afterSetOptions() calls reset(), which validates, so
replacing the options announced a failure for an untouched field.
The announcement is now guarded on isInitialized and on a new isRefreshingOptions
flag scoped to the reset() inside afterSetOptions(), released in a finally for the
same reason isClosing is - a stuck flag would silently suppress every later error,
which is harder to diagnose than an exception.
The state itself is still exposed unconditionally: aria-invalid and the visible
message appear as before. Only the announcement waits. The interactive paths the
previous commit fixed - the clear button, dropping below minValues, and an
explicit validate() from the application - must still announce, and three spec
cases hold that line so a future widening of the guard cannot regress them.
Attribution, so nobody mis-blames the previous commit: the construction path is
pre-existing, since setErrorMessage() was never guarded and the selection summary
was already skipped at init. What that commit changed was only *which* message the
refresh path spoke. Both were wrong.
--- WR-10: an explicit undefined defeated a page-level policy ---
Object.assign copies own enumerable keys including ones whose value is undefined,
so `enableSecureText: wrapper.sanitizeValues` with an unset wrapper property
overwrote setGlobalDefaults({ enableSecureText: true }) instead of falling back to
it. A host could enable escaping page-wide and still get it off at every such call
site, with nothing to show it had been overridden - and that is exactly the shape
the OutSystems wrapper produces. undefined-valued keys are now dropped before the
merge, which also makes it agree with this method's own resolve() helper; the two
disagreed inside one function.
This changes undefined for every prop, not just enableSecureText: it now means
"fall back" rather than "override with undefined". That is the intended reading,
it matches resolve(), and it fixes cases like placeholder: undefined previously
rendering the string "undefined". It belongs in the upgrade notes.
Still open, deliberately: setGlobalDefaults({}) does not clear and a non-object
silently wipes the policy (WR-10's other halves). Fixing those means deciding the
intended reset semantics of a public API, which is a design call rather than a
bug fix. It bit this work once - a stale global leaked between checks in a manual
sweep and made an unrelated case look broken - so it is worth settling soon.
Verification: 6 assertions red before, 21 green after; a 22-assertion re-run of
the AI-20/AI-22/AI-23/AI-24 work; and a sweep over the 36 demo instances - 198
data-value round-trips, 34 announcements and 6 accessible names, none carrying
markup or an escape sequence, with every instance silent on load. tsc, eslint,
stylelint and the scripts/ci tests pass.
`.github/README.md` requires that build files are not committed in a PR, only generated for a release. This branch had held that line deliberately - dist/, dist-archive/ and docs/assets/ pinned to master's content in every commit, so the diff is source and tests only - until 35540fc committed all eight of them again. That is not just tidiness. Those artefacts predate the two most recent source commits (97c9590 and 9108828), so the committed bundle no longer matched src/: anyone opening the docs demo, or trusting dist/, got pre-fix code. It cost real time here - a verification run against the stale bundle appeared to show the validation announcement being overwritten, which looked like a regression in 97c9590 and was not. Re-checked against a fresh build, one announcement per transition and the error wins whenever the field is invalid. So the rule is worth restating: after `npm run build`, `git status` will show these eight files modified. That is the correct state on a feature branch - do not commit them. Restore with `git checkout HEAD -- dist/ …` (not master, once this commit has landed they are the same). The release procedure in .github/README.md is what generates them, after the version bump.
The CI run reported "payload must not execute" on `does not let an undefined
prop defeat the global default`, 8 of 9 cases green. The component was behaving
correctly; the marker was written by a different test.
The file shared one `window.__vsGlobalXss`, reset in beforeEach. The
insecure-by-default case deliberately creates a REAL
`<img src=x onerror="window.__vsGlobalXss=true">` and asserts the element
exists - but an image error event is asynchronous and that assertion passes long
before it fires. Measured against the built bundle, from the moment the
assertion passed:
first <img> +7 ms still inside its own test
second <img> +313 ms inside the `undefined prop` test, after its reset
Two images because the dropbox re-renders when its 300 ms open animation
finishes (showDuration: 300), creating a second live element. At 134 ms per
test, that second write lands two tests later. Timing-dependent, which is why
the other eight cases were green.
Two assumptions let it through, and the second corrects a note I had written in
ACTION-ITEMS: testIsolation is false AND baseUrl ends in `#/`, so
cy.visit('get-started') is a hash-only navigation that never reloads the
document. One window for the whole spec - the file already relied on that, since
it resets VirtualSelect.secureTextWarningShown by hand. So `cy.visit` was not
clearing anything between cases.
Fix, spec-only, no product change. Each test mints its own marker, so a late
detonation writes the marker of the test that caused it and cannot fail another.
Waiting it out was the alternative and was rejected: it swaps a real assertion
for a sleep and is still a race.
Two assertions added while here, both about tests that could pass for the wrong
reason:
- the insecure-by-default case now asserts its payload *does* execute, not
merely that an <img> element exists. That is the claim the case exists to
make, and it drains the first detonation inside its own test.
- the `undefined prop` case asserts the option rendered as text *before*
asserting no <img> exists. On its own, "no img" is equally true of a list
that never rendered at all.
resetGlobals() also could not do its job: setGlobalDefaults({}) merges, so
`enableSecureText: true` leaked forward from the previous case and only test
order kept it harmless - it contaminated my own diagnostic sweep twice. It now
resets by supplying undefined, which the merge treats as "not supplied": the
contract AI-25's WR-10 fix established, and independent of how the still-open
`setGlobalDefaults({})` clearing question is settled.
Verified by replaying all nine cases in a real browser back to back with no
waits between them - the hostile ordering: 9/9 pass, and after letting every
late detonation land, the markers of the two escaping cases are still unset
while the two that should detonate are true. tsc, eslint and prettier pass.
…-27) All four are in the safety net rather than the product, which is why they came before the remaining WR- items: every other conclusion on this branch leans on these specs. No product code changes here. Each rewritten case was replayed against the built bundle with real CDP key events - 30 assertions, all green - because Cypress still cannot start in this environment. --- WR-14: a case that documented the contract this branch removed --- `Arrow key behavior - no option navigation when search input focused` was left behind when 04abbe9 deliberately made Up/Down navigate options *from* the search input. Its name, its comments ("arrow key should move cursor, not navigate options") and a dead searchInputSelector variable all describe the old contract, and because its assertions only checked that DOM focus stayed in the field - still true - it passed while telling the next maintainer that the new behaviour is the bug. It also could not distinguish "navigates without stealing focus" from "does nothing at all". Renamed, and it now pins the whole contract: the highlight moves on Down, comes back on Up, DOM focus never leaves the field, and aria-activedescendant follows the highlight - the last of these being what makes the navigation perceivable to a screen reader while focus stays in the input (WCAG 4.1.2). cy.realPress() rather than cy.pressKeys() on purpose: pressKeys() focuses the field before pressing, which would make the focus assertion self-fulfilling. The cy.wait(100) is replaced by asserting the field's value and the filtered option count, so the case waits on a state instead of a clock. --- WR-15: an assertion that never ran --- "removes the measurer once the last instance is destroyed" guarded its only assertion behind `if (remaining === 0)`. Measured: the docs page keeps 2 instances alive, so that is false on every run. The case reported green while asserting nothing, and Utils.removeTextMeasurer() was never exercised. It now creates the state instead of testing for it - destroys every active instance, asserts the set is empty, then asserts the node is gone. Safe because this describe runs with testIsolation: true and it is the last case in the file. It also asserts there were instances to destroy, so the teardown cannot quietly become a no-op the way the old guard did. --- WR-16: .type() chained onto virtualised nodes, in two places --- The pattern 9d7d35c removed from four cases survived in the Option group suite with the count raised to 20: each iteration re-queried `.focused` and typed into a node renderOptions() may have replaced in between. The neighbouring case has the same defect and is not in the review's list, so it is fixed here too - same fault, 25 lines up. Both now use pressKeys() and a fresh query per assertion, with no aliases for option nodes. The alias mattered for more than the keystroke: a detached node keeps its classes, so `cy.get('@lastoption').should('have.class', 'focused')` would have passed without checking anything. One note on evidence: confirming the clamp with 20 synthetic keydowns in a tight loop showed the highlight back at index 0, which looks like wrap-around. That was a harness artefact - no re-render between presses. With real presses spaced as Cypress commands are, 20 presses land on the last group option and a 21st leaves it there. --- WR-17: pressKeys() broke on any search: false instance --- It resolved .vscomp-search-input unconditionally - the exact case openFresh() guards two commands above. Nothing currently mounts that way and calls pressKeys(), so this was a latent trap: the next such case would have failed with an opaque "expected to find element". It now focuses $searchInput || $wrapper. The wrapper is the right fallback rather than a workaround: it carries role="combobox" and tabindex="0", and it is where onKeyDown is bound ($allWrappers), which is how a keystroke in the search input reaches the handler in the first place - by bubbling up to it. Verified on a real search: false instance: ArrowDown navigates, a second press moves on, Enter selects. The invalid-key error also interpolated the caller's own keys where it meant the valid set, so a typo produced `Invalid key provided: "Foo". Must be one of: Foo`. A single SPECIAL_KEYS constant now feeds the type, the guard and the message, so they cannot drift. tsc passes. Prettier: both touched files were already unformatted on HEAD (the repo does not run prettier over cypress/), so they are left as they are rather than reformatted wholesale, which would bury the change - but every added line is within the configured 120-column width.
…ge (AI-27) My WR-15 fix hung the Cypress runner. It created the "last instance destroyed" state by destroying every active instance, including the two the docs page owns, and the run stopped dead on that case with nothing printed after it. The mechanism is unexplained and is not in the component. Replayed outside Cypress against the built bundle, that same teardown is clean: three destroys, no throw, no re-entry, no page error, the shared MutationObserver and the page-level listeners both released, the measurer node removed and nothing left tagged .vscomp-ele. So something about destroying the AUT page's own components from inside a test upsets the runner rather than the code under test. Noted as unknown rather than guessed at - and worth not repeating. The other route is better anyway: mount on a docs page that has no demos of its own, and our instance is genuinely the last one, so plain unmountVs() reaches the state the case is named for. Measured instances per page - properties 0 (with 13.7k characters of real content, so it is a real page and loads the same bundle), methods 0, get-started 2, events 3. This uses properties. Nothing outside the test is torn down now, and the precondition is asserted rather than assumed: if that page ever gains a demo, the case fails loudly instead of quietly going vacuous again, which was the original defect. Verified 7/7 in a real browser: the page starts with no instances, the measurer appears once a tag is measured, our mount is the only instance, unmounting reaches zero, the node is removed, the shared observer and global listeners go with it, and the docs page is left intact. tsc and prettier pass.
PR Test Results — ✅ all checks passed
Tested commit: |
…sted combobox (AI-28)
Three findings from a review of the whole branch diff. All three are in this
branch's own additions rather than in pre-existing code: each ships a new
public surface - a DOM node, an ARIA role, a static method - and each had a
defect in that new surface. Two of them also close questions this branch had
left open as design calls, which matters more than the fixes themselves.
1. The live region (AI-6) and the validation message (AI-4) were rendered
inside .vscomp-wrapper, which carries role="combobox". An unlabelled
combobox is named from its contents, and visually-hidden text still counts
toward that computation - so the field's own accessible name absorbed the
search result count and changed as the user typed. Both nodes are now
siblings of the wrapper, inside the host element. The aria-describedby
association and the role="status" announcement are unaffected; neither
depends on nesting. The reveal rule becomes a sibling selector, so the
stylesheet delta stays at five hunks - the rule changed shape, none was
added.
2. The search input is no longer a second role="combobox". AI-7 gave it the
role while the wrapper kept it for the closed control; the nesting was
flagged twice as an open question and deferred to AI-13. It did not need
AI-13 - the resolution is a subtraction. Everything the navigation depends
on (aria-autocomplete, aria-controls, aria-activedescendant) is supported
by the implicit textbox role, and aria-expanded, which is not, is dropped.
This closes WR-08 by removing the class of bug rather than re-syncing the
state: the input's aria-expanded was hard-coded false and updated only in
the open/close paths, neither of which runs under keepAlwaysOpen, so the
wrapper reported true while the input reported false for the same listbox.
The review proposed writing both elements from one place; deleting one of
the two writers leaves no second state to drift.
3. setGlobalDefaults() ignores a non-object instead of clearing every default,
and resetGlobalDefaults() is added for explicit clearing. This settles the
two questions left open under WR-10 while the API is still unreleased.
Wiping a page-wide security policy is the worst outcome for what is
usually a host forwarding an unset config variable; and {} meaning "clear"
would make a merge method inconsistent with itself. A single key can still
be cleared by passing it as undefined, which is the contract AI-25
established, so the two mechanisms now agree.
Verified as one red-then-green cycle, not three, so the commit matches the
verification boundary rather than being subdivided after the fact: 6
assertions failing first, all green after. WR-08's new keepAlwaysOpen case was
separately confirmed red against a build with the two attributes restored -
the failure message is the contradiction itself.
Full suite green for the first time: 406/406 across 25 specs, including the 54
security cases that had only ever been verified by driving a browser. Cypress
could not start on this machine because the editor host exports
ELECTRON_RUN_AS_NODE=1, which makes the Electron binary run as plain Node and
reject its own flags; env -u ELECTRON_RUN_AS_NODE works. eslint, stylelint and
tsc clean. No build output committed.
7c8249b to
cbdc5de
Compare
There was a problem hiding this comment.
Pull request overview
This PR substantially improves the VirtualSelect component’s accessibility (keyboard + ARIA semantics + live announcements + validation messaging), hardens multiple attribute-injection/XSS-adjacent surfaces, and reduces interaction-path performance costs (notably scrolling and tag tooltip measurement).
Changes:
- Adds/updates accessibility behavior and semantics: arrow-key navigation from search, Escape close behavior, live region status announcements, required/invalid state exposure, reduced-motion support, multiselectable listbox, and target-size CSS.
- Security hardening: escapes at attribute boundaries, sanitizes accessible-name sources, addresses
__proto__key collisions, and introduces page-levelsetGlobalDefaults()to enforce escaping centrally. - Performance improvements: coalesces scroll re-renders, avoids repeated O(n) ARIA scans, and reuses a shared off-screen text measurer node.
Reviewed changes
Copilot reviewed 30 out of 33 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/virtual-select.types.js | Documents new localisation/announcement/validation text props. |
| src/utils/utils.js | Adds shared text measurer + reduced-motion helper + attribute/ARIA label text utilities. |
| src/utils/dom-utils.js | Adds attribute removal + ARIA toggle helper; hardens attribute serialization by escaping quotes. |
| src/sass/partials/virtual-select.scss | Styles for error message, Select All hit target, and reduced-motion transitions. |
| src/sass/partials/variable.scss | Introduces $min-target-size and updates tag clear sizing baseline. |
| src/virtual-select.js | Core behavior changes (global defaults API, teardown cleanup, ARIA/keyboard/security/perf behavior). |
| docs/properties.md | Updates security guidance for what is escaped and how to enforce it globally. |
| docs/methods.md | Documents new VirtualSelect.setGlobalDefaults() API. |
| docs/assets/virtual-select.min.css | Updates built/minified docs CSS output. |
| cypress/support/mount.ts | Adds shared mount/unmount helpers for the new regression suite. |
| cypress/support/index.d.ts | Extends HTMLElement + Cypress command typing for VirtualSelect test driving. |
| cypress/support/commands.ts | Adds openFresh() and improves key handling + error messaging for test stability. |
| cypress/e2e/examples.cy.ts | Updates existing a11y tests to new keyboard/ARIA behavior and reduces flake risk. |
| cypress/e2e/a11y-search-arrow-navigation.cy.ts | New regression spec for search-input-driven navigation + activedescendant behavior. |
| cypress/e2e/a11y-escape-close.cy.ts | New regression spec for Escape close behavior across layouts. |
| cypress/e2e/a11y-close-clears-highlight.cy.ts | New regression spec for synchronous highlight clearing on close/reopen edge cases. |
| cypress/e2e/a11y-live-region.cy.ts | New regression spec for status announcements via live region. |
| cypress/e2e/a11y-required-error.cy.ts | New regression spec for required/invalid state, messaging, and announcement behavior. |
| cypress/e2e/a11y-select-all.cy.ts | New regression spec for Select All checkbox semantics + keyboard behavior. |
| cypress/e2e/a11y-target-size.cy.ts | New regression spec for 24×24 pointer target minimums. |
| cypress/e2e/a11y-reduced-motion.cy.ts | New regression spec ensuring reduced-motion is honored in JS durations + CSS. |
| cypress/e2e/a11y-listbox-multiselectable.cy.ts | New regression spec for aria-multiselectable exposure in multiple mode. |
| cypress/e2e/a11y-aria-label.cy.ts | New regression spec ensuring accessible names are plain text and quote-safe. |
| cypress/e2e/security-hidden-input-name.cy.ts | New regression spec preventing name attribute injection/breakage and verifying FormData submission. |
| cypress/e2e/security-proto-value.cy.ts | New regression spec for __proto__/prototype-member value handling via APIs. |
| cypress/e2e/security-quote-escaping.cy.ts | New regression spec validating quote escaping at attribute boundary and search correctness. |
| cypress/e2e/security-ampersand-storage.cy.ts | New regression spec ensuring values are stored verbatim and searchable even with &, <, etc. |
| cypress/e2e/security-chrome-label-props.cy.ts | New regression spec preventing attribute breakouts via component chrome label props. |
| cypress/e2e/security-global-defaults.cy.ts | New regression spec for enforcing escaping via page-level global defaults. |
| cypress/e2e/perf-scroll-aria.cy.ts | New regression spec preventing O(n) ARIA work on scroll + coalescing renders. |
| cypress/e2e/perf-text-measurer.cy.ts | New regression spec ensuring single shared measurer node + cleanup on last destroy. |
Files not reviewed (1)
- docs/assets/virtual-select.min.css: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
`.github/README.md` requires that build files are not committed in a PR, only generated for a release, and 6762b0c re-pinned all eight of them for that reason. `8f280c1 Update docs` then committed the three `docs/assets/` artefacts again, which is what the review flagged: a minified diff in the PR, and a bundle that goes stale against `src/` as soon as the next source commit lands. Restored to master's content. `dist/`, `dist-archive/` and `docs/assets/` all show as modified after a local `npm run build` - that is the correct state on a feature branch, not something to commit. The release procedure in `.github/README.md` regenerates them after the version bump.
The `setGlobalDefaults()` security note listed `label`, `value` and `description` as raw HTML that `enableSecureText` escapes. Only `label` and `description` are: since faef03c (AI-22) `value` is stored verbatim and reaches no HTML sink - it is compared, used as a map key, and written to `data-value`, which is escaped at that boundary. `properties.md` already states this in both the security note and the prop table, so the methods page was the last place contradicting it - and the one a consumer reads when sizing up their XSS exposure and what turning the flag on actually buys them.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 33 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- docs/assets/virtual-select.min.css: Generated file
Suppressed comments (2)
docs/methods.md:281
- The security note here still states that option
valueis inserted as raw HTML. Elsewhere in the docs (properties.md) and in the implementation rationale,valueis not rendered as HTML (only compared / used as a key and escaped at attribute boundaries), so this description is inaccurate and could mislead consumers about what enableSecureText protects.
The main use is security. Option `label`, `value` and `description` are inserted as raw HTML and
are only escaped when `enableSecureText` is on, which is **not** the default (escaping runs per
option and is measurable on 10k-100k+ lists). If any option text in your app can come from
untrusted input, turn it on once during startup:
docs/assets/virtual-select.min.css:6
- This file is a generated build artifact under
docs/assets/, but the repo PR guidelines explicitly say not to commit build files (they should be generated only for a release). The PR description also states that no build output is committed, yet this file is modified. It also includes a UTF-8 BOM before@keyframesand still declaresVirtual Select v1.3.0in the header, suggesting the committed artifact may be stale or inconsistently generated.
…adings Both statics landed in cbdc5de (AI-28) documented only as prose inside the `setGlobalDefaults()` section. Every other method on the page has its own `###` heading, and docsify builds the page ToC and the heading anchors from those - so neither was scannable, and nothing could link to them the way `properties.md` links to `methods.md#virtualselectsetglobaldefaults`. No behaviour change. Two small gains beyond the headings: the reset note now links to its section rather than just naming the method, and `getGlobalDefaults()` is described as returning a *shallow* copy, which is what it returns (`{ ...VirtualSelect.globalDefaults }`, src/virtual-select.js:4325) - the previous one-liner said "a copy", which would have been read as protecting nested values it does not protect.
| * @static | ||
| * @returns {HTMLElement} | ||
| */ | ||
| static getTextMeasurer() { |
There was a problem hiding this comment.
I would say this is too demanding if there are a lot of options selected... why not having an inline css variable assigned to the parent with it's width and tags will inherit it value as max value?
I know this is an old code with several improvements, however and since we're tacking that much files I would say we could remove this cost of being adding hidden elements, managing them just to ensure the text width... Not a blocker, more like challenging the approach...
There was a problem hiding this comment.
Worth challenging, and you're right that the hidden node can go — though not quite by the route in the suggestion.
The tags already truncate purely in CSS: .vscomp-value-tag has max-width: 100% plus the ellipse mixin, and .vscomp-value-tag-content is width: calc(100% - 24px) with the same mixin. So an inline variable carrying the parent width would be a second spelling of a cap that is already there. The measurement isn't sizing anything — it only decides whether that tag gets a tooltip, and CSS can truncate but can't report that it truncated.
Where your instinct does land: the check doesn't need an off-screen clone. DomUtils.hasEllipsis() (scrollWidth > offsetWidth) already exists and is already used for the non-tag value text, this.$valueTags is already collected right after the innerHTML write, and setValueTagAttr() already loops over every tag. Moving the check there measures the real rendered box in one batched read pass, instead of the current one forced layout per tag.
It would also fix two accuracy bugs that predate this branch. The comparison uses .vscomp-toggle-button's clientWidth, which is ~73px more than the text actually gets (button padding 4px 22px 0 10px, tag border/padding/margin ~17px, 24px clear button), while the font is copied from the wrapper at 14px when tags render at 12px — so it under-reports the available width and over-reports the text width at the same time.
Not folding it into this PR: it changes which tags get tooltips (more of them, correctly), it's pre-existing on 1.3.0, and it deserves its own before/after check. Agreed it's not a blocker.
Follow-up issue created: #487 — Tag tooltips: the overflow check measures the wrong box, so tooltips are both missed and added unnecessarily, with the measurement table and the hasEllipsis() approach written up.
os-davidlourenco
left a comment
There was a problem hiding this comment.
LGTM - Overall, it seems a very good update, tested all the cases and worked well. Kept the doubt regarding the ServerSide Search ones not being announced, as well as the impact of setting a min-width/min-height on our internal side.
Issue number: resolves #
Every use case below ships with a regression spec in
cypress/e2e/that was written first and confirmed failing against the previous build. Suite: 406 tests across 25 specs, all passing.tsc,eslint srcandstylelintare clean. No build output is committed, per.github/README.md.How to run everything:
For the manual steps below,
npm run build && npm run docsthen open the served site. A screen reader is needed only where stated (NVDA + Firefox, or VoiceOver + Safari).What is the current behavior?
Twelve areas are affected. Grouped by use case, with the problem, the change, and how to verify it.
1. Keyboard operation of the option list
Problem. The component could not be operated from the keyboard at all in its default configuration — WCAG 2.1.1 (A):
dropboxWrapper: 'self'. Containment was tested against$dropboxWrapper, which isundefinedin that configuration, so the branch never ran — WCAG 2.1.2 (A)..focusedoption,focusedOptionIndexandaria-activedescendantall survived, so reopening inside that window resumed navigation one row past where the user left off. On a grouped multi-select that meant Enter toggled the first child option instead of the group header..focus()on the element it had just un-highlighted, which on close pulled focus into a dropbox being hidden and fought the wrapper refocus — leaving keyboard position on<body>.New behavior. Up/Down drive the list from the search input following the WAI-ARIA APG editable-combobox pattern: focus stays in the field so typing keeps working, and the active row is published as
aria-activedescendant. Caret movement is Home / End / Left / Right. Escape closes in every layout. The highlight is dropped synchronously on close, and clearing a highlight no longer moves DOM focus.How to test.
<body>.Specs:
a11y-search-arrow-navigation.cy.ts,a11y-escape-close.cy.ts,a11y-close-clears-highlight.cy.ts.2. Screen reader status announcements
Problem. The stylesheet shipped a
.vscomp-live-regionrule but no JavaScript ever created the element, so there were zero live regions in the DOM. Result counts, "no results", server-search loading and every selection change were conveyed visually only — WCAG 4.1.3 (AA). A screen reader user typing into the search box heard nothing at all.New behavior. A per-instance
role="status" aria-live="polite" aria-atomic="true"region announces the filtered result count, no-results, server-search loading, and selection changes. Seven new overridable props for localisation:searchResultsText,searchResultText,noOptionsSelectedText,selectedText,loadingText, plusrequiredErrorText/minValuesErrorTextbelow.Announcements are deliberately scoped to things the user did: nothing is spoken on construction (an initial value does not announce on page load) or for a programmatic
setOptions()refresh. Repeating the same message is a no-op, so "No results found" is not re-read on every further keystroke that still matches nothing. Messages are reduced to plain text, so a label containing markup or an escape sequence is spoken as words rather than asTom & Jerryor tag soup.How to test. With a screen reader on:
Without a screen reader, inspect
.vscomp-live-regioninside the host element and watch itstextContentchange as you interact.Spec:
a11y-live-region.cy.ts.3. Required fields and validation errors
Problem.
requiredwas never exposed to assistive technology, and a failedvalidate()only toggled ahas-errorclass that reddened the toggle button's border. That is a failure signalled by colour alone — WCAG 1.4.1 (A) — with noaria-required, noaria-invalid, no text message and no announcement, so WCAG 3.3.1 (A) was unmet too.Two further defects sat behind it:
setValue()announced the validation message and then overwrote the region with the selection summary in the same tick (a polite region is read from its final content, so the error was never actually spoken on any interactive path), and a native form reset removed only thehas-errorclass — leavingaria-invalid="true"andaria-describedbypointing at a populated-but-hidden message, so the control stayed announced as invalid with no way to clear it.New behavior.
aria-requiredreflects the constraint;aria-invalidandaria-describedbyreflect the failure; a text message renders with a ⚠ glyph as a second, non-colour cue; and the message is announced. Two new overridable props:requiredErrorText(defaultThis field is required) andminValuesErrorText(Select at least {count} options, where{count}is substituted). A form reset now clears the class,aria-invalid,aria-describedbyand the message text together.How to test.
{ required: true }. Inspect the wrapper —aria-required="true"is present; on an optional field the attribute is absent, not"false".validate()with nothing selected — a red message with a ⚠ appears below the field,aria-invalid="true"is set, andaria-describedbypoints at the message'sid.{ multiple: true, required: true, minValues: 3 }, select two — the message reads "Select at least 3 options".aria-invalidandaria-describedbyall clear.<form>and press a native reset button — every error affordance clears, and the control is no longer announced as invalid.Spec:
a11y-required-error.cy.ts.4. Select All control
Problem. Select All was a bare
<span>with anaria-label. It exposed as a generic element with no role and no state, so every select/deselect was silent to assistive technology — WCAG 4.1.2 and 1.3.1 (A). Space did not activate it (only Enter did) and instead scrolled the page — WCAG 2.1.1 (A).New behavior.
role="checkbox"witharia-checkedkept in sync from the single method every selection path funnels through (select all, deselect all, per-option clicks, group toggles,setValue,reset), so the exposed state cannot drift. Space activates it, withpreventDefault()so the page no longer scrolls; Enter still works.How to test.
.vscomp-toggle-all-button—role="checkbox"andaria-checked="false".aria-checkedbecomes"true", and the page does not scroll.aria-checkedback to"false".aria-checkedreturns to"false"on its own.aria-checkedflips to"true"without touching Select All.Spec:
a11y-select-all.cy.ts.5. Accessible names
Problem.
aria-labelon options and group headers was built by interpolating the raw label, so an icon or<br>in a label was announced as literal tag soup (<i class="flag"></i> France), and a double quote in a label closed the attribute early — truncating the accessible name to the prefix and turning the remainder into markup. WCAG 4.1.2 / 1.1.1 (A).The same applied to six of the component's own label props, none of which passed through
secureText()— soenableSecureText: truedid not protect them either:ariaLabelText,ariaLabelledby,ariaLabelClearButtonText,ariaLabelSearchClearButtonText,selectAllText(itsaria-label) andsearchPlaceholderText.New behavior. Accessible names are plain text: markup is stripped with tags collapsing to a single space (so
Paris<br>FrancebecomesParis France, notParisFrance), and&and"are escaped at the attribute boundary. This works in both escaping modes — previously the strip was a no-op withenableSecureText: true, because the label arrived already escaped and there was no<left to match. A no-break space is read as a space rather than as .selectAllText's visible label still renders HTML, which is unchanged and intentional.How to test.
<i class="flag"></i> France. Inspect itsaria-label— readsFrance, with no tags. Repeat withenableSecureText: true— same result.The "City" of Light. Thearia-labelcarries the whole string and no stray attributes appear on the option row.selectAllText: 'Pick <b>all</b>'— thearia-labelisPick all, while the visible label still renders a real bold all.x" data-pwned="1" y="as all six label props withenableSecureText: true—document.querySelectorAll('[data-pwned]')returns empty, and each accessible name / placeholder contains the full string.Specs:
a11y-aria-label.cy.ts,security-chrome-label-props.cy.ts.6. Combobox structure and exposed state
Problem. Two structural ARIA defects:
role="combobox". A combobox with noaria-label/aria-labelledbytakes its accessible name from its contents, and visually-hidden text still counts toward that computation — so an unlabelled instance absorbed the result count and the error message into the field's own name, which then changed as the user typed.role="combobox"nested inside the first, with its ownaria-expandedrendered hard-coded asfalseand updated only inside the open/close paths. Neither of those runs underkeepAlwaysOpen, so such an instance permanently reported two contradictory states for one listbox: the wrapper said expanded, the input — the element actually holding focus — said collapsed. WCAG 4.1.2 (A).New behavior. The live region and error message render as siblings of
.vscomp-wrapperinside the host element, so they cannot contribute to its name; thearia-describedbyassociation and therole="status"announcement are unaffected, since neither depends on nesting. The wrapper is the singlerole="combobox"and the single carrier ofaria-expanded. The search input keepsaria-autocomplete="list",aria-controlsandaria-activedescendant, all supported by its implicittextboxrole, and dropsaria-expanded, which is not.aria-activedescendantis no longer written to the role-less dropbox container, where it was meaningless.How to test.
ariaLabelTextorariaLabelledby. Type in the search box, then read the combobox's computed accessible name in the browser's accessibility inspector — it does not contain the result count and does not change as you type.[role="combobox"] .vscomp-live-regionand[role="combobox"] .vscomp-error-messageboth match nothing; both elements exist as children of the host.roleattribute, noaria-expanded, andaria-controlspoints at the options container'sid..vscomp-wrapperhasaria-expanded="true". Press Esc — it becomes"false".{ keepAlwaysOpen: true }— the wrapper reportsaria-expanded="true"and the input carries no contradicting attribute..vscomp-dropbox-containerhas noaria-activedescendant.Specs:
a11y-search-arrow-navigation.cy.ts,a11y-live-region.cy.ts,a11y-required-error.cy.ts,a11y-listbox-multiselectable.cy.ts.7. Target sizes
Problem. Select All collapsed to its 25×15 content box and the per-tag clear button was 20×20, both below the 24×24 CSS px minimum — WCAG 2.5.8 (AA).
New behavior.
min-height/min-width: 24pxon Select All and 24×24 on the tag clear button. Hit area only — the checkbox glyph and the visible tag are unchanged, and the checkbox stays left-aligned with the option checkboxes below it rather than being centred in the enlarged box.How to test. Open a multi-select with Select All and inspect
.vscomp-toggle-all-button— computed box is at least 24×24. WithshowValueAsTagsand a value selected, inspect.vscomp-value-tag-clear-button— 24×24. Visually confirm the Select All checkbox still lines up with the option checkboxes underneath it.Spec:
a11y-target-size.cy.ts.8. Reduced motion
Problem. The open/close animation is driven from JavaScript as well as CSS, so a stylesheet
prefers-reduced-motionrule alone would still leave the popover animating forshowDuration/hideDurationmilliseconds. Neither existed. WCAG 2.3.3 (AAA).New behavior. A
@media (prefers-reduced-motion: reduce)block zeroes the transition durations, andshowDuration/hideDurationresolve to0when the preference is set. The options loader keeps spinning on purpose — it is a status indicator, and a frozen spinner reads as a broken UI; screen reader users get the same information from the live region.How to test. Enable "reduce motion" in the OS (or emulate it in DevTools → Rendering → Emulate CSS prefers-reduced-motion). Open a dropdown — it appears and disappears instantly with no slide. Read the instance's
showDuration— it is0. Confirm the loader still animates during a server search.Spec:
a11y-reduced-motion.cy.ts.9. Option identity and search under
enableSecureText: trueProblem. Escaping was applied to the text the library stores rather than to what it renders, which corrupted the option's own identity.
valuenever reaches an HTML sink — it is only compared, used as a map key, and written to adata-valueattribute — so escaping it protected nothing and broke the API's contract with itself:{ label: 'Tom & Jerry', value: 'a&b' }option.valuea&bsetValue(['a&b'])— the caller's own valueelement.value, feed it backsetDisabledOptions(['a&b'])Tom & JerryThe "City" of LightThe "City" of Light— visible mojibake"City"The normalised search keys derived from the escaped text, which is why search was unreachable for
",&,<and>.New behavior. Escaping applies to what is rendered as HTML:
labelanddescriptionstay escaped;valueis stored exactly as supplied; and quotes are escaped at each attribute the library writes rather than in the stored text. Search keys derive from the raw text. The XSS protection is unchanged —<img src=x onerror=…>in a label is still inert, with regression specs for it.How to test. With
enableSecureText: trueand an option{ label: 'Tom & "Jerry"', value: 'a&b' }:option.value—a&b, exactly as supplied.setValue(['a&b'])— selects the option.element.value, thensetValue()it back — the selection round-trips.setDisabledOptions(['a&b'])— the option is disabled.Tom &, then"Jerry"— both find it.Tom & "Jerry", not".<img src=x onerror="window.__x=true">— no<img>is created andwindow.__xstays undefined.Specs:
security-ampersand-storage.cy.ts,security-quote-escaping.cy.ts,secure-text-warning.cy.ts.10. Attribute-injection paths
Problem. Six sinks interpolated untrusted or developer-supplied strings straight into attributes, most of them regardless of
enableSecureText:name—name="${this.name}"in the template. A quote closed the attribute, soitems["a"]submitted asitems[— a wrong-but-plausible field name, which is worse than an obvious failure. Worse still,nameis interpolated before the field'sclass, so a payload that closed the tag swallowed the class too;querySelector('.vscomp-hidden-input')then returnednulland the firstsetValue()threw inside the constructor — the control never rendered at all.data-valueon the option row — a value ofx" data-pwned="1" z="put a live attribute on the element.data-tooltipon a value tag — escaped only when the label happened to contain an HTML tag, so a tag-free payload went in raw. The same gap affected aplaceholderorclearButtonTextcontaining a quote.customData.group_name/.descriptioninterpolated intoaria-label.classNamesbreaking out of theclassattribute.Separately, value-keyed lookups used plain objects.
mapping['__proto__']returns the inheritedObject.prototype— truthy, but never=== true, which is exactly what those lookups test. So an option whose value is__proto__was selectable by click but invisible tosetValue,setDisabledOptionsandsetEnabledOptions;setEnabledOptions(['__proto__'])disabled every option; and withallowNewOptionit was silently duplicated. (No prototype pollution was possible either before or after —mapping['__proto__'] = truecalls the inherited setter, which ignores a non-object value.)New behavior.
nameis assigned as a DOM property after render, so there is no HTML parsing to break out of. Quotes are escaped at every attribute boundary, unconditionally.data-valueescapes&and"so it still round-trips.customDataandclassNamesare escaped/sanitised. All value-keyed maps useObject.create(null).How to test.
name: 'items["a"]', put it in a<form>, select a value, and readnew FormData(form)— the key isitems["a"]exactly.name: 'x" data-pwned="1"><img src=x>'and an initial value — the control renders normally anddocument.querySelectorAll('[data-pwned]')is empty.value: 'x" data-pwned="1" z="'— no injected attribute on the option row;dataset.valuereads back the original string.showValueAsTags, select an option whose long label containsx" data-pwned="1" y="— no injected attribute on the tag.value: '__proto__'—setValue(['__proto__'])selects it,setDisabledOptionsandsetEnabledOptionsboth target it correctly, andObject.getPrototypeOf({})is untouched.customData.descriptionandclassNamescontaining a payload — nothing executes and no attribute is injected.Specs:
security-hidden-input-name.cy.ts,security-proto-value.cy.ts,security-quote-escaping.cy.ts,security-customdata-xss.cy.ts,security-classnames-xss.cy.ts.11. Page-level control over escaping
Problem. Option
labelanddescriptionare interpolated intoinnerHTMLand are only escaped whenenableSecureTextis on, which it is not by default (escaping runs per option and is measurable on 10k–100k lists). A host application that renders untrusted option text had to remember the flag at every one of possibly hundreds of call sites, with no way to enforce it centrally.A related defect made any such policy unreliable:
Object.assigncopies keys whose value isundefined, so a wrapper forwarding an unset variable (enableSecureText: config.sanitize) overrode the default rather than falling back to it.New behavior.
VirtualSelect.setGlobalDefaults({ enableSecureText: true })sets page-level defaults for every instance created afterwards;getGlobalDefaults()returns a copy;resetGlobalDefaults()clears them. Precedence is per-instance options → page-level globals → built-in defaults. A prop passed asundefinednow means "not supplied" and falls back through that chain. A non-object argument tosetGlobalDefaults()is ignored rather than treated as "clear", so a host forwarding an unset config variable cannot silently disable a security policy it had enabled.eleandoptionsare ignored, being inherently per-instance.How to test.
VirtualSelect.setGlobalDefaults({ enableSecureText: true }), then init a dropdown with a payload label and noenableSecureText— the payload renders as text and does not execute.enableSecureText: false— it still renders raw HTML, confirming these are defaults and not overrides.enableSecureText: undefined— it picks up the globaltrue.setGlobalDefaults({ placeholder: 'Pick one' })and confirmgetGlobalDefaults()reports both keys — calls merge.setGlobalDefaults(null)and confirmgetGlobalDefaults().enableSecureTextis stilltrue.resetGlobalDefaults()and confirmgetGlobalDefaults()is{}.getGlobalDefaults()and confirm internal state is unaffected.Spec:
security-global-defaults.cy.ts. Documented indocs/methods.md.12. Rendering performance
Problem. Three costs on the interaction path, measured at 100k options:
aria-setsize/aria-posinsetonly change when the filtered set or its order changes.willTextOverflow()created a<div>, readgetComputedStyletwice, appended it to<body>, readclientWidthand removed it — once per selected tag. Each mutation invalidates layout for the read that follows, so rendering many tags meant a burst of forced synchronous layouts.New behavior. Scroll re-renders coalesce to at most one per animation frame, with the pending frame cancelled in
destroy()so it cannot run against a torn-down instance. The ARIA scan runs only when the filtered set or its order changes, driven by a dirty flag that every mutating path sets. One shared,aria-hidden, off-screen measuring node is reused and removed with the last instance; it now also accounts forfont-weightandletter-spacing, which affect advance width and previously caused overflow to be under-reported.How to test.
calculateAriaMetadataand scroll — it is not called per tick; it is called after typing a filter or changing sort order.aria-setsize/aria-posinsetare still correct after filtering, sorting, and adding a new option withallowNewOption.showValueAsTags, select many options and confirm exactly one.vscomp-text-measurernode exists in the DOM.Specs:
perf-scroll-aria.cy.ts,perf-text-measurer.cy.ts.What is the new behavior?
Summarised — the detail and test steps are per use case above.
Added
VirtualSelect.setGlobalDefaults()/getGlobalDefaults()/resetGlobalDefaults()for page-level defaults.role="status"live region, with five new overridable announcement props.aria-required,aria-invalid,aria-describedbyand a validation message with a non-colour cue, plusrequiredErrorTextandminValuesErrorText.aria-multiselectableon the listbox in multiple mode.aria-activedescendanton the combobox, tracking the highlighted option.prefers-reduced-motionsupport in CSS and in the JS durations.Changed
role="checkbox"with syncedaria-checked, activated by Space.enableSecureTextescapes only what is rendered as HTML;option.valueis stored verbatim; quotes are escaped at the attribute boundary..vscomp-wrapper; the wrapper is the solerole="combobox"and sole carrier ofaria-expanded.Fixed
<body>.name,data-value,data-tooltip,customData,classNamesand six label props can no longer break out of their attribute.valueis__proto__, or contains&,<,>or", is settable, disablable and restorable.undefinedno longer overrides a page-level default.Performance — scroll re-renders coalesced per frame; ARIA scan off the render path; one shared text measurer.
Does this introduce a breaking change?
No JavaScript API changed shape — no prop, method or event gained, lost or renamed a parameter, and nothing was removed. Three things are observable:
Up/Down in the search input navigate options instead of moving the caret. Caret movement is Home / End / Left / Right. This is the only change every user meets regardless of configuration, and it was kept because it is the only fix for a Level A keyboard failure that otherwise leaves the component unusable from the keyboard. Keyboard E2E tests that typed
{downarrow}to move a caret will now change the highlight instead.Under
enableSecureText: trueonly, escaping applies to what is rendered rather than what is stored. If you do not set that flag, nothing here affects you.option.valuea&ba&boption.labelTom & &quot;Jerry&quot;Tom & "Jerry"Tom & "Jerry"Tom & "Jerry"setValue(['a&b'])element.value, set it backTom &or"Jerry"&,<,>or"tosetValue/setDisabledOptions/setEnabledOptionsa&bin a database) will no longer match and need a one-off migration"rather than". Update the snapshotsoption.labelback for displaylabelRenderer/selectedLabelRendererd.label, so building HTML from it is as safe as it wasA prop passed as
undefinednow means "not supplied" and falls back to the page-level default and then the built-in one, which is what the library's own precedence helper already assumed. Previously it overrode the default, soinit({ placeholder: undefined })rendered the literal string "undefined". This applies to every prop; passfalse/''/0to force a value.BREAKING.mddoes not exist in this repository, so item 1 is recorded as the lead item under Changed in the release notes. Happy to add the file if that is preferred before tagging.Other information
The CSS delta is four changes / five diff hunks, all from the accessibility work — verify with
git diff master..HEAD -- src/sass | grep -c '^@@':$value-tag-clear-width20px → 24px.vscomp-toggle-all-buttongainsmin-height/min-width: 24px.vscomp-error-messagerule, hidden until.has-error ~matchesvalidate()fails@media (prefers-reduced-motion: reduce)blockDeliberately out of scope. The default-theme contrast fixes and the missing combobox focus indicator are WCAG AA gaps, but each changes the theme's appearance unconditionally, so they belong in a visual pass with design sign-off rather than inside an otherwise appearance-neutral release. Group-header roles and
aria-setsizevalues, removing the defaultaria-label="Options list", Space-to-open plus Home/End/PageUp/PageDown,em-based sizing with a measuredoptionHeight, and Backspace removing the last tag rather than clearing everything are all held for 2.0.0 — each changes an observable contract, so shipping them separately would mean several releases each carrying its own migration note.One known residue, also held for 2.0.0:
option.labelandoption.descriptionare still stored escaped, so reading them back underenableSecureText: truereturnsTom & Jerry. Everything functional is fixed here — the value is verbatim, search matches the supplied text, and rendering is correct — so what remains is a read-back inconvenience. Finishing it means storing those two fields raw and escaping at render, which is breaking for a specific and easily-missed reason:labelRenderercurrently receives a pre-escapedd.label, so the common(d) => '<b>' + d.label + '</b>'is safe today and would silently become an injection. As a bonus it would make escaping cheaper — per visible row instead of per stored option, which is the cost cited for keepingenableSecureTextoff by default.Build output.
dist/,dist-archive/anddocs/assets/are not included, per the contributor note in.github/README.md.pr-tests.ymlrunsnpm run buildbefore E2E andnpm-publish.ymlbuilds before publishing, so nothing consumes the committed bundles; they should be regenerated at release time, after bumpingpackage.jsonto1.4.0.