fix(a11y): clear stale Loading aria-label + name title controls#153
Open
JohnMcLear wants to merge 1 commit into
Open
fix(a11y): clear stale Loading aria-label + name title controls#153JohnMcLear wants to merge 1 commit into
JohnMcLear wants to merge 1 commit into
Conversation
Three accessibility regressions called out in the 2026-05-16 follow-up on ether/etherpad#7255 (the screen-reader-inspector screenshots in the issue comment): (1) The <a> wrapping the loading text carried data-l10n-id="pad.loading". After etherpad PR #7584, html10n auto-populates aria-label on any node with a data-l10n-id, leaving the anchor with `aria-label="Loading..." data-l10n-aria-label="true"`. The plugin's three title-swap sites (initial render, recieveTitleMessage, save click) only cleared data-l10n-id, so the anchor's aria-label stayed "Loading..." forever — AT users heard "Loading..." even after the pad title was set to e.g. "Test". Move data-l10n-id onto an inner <span> instead of the <a>, so html10n's auto-aria-label lands on the span (which is replaced wholesale on title swap) rather than persisting on the anchor. Centralise the three title-swap sites through a single `applyLiveTitle` helper that also defensively strips aria-label / data-l10n-aria-label off the anchor in case an older template version is in play. (2) The pencil-icon edit button was a <div> with no accessible name and no native focus / Enter / Space handling — AT users couldn't tell it was a button and couldn't activate it from the keyboard. Promote it to <button type="button"> and reset its native chrome in CSS so the glyph keeps centering the same way it did when it was a div. Same fix for the OK save button (already <button>, just adds aria-labelledby). (3) The text input had no associated label — screen readers announced just "edit text". Add aria-labelledby pointing at a translated visually-hidden span so the input's accessible name is "Pad title". The three new labels are visually hidden via a plugin-scoped .ep_set_title_sr_only class (clip-rect pattern). Adds en.json entries for the three labels and a regression Playwright spec that asserts both the aria-labelledby wiring and the absence of stale aria-label attributes after the title swap. Refs ether/etherpad#7255 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Merged
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three accessibility regressions called out in the 2026-05-16 follow-up on
ether/etherpad#7255 (the
screen-reader-inspector screenshots in the issue comment).
1. Stale
aria-label=\"Loading...\"on the title anchorThe
<a>wrapping the loading text carrieddata-l10n-id=\"pad.loading\". Afteretherpad PR #7584, html10n auto-populates
aria-labelon any node with adata-l10n-id, leaving the anchor witharia-label=\"Loading...\" data-l10n-aria-label=\"true\". The three title-swap sites (initial render,recieveTitleMessage, save click) only cleareddata-l10n-id, so thearia-labelstayed\"Loading...\"forever — AT users heard\"Loading...\"even after the pad title was set (e.g. to
\"Test\").This PR moves
data-l10n-idonto an inner<span>so html10n's auto-aria-labellands on the span (which gets replaced wholesale on title swap) rather than
persisting on the anchor. Centralises the three title-swap sites through a
single
applyLiveTitlehelper that also defensively stripsaria-label/data-l10n-aria-labeloff the anchor.2. Pencil-icon edit button had no accessible name
The edit button was a
<div>with no accessible name and no native focus /Enter / Space handling — AT users couldn't tell it was a button and couldn't
activate it from the keyboard. Promote it to
<button type=\"button\">andreset its native chrome in CSS so the glyph keeps centering the same way it did
when it was a div.
3. Title input had no associated label
The text input had no label — screen readers announced just
\"edit text\".Add
aria-labelledbypointing at a translated visually-hidden span so theinput's accessible name is
\"Pad title\".Same
aria-labelledbytreatment for the OK save button.The three new labels are visually hidden via a plugin-scoped
.ep_set_title_sr_onlyclass (clip-rect pattern).Test plan
pnpm run linttitle.spec.tsregression test still passes (#pad_title > #title > h1 > ahas "JohnMcLear" after save)#edit_titlehas role=button + aria-labelledby#input_titlehas aria-labelledbyaria-label,data-l10n-aria-label, ordata-l10n-id