fix(a11y): name role=toolbar regions, hide linemetricsdiv from AT (#7255) (#7777)

* fix(a11y): name role=toolbar regions, hide linemetricsdiv from AT (#7255)

Two regressions called out in the 2026-05-16 follow-up on #7255, after the
firefox accessibility inspector flagged them:

(1) The "Ether X" announcement between the editor and chat button was the
outer ace iframe (titled "Ether") plus a single 'x' text leaf the renderer
appends to outerdocbody for line-height measurement (linemetricsdiv in
ace.ts). Add aria-hidden=true on creation so AT skips the measurement node
entirely. Same approach we used for sidediv in PR #7758.

(2) The two formatting/actions <ul role="toolbar"> regions and the
history-mode role=toolbar div had no accessible name. Lighthouse + the
firefox a11y panel both flagged this. Putting data-l10n-id directly on
the <ul> would either destroy its <li> children (textContent branch) or
not populate aria-label (the html10n auto-aria-label code path skips
non-form-control elements), and a hidden <span> child inside the <ul>
would be invalid HTML. Solution: three visually-hidden <span> labels
sitting just before #editbar, each carrying data-l10n-id for translation,
referenced from the toolbars via aria-labelledby. Apply the same treatment
to .show-more-icon-btn, whose aria-label was previously hardcoded English
(no data-l10n-id, so untranslated).

Adds Playwright assertions for linemetricsdiv aria-hidden and the resolved
accessible-name text of each toolbar. Updates the existing show-more test
to expect aria-labelledby (it previously asserted hardcoded English
aria-label).

Refs #7255

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

* fix(a11y): reuse translated history-controls label key (Qodo PR review)

Qodo flagged: adding `aria-labelledby="editbar-history-label"` on
#history-controls overrode the `aria-label` that pad_mode.ts sets from
`pad.historyMode.controlsLabel`. That key is already translated in
multiple locales (en/de/nl/...); the new `pad.editor.toolbar.history`
key was English-only, so non-English users would regress from a
localized history-toolbar name to the English fallback.

Point the hidden label span at the existing translated key instead of
minting a new one, drop the new key from en.json, and update the
Playwright expectation to match the translated string ("Pad history
controls"). The aria-label that pad_mode.ts still writes to
#history-controls is now redundant (aria-labelledby wins) but harmless,
and leaving it preserves the runtime relocalization path.

Refs ether/etherpad#7777

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

* 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 ether/etherpad#7255

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

* fix(a11y): label #online_count for AT (#7255 - "number next to the user icon")

Murphy's 2026-05-16 follow-up cut off mid-bullet ("It's not clear what the
number next to the …"). Best guess: the user-count badge in the showusers
toolbar button. Currently it exposes a bare digit to AT — "5" with no
context — because the visible badge text is also the entire accessible
content.

Append a localized aria-label generated from a new pad.userlist.onlineCount
key (plural macro for one / other) whenever the count updates, so AT
announces "5 connected users" instead of the bare digit. Add role=status
and aria-live=polite so the count change is announced inline without
forcing the user to refocus the button.

Visible badge digit unchanged. html10n.get is null-safe (falls back to
an English template so AT never gets back "undefined" before the locale
bundle has loaded).

Adds a Playwright spec verifying role/aria-live and that the aria-label
contains "connected user".

Refs ether/etherpad#7255

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

* fix(a11y): tighten #online_count + plugin-emitted toolbar <li>s (#7255)

Two small follow-ups on top of the toolbar/online-count work:

(1) #online_count had no accessible label on solo-author pads.
updateNumberOfOnlineUsers — which writes the localized aria-label —
only fires on userJoin/userLeave/status change, never on initial
single-author load. Call it at the end of init() so the badge ships
with its label on first paint. Also bind html10n's 'localized' event
so non-English users get the translated label after the locale bundle
arrives (matches the keyboard-hint / history-toolbar pattern).
Harden the Playwright spec to use polling toHaveAttribute instead of
one-shot getAttribute.

(2) Sweep role="presentation" onto plugin-emitted toolbar <li>s in
pad_editbar.ts init(). Core's toolbar.ts emits its <li>s with the role
already, but plugins (ep_headings2, ep_align, ep_font_*, ep_print, ...)
ship their own editbarButtons.ejs templates that emit <li> directly,
so Lighthouse's listitem rule kept firing on the "with plugins" test
runs. Runtime sweep covers anything in the editbar at init time, no
plugin coordination needed.

Refs ether/etherpad#7255

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
John McLear 2026-05-16 13:08:33 +01:00 committed by GitHub
parent 04045fe532
commit 924059257d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 186 additions and 10 deletions

View file

@ -362,6 +362,9 @@
"pad.historyMode.users.authorsHeader": "Authors at this revision",
"pad.editor.skipToContent": "Skip to editor",
"pad.editor.keyboardHint": "Press Escape to exit the editor. Press Alt+F9 to access the toolbar.",
"pad.editor.toolbar.formatting": "Formatting toolbar",
"pad.editor.toolbar.actions": "Pad actions toolbar",
"pad.editor.toolbar.showMore": "Show more toolbar buttons",
"timeslider.toolbar.authors": "Authors:",
"timeslider.toolbar.authorsList": "No Authors",
"timeslider.toolbar.exportlink.title": "Export",
@ -398,6 +401,7 @@
"pad.savedrevs.timeslider": "You can see saved revisions by visiting the timeslider",
"pad.userlist.entername": "Enter your name",
"pad.userlist.unnamed": "unnamed",
"pad.userlist.onlineCount": "{[ plural(count) one: {{count}} connected user, other: {{count}} connected users ]}",
"pad.editbar.clearcolors": "Clear authorship colors on entire document? This cannot be undone",
"pad.impexp.importbutton": "Import Now",

View file

@ -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',
});
}
}

