From ab70d57c7df8c4abbc7cec387d7f6851943d0e66 Mon Sep 17 00:00:00 2001 From: gnbm Date: Tue, 11 Aug 2026 10:24:29 +0100 Subject: [PATCH 1/2] fix(a11y): stop marking the dropbox aria-hidden while it is still open closeDropbox() set aria-hidden="true" on the dropbox immediately, but the popover keeps it on screen for the ~200ms hide transition and the `closed` class - with the display:none that actually removes it - only lands in afterHidePopper(). For that window the dropbox was declared absent to assistive technology while still visible, still hit-testable and still isOpened() === true. Chrome refuses aria-hidden over the focused element ("Blocked aria-hidden on an element because its descendant retained focus"), so the dropbox stayed exposed after the component believed it had hidden it - the component and the accessibility tree disagreed, and a screen reader follows the tree. Selecting an option in a single select was enough to trigger it: the pointer is still over the list while it fades out, and hovering an option focuses it. Three coordinated changes: 1. aria-hidden now flips in afterHidePopper() only, together with the rest of the closed state, so "hidden from assistive technology" and "hidden on screen" can no longer disagree. tabindex still goes to -1 at close time - it was never part of the conflict (it does not block a programmatic focus()), so when the dropbox leaves the tab order is unchanged. 2. A closing dropbox ignores the pointer. isOpened() stays true for the whole hide transition, so hover kept driving the fading list - re-highlighting options, re-writing aria-activedescendant onto a combobox that had just announced itself collapsed, and pulling DOM focus back into the subtree about to be hidden. isClosingTransition gates onOptionsMouseOver() from hide start to hide end (and is cleared on reopen). Clicks and arrow keys are deliberately not gated - that would change what mid-fade interactions do. 3. releaseFocusFromDropbox() releases whatever focus is still inside when the dropbox is actually hidden - the pointer gate cannot cover a host focusing into the dropbox mid-fade. Focus is handed back to the combobox (WCAG 2.4.3 Focus Order; previously it silently fell to ), or only blurred when the close was caused by focus moving elsewhere (another dropdown opening, an outside click), so this instance never steals focus from where it now belongs. It runs after the `closed` class is committed and before the aria-hidden writes, and those writes stand down when a focus handler reopened the dropdown meanwhile. isFocusInsideDropbox() documents why this containment test is narrower than closeDropbox()'s within-component check. The stand-down guard is isOpened() rather than "is this instance back in openInstances", deliberately: the two differ for a hide that lost its race with an earlier reopen, and that hide has to complete. The popover has already taken the dropbox off the screen by then, so standing down would leave the wrapper without its `closed` class and every later toggle closing a dropdown that is not there. WCAG 4.1.2 Name, Role, Value (A), 1.3.1 Info and Relationships (A), 2.4.3 Focus Order (A). test(a11y): pin that focus never sits inside an aria-hidden subtree a11y-aria-hidden-focus.cy.ts installs the invariant Chrome enforces, from both directions - focus moving into a hidden subtree, and aria-hidden applied over a subtree that still holds focus - by patching Element.prototype.setAttribute (with String() coercion: setAria() forwards a boolean) so the offending call site is captured synchronously. Ten cases: the reported hover-while-fading flow and its multi-select variant, a silent close over the focused search input, a host focusing into the dropbox mid-fade, Tab onto an option followed by an outside click, Escape mid navigation, the open/hidden state coherence, a focusin handler that reopens during the close, a hide that lost a race with a reopen, and a deferred re-render after the close. The mid-fade cases pin the transition window (isOpened() asserted true) and dispatch mouseover synchronously, so they cannot pass vacuously on a slow runner. The guard installs idempotently and judges only vscomp nodes. --- cypress/e2e/a11y-aria-hidden-focus.cy.ts | 517 +++++++++++++++++++++++ src/virtual-select.js | 163 ++++++- 2 files changed, 670 insertions(+), 10 deletions(-) create mode 100644 cypress/e2e/a11y-aria-hidden-focus.cy.ts diff --git a/cypress/e2e/a11y-aria-hidden-focus.cy.ts b/cypress/e2e/a11y-aria-hidden-focus.cy.ts new file mode 100644 index 0000000..5157334 --- /dev/null +++ b/cypress/e2e/a11y-aria-hidden-focus.cy.ts @@ -0,0 +1,517 @@ +/** cSpell:ignore vscomp */ + +/** + * Focus must never sit inside an `aria-hidden="true"` subtree. + * + * Chrome refuses to apply aria-hidden when a descendant holds focus and logs "Blocked + * aria-hidden on an element because its descendant retained focus" - so the dropbox the + * component believes it hid stays exposed to assistive technology, which follows the tree, + * not the component. WAI-ARIA: aria-hidden must not be used on an ancestor of the focused + * element. + * + * Two windows produced it, both inside the popover's ~200ms hide transition, while the + * dropbox is still visible, still hit-testable and still `isOpened() === true`: + * + * 1. closeDropbox() marked the dropbox aria-hidden immediately, 200ms before it left the + * screen - fixed by deferring the attribute to afterHidePopper(); + * 2. handlers gated on isOpened() kept driving the fading dropbox - hovering an option + * focuses it (onOptionsMouseOver -> focusOption) - so focus could re-enter after the + * close and still be there when the attribute finally landed. Fixed twice over: the + * pointer is gated during the transition (isClosingTransition), and whatever a host + * still pulls in programmatically is released at hide-end (releaseFocusFromDropbox). + * + * WCAG 4.1.2 Name, Role, Value (A), 1.3.1 Info and Relationships (A), 2.4.3 Focus Order (A). + */ + +import { mountVs, unmountVs, makeOptions } from '../support/mount'; + +type Violation = { kind: string; focused: string; ancestor: string; stack: string }; + +/** the sink lives on the window so re-installing the guard repoints it instead of stacking */ +type GuardHost = Window & { __ariaHiddenFocusSink?: Violation[] }; + +const describeEle = (el: Element | null) => + (el ? `${el.tagName.toLowerCase()}.${(el.getAttribute('class') || '').split(' ').join('.')}` : 'null'); + +/** + * The guard judges only the component under test: the docsify page hosting these specs has + * aria-hidden chrome of its own (sidebar, cover), and a violation there is not this + * component's defect. + */ +const isComponentNode = (el: Element | null) => !!el && !!el.closest('[class*="vscomp"]'); + +function hiddenAncestor(el: Element | null): Element | null { + let node: Element | null = el; + + while (node) { + if (node.getAttribute && node.getAttribute('aria-hidden') === 'true') { + return node; + } + + node = node.parentElement; + } + + return null; +} + +const callSite = (win: Window) => { + const raw = new (win as unknown as { Error: ErrorConstructor }).Error().stack || ''; + + return raw.split('\n').slice(2, 8).map((line) => line.trim()).join(' | '); +}; + +/** + * Reproduces what Chrome itself checks, from both directions: focus moving into an already + * hidden subtree, and aria-hidden being applied over a subtree that still holds focus. + * `Element.prototype.setAttribute` is patched rather than observed with a MutationObserver so + * the offending call site is captured synchronously - that is what pinned the root cause to + * `toggleOptionFocusedState` in the first place. + */ +function installAriaHiddenFocusGuard(win: Window, sink: Violation[]): void { + const host = win as GuardHost; + + if (host.__ariaHiddenFocusSink) { + /** already patched this window: repoint the sink rather than stacking a second patch */ + host.__ariaHiddenFocusSink = sink; + return; + } + + host.__ariaHiddenFocusSink = sink; + + win.document.addEventListener( + 'focusin', + (e) => { + const ancestor = hiddenAncestor(e.target as Element); + + if (ancestor && isComponentNode(ancestor)) { + host.__ariaHiddenFocusSink?.push({ + kind: 'focus moved into an aria-hidden subtree', + focused: describeEle(e.target as Element), + ancestor: describeEle(ancestor), + stack: callSite(win), + }); + } + }, + true, + ); + + const proto = (win as unknown as { Element: { prototype: Element } }).Element.prototype; + const nativeSetAttribute = proto.setAttribute; + + proto.setAttribute = function patchedSetAttribute(this: Element, name: string, value: string) { + nativeSetAttribute.call(this, name, value); + + /** + * String(): DomUtils.setAria() forwards the boolean `true`, not the string. The first + * version of this guard compared `value !== 'true'` and therefore never fired for any + * element the component hides - the whole "attribute applied over focus" direction was + * dead code, and deleting releaseFocusFromDropbox() left this spec green (review + * finding on PR #492). + */ + if (name !== 'aria-hidden' || String(value) !== 'true') { + return; + } + + const active = win.document.activeElement; + + if (active && active !== win.document.body && this.contains(active) && isComponentNode(this)) { + host.__ariaHiddenFocusSink?.push({ + kind: 'aria-hidden applied over the focused element', + focused: describeEle(active), + ancestor: describeEle(this), + stack: callSite(win), + }); + } + }; +} + +describe('A11y: aria-hidden is never applied over the focused element', { testIsolation: true }, () => { + const mountId = 'vs-aria-hidden-focus'; + const secondMountId = 'vs-aria-hidden-focus-2'; + const violations: Violation[] = []; + + /** + * `dropboxWrapper: 'body'` portals the dropbox out of the host element, which is the layout + * the report came from and the only one where the dropbox has a wrapper of its own to carry + * aria-hidden. Every dropbox query below therefore goes through the instance, not the DOM. + */ + const mount = (extra: Record = {}) => { + cy.viewport(1280, 800); + cy.visit('get-started'); + cy.window().then((win) => { + violations.length = 0; + installAriaHiddenFocusGuard(win, violations); + mountVs(win, mountId, { options: makeOptions(50), search: true, dropboxWrapper: 'body', ...extra }); + }); + }; + + const dropbox = () => cy.get(`#${mountId}`).then(($ele) => cy.wrap($ele[0].virtualSelect.$dropboxWrapper)); + + const open = () => { + cy.get(`#${mountId}`).find('.vscomp-toggle-button').click(); + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('not.have.class', 'closed'); + }; + + /** opening focuses the search input asynchronously (popover afterShow); wait for it */ + const waitForSearchFocus = () => { + cy.get(`#${mountId}`).should(($ele) => { + expect($ele[0].ownerDocument.activeElement, 'search input focused after open') + .to.equal($ele[0].virtualSelect.$searchInput); + }); + }; + + /** + * Dispatch a real bubbling mouseover synchronously - no Cypress command hop, so it is + * guaranteed to land inside the hide transition it is aimed at, and no actionability check + * can reject the half-faded element (the mid-fade state is exactly what is under test). + */ + const hoverOptionNow = (vs: any, index: number) => { + const $option = vs.$dropboxContainer.querySelector(`.vscomp-option[data-index="${index}"]`); + const win = vs.$ele.ownerDocument.defaultView; + + $option.dispatchEvent(new win.MouseEvent('mouseover', { bubbles: true })); + }; + + /** the hide transition is ~200ms; wait it out so late callbacks are included */ + const assertNoViolations = () => { + cy.wait(700); + cy.then(() => { + expect(violations, JSON.stringify(violations, null, 2)).to.deep.equal([]); + }); + }; + + afterEach(() => { + cy.window().then((win) => { + unmountVs(win, mountId); + unmountVs(win, secondMountId); + }); + }); + + /** + * The reported case. Selecting in a single select closes the dropbox, but the pointer is + * still over the list while it fades out. The fading list must ignore the pointer: no new + * highlight, no aria-activedescendant on a combobox that just announced itself collapsed, + * no DOM focus pulled into a subtree about to be marked hidden - and focus must end on the + * combobox, not fall to . + */ + it('ignores the pointer over the fading list and lands focus on the combobox', () => { + mount(); + open(); + + dropbox().find('.vscomp-option[data-index="2"]').click(); + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + /** + * One Cypress command hop after the click, against a 200ms transition. The pin keeps + * the test honest: a closed dropbox ignores mouseover, so on a runner slow enough to + * outlive the fade the hover would silently become a no-op and the test would pass + * vacuously - this fails loudly with the reason instead. + */ + expect(vs.isOpened(), 'still mid hide-transition').to.equal(true); + + hoverOptionNow(vs, 3); + + expect(vs.$dropboxContainer.querySelector('.vscomp-option.focused'), 'highlight after hover').to.equal(null); + expect(vs.$wrapper.getAttribute('aria-activedescendant'), 'aria-activedescendant after hover').to.equal(null); + }); + + assertNoViolations(); + + cy.get(`#${mountId}`).then(($ele) => { + expect($ele[0].ownerDocument.activeElement, 'focus once closed').to.equal($ele[0].virtualSelect.$wrapper); + }); + }); + + it('ignores the pointer when a multi-select closes from the toggle button', () => { + mount({ multiple: true }); + open(); + + cy.get(`#${mountId}`).find('.vscomp-toggle-button').click(); + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + expect(vs.isOpened(), 'still mid hide-transition').to.equal(true); + + hoverOptionNow(vs, 4); + + expect(vs.$dropboxContainer.querySelector('.vscomp-option.focused'), 'highlight after hover').to.equal(null); + }); + + assertNoViolations(); + }); + + /** + * The silent close: opening one instance closes every other one synchronously, straight + * through afterHidePopper() - while the first instance's search input still holds focus. + * The attribute write happens in the same script block, before the browser's style recalc + * can drop focus to , so without an explicit release aria-hidden lands over the + * focused input. shouldFocusWrapperOnClose is false here, so the release must blur, not + * steal focus back from the instance the user just opened. + */ + it('releases focus from a dropdown that another instance closes silently', () => { + mount(); + cy.window().then((win) => { + mountVs(win, secondMountId, { options: makeOptions(50), search: true, dropboxWrapper: 'body' }); + }); + open(); + waitForSearchFocus(); + + cy.get(`#${secondMountId}`).then(($ele2) => $ele2[0].virtualSelect.openDropbox()); + + assertNoViolations(); + + cy.get(`#${mountId}`).should(($ele) => { + const vs = $ele[0].virtualSelect; + const active = $ele[0].ownerDocument.activeElement; + + expect(vs.$dropboxWrapper.contains(active), 'focus inside the silently closed dropbox').to.equal(false); + expect(vs.$dropboxWrapper.getAttribute('aria-hidden'), 'closed dropbox hidden').to.equal('true'); + }); + }); + + /** + * The pointer gate cannot cover host-driven focus: anything a host runs mid-fade that ends + * in focusOption() (a programmatic search, a value write) focuses the option it highlights, + * and a host can also focus() an element in the dropbox directly. The hide-end release is + * the backstop for that whole class - focus must be back on the combobox before the subtree + * is marked hidden. + * + * Driven with a direct focus() rather than through setSearchValue(): the search path + * re-renders the options (innerHTML), which destroys the focused node and races the + * deferred re-focus timer in afterRenderOptions() - where focus sits at hide-end then + * depends on which fired last. The direct call is the distilled, deterministic form of + * every path in the class. + */ + it('releases focus a host action pulled into the dropbox mid-fade', () => { + mount(); + open(); + waitForSearchFocus(); + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + vs.closeDropbox(); + expect(vs.isOpened(), 'still mid hide-transition').to.equal(true); + + const $option = vs.$dropboxContainer.querySelector('.vscomp-option[data-index="3"]'); + + $option.focus(); + + /** precondition: DOM focus genuinely re-entered the fading dropbox */ + expect($ele[0].ownerDocument.activeElement, 'focus pulled into the fading dropbox').to.equal($option); + }); + + assertNoViolations(); + + cy.get(`#${mountId}`).then(($ele) => { + expect($ele[0].ownerDocument.activeElement, 'focus once closed').to.equal($ele[0].virtualSelect.$wrapper); + }); + }); + + /** + * Tab from the search input is the one documented path that puts real DOM focus on an + * option while a search input exists (onKeyDown -> focusFirstVisibleOption). The outside + * click's mousedown blurs it before the close, so this pins that nothing during the hide + * transition pulls focus back into the component. + */ + it('keeps focus out of the dropbox when it closes with an option focused', () => { + mount(); + open(); + + cy.get(`#${mountId}`).pressKeys('Tab'); + dropbox().find('.vscomp-option.focused').should('exist'); + + cy.get('body').click(5, 5); + + assertNoViolations(); + + cy.window().then((win) => { + const active = win.document.activeElement as Element; + + expect(active.closest('[class*="vscomp"]'), 'focus left inside the component').to.equal(null); + }); + }); + + it('keeps focus out of the dropbox when Escape closes it mid navigation', () => { + mount({ search: false }); + open(); + + cy.get(`#${mountId}`).pressKeys('ArrowDown'); + cy.get(`#${mountId}`).pressKeys('ArrowDown'); + cy.get(`#${mountId}`).find('.vscomp-wrapper').trigger('keydown', { key: 'Escape', keyCode: 27, which: 27 }); + + assertNoViolations(); + + cy.get(`#${mountId}`).should(($ele) => { + expect($ele[0].ownerDocument.activeElement, 'focus once closed').to.equal($ele[0].virtualSelect.$wrapper); + }); + }); + + /** + * The state behind all of the above: for as long as the component reports the dropbox as + * open - and keeps answering clicks and arrow keys on it - it must not also be telling + * assistive technology that the dropbox is not there. + */ + it('does not mark the dropbox hidden while it still reports itself open', () => { + mount(); + open(); + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + vs.closeDropbox(); + + expect(vs.isOpened(), 'still mid hide-transition').to.equal(true); + expect(vs.$dropboxWrapper.getAttribute('aria-hidden'), 'aria-hidden while still open').to.not.equal('true'); + }); + + /** and it must be hidden once the transition has actually finished */ + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('have.class', 'closed'); + cy.get(`#${mountId}`).should(($ele) => { + const vs = $ele[0].virtualSelect; + + expect(vs.$dropboxWrapper.getAttribute('aria-hidden'), 'aria-hidden once closed').to.equal('true'); + expect(vs.$dropboxWrapper.getAttribute('tabindex'), 'tabindex once closed').to.equal('-1'); + }); + }); + + /** + * The focus release fires `focusin` on the wrapper synchronously, mid-afterHidePopper(). A + * consumer handler reacting to it by reopening the dropdown ("restore the last dropdown" + * hosts) used to be clobbered: the hiding writes that follow the release landed on top of + * openDropbox()'s and left a visible, expanded dropdown carrying aria-hidden="true". The + * writes must stand down when the dropdown is open again by the time they run. + * + * (`afterClose` cannot reproduce this: DomUtils.dispatchEvent() defers events through + * setTimeout(0), so an afterClose handler always runs after afterHidePopper() finished.) + */ + it('does not hide a dropdown a focus handler reopens during the close', () => { + mount(); + open(); + waitForSearchFocus(); + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + /** attached after closeDropbox() so the close-time wrapper refocus does not trigger it */ + vs.closeDropbox(); + expect(vs.isOpened(), 'still mid hide-transition').to.equal(true); + + /** put focus back inside so the hide-end release has something to hand to the wrapper */ + vs.$dropboxContainer.querySelector('.vscomp-option[data-index="3"]').focus(); + vs.$wrapper.addEventListener('focusin', () => $ele[0].open?.(), { once: true }); + }); + + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('not.have.class', 'closed'); + + cy.get(`#${mountId}`).should(($ele) => { + const vs = $ele[0].virtualSelect; + + expect(vs.isOpened(), 'reopened').to.equal(true); + expect(vs.$dropboxWrapper.getAttribute('aria-hidden'), 'aria-hidden on the reopened dropbox').to.not.equal('true'); + expect(vs.$dropboxWrapper.getAttribute('tabindex'), 'tabindex on the reopened dropbox').to.equal('0'); + }); + + cy.then(() => { + expect(violations, JSON.stringify(violations, null, 2)).to.deep.equal([]); + }); + }); + + /** + * A hide that lost a race with a reopen still lands. A characterisation guard, not a + * red-then-green regression case - it passes on the code that preceded it, and pins two + * things a plausible "fix" in this area would break. + * + * `open()` during the fade does not cancel the popover's pending hide: its `show()` + * early-returns while `pop-comp-active` is still set, so the popper's hide timer is never + * cleared and afterHidePopper() runs ~200ms later against an instance that is back in + * `openInstances` (see AI-33 in ACTION-ITEMS.md - the reopen is silently lost). Reopening + * also lifts the pointer gate, so the still-rendered list can take DOM focus again in + * between. + * + * 1. No aria-hidden violation on that path. It holds today only because the popover + * applies display:none *before* calling back, so the browser has already moved focus + * out - an ordering this component does not control, which is exactly why it is + * pinned rather than assumed. + * 2. The dropdown stays reopenable. The stale hide has to *complete* rather than stand + * down: skipping it (e.g. gating afterHidePopper on `openInstances`) leaves the + * wrapper without its `closed` class while the popover has already gone invisible, so + * every later toggle sees `isOpened() === true` and closes a dropdown that is not + * there. Confirmed by building that variant - these last assertions are what failed. + */ + it('releases focus when a hide that lost a race with a reopen finally lands', () => { + mount(); + open(); + waitForSearchFocus(); + + cy.window().then((win) => { + const vs = win.document.getElementById(mountId)!.virtualSelect; + + vs.closeDropbox(); + win.document.getElementById(mountId)!.open?.(); + + /** precondition: the reopen registered, and the pointer gate is back off */ + expect((win as any).VirtualSelect.openInstances.has(vs), 'reopened instance tracked').to.equal(true); + expect(vs.isClosingTransition, 'pointer gate lifted by the reopen').to.equal(false); + + hoverOptionNow(vs, 3); + + /** precondition: the hover genuinely put DOM focus back inside the fading dropbox */ + expect( + vs.$dropboxContainer.contains(win.document.activeElement), + 'focus back inside after the reopen', + ).to.equal(true); + }); + + assertNoViolations(); + + cy.window().then((win) => { + const vs = win.document.getElementById(mountId)!.virtualSelect; + + expect(vs.$dropboxWrapper.contains(win.document.activeElement), 'focus left in the hidden dropbox') + .to.equal(false); + }); + + /** the stale hide must leave the component closed, so the next open still works */ + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('have.class', 'closed'); + cy.get(`#${mountId}`).find('.vscomp-toggle-button').click(); + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('not.have.class', 'closed'); + cy.get(`#${mountId}`).should(($ele) => { + expect($ele[0].virtualSelect.$dropboxContainer.getBoundingClientRect().height, 'reopened dropbox height') + .to.be.greaterThan(0); + }); + }); + + /** + * afterRenderOptions() re-focuses the highlighted option on a timer. The dropbox can close + * inside that delay, so the decision has to hold when the timer fires, not when it was + * scheduled. + */ + it('does not pull focus back into a closed dropbox from a deferred re-render', () => { + mount({ search: false }); + open(); + + cy.get(`#${mountId}`).pressKeys('ArrowDown'); + dropbox().find('.vscomp-option.focused').should('exist'); + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + /** schedule the deferred focus, then close before it fires */ + vs.renderOptions(); + vs.closeDropbox(); + }); + + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('have.class', 'closed'); + assertNoViolations(); + + cy.window().then((win) => { + const active = win.document.activeElement; + + expect(active && active.classList.contains('vscomp-option'), 'focus on an option').to.not.equal(true); + }); + }); +}); diff --git a/src/virtual-select.js b/src/virtual-select.js index 2b6a02c..c861097 100644 --- a/src/virtual-select.js +++ b/src/virtual-select.js @@ -889,7 +889,17 @@ export class VirtualSelect { onOptionsMouseOver(e) { const $ele = e.target.closest('.vscomp-option'); - if ($ele && this.isOpened()) { + /** + * isClosingTransition: isOpened() stays true for the ~200ms of the popover's hide + * transition, so without the extra gate a pointer resting over the fading list kept + * driving it - re-highlighting options, re-writing aria-activedescendant on a combobox + * that had just announced itself collapsed, and (because taking the highlight takes DOM + * focus) pulling focus back into a subtree about to be marked aria-hidden. A closing + * dropbox ignores the pointer. Clicks and arrow keys are deliberately not gated - that + * would change what mid-fade interactions *do*, where this only stops a passive hover + * from mutating state; see AI-32 in ACTION-ITEMS.md. + */ + if ($ele && this.isOpened() && !this.isClosingTransition) { if (this.shouldSkipOptionInNavigation($ele)) { this.removeOptionFocus(); } else { @@ -3015,6 +3025,9 @@ export class VirtualSelect { */ this.isSilentServerSearch = false; + /** a reopen during a still-running hide transition puts the pointer back in charge */ + this.isClosingTransition = false; + DomUtils.setAttr(this.$dropboxWrapper, 'tabindex', '0'); DomUtils.setAria(this.$dropboxWrapper, 'hidden', false); @@ -3095,9 +3108,7 @@ export class VirtualSelect { // Return focus to wrapper only when no other meaningful element currently has focus const active = document.activeElement; - const withinComponent = - (active && this.$wrapper.contains(active)) || - (this.hasDropboxWrapper && active && this.$dropboxWrapper.contains(active)); + const withinComponent = (active && this.$wrapper.contains(active)) || this.isFocusInsideDropbox(active); const shouldRefocus = this.shouldFocusWrapperOnClose && VirtualSelect.lastInteractedInstance === this && @@ -3130,17 +3141,39 @@ export class VirtualSelect { this.setActiveDescendant(''); } + /** + * Taking the dropbox out of the tab order happens here, but hiding it from assistive + * technology is left to afterHidePopper() - where the rest of the closed state lands, the + * `closed` class and with it the `display: none` that actually takes the dropbox off the + * screen. + * + * aria-hidden used to be set here too, so for the ~200ms of the popover's hide transition + * the dropbox was marked absent while still visible, still hit-testable and still + * `isOpened() === true`. Every handler gated on isOpened() therefore kept running against a + * subtree already declared hidden - and onOptionsMouseOver() -> focusOption() moves DOM + * focus onto the option it highlights. Chrome refuses to apply aria-hidden over the focused + * element ("Blocked aria-hidden on an element because its descendant retained focus"), so + * the dropbox stayed exposed anyway: the component and the accessibility tree disagreed, + * and a screen reader follows the tree. Selecting an option in a single select was enough + * to hit it - the pointer is still over the list while it fades out. + * + * isClosingTransition covers the transition itself: without it the pointer kept driving + * the fading list - re-highlighting options, re-writing the aria-activedescendant this + * close just cleared onto a combobox already announcing itself collapsed, and pulling DOM + * focus back in. What the pointer gate cannot stop (a host focusing into the dropbox, or + * highlight paths it drives programmatically), releaseFocusFromDropbox() releases at + * hide-end, before the attribute lands. For those 200ms the list genuinely is still on + * screen, so exposing it to AT until it leaves matches what a sighted user sees. tabindex + * stays here because it was never part of the conflict - it does not block a programmatic + * focus() - and moving it would change when the dropbox leaves the tab order. + */ if (this.dropboxPopover && !isSilent) { + this.isClosingTransition = true; this.dropboxPopover.hide(); DomUtils.setAttr(this.$dropboxWrapper, 'tabindex', '-1'); - DomUtils.setAria(this.$dropboxWrapper, 'hidden', true); - DomUtils.setAttr(this.$dropboxContainerTop, 'tabindex', '-1'); - DomUtils.setAria(this.$dropboxContainerTop, 'hidden', true); - DomUtils.setAttr(this.$dropboxContainerBottom, 'tabindex', '-1'); - DomUtils.setAria(this.$dropboxContainerBottom, 'hidden', true); } else { this.afterHidePopper(); } @@ -3192,6 +3225,7 @@ export class VirtualSelect { afterHidePopper() { const isSilent = this.isSilentClose; this.isSilentClose = false; + this.isClosingTransition = false; DomUtils.removeClass(this.$allWrappers, 'focused'); this.removeOptionFocus(); @@ -3203,6 +3237,21 @@ export class VirtualSelect { DomUtils.addClass(this.$allWrappers, 'closed'); + /** + * After the closed class, so a focus handler reacting to the wrapper refocus observes a + * dropdown that is really closed, and before the aria-hidden writes below - nothing may + * hold focus inside the subtree when that attribute lands, or Chrome refuses it and the + * dropbox stays exposed to AT. Within this synchronous block the browser has not yet + * recalculated style, so document.activeElement still reports the element inside the + * dropbox even though the closed class will eventually drop focus to - which is + * exactly the focus loss the release turns into a deliberate hand-back. + * + * (afterClose needs no such ordering care: DomUtils.dispatchEvent() defers events through + * setTimeout(0), so a consumer's afterClose handler always runs after this method has + * finished.) + */ + this.releaseFocusFromDropbox(); + if (!isSilent) { DomUtils.dispatchEvent(this.$ele, 'afterClose'); } @@ -3210,7 +3259,29 @@ export class VirtualSelect { // Reset for next close this.shouldFocusWrapperOnClose = true; - // Restore accessibility attributes that were inadvertently removed + /** + * Stand down if a focus handler above reopened the dropdown synchronously: openDropbox() + * has already made the dropbox visible and focusable again, and hiding it now would leave + * it on screen but absent from the accessibility tree. The next close re-applies these + * through its own afterHidePopper(). + * + * Deliberately isOpened() and not `VirtualSelect.openInstances.has(this)`, even though the + * latter also means "reopened". The two differ for a hide that lost a race with an earlier + * reopen - `open()` called during the fade, which does not cancel the popover's pending + * hide because its show() early-returns while `pop-comp-active` is still set. That hide + * has to *complete*: the popover has already taken the dropbox off the screen by the time + * it calls back, so standing down would leave the wrapper without its `closed` class, + * `isOpened()` true against an invisible dropbox, and every later toggle closing a + * dropdown that is not there - unreopenable. Verified by building it: the openInstances + * form fails the recovery assertions in a11y-aria-hidden-focus.cy.ts. + * + * isOpened() is false here for that stale case (addClass above set it) and true only for + * the synchronous reopen this guard is for, which is exactly the split that is wanted. + */ + if (this.isOpened()) { + return; + } + DomUtils.setAttr(this.$dropboxWrapper, 'tabindex', '-1'); DomUtils.setAria(this.$dropboxWrapper, 'hidden', true); @@ -3388,6 +3459,78 @@ export class VirtualSelect { this.toggleFocusedProp(null); } + /** + * Whether the node is inside the dropbox proper - the subtree that is hidden on close. + * + * The portalled wrapper when there is one (`dropboxWrapper` option): it is the element that + * carries aria-hidden and everything in it goes away on close. The container otherwise: it + * lives inside $wrapper, whose toggle button and value display survive a close, so testing + * against $wrapper here would wrongly treat "focus on the combobox itself" as focus that + * needs releasing. + * + * Deliberately narrower than closeDropbox()'s within-component test (which also counts the + * combobox, to decide whether the user's focus deserves restoring), and different again from + * the Escape containment in onKeyDown() (which resolves the element hosting the keydown + * listener). The three answer different questions and are not interchangeable. + * + * @param {Element | null} $node + * @returns {boolean} + */ + isFocusInsideDropbox($node) { + const $root = this.$dropboxWrapper || this.$dropboxContainer; + + return !!$node && $root.contains($node); + } + + /** + * Take DOM focus out of the dropbox at the moment it is actually hidden. + * + * closeDropbox()'s own refocus runs when the close is *requested*; during the hide + * transition focus can re-enter the dropbox (a host focusing into it, or any host-driven + * path ending in focusOption(), which focuses the option it highlights). This runs from + * afterHidePopper(), immediately before the subtree is marked aria-hidden, and hands focus + * back to the combobox (WCAG 2.4.3 Focus Order) - re-asserting the decision the close-time + * refocus already made before something mid-fade overrode it. Without it, the closed + * class's display:none silently drops that focus to . + * + * Two deliberate divergences from closeDropbox()'s refocus guards: + * - no lastInteractedInstance / isSilent conditions: by hide-end those describe the close + * that *started* the transition, not where focus is now. The only question left is "is + * focus about to be trapped inside a hidden subtree" - if it is, it cannot stay there, + * whoever caused the close. + * - shouldFocusWrapperOnClose decides *where* focus goes, not *whether* it moves: when the + * close was caused by focus moving elsewhere (another dropdown opening, an outside + * click), the element is only blurred, so this instance does not steal focus from + * wherever it now belongs. + * + * No "has this instance been reopened" guard, deliberately. It reads as the safe thing to + * add - a stale hide should not yank focus out of a dropdown the user is looking at - but a + * reopen that genuinely put the dropbox back on screen cannot have a hide still pending + * against it: the popover clears `pop-comp-active` before calling back, so a successful + * show() means the hide already finished. What such a guard actually suppresses is the one + * case that needs the release most - the reopen that lost its race with the fade, where the + * popover has already applied display:none and focus inside the dropbox is about to be lost + * to . The caller decides whether the dropbox is being hidden; this only makes sure + * nothing is focused inside it when that happens. + * + * preventScroll because this runs ~200ms after the user's action - if they have scrolled in + * the meantime, focus restoration must not scroll the combobox back into view (the + * deferred re-focus in afterRenderOptions() makes the same call). + */ + releaseFocusFromDropbox() { + const $active = document.activeElement; + + if (!this.isFocusInsideDropbox($active)) { + return; + } + + if (this.shouldFocusWrapperOnClose) { + this.$wrapper.focus({ preventScroll: true }); + } else { + $active.blur(); + } + } + selectOption($ele, { event } = {}) { if (!$ele) { return; From a7dccc913a7e915a55334a491424a5a99db25272 Mon Sep 17 00:00:00 2001 From: gnbm Date: Tue, 11 Aug 2026 21:35:27 +0100 Subject: [PATCH 2/2] fix: honour an open that arrives while the dropbox is still closing PopoverComponent.show() early-returns for as long as `pop-comp-active` is on the element, and only its own afterHide removes that class. So openDropbox() called during the ~200ms hide transition did all of its own work - beforeOpen, aria-expanded="true", the instance back in openInstances - while popper.show() quietly did nothing. afterShowPopper() never ran, so there was no `focused` class, no focus moved into the list and no afterOpen; the hide that was still pending then completed on top of it. The dropdown ended up shut with its combobox still announcing aria-expanded="true", and focus inside it dropped to . The host's open() was silently lost - it looks like nothing happened, which is the worst shape for a bug to have. Reachable through the public open() API only: toggleDropbox() sees isOpened() === true for the whole transition, so a second click closes again rather than reopening. openDropbox() now queues instead of running when the popover is mid-hide, and afterHidePopper() replays the open once the popover is idle again - immediately before the existing "stand down if the dropdown is open again" guard, so the replay reuses the one path that already knows not to hide an open dropdown rather than adding a second. Both conditions are required to queue. isClosingTransition alone would strand every later open if it were ever left set with no hide actually pending; isShown() alone is true of a dropdown that is simply already open, where re-opening must stay a no-op rather than a queued one. Together they mean exactly "a hide is running and the popover will refuse to show". Nothing reaches into dropboxPopover.popper to cancel its timer - that would couple this component to the plugin's internals - so the cost is one extra fade before the dropdown appears, which is inherent: the popover cannot show while it is hiding. A queued open is not a promise to open regardless of what happens next, so it has to be reachable by everything that ends the lifecycle, and its effects have to stay scoped to itself: - The queued instance registers in openInstances. That set already means "open, or about to be", and it is how onDocumentClick() and the "close all others" loop find an instance - the close that started the hide had already removed it. Without this the queue outlives an outside click or a second dropdown opening: the dropdown springs open ~200ms after the user dismissed it and, through that same loop, shuts the one they had just opened. - Both of those loops write `shouldFocusWrapperOnClose = false` on every instance they find before closing it - correct for the dropdown they are actually closing, wrong for a merely-queued one, whose real close is a separate, already-pending hide that made its own decision about where focus should land. closeDropbox() snapshots the flag when an open is queued and restores it when the queue is cancelled, so an unrelated later event cannot silently overwrite a decision that belongs to a different close. - closeDropbox() treats a merely-queued instance as not open and only cancels the queue, returning before isSilentClose is assigned. That flag belongs to the close that started the hide, and overwriting it would lose that close's afterClose; returning early also avoids a second beforeClose for a dropdown that is already closing. - The replay is gated on !isOpened(), because both reopen routes can converge on one close - a consumer focus handler reacting to the wrapper refocus in releaseFocusFromDropbox() may already have reopened the dropdown synchronously, and replaying on top of that would dispatch a second beforeOpen for a single open. The same reopen can leave a stale afterClose describing a dropdown that is, by the time the event fires, open again; afterClose is now gated on the identical !isOpened() check. Queueing before any work is what keeps the event contract intact more generally: letting openDropbox() run and re-running it from afterHidePopper() would dispatch beforeOpen twice for one open(), which would make a host that lazy-loads options on beforeOpen fetch twice. test: pin the reopen against the popover's hide transition reopen-during-hide-transition.cy.ts, eleven cases: the reopen ends up open and on screen, the open actually finishes (focus in the search input, the `focused` class), beforeOpen and afterOpen each fire exactly once, a close following the queued open cancels it, an outside click cancels it, another dropdown opening cancels it and stays open itself without corrupting where the cancelled instance's own close sends focus, a focus handler reopening alongside the queue still yields one beforeOpen and no stale afterClose, the same queue works on the default non-portalled layout, and an ordinary open with no hide running is still applied synchronously rather than deferred. Six of the eleven were red against the previous behaviour. Each waits out the hide and the show that has to follow it before asserting: every assertion also holds during the fade - openDropbox() sets aria-expanded and clears aria-hidden synchronously, and the popover has not yet applied display:none - so a retrying should() would otherwise latch onto that transient and pass against the bug. One case reads document.activeElement in a one-shot check immediately after a retrying class assertion rather than inside the same retrying callback, so a slower retry landing after an unrelated second dropdown's own focus change can't be mistaken for the result under test. a11y-aria-hidden-focus.cy.ts's mid-transition case is rewritten. Its premise - that the reopen is lost - no longer holds, so it now pins the accessibility half of the queued path: the queue must hold the pointer gate up for the rest of the fade, so the list cannot take DOM focus back while the hide is still on its way to marking the subtree hidden. Its deferred-re-render case is removed. It could not fail: closeDropbox() clears `.focused` synchronously, and even when forced to have something, the `closed` class makes the subtree display:none, so focus() on an option inside it is a no-op - while before that point aria-hidden has not been applied yet. The invariant is unviolatable through that timer, so a fire-time guard there would be unreachable code. The reasoning and the condition that would make it reachable again are recorded in the spec's docblock rather than left as a case that passes whatever the code does. --- cypress/e2e/a11y-aria-hidden-focus.cy.ts | 99 ++-- cypress/e2e/a11y-close-clears-highlight.cy.ts | 14 +- .../e2e/reopen-during-hide-transition.cy.ts | 422 ++++++++++++++++++ src/virtual-select.js | 169 ++++++- 4 files changed, 606 insertions(+), 98 deletions(-) create mode 100644 cypress/e2e/reopen-during-hide-transition.cy.ts diff --git a/cypress/e2e/a11y-aria-hidden-focus.cy.ts b/cypress/e2e/a11y-aria-hidden-focus.cy.ts index 5157334..13b049a 100644 --- a/cypress/e2e/a11y-aria-hidden-focus.cy.ts +++ b/cypress/e2e/a11y-aria-hidden-focus.cy.ts @@ -21,6 +21,15 @@ * still pulls in programmatically is released at hide-end (releaseFocusFromDropbox). * * WCAG 4.1.2 Name, Role, Value (A), 1.3.1 Info and Relationships (A), 2.4.3 Focus Order (A). + * + * Deliberately not covered: the deferred re-focus in afterRenderOptions(), which fires ~20ms after + * a render and focuses whatever still carries `.focused`. It looks like a third way in, and a case + * for it lived here briefly - but it could not fail. Once the dropbox is closed the `closed` class + * makes it `display: none`, so focus() on an option inside it is a no-op; before that, aria-hidden + * has not been applied yet. A fire-time guard there would be unreachable code, and the case that + * pretended to pin it passed whatever afterRenderOptions() did. Removed rather than left as false + * assurance. If the `closed` rule ever stops hiding the subtree, this becomes reachable and the + * guard becomes real. */ import { mountVs, unmountVs, makeOptions } from '../support/mount'; @@ -421,97 +430,47 @@ describe('A11y: aria-hidden is never applied over the focused element', { testIs }); /** - * A hide that lost a race with a reopen still lands. A characterisation guard, not a - * red-then-green regression case - it passes on the code that preceded it, and pins two - * things a plausible "fix" in this area would break. - * - * `open()` during the fade does not cancel the popover's pending hide: its `show()` - * early-returns while `pop-comp-active` is still set, so the popper's hide timer is never - * cleared and afterHidePopper() runs ~200ms later against an instance that is back in - * `openInstances` (see AI-33 in ACTION-ITEMS.md - the reopen is silently lost). Reopening - * also lifts the pointer gate, so the still-rendered list can take DOM focus again in - * between. + * An open arriving mid-fade is queued behind the running hide (see + * reopen-during-hide-transition.cy.ts for the reopen behaviour itself). This is the a11y half + * of that path: the queue must hold the pointer gate up for the rest of the fade, so the list + * cannot take DOM focus back while the hide is still on its way to marking the subtree hidden. * - * 1. No aria-hidden violation on that path. It holds today only because the popover - * applies display:none *before* calling back, so the browser has already moved focus - * out - an ordering this component does not control, which is exactly why it is - * pinned rather than assumed. - * 2. The dropdown stays reopenable. The stale hide has to *complete* rather than stand - * down: skipping it (e.g. gating afterHidePopper on `openInstances`) leaves the - * wrapper without its `closed` class while the popover has already gone invisible, so - * every later toggle sees `isOpened() === true` and closes a dropdown that is not - * there. Confirmed by building that variant - these last assertions are what failed. + * Worth pinning separately because the obvious way to write the queue - run openDropbox() + * eagerly and repair afterwards - lifts `isClosingTransition` and hands the fading list back + * to the pointer, which is exactly how focus used to end up inside an aria-hidden subtree. */ - it('releases focus when a hide that lost a race with a reopen finally lands', () => { + it('keeps the pointer gate up while an open waits behind the hide transition', () => { mount(); open(); waitForSearchFocus(); cy.window().then((win) => { - const vs = win.document.getElementById(mountId)!.virtualSelect; + const $ele = win.document.getElementById(mountId)!; + const vs = $ele.virtualSelect; vs.closeDropbox(); - win.document.getElementById(mountId)!.open?.(); + expect(vs.isOpened(), 'still mid hide-transition when the reopen arrives').to.equal(true); - /** precondition: the reopen registered, and the pointer gate is back off */ - expect((win as any).VirtualSelect.openInstances.has(vs), 'reopened instance tracked').to.equal(true); - expect(vs.isClosingTransition, 'pointer gate lifted by the reopen').to.equal(false); + $ele.open?.(); + + expect(vs.isClosingTransition, 'pointer gate held while the open waits').to.equal(true); hoverOptionNow(vs, 3); - /** precondition: the hover genuinely put DOM focus back inside the fading dropbox */ - expect( - vs.$dropboxContainer.contains(win.document.activeElement), - 'focus back inside after the reopen', - ).to.equal(true); + expect(vs.$dropboxContainer.querySelector('.vscomp-option.focused'), 'highlight after hover').to.equal(null); + expect(vs.$dropboxContainer.contains(win.document.activeElement), 'focus after hover').to.equal(false); }); assertNoViolations(); - cy.window().then((win) => { - const vs = win.document.getElementById(mountId)!.virtualSelect; - - expect(vs.$dropboxWrapper.contains(win.document.activeElement), 'focus left in the hidden dropbox') - .to.equal(false); - }); - - /** the stale hide must leave the component closed, so the next open still works */ - cy.get(`#${mountId}`).find('.vscomp-wrapper').should('have.class', 'closed'); - cy.get(`#${mountId}`).find('.vscomp-toggle-button').click(); + /** and the queued open still lands, on a dropbox that was never marked hidden underneath it */ cy.get(`#${mountId}`).find('.vscomp-wrapper').should('not.have.class', 'closed'); cy.get(`#${mountId}`).should(($ele) => { - expect($ele[0].virtualSelect.$dropboxContainer.getBoundingClientRect().height, 'reopened dropbox height') - .to.be.greaterThan(0); - }); - }); - - /** - * afterRenderOptions() re-focuses the highlighted option on a timer. The dropbox can close - * inside that delay, so the decision has to hold when the timer fires, not when it was - * scheduled. - */ - it('does not pull focus back into a closed dropbox from a deferred re-render', () => { - mount({ search: false }); - open(); - - cy.get(`#${mountId}`).pressKeys('ArrowDown'); - dropbox().find('.vscomp-option.focused').should('exist'); - - cy.get(`#${mountId}`).then(($ele) => { const vs = $ele[0].virtualSelect; - /** schedule the deferred focus, then close before it fires */ - vs.renderOptions(); - vs.closeDropbox(); - }); - - cy.get(`#${mountId}`).find('.vscomp-wrapper').should('have.class', 'closed'); - assertNoViolations(); - - cy.window().then((win) => { - const active = win.document.activeElement; - - expect(active && active.classList.contains('vscomp-option'), 'focus on an option').to.not.equal(true); + expect(vs.$dropboxWrapper.getAttribute('aria-hidden'), 'aria-hidden on the reopened dropbox').to.not.equal('true'); + expect(vs.$dropboxContainer.getBoundingClientRect().height, 'reopened dropbox height').to.be.greaterThan(0); }); }); + }); diff --git a/cypress/e2e/a11y-close-clears-highlight.cy.ts b/cypress/e2e/a11y-close-clears-highlight.cy.ts index d121cc2..bedf49d 100644 --- a/cypress/e2e/a11y-close-clears-highlight.cy.ts +++ b/cypress/e2e/a11y-close-clears-highlight.cy.ts @@ -72,10 +72,16 @@ describe('A11y: closing the dropbox clears option navigation state', { testIsola }); /** - * Reopened in the *same tick* as the close, deliberately. Waiting for the `closed` class - * first would wait out `afterHidePopper()`, which clears the highlight on its own - so the - * case could never observe the bug it exists for. This is the user-visible symptom: reopen - * before the hide transition finishes and navigation must still start at the top. + * `openDropbox()` is called in the *same tick* as the close, deliberately. Waiting for the + * `closed` class first would wait out `afterHidePopper()`, which clears the highlight on its + * own - so the case could never observe the bug it exists for. + * + * The call itself now queues rather than reopening synchronously (a reopen mid-fade waits for + * the running hide to finish - see reopen-during-hide-transition.cy.ts), but that does not + * weaken what this pins: `isOpened()` stays true for the whole hide transition regardless, so + * the ArrowDown below reaches `focusOption()` either way, on a dropbox whose highlight + * `closeDropbox()` already cleared. What is under test is that clearing, not whether the + * queued open has landed by the time the key is pressed. */ it('starts navigation at the first option again when reopened mid hide-transition', () => { mount(); diff --git a/cypress/e2e/reopen-during-hide-transition.cy.ts b/cypress/e2e/reopen-during-hide-transition.cy.ts new file mode 100644 index 0000000..4ce5f79 --- /dev/null +++ b/cypress/e2e/reopen-during-hide-transition.cy.ts @@ -0,0 +1,422 @@ +/** cSpell:ignore vscomp popcomp */ + +/** + * A reopen that arrives while the dropbox is still closing must still open it. + * + * The popover refuses to show while its own hide transition is running: `show()` early-returns + * for as long as `pop-comp-active` is on the element, and only its `afterHide` removes that + * class. So `openDropbox()` during the ~200ms fade did all of its own work - `beforeOpen`, + * `aria-expanded="true"`, the instance back in `openInstances` - while `popper.show()` quietly + * did nothing and `afterShowPopper()` never ran. The pending hide then completed on top of it. + * + * The dropdown ended up shut with its combobox still announcing `aria-expanded="true"`, no + * `afterOpen` ever dispatched, and focus dropped to `` (the popover applies + * `display: none` before the component gets any callback, so nothing can hand it back). The + * host's `open()` was silently lost - it only looks like nothing happened, which is the worst + * shape for a bug to have. + * + * Reachable through the public `open()` API only: `toggleDropbox()` sees `isOpened() === true` + * for the whole transition, so a second click closes again rather than reopening. + */ + +import { mountVs, unmountVs, makeOptions } from '../support/mount'; + +describe('Reopening while the dropbox is still closing', { testIsolation: true }, () => { + const mountId = 'vs-reopen-during-hide'; + const secondMountId = 'vs-reopen-during-hide-2'; + + const mount = (extra: Record = {}) => { + cy.viewport(1280, 800); + cy.visit('get-started'); + cy.window().then((win) => { + mountVs(win, mountId, { options: makeOptions(50), search: true, dropboxWrapper: 'body', ...extra }); + }); + }; + + const open = () => { + cy.get(`#${mountId}`).find('.vscomp-toggle-button').click(); + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('not.have.class', 'closed'); + }; + + /** + * Close and reopen inside the same tick, so the reopen is guaranteed to land inside the hide + * transition rather than racing a Cypress command hop. `isOpened()` is asserted between the + * two to prove the transition really is still running - otherwise the case could pass by + * testing an ordinary closed-then-open sequence. + */ + const closeThenReopen = () => { + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + vs.closeDropbox(); + expect(vs.isOpened(), 'still mid hide-transition when the reopen arrives').to.equal(true); + $ele[0].open?.(); + }); + }; + + /** + * Settle first, then assert. + * + * Every assertion below also holds *during* the fade - `openDropbox()` sets aria-expanded and + * clears aria-hidden synchronously, and the popover has not yet applied `display: none` - so a + * retrying `should()` on its own would latch onto that transient and pass against the very bug + * this spec exists for. The wait covers the hide (~200ms) plus the show that has to follow it. + */ + const assertOpenAndVisible = () => { + cy.wait(700); + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('not.have.class', 'closed'); + cy.get(`#${mountId}`).should(($ele) => { + const vs = $ele[0].virtualSelect; + + expect(vs.isOpened(), 'isOpened()').to.equal(true); + expect(vs.$dropboxContainer.style.display, 'container display').to.not.equal('none'); + expect(vs.$dropboxContainer.getBoundingClientRect().height, 'rendered height').to.be.greaterThan(0); + expect(vs.$wrapper.getAttribute('aria-expanded'), 'aria-expanded').to.equal('true'); + expect(vs.$dropboxWrapper.getAttribute('aria-hidden'), 'aria-hidden').to.not.equal('true'); + expect(vs.$dropboxWrapper.getAttribute('tabindex'), 'tabindex').to.equal('0'); + }); + }; + + /** opening focuses the search input asynchronously (popover afterShow); wait for it */ + const waitForSearchFocus = () => { + cy.get(`#${mountId}`).should(($ele) => { + expect($ele[0].ownerDocument.activeElement, 'search input focused after open') + .to.equal($ele[0].virtualSelect.$searchInput); + }); + }; + + afterEach(() => { + cy.window().then((win) => { + unmountVs(win, mountId); + unmountVs(win, secondMountId); + }); + }); + + it('ends up open and on screen', () => { + mount(); + open(); + closeThenReopen(); + + assertOpenAndVisible(); + }); + + /** + * afterShowPopper() is where the dropdown finishes opening - the `focused` class, the scroll + * position, and the focus move onto the search input. A reopen that never reaches it leaves a + * dropdown nobody can type into. + */ + it('finishes the open: focus lands in the search input', () => { + mount(); + open(); + closeThenReopen(); + + assertOpenAndVisible(); + + cy.get(`#${mountId}`).should(($ele) => { + const vs = $ele[0].virtualSelect; + + expect($ele[0].ownerDocument.activeElement, 'focus after the reopen').to.equal(vs.$searchInput); + expect(vs.$wrapper.classList.contains('focused'), 'focused class').to.equal(true); + }); + }); + + /** + * One open() call is one open, however the reopen has to be sequenced internally. A host + * loading options on beforeOpen must not be asked twice, and afterOpen has to arrive - it is + * the only signal that the dropdown is actually usable. + */ + it('dispatches beforeOpen once and afterOpen once', () => { + mount(); + open(); + + const events: string[] = []; + + cy.get(`#${mountId}`).then(($ele) => { + ['beforeOpen', 'afterOpen', 'beforeClose', 'afterClose'].forEach((name) => { + $ele[0].addEventListener(name, () => events.push(name)); + }); + }); + + closeThenReopen(); + assertOpenAndVisible(); + + /** let any late duplicate arrive before counting - dispatchEvent defers through setTimeout */ + cy.wait(300); + cy.then(() => { + expect(events.filter((name) => name === 'beforeOpen').length, `beforeOpen (${events.join(', ')})`).to.equal(1); + expect(events.filter((name) => name === 'afterOpen').length, `afterOpen (${events.join(', ')})`).to.equal(1); + }); + }); + + /** a close arriving after the queued reopen wins - the dropdown must not spring back open */ + it('does not reopen when a close follows the reopen', () => { + mount(); + open(); + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + vs.closeDropbox(); + $ele[0].open?.(); + vs.closeDropbox(); + }); + + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('have.class', 'closed'); + cy.wait(600); + cy.get(`#${mountId}`).should(($ele) => { + const vs = $ele[0].virtualSelect; + + expect(vs.isOpened(), 'isOpened() after the trailing close').to.equal(false); + expect(vs.$dropboxWrapper.getAttribute('aria-hidden'), 'aria-hidden').to.equal('true'); + }); + }); + + /** + * A queued open is not a promise to open regardless of what happens next. The page-level close + * paths reach an instance through `VirtualSelect.openInstances`, and a queued instance is not + * in it by default - closeDropbox() removed it before the open was ever queued - so an outside + * click would sail past and the dropdown would spring open ~200ms after the user dismissed it. + */ + it('cancels a queued open when the user clicks outside', () => { + mount(); + open(); + closeThenReopen(); + + cy.get('body').click(5, 5); + + cy.wait(700); + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('have.class', 'closed'); + cy.get(`#${mountId}`).should(($ele) => { + const vs = $ele[0].virtualSelect; + + expect(vs.isOpened(), 'isOpened() after the outside click').to.equal(false); + expect(vs.$dropboxWrapper.getAttribute('aria-hidden'), 'aria-hidden').to.equal('true'); + }); + }); + + /** + * Same rule for the other page-level close: opening a second dropdown closes every other one. + * A queue that survived it would reopen ~200ms later and, through its own "close all others" + * loop, shut the dropdown the user had just opened. + */ + it('cancels a queued open when another dropdown opens', () => { + mount(); + cy.window().then((win) => { + mountVs(win, secondMountId, { options: makeOptions(50), search: true, dropboxWrapper: 'body' }); + }); + open(); + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + vs.closeDropbox(); + expect(vs.isOpened(), 'still mid hide-transition when the reopen arrives').to.equal(true); + $ele[0].open?.(); + }); + + cy.get(`#${secondMountId}`).then(($ele2) => $ele2[0].virtualSelect.openDropbox()); + + cy.wait(800); + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('have.class', 'closed'); + cy.get(`#${secondMountId}`).find('.vscomp-wrapper').should('not.have.class', 'closed'); + cy.get(`#${secondMountId}`).should(($ele2) => { + expect($ele2[0].virtualSelect.isOpened(), 'the dropdown the user opened stayed open').to.equal(true); + }); + }); + + /** + * Cancelling a queued open must cancel only the open - not silently rewrite the decision the + * *original* close already made about where focus goes once its own hide finishes. + * + * onDocumentClick() and openDropbox()'s "close all other instances" loop both do + * `instanceObj.shouldFocusWrapperOnClose = false` before calling `closeDropbox()` on every + * entry in openInstances - and a queued instance is in that set (this is exactly what makes + * the previous two cases able to cancel it at all). Before this fix, `closeDropbox()`'s + * pendingOpen branch cancelled the queue but never touched that flag back, so it stayed + * corrupted for the *original* close's own afterHidePopper() - which fires later, once the + * hide that started all this actually finishes. + * + * Reproduced with a second dropdown opening (not an outside click): opening B runs + * "close all others" synchronously, at the very top of B.openDropbox() - well before B's own + * ~300ms show transition could plausibly move focus into B's search input and mask the + * corruption by overwriting document.activeElement first. The assertion reads + * document.activeElement the moment A's own hide finishes (~200ms), safely inside that + * window. + */ + it('does not let a cancelled queue corrupt where the original close sends focus', () => { + mount(); + cy.window().then((win) => { + mountVs(win, secondMountId, { options: makeOptions(50), search: true, dropboxWrapper: 'body' }); + }); + open(); + waitForSearchFocus(); + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + // A's own close: default shouldFocusWrapperOnClose (true) - this close wants focus back. + vs.closeDropbox(); + expect(vs.isOpened(), 'still mid hide-transition').to.equal(true); + expect(vs.shouldFocusWrapperOnClose, 'A wants focus back, before anything cancels the queue') + .to.equal(true); + + // Something puts focus back inside A's still-visible dropbox mid-fade. + vs.$dropboxContainer.querySelector('.vscomp-option[data-index="3"]').focus(); + + // Queue a reopen of A, then cancel it by opening a second, unrelated dropdown. + $ele[0].open?.(); + }); + + cy.get(`#${secondMountId}`).then(($ele2) => $ele2[0].virtualSelect.openDropbox()); + + /** + * Read document.activeElement in a one-shot .then() the instant A's own hide finishes, not + * inside the retrying .should() above: B's own show transition also completes on its own + * ~300ms timer and steals focus to its search input, and a retry that happens to land after + * that would see B's focus instead of the value under test - passing or failing for the + * wrong reason either way. A single check right after the class appears is the only way to + * observe the moment this method actually runs. + */ + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('have.class', 'closed'); + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + expect( + $ele[0].ownerDocument.activeElement, + 'focus once A\'s own close finishes - it wanted focus back, and nothing since then asked for anything else', + ).to.equal(vs.$wrapper); + }); + }); + + /** + * Two reopen routes can converge on one close: the queued open, and a consumer focus handler + * reacting to the wrapper refocus that releaseFocusFromDropbox() performs. Whichever gets there + * first, the dropdown opens once. + */ + it('dispatches beforeOpen once when a focus handler also reopens during the close', () => { + mount(); + open(); + waitForSearchFocus(); + + const events: string[] = []; + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + $ele[0].addEventListener('beforeOpen', () => events.push('beforeOpen')); + + vs.closeDropbox(); + expect(vs.isOpened(), 'still mid hide-transition').to.equal(true); + $ele[0].open?.(); + + /** focus inside, so the hide-end release has something to hand back and fires focusin */ + vs.$dropboxContainer.querySelector('.vscomp-option[data-index="3"]').focus(); + vs.$wrapper.addEventListener('focusin', () => $ele[0].open?.(), { once: true }); + }); + + assertOpenAndVisible(); + + cy.wait(300); + cy.then(() => { + expect(events.length, `beforeOpen count (${events.join(', ')})`).to.equal(1); + }); + }); + + /** + * A consumer focus handler that reopens the dropdown synchronously - the same pattern the + * a11y spec's "does not hide a dropdown a focus handler reopens during the close" pins - runs + * from inside releaseFocusFromDropbox(), which afterHidePopper() calls before its own + * `afterClose` dispatch. By the time that reentrant open returns, isOpened() is already true + * again - but afterClose used to fire anyway, describing a close that the same synchronous + * call stack had already undone. A host cleaning up "now that it's closed" would run that + * cleanup against a dropdown that is, in fact, open. + * + * `beforeOpen`/`afterOpen` are asserted too, so a fix that suppressed afterClose by also + * suppressing the reopen's own events would not pass this by accident. + */ + it('does not dispatch afterClose for a dropdown a focus handler reopened synchronously', () => { + mount(); + open(); + waitForSearchFocus(); + + const events: string[] = []; + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + ['beforeOpen', 'afterOpen', 'afterClose'].forEach((name) => { + $ele[0].addEventListener(name, () => events.push(name)); + }); + + vs.closeDropbox(); + expect(vs.isOpened(), 'still mid hide-transition').to.equal(true); + + vs.$dropboxContainer.querySelector('.vscomp-option[data-index="3"]').focus(); + vs.$wrapper.addEventListener('focusin', () => $ele[0].open?.(), { once: true }); + }); + + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('not.have.class', 'closed'); + cy.get(`#${mountId}`).should(($ele) => { + expect($ele[0].virtualSelect.isOpened(), 'reopened').to.equal(true); + }); + + /** + * Retries against the live `events` array rather than a fixed wait: the reentrant open's own + * show transition and its deferred event dispatches take a variable amount of wall-clock + * time (the ~300ms show plus setTimeout(0) scheduling), and a one-shot check timed to "enough + * margin in the common case" is exactly the flaky pattern this suite has hit before. + */ + cy.wrap(null).should(() => { + expect(events, `events (${events.join(', ')})`).to.deep.equal(['beforeOpen', 'afterOpen']); + }); + }); + + /** + * Every other case here mounts with `dropboxWrapper: 'body'` (the portalled layout the + * reported Chrome warning came from), which gives the dropbox its own wrapper element and + * makes `isFocusInsideDropbox()` take its `$dropboxWrapper` branch. The default, + * non-portalled layout takes that helper's `$dropboxContainer` fallback instead - the same + * queue, on the layout most consumers actually use. + * + * Not `assertOpenAndVisible()`: `$dropboxWrapper` is only ever assigned when `dropboxWrapper` + * is set (renderDropbox() leaves it undefined otherwise), and the aria-hidden/tabindex writes + * that helper checks are themselves no-ops on this layout - DomUtils.setAria()/setAttr() both + * guard on a falsy element. There is nothing to carry aria-hidden without a wrapper of its + * own; what this layout still needs is the reopen itself. + */ + it('also queues and replays the open on the default, non-portalled layout', () => { + mount({ dropboxWrapper: 'self' }); + open(); + closeThenReopen(); + + cy.wait(700); + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('not.have.class', 'closed'); + cy.get(`#${mountId}`).should(($ele) => { + const vs = $ele[0].virtualSelect; + + expect(vs.isOpened(), 'isOpened()').to.equal(true); + expect(vs.$dropboxContainer.style.display, 'container display').to.not.equal('none'); + expect(vs.$dropboxContainer.getBoundingClientRect().height, 'rendered height').to.be.greaterThan(0); + expect(vs.$wrapper.getAttribute('aria-expanded'), 'aria-expanded').to.equal('true'); + expect($ele[0].ownerDocument.activeElement, 'focus after the reopen').to.equal(vs.$searchInput); + }); + }); + + /** the ordinary path must not be routed through the queue */ + it('still opens immediately when no hide is running', () => { + mount(); + open(); + + cy.get(`#${mountId}`).find('.vscomp-toggle-button').click(); + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('have.class', 'closed'); + + cy.get(`#${mountId}`).then(($ele) => { + $ele[0].open?.(); + + /** synchronous: an open with nothing to wait for must not be deferred */ + expect($ele[0].virtualSelect.isOpened(), 'open applied in the same tick').to.equal(true); + }); + + assertOpenAndVisible(); + }); +}); diff --git a/src/virtual-select.js b/src/virtual-select.js index c861097..c63eb29 100644 --- a/src/virtual-select.js +++ b/src/virtual-select.js @@ -1404,6 +1404,10 @@ export class VirtualSelect { this.uniqueId = this.getUniqueId(); this.shouldFocusWrapperOnClose = true; // Initialize focus management property this.isClosing = false; + /** true only for the ~200ms a popover-backed close is actually running - see closeDropbox() */ + this.isClosingTransition = false; + /** an open that arrived mid hide-transition and is waiting for it - see openDropbox() */ + this.pendingOpen = false; /** true from closeDropbox() until the next openDropbox() - see closeDropbox() */ this.isSilentServerSearch = false; this.ariaSetSize = 0; @@ -2993,6 +2997,78 @@ export class VirtualSelect { openDropbox(isSilent) { // Set this instance as the last interacted one immediately VirtualSelect.lastInteractedInstance = this; + + /** + * Queue the open when the popover is still running its hide transition, and let + * afterHidePopper() replay it once the popover is idle again. + * + * PopoverComponent.show() early-returns for as long as `pop-comp-active` is on the element, + * and only its own afterHide removes that class - so opening during the ~200ms fade used to + * do all of this method's work while popper.show() quietly did nothing. afterShowPopper() + * never ran (no `focused` class, no focus moved into the list, no afterOpen), and the hide + * that was still pending then completed on top of it: the dropdown ended up shut with its + * combobox announcing aria-expanded="true". The host's open() was silently lost. + * + * Both state conditions are required. isClosingTransition alone would strand every later + * open if it were ever left set with no hide actually pending; isShown() alone is true of a + * dropdown that is simply already open, where re-opening must stay a no-op rather than a + * queued one. Together they mean exactly "a hide is running and the popover will refuse to + * show". + * + * `!isSilent` is not a fourth safety check, it is the scope of the problem: a silent open + * never calls dropboxPopover.show() at all (see the branch at the end of this method - it + * sets display directly and runs afterShowPopper() synchronously), so the refusal this + * queue works around cannot happen to it. A silent open landing mid-hide would still be + * clobbered, but by something else - the stale afterHidePopper() adding `closed` and + * aria-hidden on top of it - and it would need its own fix, not this one. Deliberately not + * written: no caller passes isSilent to this method, in-tree or through the public + * $ele.open(), and none ever has. Guarding an unreachable path here would be untested code + * defending against a call that does not exist. + * + * The visible cost, measured rather than assumed: the dropbox completes its fade to + * opacity 0 and then fades back in, so a reopen takes hideDuration + showDuration before + * the list is on screen again. It is an opacity dip, not a layout flash - `display: none` + * is never painted, because the replay's removeClass('closed') and the popper restoring + * `inline-flex` both land in the same synchronous block as the hide that set them. + * + * That is a deliberate trade, not a limitation. PopperComponent.show() does start with + * clearTimeout(hideDurationTimeout), so the hide *is* cancellable and the dip could be + * removed by calling `this.dropboxPopover.popper.show({ resetPosition: true })` directly. + * What stands in the way is only PopoverComponent.show()'s own `if (this.isShown()) return` + * one layer above it. Reaching past that means depending on two levels of undocumented + * plugin internals - and every defect this queue exists to fix came from mis-modelling this + * plugin's internal state in the first place. No user gesture can even reach the dip: + * toggleDropbox() reads isOpened() as true for the whole transition, so a second click + * closes again rather than reopening, and only a programmatic close()-then-open() gets + * here. A cosmetic gain on that path is not worth the coupling. + */ + if (!isSilent && this.isClosingTransition && this.dropboxPopover && this.dropboxPopover.isShown()) { + this.pendingOpen = true; + + /** + * Registered as open even though nothing is on screen yet, because that is how the + * page-level closes find an instance: onDocumentClick() and the "close all others" loop + * below both iterate openInstances, and the close that started this hide had already + * removed it. Without this the queue outlives an outside click or a second dropdown + * opening, and the dropdown springs open ~200ms after the user dismissed it - taking the + * one they had just opened down with it, through this very loop. closeDropbox() knows a + * queued instance is not really open and only cancels the queue. + * + * Both of those loops also do `instanceObj.shouldFocusWrapperOnClose = false` immediately + * before calling closeDropbox() on every entry they find - correct for the dropdown they + * are actually closing, wrong for a merely-queued one, whose real close is a separate, + * still-pending hide that already made its own decision about where focus should land. + * Snapshotting it here lets closeDropbox()'s cancel branch undo that mutation rather than + * leaving it to silently overwrite a decision an unrelated, later event has no business + * revisiting. + */ + this.queuedShouldFocusWrapperOnClose = this.shouldFocusWrapperOnClose; + VirtualSelect.openInstances.add(this); + + return; + } + + this.pendingOpen = false; let originalTransition = ''; // Disable transitions for programmatic opening if (!isSilent) { @@ -3092,6 +3168,30 @@ export class VirtualSelect { } closeDropbox(isSilent) { + /** + * An instance whose open is only queued behind a running hide is not open: that hide is + * already doing the closing, so a close aimed at it has nothing to do but cancel the queue. + * + * Before isSilentClose is touched, deliberately. The afterHidePopper() still pending belongs + * to the close that started the hide; overwriting the flag here would hand it this call's + * silentness instead, and its afterClose would go missing (or appear when it should not). + * Returning early also keeps this from dispatching a second beforeClose for a dropdown that + * is already closing. + * + * shouldFocusWrapperOnClose is restored to what it was when the open was queued, undoing + * whatever this call's caller just set it to. onDocumentClick() and the "close all others" + * loop below both write `false` to it immediately before calling closeDropbox() - a decision + * that belongs to the close THIS call would have performed, not to the hide that is actually + * still running for this instance and will read the flag itself once it finishes. + */ + if (this.pendingOpen) { + this.pendingOpen = false; + this.shouldFocusWrapperOnClose = this.queuedShouldFocusWrapperOnClose; + VirtualSelect.openInstances.delete(this); + + return; + } + this.isSilentClose = isSilent; // Remove from open instances @@ -3224,7 +3324,11 @@ export class VirtualSelect { afterHidePopper() { const isSilent = this.isSilentClose; + /** read before anything can queue a new one, and cleared either way - see openDropbox() */ + const shouldReopen = this.pendingOpen; + this.isSilentClose = false; + this.pendingOpen = false; this.isClosingTransition = false; DomUtils.removeClass(this.$allWrappers, 'focused'); @@ -3246,13 +3350,24 @@ export class VirtualSelect { * dropbox even though the closed class will eventually drop focus to - which is * exactly the focus loss the release turns into a deliberate hand-back. * - * (afterClose needs no such ordering care: DomUtils.dispatchEvent() defers events through - * setTimeout(0), so a consumer's afterClose handler always runs after this method has - * finished.) + * (This ordering protects the *reader* of activeElement, not afterClose's own dispatch - + * DomUtils.dispatchEvent() defers every event through setTimeout(0), so a consumer's + * afterClose handler itself always runs after this method has finished and cannot reenter + * it.) */ this.releaseFocusFromDropbox(); - if (!isSilent) { + /** + * !isOpened() because the focus() call inside the release above can, by itself, reopen the + * dropdown: it fires a synchronous native focusin, and a consumer handler reacting to it by + * calling open() runs to completion (including removing the `closed` class) before control + * returns here. Dispatching afterClose after that would describe a dropdown the same + * synchronous call stack had already reopened - stale the instant it fires, and ordered + * before the reopen's own beforeOpen/afterOpen only because dispatchEvent's setTimeout(0) + * calls preserve scheduling order, not because it happened first. The replay guard just + * below makes the identical check for the identical reason. + */ + if (!isSilent && !this.isOpened()) { DomUtils.dispatchEvent(this.$ele, 'afterClose'); } @@ -3260,23 +3375,30 @@ export class VirtualSelect { this.shouldFocusWrapperOnClose = true; /** - * Stand down if a focus handler above reopened the dropdown synchronously: openDropbox() - * has already made the dropbox visible and focusable again, and hiding it now would leave - * it on screen but absent from the accessibility tree. The next close re-applies these - * through its own afterHidePopper(). + * The hide has finished, so the popover is idle and will accept a show again: replay the + * open that arrived while it was still running. Placed before the guard below rather than + * after it, so the reopen goes through the one path that already knows not to hide a + * dropdown that is open - openDropbox() leaves isOpened() true, and the guard stands the + * hiding writes down for it exactly as it does for a synchronous focus-handler reopen. * - * Deliberately isOpened() and not `VirtualSelect.openInstances.has(this)`, even though the - * latter also means "reopened". The two differ for a hide that lost a race with an earlier - * reopen - `open()` called during the fade, which does not cancel the popover's pending - * hide because its show() early-returns while `pop-comp-active` is still set. That hide - * has to *complete*: the popover has already taken the dropbox off the screen by the time - * it calls back, so standing down would leave the wrapper without its `closed` class, - * `isOpened()` true against an invisible dropbox, and every later toggle closing a - * dropdown that is not there - unreopenable. Verified by building it: the openInstances - * form fails the recovery assertions in a11y-aria-hidden-focus.cy.ts. + * isOpened() because both reopen routes can converge on one close: a consumer focus handler + * reacting to the refocus in releaseFocusFromDropbox() above may already have reopened the + * dropdown synchronously. Replaying on top of that would run a second openDropbox() and + * dispatch a second beforeOpen for a single open. + */ + if (shouldReopen && !this.isDestroyed && !this.isOpened()) { + this.openDropbox(); + } + + /** + * Stand down if the dropdown is open again by the time these writes would run - either the + * replay above, or a focus handler that reopened it synchronously from the wrapper refocus. + * openDropbox() has already made the dropbox visible and focusable in both cases, and + * hiding it now would leave it on screen but absent from the accessibility tree. The next + * close re-applies all of this through its own afterHidePopper(). * - * isOpened() is false here for that stale case (addClass above set it) and true only for - * the synchronous reopen this guard is for, which is exactly the split that is wanted. + * A state read rather than a record of what happened, and evaluated last, so it cannot + * disagree with the reopen paths the way an "did someone call open()" flag would. */ if (this.isOpened()) { return; @@ -3507,11 +3629,10 @@ export class VirtualSelect { * add - a stale hide should not yank focus out of a dropdown the user is looking at - but a * reopen that genuinely put the dropbox back on screen cannot have a hide still pending * against it: the popover clears `pop-comp-active` before calling back, so a successful - * show() means the hide already finished. What such a guard actually suppresses is the one - * case that needs the release most - the reopen that lost its race with the fade, where the - * popover has already applied display:none and focus inside the dropbox is about to be lost - * to . The caller decides whether the dropbox is being hidden; this only makes sure - * nothing is focused inside it when that happens. + * show() means the hide already finished, and an open arriving before that is queued rather + * than applied (see openDropbox). All such a guard could do is suppress the release on a + * path where the dropbox is being hidden anyway. The caller decides whether the dropbox is + * being hidden; this only makes sure nothing is focused inside it when that happens. * * preventScroll because this runs ~200ms after the user's action - if they have scrolled in * the meantime, focus restoration must not scroll the combobox back into view (the