Skip to content

Give formatting buttons fallback accessible names - #115

Open
Ehsan-hash wants to merge 4 commits into
github:mainfrom
Ehsan-hash:fix/formatting-button-names
Open

Ehsan-hash wants to merge 4 commits into
github:mainfrom
Ehsan-hash:fix/formatting-button-names

Conversation

@Ehsan-hash

@Ehsan-hash Ehsan-hash commented Oct 15, 2026 •

Copy link
Copy Markdown

Icon-only built-in formatting buttons can expose an empty accessible name. Give unnamed built-in buttons a descriptive default title, while preserving supplied text, image alternative text, ARIA names and explicit titles, including empty titles.

The initial-name check ignores hidden decoration and resolves label references in the button's document or shadow root. When the document is still loading, it waits for parser-created children before deciding whether to add a title. This prevents an English tooltip from being added before localized text arrives. The README explains localization, suppression and the connection-time limit. Custom data-md-button controls are unchanged.

Validation: lint, TypeScript build and all 156 ChromeHeadless tests pass. Regressions fail on the original code and the corresponding pre-correction versions. Actual Chromium checks cover 18 name/title/description cases, hidden controls before and after reveal, four parser-created cases and Enter formatting for named and unnamed buttons. Fresh independent review covers the complete current contribution. OS screen-reader behavior was not tested.

Fixes #43.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The library-wide tooltip and accessible-description changes need maintainer accessibility sign-off.

1 open finding
What changed in this PR

Adds fallback accessible names for built-in Markdown toolbar buttons to address #43.

Changes:

  • Adds default titles for all 13 built-in controls while preserving explicit titles, including empty ones.
  • Tests default titles and preservation behavior.
  • Documents English tooltips, localization, and suppression.
File Description
test/​test.js Adds default-title and title-preservation regression tests.
src/​index.ts Sets built-in button titles when absent.
README.md Explains fallback naming and tooltip customization.

🧠 Review effort: Max


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread README.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The naming checks can still leave built-in buttons without accessible names.

3 open findings
1 resolved since last review

🧠 Review effort: Max

Comment thread src/index.ts Outdated
const hasAccessibleName = [
// innerText excludes hidden descendants from rendered button text.
// eslint-disable-next-line github/no-innerText
this.innerText,
Comment thread src/index.ts Outdated
Comment on lines +152 to +158
const hasAccessibleName = [
// innerText excludes hidden descendants from rendered button text.
// eslint-disable-next-line github/no-innerText
this.innerText,
this.getAttribute('aria-label'),
this.getAttribute('aria-labelledby')
].some(value => value?.trim())
Comment thread src/index.ts Outdated
Comment on lines +152 to +162
const hasAccessibleName = [
// innerText excludes hidden descendants from rendered button text.
// eslint-disable-next-line github/no-innerText
this.innerText,
this.getAttribute('aria-label'),
this.getAttribute('aria-labelledby')
].some(value => value?.trim())
if (!this.hasAttribute('title') && !hasAccessibleName) {
const title = defaultTitles[this.localName]
if (title) this.setAttribute('title', title)
}
@Ehsan-hash

Copy link
Copy Markdown
Author

Addressed the three naming and parser-timing findings. Hidden decoration and unresolved label references now retain the default fallback, while image alternative text and valid labels suppress the English tooltip. Parser-created buttons wait for their children, and hidden buttons retain their supplied names when revealed. Explicit titles, including empty titles, are preserved.

All 156 ChromeHeadless tests, lint and TypeScript build pass. Chromium accessibility-tree checks, real parser cases, keyboard formatting and independent review also pass. These are browser checks, not a manual screen-reader test.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Buttons must have discernible text

3 participants