View file

@ -243,6 +243,11 @@ const Ace2Editor = function () {
sideDiv.appendChild(sideDivInner);
const lineMetricsDiv = outerDocument.createElement('div');
lineMetricsDiv.id = 'linemetricsdiv';
// Measurement-only node: holds a single "x" so the renderer can read
// its computed line height. Without aria-hidden, AT exposes the stray
// glyph as a "text leaf" sandwiched between the editor iframe and the
// chat button — see ether/etherpad#7255 (comment "Ether X" announcement).
lineMetricsDiv.setAttribute('aria-hidden', 'true');
lineMetricsDiv.appendChild(outerDocument.createTextNode('x'));
outerDocument.body.appendChild(lineMetricsDiv);

View file

@ -155,6 +155,24 @@ exports.padeditbar = new class {
});
});
// Lighthouse / axe-core's `listitem` rule fires on every <li> child of an
// element whose role isn't `list` — and role="toolbar" on our <ul>s
// overrides the implicit list role. Core's toolbar.ts emits its <li>s
// with role="presentation" already; plugins ship their own editbarButtons
// EJS templates (ep_headings2, ep_align, ep_font_*, ep_print, ...) that
// emit <li> directly without the role, which kept the Lighthouse error
// alive. Sweep here once the toolbar DOM is fully built — runs before
// postToolbarInit hooks so anything plugins add even later still has the
// role attribute added to it the moment it lands (covered by the same
// sweep on the next init() call). role="presentation" only affects the
// accessibility tree, so CSS / JS selectors keep working. #7255.
$('#editbar .menu_left > li, #editbar .menu_right > li, #history-controls > li')
.filter((_i, el) => !el.hasAttribute('role'))
.attr('role', 'presentation');
$('#editbar .menu_left > li > a, #editbar .menu_right > li > a, #history-controls > li > a')
.filter((_i, el) => !el.hasAttribute('role'))
.attr('role', 'presentation');
$('body:not(#editorcontainerbox)').on('keydown', (evt) => {
this._bodyKeyEvent(evt);
});

View file

