From 6d0e46d537f2bd9f93dd57b402c7ffd061cc3541 Mon Sep 17 00:00:00 2001
From: Logan <logan@ljones.me>
Date: Wed, 12 Aug 2026 23:08:49 -0400
Subject: [PATCH] fix(a11y): wire the combobox's roles to each other

a11y.14: role=combobox, role=listbox and role=option were all present
and nothing connected them -- no ids, no aria-controls, no
aria-activedescendant -- so arrowing through nineteen options moved a
visual highlight and announced nothing.

Reproduced on the smart-playlist rule editor against the browser's own
computation rather than a snapshot: getFullAXTree reported no
activedescendant and no controls on any of the five comboboxes on the
page. After, the same node carries both.

aria-selected also meant 'highlighted', which is the one thing it does
not mean: a user arrowing past an option heard it announced as selected
while the value they had chosen was announced as unselected. It is the
chosen value now, and the highlight is what activedescendant points at.

The IDREF tests assert the link rather than the attribute -- an
activedescendant naming an id no element carries is exactly as silent as
no attribute, and reads as fixed.
---
 frontend/src/components/combobox/combobox.ts  |  44 +++++++-
 .../test/components/combobox-aria.test.ts     | 103 ++++++++++++++++++
 2 files changed, 145 insertions(+), 2 deletions(-)
 create mode 100644 frontend/test/components/combobox-aria.test.ts

diff --git a/frontend/src/components/combobox/combobox.ts b/frontend/src/components/combobox/combobox.ts
index db0544d..94b166e 100644
--- a/frontend/src/components/combobox/combobox.ts
+++ b/frontend/src/components/combobox/combobox.ts
@@ -10,6 +10,25 @@ import { designTokens } from '../../styles/tokens.css';
  * Key implementation detail: option `<li>` elements use `@mousedown` with
  * `e.preventDefault()` so that the input's `blur` event does not close the
  * dropdown before the click registers.
