fix(a11y): mark toolbar li/a wrappers presentational (Lighthouse, #7255)

Lighthouse's axe-core `listitem` rule fires on every toolbar button
because role="toolbar" on the <ul> overrides its implicit role="list",
leaving the <li> children "orphaned" by axe's heuristic. Murphy's
2026-05-16 follow-up on #7255 attached the Chrome DevTools Lighthouse
panel screenshot of this exact failure.

Marking the <li>+<a> wrappers role="presentation" tells axe-core they
are layout scaffolding for the toolbar role, while the inner <button>
keeps its semantics for AT. Same treatment for SelectButton's <li>
wrapper. The Separator's <li> also gets aria-hidden=true so AT does
not announce an empty list item between toolbar buttons.

CSS and JS selectors that still target `.toolbar ul li` continue to
work — role="presentation" only affects the accessibility tree, not
the DOM tree. No visual or behavioral change for sighted users.

Adds a Playwright spec that walks every rendered toolbar <li>/<a>
and asserts role="presentation" so future toolbar.ts tweaks can't
silently re-introduce the Lighthouse failure.

Refs #7255

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: JohnMcLear

src/node/utils/toolbar.ts

@@ -113,12 +113,23 @@ class Button {
     }
 
     render() {
+        // role="presentation" on the <li> + <a> wrappers tells axe-core /
+        // Lighthouse they are layout scaffolding for the role="toolbar"
+        // parent, not list items. Without it the listitem rule fires
+        // because role="toolbar" overrides the implicit role="list" on
+        // the <ul>, leaving every <li> "orphaned" by axe's heuristic.
+        // The inner <button> keeps its semantics. See ether/etherpad#7255.
         const liAttributes = {
             'data-type': 'button',
             'data-key': this.attributes.command,
+            'role': 'presentation',
         };
         return tag('li', liAttributes,
-            tag('a', {'class': this.grouping, 'data-l10n-id': this.attributes.localizationId},
+            tag('a', {
+                    'class': this.grouping,
+                    'role': 'presentation',
+                    'data-l10n-id': this.attributes.localizationId,
+                },
                 tag('button', {
                     'class': ` ${this.attributes.class}`,
                     'data-l10n-id': this.attributes.localizationId,
@@ -167,6 +178,8 @@ class SelectButton extends Button {
             'id': this.attributes.id,
             'data-key': this.attributes.command,
             'data-type': 'select',
+            // See Button.render() above for the role="presentation" rationale.
+            'role': 'presentation',
         };
         return tag('li', attributes, this.select({id: this.attributes.selectId}));
     }
@@ -184,7 +197,14 @@ class Separator {
     }
 
     public render() {
-        return tag('li', {class: 'separator'});
+        // See Button.render() above for the role="presentation" rationale.
+        // The separator is purely visual; aria-hidden makes that explicit so
+        // AT does not announce an empty list item between toolbar buttons.
+        return tag('li', {
+            'class': 'separator',
+            'role': 'presentation',
+            'aria-hidden': 'true',
+        });
 
     }
 }

src/tests/frontend-new/specs/a11y_dialogs.spec.ts

@@ -173,6 +173,29 @@ test('editbar toolbars have role=toolbar with accessible names (#7255)', async (
   }
 });
 
+test('toolbar <li>/<a> wrappers are presentational (Lighthouse listitem rule, #7255)', async ({page}) => {
+  // Lighthouse / axe-core's `listitem` rule flags <li> children of any
+  // element whose role isn't `list` — and role="toolbar" on the <ul>
+  // overrides the implicit list role. Murphy's #7255 follow-up included
+  // the Lighthouse screenshot of this exact failure. role="presentation"
+  // tells axe-core the <li>+<a> wrappers are layout scaffolding, while
+  // the inner <button> retains button semantics for AT.
+  const listItems = page.locator('.menu_left > li, .menu_right > li');
+  const count = await listItems.count();
+  expect(count).toBeGreaterThan(0);
+  for (let i = 0; i < count; i++) {
+    await expect(listItems.nth(i)).toHaveAttribute('role', 'presentation');
+  }
+  // Non-separator items wrap their <button> in an <a> — that <a> is also
+  // presentational so AT focus lands on the <button>, not the empty link.
+  const anchors = page.locator('.menu_left > li:not(.separator) > a, .menu_right > li:not(.separator) > a');
+  const aCount = await anchors.count();
+  expect(aCount).toBeGreaterThan(0);
+  for (let i = 0; i < aCount; i++) {
+    await expect(anchors.nth(i)).toHaveAttribute('role', 'presentation');
+  }
+});
+
 test('linemetricsdiv is hidden from screen readers (#7255)', async ({page}) => {
   // The "Ether X" announcement in the issue's a11y-inspector screenshot was
   // the outer iframe (titled "Ether") plus a single "x" text leaf from