@ -354,7 +354,25 @@ const paduserlist = (() => {
self.setMyUserInfo(myInitialUserInfo);
if ($('#online_count').length === 0) {
$('#editbar [data-key=showusers] > a').append('<span id="online_count">1</span>');
// role="status" + aria-live="polite" announces the count when it
// changes; the localized aria-label (set by updateNumberOfOnlineUsers
// below) turns the bare badge digit into "N connected users" so AT
// users get context, not a stray "5". See ether/etherpad#7255.
$('#editbar [data-key=showusers] > a').append(
'<span id="online_count" role="status" aria-live="polite">1</span>');
}
// Set the initial aria-label. updateNumberOfOnlineUsers otherwise only
// fires on userJoin / userLeave / status change — on a solo-author pad
// those events never come, and the badge would ship to AT with no
// accessible name (the regression Playwright caught on PR #7777).
self.updateNumberOfOnlineUsers();
// Re-localize on runtime language changes. updateNumberOfOnlineUsers
// reads html10n.get, which returns undefined before the bundle loads
// and returns translated text afterwards; binding here keeps the
// accessible name in sync with applyLanguage(). Same pattern used
// by the keyboard hint in ace.ts and the history toolbar labels.
if (html10n && typeof (html10n as any).bind === 'function') {
(html10n as any).bind('localized', () => self.updateNumberOfOnlineUsers());
}
$('#otheruserstable tr').remove();
@ -547,7 +565,15 @@ const paduserlist = (() => {
localStorage.setItem('recentPads', JSON.stringify(recentPadsList));
}
$('#online_count').text(online);
// Set both visible text (the badge digit) and the accessible name in
// one place so they can't drift. html10n.get returns undefined if the
// locale bundle hasn't loaded yet — fall back to an English template
// so AT never reads back "undefined".
const $count = $('#online_count');
$count.text(online);
const label = html10n.get('pad.userlist.onlineCount', {count: online})
|| `${online} connected user${online === 1 ? '' : 's'}`;
$count.attr('aria-label', label);
return online;
},

View file

@ -80,10 +80,27 @@
<!----------------------------->
<!--------- TOOLBAR ----------->
<!----------------------------->
<!-- Visually-hidden labels for the role=toolbar regions. Sitting outside the
<ul>s keeps the toolbar markup valid (HTML disallows non-<li>/<script>
children inside <ul>) while still giving each toolbar a translated
accessible name via aria-labelledby. See ether/etherpad#7255. -->
<span id="editbar-formatting-label" class="sr-only"
data-l10n-id="pad.editor.toolbar.formatting">Formatting toolbar</span>
<!-- Reuses pad.historyMode.controlsLabel rather than minting a new key:
pad_mode.ts already set this as aria-label on #history-controls via JS,
and the existing key has translations in nl/de/etc. A new English-only
key would cause non-English locales to regress to fallback English. -->
<span id="editbar-history-label" class="sr-only"
data-l10n-id="pad.historyMode.controlsLabel">Pad history controls</span>
<span id="editbar-actions-label" class="sr-only"
data-l10n-id="pad.editor.toolbar.actions">Pad actions toolbar</span>
<span id="editbar-showmore-label" class="sr-only"
data-l10n-id="pad.editor.toolbar.showMore">Show more toolbar buttons</span>
<div id="editbar" class="toolbar">
<div id="toolbar-overlay"></div>
<ul class="menu_left" role="toolbar">
<ul class="menu_left" role="toolbar" aria-labelledby="editbar-formatting-label">
<% e.begin_block("editbarMenuLeft"); %>
<%- toolbar.menu(settings.toolbar.left, isReadOnly, 'left', 'pad') %>
<% e.end_block(); %>
@ -100,7 +117,8 @@
translation keys. We deliberately do NOT put data-l10n-id on
the icon buttons — that would set their textContent, which on
a buttonicon glyph would draw the localized string on screen. -->
<div id="history-controls" class="history-controls" role="toolbar" hidden>
<div id="history-controls" class="history-controls" role="toolbar"
aria-labelledby="editbar-history-label" hidden>
<button type="button" id="history-playpause" class="buttonicon buttonicon-play"
aria-pressed="false"></button>
<button type="button" id="history-leftstep" class="buttonicon buttonicon-step-backward">
@ -137,12 +155,14 @@
<option value="1000" data-l10n-id="timeslider.settings.playbackSpeed.1000ms">1000 ms</option>
</select>
</div>
<ul class="menu_right" role="toolbar">
<ul class="menu_right" role="toolbar" aria-labelledby="editbar-actions-label">
<% e.begin_block("editbarMenuRight"); %>
<%- toolbar.menu(settings.toolbar.right, isReadOnly, 'right', 'pad') %>
<% e.end_block(); %>
</ul>
<button type="button" class="show-more-icon-btn" aria-label="Show more toolbar buttons" aria-expanded="false"></button> <!-- use on small screen to display hidden toolbar buttons -->
<button type="button" class="show-more-icon-btn"
aria-labelledby="editbar-showmore-label"
aria-expanded="false"></button> <!-- use on small screen to display hidden toolbar buttons -->
</div>
<% e.begin_block("afterEditbar"); %><% e.end_block(); %>

View file

@ -139,14 +139,97 @@ test('otherusers region has aria-live and aria-label (no aria-role typo)', async
expect(ariaRole).toBeNull();
});
test('show-more toolbar button has aria-label and aria-expanded', async ({page}) => {
test('show-more toolbar button has an accessible name and aria-expanded', async ({page}) => {
const btn = page.locator('.show-more-icon-btn');
const tag = await btn.evaluate((el) => el.tagName.toLowerCase());
expect(tag).toBe('button');
await expect(btn).toHaveAttribute('aria-label', 'Show more toolbar buttons');
// The accessible name is supplied by aria-labelledby pointing at a hidden
// localized span (so html10n can translate it). Verify the relationship
// resolves and produces the expected English string with locale=en-US.
await expect(btn).toHaveAttribute('aria-labelledby', 'editbar-showmore-label');
await expect(page.locator('#editbar-showmore-label')).toHaveText('Show more toolbar buttons');
await expect(btn).toHaveAttribute('aria-expanded', 'false');
});
test('editbar toolbars have role=toolbar with accessible names (#7255)', async ({page}) => {
// Lighthouse + AT tooling (firefox a11y inspector) flagged both <ul> toolbars
// as unnamed in the 2026-05-16 follow-up on #7255. Each toolbar role now
// points to a hidden localized span via aria-labelledby; if either span is
// ever removed, getAttribute returns an id with no matching element and the
// toolbar becomes unnamed again — so assert the resolved string, not just
// the attribute wiring.
const cases: Array<[string, string, string]> = [
['.menu_left', 'editbar-formatting-label', 'Formatting toolbar'],
['.menu_right', 'editbar-actions-label', 'Pad actions toolbar'],
// History toolbar reuses pad.historyMode.controlsLabel (already
// translated in multiple locales) instead of a new English-only key.
['#history-controls', 'editbar-history-label', 'Pad history controls'],
];
for (const [sel, labelId, expected] of cases) {
const toolbar = page.locator(sel);
await expect(toolbar).toHaveAttribute('role', 'toolbar');
await expect(toolbar).toHaveAttribute('aria-labelledby', labelId);
await expect(page.locator(`#${labelId}`)).toHaveText(expected);
}
});
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');
}
// Core's toolbar.ts emits items as <li><a><button>...</button></a></li>;
// for those, the wrapping <a> is presentational so AT focus lands on the
// <button>, not the empty link. Plugins may emit anchors with their own
// role (e.g. ep_subscript_and_superscript renders <a role="button">), so
// scope this assertion to core's button-wrappers only — `:has(> button)`
// matches the <a> that contain a <button> child, which is what core emits.
const anchors = page.locator(
'.menu_left > li:not(.separator) > a:has(> button), ' +
'.menu_right > li:not(.separator) > a:has(> button)');
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('online_count badge has a localized accessible label (#7255)', async ({page}) => {
// The user-count badge in the showusers toolbar button used to expose a
// bare digit ("5") to AT, with no clue it was a user count. Now the badge
// carries an aria-label generated from pad.userlist.onlineCount that
// updates whenever the count changes. role=status + aria-live=polite
// means AT announces the change without the user having to refocus.
const badge = page.locator('#online_count');
await expect(badge).toHaveAttribute('role', 'status');
await expect(badge).toHaveAttribute('aria-live', 'polite');
// toHaveText / toHaveAttribute poll so the assertions survive the
// initial userlist init() pass (which appends the span and then sets
// its aria-label asynchronously after html10n + setMyUserInfo land).
await expect(badge).toHaveText(/^\d+$/);
// English plural form contains "connected user" — covers both singular
// and plural without baking the exact count into the test.
await expect(badge).toHaveAttribute('aria-label', /connected user/);
});
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
// ace.ts's linemetricsdiv. linemetricsdiv is a measurement-only node — it
// holds a single "x" so the renderer can read its computed line height —
// and must stay out of the AT tree.
const outerFrame = page.frameLocator('iframe[name="ace_outer"]');
await expect(outerFrame.locator('#linemetricsdiv')).toHaveAttribute('aria-hidden', 'true');
});
test('skip-to-content link bypasses toolbar (WCAG 2.4.1, #7255)', async ({page}) => {
const skip = page.locator('#skip-to-content');
// It exists in the DOM and is hidden from sighted users by default —