+ *
+ * `a11y.14`: the roles were right and the wiring between them was
+ * missing, so arrowing through the list moved a visual highlight and
+ * announced nothing — confirmed against the browser's own computation
+ * (`Accessibility.getFullAXTree` reported no `activedescendant` and no
+ * `controls` on any of the five comboboxes on the page). Three things
+ * carry it now: ids on the listbox and every option, `aria-controls`,
+ * and `aria-activedescendant` naming the highlighted option.
+ *
+ * `aria-selected` used to mean "highlighted", which is the one thing it
+ * does not mean. It is the *chosen* value now; the highlight is what
+ * `aria-activedescendant` points at, which is the distinction the whole
+ * pattern rests on.
+ *
+ * Unlike `config-section`'s disclosure, this `aria-controls` IDREF is
+ * allowed to dangle while the popup is closed: the listbox genuinely
+ * does not exist then, and `aria-expanded="false"` says so. A
+ * disclosure's body exists either way, which is why that one renders
+ * unconditionally and hides with `hidden`.
  */
 @customElement('yj-combobox')
 export class YjCombobox extends LitElement {
@@ -45,6 +64,22 @@ export class YjCombobox extends LitElement {
     @state()
     private highlightedIndex = -1;
 
+    /**
+     * Per-instance id prefix for the IDREFs below.
+     *
+     * The ids only have to be unique within this shadow root — an IDREF
+     * does not cross one — but two comboboxes render side by side in a
+     * smart-playlist rule row, so a counter costs nothing and keeps the
+     * DOM readable when one of these is being debugged.
+     */
+    private readonly uid = `yj-combobox-${(YjCombobox.instances += 1)}`;
+
+    private static instances = 0;
+
+    private optionId(i: number): string {
+        return `${this.uid}-opt-${i}`;
+    }
+
     // ── Computed ────────────────────────────────────────────────────
 
     /** Options that match the current filterText (case-insensitive substring). */
@@ -291,15 +326,20 @@ export class YjCombobox extends LitElement {
                     role="combobox"
                     aria-expanded=${this.open}
                     aria-autocomplete="list"
+                    aria-controls=${`${this.uid}-listbox`}
+                    aria-activedescendant=${this.open && this.highlightedIndex >= 0
+                        ? this.optionId(this.highlightedIndex)
+                        : nothing}
                 />
                 ${this.open && opts.length > 0
                     ? html`
-                          <ul class="dropdown" role="listbox">
+                          <ul class="dropdown" role="listbox" id=${`${this.uid}-listbox`}>
                               ${opts.map(
                                   (opt, i) => html`
                                       <li
                                           role="option"
-                                          aria-selected=${i === this.highlightedIndex}
+                                          id=${this.optionId(i)}
+                                          aria-selected=${opt === this.value}
                                           class=${i === this.highlightedIndex
                                               ? 'highlighted'
                                               : ''}
diff --git a/frontend/test/components/combobox-aria.test.ts b/frontend/test/components/combobox-aria.test.ts
new file mode 100644
index 0000000..0110ccf
--- /dev/null
+++ b/frontend/test/components/combobox-aria.test.ts
@@ -0,0 +1,103 @@
+/**
+ * `a11y.14` — the combobox announces the option it has moved to.
+ *
+ * Reproduced in the running app first, on the smart-playlist rule
+ * editor, and confirmed against the browser's own computation rather
+ * than a snapshot: `Accessibility.getFullAXTree` reported no
+ * `activedescendant` and no `controls` on any of the five comboboxes on
+ * the page, so arrowing through nineteen options moved a visual
+ * highlight and told a screen reader nothing.
+ *
+ * The IDREF cases assert the *link*, not the attribute: an
+ * `aria-activedescendant` naming an id that no element carries is
+ * exactly as silent as no attribute at all, and reads as fixed.
+ */
+import { describe, expect, it } from 'vitest';
+
+import '@components/combobox/combobox';
+import type { YjCombobox } from '@components/combobox/combobox';
+import { fixture, shadow, shadowAll } from '@test/support/render';
+
+const OPTIONS = ['Artist', 'Album', 'Genre', 'Year'];
+
+type Combobox = YjCombobox;
+
+async function open(value = ''): Promise<Combobox> {
+  const el = await fixture<Combobox>('yj-combobox', {
+    options: OPTIONS,
+    value,
+  });
+
+  shadow<HTMLInputElement>(el, 'input')?.focus();
+  await el.updateComplete;
+
+  return el;
+}
+
+async function arrowDown(el: Combobox, times = 1): Promise<void> {
+  for (let i = 0; i < times; i++) {
+    shadow<HTMLInputElement>(el, 'input')?.dispatchEvent(
+      new KeyboardEvent('keydown', { key: 'ArrowDown', bubbles: true }),
+    );
+    await el.updateComplete;
+  }
+}
+
+describe('<yj-combobox> ARIA', () => {
+  it('points aria-controls at the listbox it opens', async () => {
+    const el = await open();
+
+    const controls = shadow(el, 'input')?.getAttribute('aria-controls');
+
+    expect(shadow(el, `ul#${controls}`)).not.toBeNull();
+  });
+
+  it('names the highlighted option, and that option exists', async () => {
+    const el = await open();
+
+    await arrowDown(el, 2);
+
+    const active = shadow(el, 'input')?.getAttribute('aria-activedescendant');
+    const target = active ? shadow(el, `#${active}`) : null;
+
+    expect([target?.getAttribute('role'), target?.textContent?.trim()]).toEqual(
+      ['option', 'Album'],
+    );
+  });
+
+  it('moves the pointer as the highlight moves', async () => {
+    const el = await open();
+
+    await arrowDown(el);
+    const first = shadow(el, 'input')?.getAttribute('aria-activedescendant');
+
+    await arrowDown(el);
+    const second = shadow(el, 'input')?.getAttribute('aria-activedescendant');
+
+    expect(first).not.toBe(second);
+  });
+
+  it('carries no pointer while nothing is highlighted', async () => {
+    const el = await open();
+
+    expect(shadow(el, 'input')?.hasAttribute('aria-activedescendant')).toBe(
+      false,
+    );
+  });
+
+  // aria-selected means *chosen*. It used to mean "highlighted", which
+  // is the one thing it does not mean — so a user arrowing past an
+  // option heard it announced as selected when it was not, and the
+  // value they had actually chosen was announced as unselected.
+  it('marks the chosen value as selected, not the highlighted one', async () => {
+    const el = await open('Genre');
+
+    await arrowDown(el);
+
+    const selected = shadowAll(el, 'li')
+      .filter((li) => li.getAttribute('aria-selected') === 'true')
+      .map((li) => li.textContent?.trim());
+
+    expect(selected).toEqual(['Genre']);
+  });
+});