Skip to content

Add screen-reader integration test#21205

Open
calixteman wants to merge 1 commit intomozilla:masterfrom
calixteman:screenreader_test
Open

Add screen-reader integration test#21205
calixteman wants to merge 1 commit intomozilla:masterfrom
calixteman:screenreader_test

Conversation

@calixteman
Copy link
Copy Markdown
Contributor

@calixteman calixteman commented Apr 30, 2026

Introduce a new test/screenreader/ suite that drives a real screen reader (NVDA on Windows, VoiceOver on macOS) via @guidepup/guidepup, and use it to cover bug 2034568.

@calixteman calixteman added test accessibility infra Infrastructure related labels Apr 30, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 30, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.97%. Comparing base (1ebaa03) to head (642c763).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #21205      +/-   ##
==========================================
- Coverage   55.98%   55.97%   -0.02%     
==========================================
  Files         218      218              
  Lines       59059    59059              
==========================================
- Hits        33066    33060       -6     
- Misses      25993    25999       +6
Flag Coverage Δ
fonttest 8.66% <ø> (ø)
unittest 55.22% <ø> (-0.02%) ⬇️
unittestcli 55.72% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@calixteman
Add screen-reader integration test
642c763 
Introduce a new `test/screenreader/` suite that drives a real screen
reader (NVDA on Windows, VoiceOver on macOS) via `@guidepup/guidepup`,
and use it to cover bug 2034568.
timvandermeij
timvandermeij approved these changes Apr 30, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@timvandermeij timvandermeij left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me, with the merge conflict fixed and the comments addressed. It's nice that we have this now given the importance of software accessibility, so being able to continuously run tests against the real screenreaders is quite helpful. Nice work!

Comment thread test/screenreader/screenreader_spec.mjs
beforeEach(async () => {
pages = await loadAndWait(
"tracemonkey.pdf",
"#viewsManagerToggleButton"
Copy link
Copy Markdown
Contributor

@timvandermeij timvandermeij Apr 30, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

It might work in this case, but given 4021d57 it might be safer to await the first page rendering here too (using .page[data-page-number = "1"] .endOfContent) to make sure that the thumbnails are in the right state before we proceed.

Comment thread test/screenreader/jasmine-boot.js
// cache: `describe`/`it`/etc. are resolved off `globalThis` inside
// `register`, against whichever Jasmine instance was just installed.
jasmine.loadConfig({
random: false,
Copy link
Copy Markdown
Contributor

@timvandermeij timvandermeij Apr 30, 2026

Choose a reason for hiding this comment

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

We should set this to true to prevent getting tests that accidentally depend on each other (as tests must always be isolated from each other).

Comment thread .github/workflows/screenreader_tests.yml
name: ${{ matrix.os }} / ${{ matrix.browser }}

# Real screen-reader automation only works on Windows (NVDA) and macOS
# (VoiceOver), so Linux is intentionally absent from the matrix.
Copy link
Copy Markdown
Contributor

@timvandermeij timvandermeij Apr 30, 2026

Choose a reason for hiding this comment

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

We could note there that there is already a feature request at guidepup/guidepup#107 to support a Linux screenreader too, so linking that there could be useful as a reference for later in case it ends up being implemented so we can adjust the workflow.

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

Reviewers

@timvandermeij timvandermeij timvandermeij approved these changes

Assignees

No one assigned

Labels

accessibility infra Infrastructure related test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@calixteman @codecov-commenter @timvandermeij