Skip to content

fix(a11y): expose plain pad lines as paragraphs to assistive tech (#7778) - #8240

Open
JohnMcLear wants to merge 2 commits into
developfrom
fix/7778-a11y-read-view
Open

JohnMcLear wants to merge 2 commits into
developfrom
fix/7778-a11y-read-view

Conversation

@JohnMcLear

@JohnMcLear JohnMcLear commented Sep 17, 2026

Copy link
Copy Markdown
Member

Fixes #7778

Root cause

#7782 removed role="textbox" from innerdocbody, but @StrangeGirlMurph's VoiceOver re-test afterwards said line-by-line reading still doesn't work. The accessibility tree shows why: every pad line is a <div class="ace-line"> with no role, and browsers expose that as an anonymous generic node. Screen readers flatten the pad into one run of text, with no line boundaries to step between.

I dumped Chromium's accessibility tree for the ace_inner frame with CDP Accessibility.getFullAXTree on a 3-line pad with a link on line 2:

Before (develop):

generic "Pad content"
generic ""   generic ""   generic ""
StaticText "First line of text" / StaticText "Second line see " / link "https://etherpad.org" / ...

After:

generic "Pad content"
paragraph ""   paragraph ""   paragraph ""
StaticText "First line" / StaticText "See " / link "https://etherpad.org" / ...

Fix

domline.ts writeHTML now sets role="paragraph" on plain lines. A line skips the role only if its rendered markup contains an element with its own block semantics (p,ul,ol,li,dl,h1h6,pre,blockquote,table,figure,hr), so lists and ep_headings2 headings keep their native roles and a list never sits inside a paragraph. Inline or styling-only wrappers from plugin line-attribute hooks don't count, so those lines stay paragraphs. #7782 had already named this as the next step.

  • The tag doesn't change, so plugin selectors on div.ace-line are unaffected. This is not the AT mirror or a shadow DOM approach.
  • The role goes only on real DOM nodes (the optDocument path). Export and the non-DOM createDomLine path are unchanged.
  • No user-facing strings, so no i18n changes.

Tests

New Playwright test pad lines are exposed to AT as separate paragraphs with navigable links (#7778) in a11y_dialogs.spec.ts:

  • 3 typed lines show up as 3 paragraph roles with the right text.
  • The URL in line 2 is a link whose accessible name is its visible text and whose href is correct.
  • Turning line 3 into a bulleted list drops its role: 2 paragraphs are left and the listitem holds "Third line".

New backend jsdom test tests/backend/specs/domline_line_role.ts covers four cases: a plain line, a list line, a plugin <h1> wrapper and a plugin inline <span> wrapper. On develop 2 of the 4 fail. After the first commit the inline-wrapper case still failed; that was Qodo's finding. With the second commit all 4 pass.

Evidence (local, chromium):

  • Without the fix the new test fails: getByRole('paragraph') expected 3, received 0.
  • With the fix: the whole a11y_dialogs.spec.ts passes, including the Inaccessibility to screenreaders #7255/a11y: Provide an AT-only read view of pad content (line-by-line + link navigation) #7778 regressions. The new test also passes on firefox.
  • Full frontend-new chromium suite: all pass. The first run's failures came from specs that hardcode port 9001 while my server ran on a different port. With the port swapped temporarily, those 46 passed too.
  • Backend mocha: 1677 passing, 0 failing (includes domline_list_start.ts). Vitest: 840 passed.
  • tsc --noEmit: no new errors.

Still needs a real screen reader

The automated tree now has line boundaries. Only a VoiceOver/NVDA user can confirm that reading line by line and moving between links actually works inside the contenteditable iframe. @StrangeGirlMurph, could you re-test on pad-dev once this is deployed?

Related AT noise I found but left out of this PR: the editor iframe documents are titled Empty (from static/empty.html), and the frames carry the hardcoded titles Ether / pad. The Pad content aria-label is hardcoded English. A screen reader may read these out when it enters the editor.

🤖 Generated with Claude Code

https://claude.ai/code/session_012kA75NPq8nGRidAwhPXeCi

)

Each pad line renders as <div class="ace-line">, which browsers expose in
the accessibility tree as an anonymous `generic` node. Screen readers then
flatten the whole pad into a single run of text, so users can't step
through it line by line or reach links inside a given line.

Give plain lines role="paragraph" in domline's writeHTML. Lines wrapped in
block markup (lists, plugin headings via aceDomLine*ProcessLineAttributes)
keep their native semantics and get no role. No tag change, so plugin
selectors on div.ace-line are unaffected. This is the follow-up step
proposed in #7782.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012kA75NPq8nGRidAwhPXeCi
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Expose plain pad lines as accessible paragraphs

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Exposes plain editor lines as paragraphs so assistive technologies preserve line boundaries.
• Preserves native semantics for lists, headings, and other block-wrapped lines.
• Adds browser coverage for paragraph roles, navigable links, and list conversion.
Diagram

graph TD
  A["Line attributes"] --> B["DOM line writer"] --> C{"Block wrapper?"}
  C -- "No" --> D["Paragraph role"] --> F["Accessibility tree"]
  C -- "Yes" --> E["Native block role"] --> F
  G["A11y browser test"] -. "verifies" .-> D
  G -. "verifies" .-> E
Loading
High-Level Assessment

The conditional ARIA role in the existing DOM-line renderer is the most targeted approach. Changing ace-line elements to native paragraph tags risks editor and plugin compatibility, while maintaining a separate assistive-technology mirror would add synchronization complexity; preserving div.ace-line and deferring to native block semantics avoids both issues.

Files changed (2) +50 / -0

Bug fix (1) +14 / -0
domline.tsAssign paragraph semantics to plain rendered lines +14/-0

Assign paragraph semantics to plain rendered lines

• Detects lines wrapped by list, heading, or plugin block markup. Plain DOM lines receive role="paragraph", while wrapped lines have any stale role removed so their native semantics remain authoritative; non-DOM rendering paths remain unchanged.

src/static/js/domline.ts

Tests (1) +36 / -0
a11y_dialogs.spec.tsTest accessible line, link, and list semantics +36/-0

Test accessible line, link, and list semantics

• Adds a Playwright regression test asserting that three plain lines appear as separate paragraphs and that an embedded URL remains an accessible link. It also converts one line into a list and verifies removal of the paragraph role and preservation of list-item semantics.

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

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 17, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Plugin lines lose paragraph navigation ✓ Resolved 🐞 Bug ≡ Correctness
Description
writeHTML treats every nonempty preHtml or postHtml value as a semantic block wrapper and
consequently omits role="paragraph". When a line-attribute plugin surrounds content with
non-semantic inline or styling markup, that line is omitted from paragraph navigation even though no
heading or list semantics replace it.
Code

src/static/js/domline.ts[223]

+    const hasBlockWrapper = !!(nonEmpty && (preHtml || postHtml));
Evidence
The two line-attribute hooks concatenate plugin-provided wrapper strings without validating their
element type, and their documented contract only describes arbitrary strings added around the
displayed HTML. The new truthiness check therefore cannot establish that the wrapper supplies list,
heading, or other block semantics before removing the paragraph role.

src/static/js/domline.ts[89-135]
src/static/js/domline.ts[221-242]
doc/api/hooks_client-side.adoc[15-57]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`hasBlockWrapper` currently assumes any hook-provided `preHtml` or `postHtml` string supplies block semantics. The hook contract permits arbitrary wrapper HTML, so inline or styling wrappers can incorrectly suppress the line's paragraph role.
## Fix Focus Areas
- src/static/js/domline.ts[89-135]
- src/static/js/domline.ts[221-242]
- doc/api/hooks_client-side.adoc[15-57]
## Recommended Fix
Suppress `role="paragraph"` only when the rendered line has a known semantic block wrapper, such as a list, heading, preformatted block, or an explicitly declared semantic wrapper. Preserve the paragraph role for inline, styling-only, empty, or otherwise non-semantic hook wrappers, and add tests covering both semantic and inline plugin wrappers.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/static/js/domline.ts Outdated
)

Address Qodo review: any non-empty preHtml/postHtml from a line-attribute
hook was treated as a block wrapper, so plugins that wrap a line in
inline/styling markup silently dropped the line out of paragraph
navigation. Check the rendered node for elements that actually carry block
semantics (lists, headings, pre, blockquote, table, ...) instead.

Adds a jsdom backend test covering plain, list, plugin-heading and
plugin-inline-wrapper lines.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012kA75NPq8nGRidAwhPXeCi
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.

a11y: Provide an AT-only read view of pad content (line-by-line + link navigation)

1 participant