[6.x] Name table cells once, after their column and row

CHANGELOG.md

@@ -28,6 +28,7 @@
 - Fixed an accessibility issue where the control panel’s skip links were hidden behind the header bar when focused. ([#19886](https://github.com/craftcms/cms/pull/19886))
 - Fixed an accessibility issue where switches in editable tables weren’t labeled by their column headers. ([#19886](https://github.com/craftcms/cms/pull/19886))
 - Fixed a bug where editable tables on the same page could label each other’s inputs, or move focus to another table’s column header when sorting. ([#19886](https://github.com/craftcms/cms/pull/19886))
+- Fixed an accessibility issue where checkboxes and select menus in form-builder table cells were named twice for screen readers, and checkboxes showed their label beside them. ([#19890](https://github.com/craftcms/cms/pull/19890))
 
 ### Administration
 

resources/js/modules/forms/CheckboxControl.vue

@@ -1,7 +1,9 @@
 <script setup lang="ts">
   import CraftCheckbox from '@craftcms/ui/components/checkbox/checkbox';
+  import {inject} from 'vue';
   import type {FormControlPayload} from './types';
   import {
+    FieldLabelSrOnly,
     ignoreModelValueInitialization,
     inputName,
     serverErrorValidators,
@@ -21,6 +23,9 @@
     required: boolean;
   }>();
   const emit = defineEmits<{(event: 'update:value', value: boolean): void}>();
+  // A hidden field label already names the input; repeating it would show it
+  // beside the box and read it twice.
+  const fieldLabelSrOnly = inject(FieldLabelSrOnly, undefined);
 
   const onModelValueChanged = ignoreModelValueInitialization((event) => {
     if (!(event.currentTarget instanceof CraftCheckbox)) {
@@ -34,7 +39,7 @@
 <template>
   <craft-checkbox
     :name="editable ? inputName(control.path) : ''"
-    :label="control.props.label ?? label"
+    :label="control.props.label ?? (fieldLabelSrOnly ? undefined : label)"
     .checked="Boolean(value)"
     .choiceValue="String(control.props.checkedValue ?? '1')"
     :disabled="!editable"

resources/js/modules/forms/ChoiceControl.vue

@@ -323,7 +323,6 @@
 <template>
   <craft-select
     v-if="control.props.presentation === 'select'"
-    :label="fieldLabelSrOnly ? label : undefined"
     :label-sr-only="fieldLabelSrOnly || undefined"
     :name="
       editable

resources/js/modules/forms/TableControl.a11y.browser.test.ts

@@ -0,0 +1,212 @@
+import {afterEach, beforeEach, expect, it, vi} from 'vite-plus/test';
+import {page} from 'vite-plus/test/browser/context';
+import {createApp, h, nextTick, type App} from 'vue';
+import {createCpComponentRegistry} from '@/bootstrap/components';
+import FormRenderer from './FormRenderer.vue';
+import {registerFormComponents} from './register';
+import type {FormNodePayload, FormPayload, FormProperties} from './types';
+
+/**
+ * Each editable table cell must be named after its column header and row
+ * (WCAG 4.1.2), and tables sharing column keys must not label each other's
+ * inputs. Runs in Chromium so the names are the browser's own.
+ */
+
+const columns = {
+  title: {heading: 'Title', type: 'singleline'},
+  enabled: {heading: 'Enabled', type: 'lightswitch'},
+  homepage: {heading: 'Homepage', type: 'checkbox'},
+  route: {
+    heading: 'Route',
+    type: 'singleline',
+    prefixSelect: {
+      key: 'routeType',
+      label: 'Route type',
+      options: [
+        {label: 'Template', value: 'template'},
+        {label: 'Route', value: 'route'},
+      ],
+    },
+  },
+};
+
+const cellControls: Array<[string, string, string, string, FormProperties?]> = [
+  ['title', 'Title', 'Text', 'craft:text'],
+  ['enabled', 'Enabled', 'Lightswitch', 'craft:lightswitch'],
+  ['homepage', 'Homepage', 'Checkbox', 'craft:checkbox'],
+  ['route', 'Route', 'Text', 'craft:text'],
+  [
+    'routeType',
+    'Route type',
+    'Choice',
+    'craft:choice',
+    {
+      options: columns.route.prefixSelect.options,
+      multiple: false,
+      presentation: 'select',
+      placeholder: false,
+    },
+  ],
+];
+
+function cells(scope: string[], deltaGroup: string[]): FormNodePayload[] {
+  return cellControls.map(([key, label, type, component, props]) => ({
+    type: 'CraftCms\\Cms\\Form\\Nodes\\Field',
+    component: 'craft:field',
+    props: {label, labelSrOnly: true},
+    control: {
+      type: `CraftCms\\Cms\\Form\\Controls\\${type}`,
+      component,
+      props: props ?? {},
+      path: [...scope, key],
+      mode: 'editable',
+      deltaGroup,
+    },
+  }));
+}
+
+function table(path: string, rowKeys: string[]): FormNodePayload {
+  return {
+    type: 'CraftCms\\Cms\\Form\\Nodes\\Field',
+    component: 'craft:field',
+    props: {},
+    control: {
+      type: 'CraftCms\\Cms\\Form\\Controls\\Table',
+      component: 'craft:table',
+      nestsForms: true,
+      path: [path],
+      deltaGroup: [path],
+      mode: 'editable',
+      props: {
+        columns,
+        keyed: true,
+        defaultValues: row,
+        rowTemplate: {scope: [], refreshable: false, nodes: cells([], [path])},
+      },
+      forms: rowKeys.map((key) => ({
+        scope: [path, key],
+        refreshable: false,
+        nodes: cells([path, key], [path]),
+      })),
+    },
+  };
+}
+
+const row = {
+  title: 'Craft',
+  enabled: true,
+  homepage: false,
+  route: '',
+  routeType: 'template',
+};
+
+// Two tables with the same column keys, the way the section form has them.
+const payload: FormPayload = {
+  scope: [],
+  refreshable: false,
+  globalErrors: [],
+  errors: [],
+  values: {
+    sites: {default: row, other: row},
+    previewTargets: {default: row},
+  },
+  nodes: [
+    table('sites', ['default', 'other']),
+    table('previewTargets', ['default']),
+  ],
+};
+
+let app: App | null = null;
+let host: HTMLElement | null = null;
+
+beforeEach(() => {
+  vi.stubGlobal(
+    'fetch',
+    vi.fn().mockResolvedValue(new Response('<svg></svg>'))
+  );
+});
+
+afterEach(() => {
+  app?.unmount();
+  app = null;
+  host?.remove();
+  host = null;
+  vi.unstubAllGlobals();
+});
+
+async function mount(): Promise<HTMLElement> {
+  host = document.createElement('div');
+  document.body.append(host);
+  app = createApp({render: () => h(FormRenderer, {payload})});
+  const registry = createCpComponentRegistry();
+  registerFormComponents(registry);
+  registry.install(app);
+  app.mount(host);
+  await nextTick();
+
+  for (const element of host.querySelectorAll('*')) {
+    if ('updateComplete' in element) {
+      await (element as {updateComplete: Promise<unknown>}).updateComplete;
+    }
+  }
+  await nextTick();
+
+  return host;
+}
+
+const roles: Record<string, 'textbox' | 'switch' | 'checkbox' | 'combobox'> = {
+  Title: 'textbox',
+  Enabled: 'switch',
+  Homepage: 'checkbox',
+  Route: 'textbox',
+  'Route type': 'combobox',
+};
+
+it('names each cell after its column header and row', async () => {
+  await mount();
+
+  // Row 1 appears in both tables, row 2 only in the first.
+  const expected: Array<[role: string, name: string, count: number]> = [
+    ...Object.entries(roles)
+      .filter(([heading]) => heading !== 'Route type')
+      .flatMap(
+        ([heading, role]): Array<[string, string, number]> => [
+          [role, `${heading}, row 1`, 2],
+          [role, `${heading}, row 2`, 1],
+        ]
+      ),
+    ['combobox', 'Route type', 3],
+  ];
+
+  for (const [role, name, count] of expected) {
+    const matches = page
+      .getByRole(role as (typeof roles)[string], {name, exact: true})
+      .elements();
+
+    expect.soft(matches, `${role} "${name}"`).toHaveLength(count);
+  }
+});
+
+it('labels inputs from their own table when tables share column keys', async () => {
+  const root = await mount();
+  const tables = [...root.querySelectorAll('table')];
+  expect(tables).toHaveLength(2);
+
+  for (const tableElement of tables) {
+    for (const element of tableElement.querySelectorAll('[aria-labelledby]')) {
+      for (const id of element.getAttribute('aria-labelledby')!.split(/\s+/)) {
+        const label = id ? document.getElementById(id) : null;
+
+        if (label) {
+          expect(
+            tableElement.contains(label),
+            `#${id} labels an input in another table`
+          ).toBe(true);
+        }
+      }
+    }
+  }
+
+  const ids = [...root.querySelectorAll('[id]')].map((element) => element.id);
+  expect(ids.length).toBe(new Set(ids).size);
+});