From f00d0c4655b673f4416a1b46585cfc0ed5e5a1dc Mon Sep 17 00:00:00 2001
From: Logan <logan@ljones.me>
Date: Thu, 13 Aug 2026 01:52:08 -0400
Subject: [PATCH] fix(a11y): name every form control in Settings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Measured with Accessibility.getFullAXTree against the running app with
all seven sections expanded: 24 of 93 controls computed an empty name.
Every config-field select and toggle, and all eighteen track-list
column checkboxes, had a <label> sitting right beside them with nothing
associating the two. Now 0 of 93.

Not in the audit, and a11y.6 says why in its own line: it scanned every
<button>, and none of these is one. Same shape as the count that sent
Phase 1 looking for an unnamed sort control — the claim was answering a
narrower question than it reads as.

The fields use `for`/`id` rather than aria-label, for what it buys
beyond the name: the label text becomes a click target for the control.
A fixed id is safe only because each config-field is its own shadow
root.

Two more are named but identify nothing, which is a11y.32's complaint
one page over: three shortcut buttons announced themselves as "S", and
thirty-six column arrows as "Move up".
---
 .../components/config-page/config-field.ts    |  33 ++++-
 .../src/components/config-page/config-page.ts |  11 +-
 .../config-page/shortcut-capture.ts           |  16 +++
 .../test/components/settings-names.test.ts    | 116 ++++++++++++++++++
 4 files changed, 171 insertions(+), 5 deletions(-)
 create mode 100644 frontend/test/components/settings-names.test.ts

diff --git a/frontend/src/components/config-page/config-field.ts b/frontend/src/components/config-page/config-field.ts
index 1d9c076..032c784 100644
--- a/frontend/src/components/config-page/config-field.ts
+++ b/frontend/src/components/config-page/config-field.ts
@@ -1,6 +1,26 @@
 import { LitElement, html, css, nothing } from 'lit';
 import { customElement, property } from 'lit/decorators.js';
 
+/**
+ * The id every field's control carries, so its `<label>` can name it.
+ *
+ * The label was a *sibling* of the control with no `for`, which names
+ * nothing — so every select, toggle and text field in Settings computed
+ * an empty accessible name. Measured on the expanded Settings page:
+ * **24 of 93 controls unnamed**, six of them here and the rest the
+ * column toggles in `config-page`. `a11y.6` is not wrong about this;
+ * it says in the same line that it scanned every `<button>`, and none
+ * of these is one.
+ *
+ * A fixed id is safe, and only because each `config-field` is its own
+ * shadow root — the whole page renders a dozen elements with
+ * `id="control"` and each `for` resolves within its own root. It is
+ * preferred over `aria-label` for what it buys beyond the name: a real
+ * label association also makes the label text a click target for the
+ * control, which is behaviour, not annotation.
+ */
+const CONTROL_ID = 'control';
+
 /**
  * Schema describing a single config field.
  *
@@ -225,7 +245,9 @@ export class ConfigField extends LitElement {
                 ${this.schema.type === 'toggle'
                     ? this.renderToggle()
                     : html`
-                          <label>${this.schema.label}</label>
+                          <label for=${CONTROL_ID}>
+                              ${this.schema.label}
+                          </label>
                           ${this.renderInput()}
                       `}
                 ${this.schema.description
@@ -257,6 +279,7 @@ export class ConfigField extends LitElement {
     private renderText() {
         return html`
             <input
+                id=${CONTROL_ID}
                 type="text"
                 .value=${String(this.value ?? '')}
                 ?disabled=${this.schema.disabled}
@@ -268,6 +291,7 @@ export class ConfigField extends LitElement {
     private renderNumber() {
         return html`
             <input
+                id=${CONTROL_ID}
                 type="number"
                 .value=${String(this.value ?? '')}
                 ?disabled=${this.schema.disabled}
@@ -281,6 +305,7 @@ export class ConfigField extends LitElement {
 
         return html`
             <select
+                id=${CONTROL_ID}
                 ?disabled=${this.schema.disabled}
                 @change=${this.onSelectChange}
             >
@@ -304,6 +329,7 @@ export class ConfigField extends LitElement {
         return html`
             <div class="color-wrapper">
                 <input
+                    id=${CONTROL_ID}
                     type="color"
                     .value=${hex}
                     ?disabled=${this.schema.disabled}
@@ -318,11 +344,13 @@ export class ConfigField extends LitElement {
         return html`
             <div class="input-row">
                 <input
+                    id=${CONTROL_ID}
                     type="text"
                     .value=${String(this.value ?? '')}
                     readonly
                 />
                 <button
+                    aria-label="Browse for ${this.schema.label}"
                     ?disabled=${this.schema.disabled}
                     @click=${this.onBrowseClick}
                 >
@@ -337,9 +365,10 @@ export class ConfigField extends LitElement {
 
         return html`
             <div class="toggle-row">
-                <label>${this.schema.label}</label>
+                <label for=${CONTROL_ID}>${this.schema.label}</label>
                 <label class="toggle-switch">
                     <input
+                        id=${CONTROL_ID}
                         type="checkbox"
                         ?checked=${checked}
                         ?disabled=${this.schema.disabled}
diff --git a/frontend/src/components/config-page/config-page.ts b/frontend/src/components/config-page/config-page.ts
index 87d3a51..f88bd5f 100644
--- a/frontend/src/components/config-page/config-page.ts
+++ b/frontend/src/components/config-page/config-page.ts
@@ -1693,6 +1693,9 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
                         const isLast =
                             idx === order.length - 1;
 
+                        const columnLabel =
+                            COLUMN_DEFS[id]?.label ?? id;
+
                         return html`
                             <li
                                 class="column-item ${checked ? 'enabled' : 'disabled'}"
@@ -1700,6 +1703,7 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
                                 <input
                                     type="checkbox"
                                     class="column-toggle"
+                                    aria-label="Show the ${columnLabel} column"
                                     .checked=${checked}
                                     ?disabled=${onlyOne}
                                     @change=${() =>
@@ -1710,9 +1714,7 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
                                 <span
                                     class="column-label"
                                 >
-                                    ${COLUMN_DEFS[id]
-                                        ?.label ??
-                                    id}
+                                    ${columnLabel}
                                 </span>
                                 <span
                                     class="column-arrows"
@@ -1723,6 +1725,7 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
                                               <button
                                                   class="column-arrow-btn"
                                                   title="Move up"
+                                                  aria-label="Move ${columnLabel} up"
                                                   @click=${() =>
                                                       this.handleColumnMove(
                                                           id,
@@ -1738,6 +1741,7 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
                                               <button
                                                   class="column-arrow-btn"
                                                   title="Move down"
+                                                  aria-label="Move ${columnLabel} down"
                                                   @click=${() =>
                                                       this.handleColumnMove(
                                                           id,
@@ -1812,6 +1816,7 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
                                         </span>
                                         <shortcut-capture
                                             .action=${action}
+                                            .label=${meta.label}
                                             .currentKey=${bindings.get(
                                                 action,
                                             ) ?? ''}
diff --git a/frontend/src/components/config-page/shortcut-capture.ts b/frontend/src/components/config-page/shortcut-capture.ts
index 13de5a4..f840bab 100644
--- a/frontend/src/components/config-page/shortcut-capture.ts
+++ b/frontend/src/components/config-page/shortcut-capture.ts
@@ -8,6 +8,17 @@ export class ShortcutCapture extends LitElement {
     @property() currentKey = '';
     @property() defaultKey = '';
 
+    /**
+     * What the binding does, for the name.
+     *
+     * The button's text is the *key* — so the shortcuts list rendered
+     * three buttons called "S", two called "Down" and one called "?",
+     * beside a visible label that named none of them (it is a sibling,
+     * in another shadow root, and nothing associated the two). The
+     * label is what a control is for; the key is its value.
+     */
+    @property() label = '';
+
     @state() private recording = false;
 
     static override styles = css`
@@ -134,6 +145,9 @@ export class ShortcutCapture extends LitElement {
                     : this.currentKey
                       ? ''
                       : 'not-set'}
+                aria-label=${this.label
+                    ? `${this.label} shortcut: ${this.currentKey || 'not set'}`
+                    : ''}
                 @click=${this.handleClick}
                 @keydown=${this.handleKeydown}
                 @blur=${this.handleBlur}
@@ -148,6 +162,8 @@ export class ShortcutCapture extends LitElement {
                           class="reset-btn"
                           @click=${this.handleReset}
                           title="Reset to default (${this.defaultKey})"
+                          aria-label="Reset ${this.label ||
+                              this.action} to ${this.defaultKey}"
                       >
                           \u21BA
                       </button>
diff --git a/frontend/test/components/settings-names.test.ts b/frontend/test/components/settings-names.test.ts
new file mode 100644
index 0000000..1563559
--- /dev/null
+++ b/frontend/test/components/settings-names.test.ts
@@ -0,0 +1,116 @@
+/**
+ * Every form control in Settings had a visible label and no name.
+ *
+ * Not in the audit, and `a11y.6` says why in its own text: it scanned
+ * every `<button>`, and a `<select>` is not one. Measured with
+ * `Accessibility.getFullAXTree` against the running app with all seven
+ * sections expanded — **24 of 93 controls unnamed**: every
+ * `config-field` select and toggle, and all eighteen track-list column
+ * checkboxes. In each case a `<label>` sat right beside the control
+ * with nothing associating the two.
+ *
+ * Two of the fixes here are about a name that exists and does not
+ * identify anything, which is `a11y.32`'s complaint one page over:
+ * three shortcut buttons announced themselves as "S", and thirty-six
+ * column arrows as "Move up" or "Move down".
+ */
+import { describe, expect, it } from 'vitest';
+
+import '@components/config-page/config-field';
+import '@components/config-page/shortcut-capture';
+import { fixture, shadow } from '@test/support/render';
+
+/** What the `<label>` in this shadow root actually points at. */
+function labelledControl(host: Element): Element | null {
+  const forId = shadow(host, 'label[for]')?.getAttribute('for');
+
+  return forId ? shadow(host, `#${CSS.escape(forId)}`) : null;
+}
+
+describe('a config field names its control', () => {
+  it('associates the label with a select', async () => {
+    const el = await fixture('config-field', {
+      schema: {
+        key: 'theme.shade',
+        label: 'Shade',
+        type: 'select',
+        options: [{ value: 'dark', label: 'Dark' }],
+      },
+      value: 'dark',
+    });
+
+    expect(labelledControl(el)?.tagName).toBe('SELECT');
+  });
+
+  it('associates the label with a toggle, which is a second label deep', async () => {
+    const el = await fixture('config-field', {
+      schema: { key: 'x', label: 'Scan on startup', type: 'toggle' },
+      value: true,
+    });
+
+    // The checkbox is wrapped in a *styling* label carrying only a
+    // span, which contributes no text and so named nothing.
+    const control = labelledControl(el) as HTMLInputElement | null;
+
+    expect(control?.type).toBe('checkbox');
+  });
+
+  it.each(['text', 'number', 'color', 'directory'] as const)(
+    'associates the label with a %s field',
+    async (type) => {
+      const el = await fixture('config-field', {
+        schema: { key: 'k', label: 'Music folder', type },
+        value: '',
+      });
+
+      expect(labelledControl(el)).toBeTruthy();
+    },
+  );
+
+  it('says which field a Browse button browses for', async () => {
+    const el = await fixture('config-field', {
+      schema: { key: 'k', label: 'Music folder', type: 'directory' },
+      value: '',
+    });
+
+    expect(shadow(el, 'button')?.getAttribute('aria-label'))
+      .toBe('Browse for Music folder');
+  });
+});
+
+describe('a shortcut button says what it binds', () => {
+  it('names the action, and keeps the key as the value', async () => {
+    const el = await fixture('shortcut-capture', {
+      action: 'player.playPause',
+      label: 'Play / Pause',
+      currentKey: 'Space',
+    });
+
+    expect(shadow(el, 'button')?.getAttribute('aria-label'))
+      .toBe('Play / Pause shortcut: Space');
+    expect(shadow(el, 'button')?.textContent?.trim()).toBe('Space');
+  });
+
+  it('says so when there is no key rather than announcing nothing', async () => {
+    const el = await fixture('shortcut-capture', {
+      action: 'tracklist.delete',
+      label: 'Delete Track',
+      currentKey: '',
+    });
+
+    expect(shadow(el, 'button')?.getAttribute('aria-label'))
+      .toBe('Delete Track shortcut: not set');
+  });
+
+  it('names the action in the reset button too', async () => {
+    const el = await fixture('shortcut-capture', {
+      action: 'player.next',
+      label: 'Next Track',
+      currentKey: 'X',
+      defaultKey: 'N',
+    });
+
+    expect(shadow(el, '.reset-btn')?.getAttribute('aria-label'))
+      .toBe('Reset Next Track to N');
+  });
+});