Skip to content

Add automated accessibility checks (axe-core + keyboard) for the demo pages - #773

Merged
joedolson merged 9 commits into
ableplayer:developfrom
blogcastAI:upstream/a11y-gate
Aug 25, 2026
Merged

joedolson merged 9 commits into
ableplayer:developfrom
blogcastAI:upstream/a11y-gate

Conversation

@blogcastAI

@blogcastAI blogcastAI commented Aug 4, 2026

Copy link
Copy Markdown

Adds an automated accessibility regression check for the demo pages: axe-core WCAG 2.1 A/AA scans plus keyboard operability tests, run in CI at four viewports.

Stacked on #771 (same as #771 stacks on #770): this branch includes the CI workflow + headless-test commits, and this PR's diff will collapse to just the accessibility additions once #771 merges. If you'd rather see it standalone, I can rebase after #770/#771 land.

What it runs

  • axe-core WCAG 2.1 A/AA scans (@axe-core/playwright) over a representative demo set — index, video1 (captions), video5 (sign language + descriptions), audio1, audio3 (interactive transcript), desc1 (audio description), youtube1 (page shell). The cross-origin YouTube iframe subtree is excluded: axe cannot audit third-party documents.
  • Keyboard operability checks on the initialized player: reachable by Tab, play/pause operable with Enter (asserted against the media element's paused state plus the accessible-name flip), and a visible focus indicator on the focused control.
  • Every spec runs at four viewports — 320×568, 768×1024, 1280×800, 3840×2160 — because reflow failures only appear at 320px and density/spacing issues only at large sizes. A violation at any size fails.
  • Scans wait for the initialized .able-wrapper / .able-controller, so the audited DOM is the real player UI, not the pre-init <video> element.

What it found

Calibration before opening this PR: zero axe violations and all keyboard checks passing on all seven pages at all four viewports (40/40). So the job lands green as a normal blocking check from day one — no pre-existing-violation baseline was needed. (Verified both locally and in Actions on my fork: blogcastAI#6.)

What it does NOT claim

Automated checks cover only a fraction of WCAG success criteria. A green run means "no violations axe can detect on these seven pages" — it does not make Able Player or its demos accessible or conformant, and manual assistive-technology testing remains essential. Pages outside the representative set are not yet scanned; the set is easy to extend one line at a time in e2e/a11y.spec.js, with the honest expectation that new pages may surface violations to triage.

Files

  • .github/workflows/ci.yml — new a11y job (build → playwright install chromiumnpm run test:a11y, Playwright report uploaded as an artifact on failure)
  • playwright.config.js — the four viewport projects + webServer wiring
  • e2e/serve.mjs — dependency-free static server for the repo root, so the demos' relative ../build and ../media references resolve exactly as deployed (single-range support for media requests)
  • e2e/a11y.spec.js, e2e/keyboard.spec.js — the specs
  • package.json — devDependencies @playwright/test + @axe-core/playwright; script test:a11y
  • No build/ artifacts committed. The only player-source files in the diff arrive with the
    stacked Fix all 7 ESLint errors reported by npm run lint #770 lint commit; this PR adds no player-source changes of its own.

Why Playwright alongside puppeteer

The repo already has puppeteer for the Jest browser project, and that stays. Playwright is here
for what this suite needs and puppeteer would have to grow by hand: viewport projects (the same
spec re-run at four sizes), the maintained @axe-core/playwright integration, and trace/HTML
report artifacts on failure. The two are scoped to separate jobs and separate commands
(npm test vs npm run test:a11y), so neither affects the other.

Known limitation

On develop the YouTube demo's embed does not initialize — initSignLanguage throws when the
media element has no <source> children, which is the case for YouTube-only embeds — so that
entry currently scans the surrounding page shell rather than a live YouTube player. I will
report the crash separately rather than fold an unrelated fix into this PR.

- ableplayer-base.js: the descriptionsAudible else-if duplicated the first
  condition (no-dupe-else-if), so the documented singular spelling
  data-description-audible was never honored. Read the singular attribute
  (options.descriptionAudible ?? data.descriptionAudible) in the second
  branch, matching the existing comment's intent.
- dialog.js: import AblePlayer for the AblePlayer.getActiveDOMElement()
  call (no-undef).
- dragdrop.js: remove unused thisObj locals in handleWindowButtonClick and
  handleMenuChoice (no-unused-vars); document the intentionally-empty
  Space/Enter branch (no-empty).
- preference.js: remove the write-only $thisLabel variable and a dead
  $thisField assignment (no-unused-vars, no-useless-assignment).

npm run lint: 0 problems. jsdom test suites: 55/55 pass. Build artifacts
intentionally omitted per contributing.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@joedolson

Copy link
Copy Markdown
Member

I'm certainly in favor of adding some regression testing. It is always possible to introduce breakage in a complex system, so this is a good way of at least ensuring some minimal baseline is stable.

Might take a little while to properly review, though. ;)

blogcastAI and others added 8 commits August 9, 2026 21:57
Per review: the singular `data-description-audible` alternate code
path was added in the feature's original commit as protection against
misspelling the attribute, but was never documented and never the
standard form. Eliminate the alternate condition rather than keep a
lint-clean version of it; `data-descriptions-audible` (plural) is the
one supported spelling.
The repository ships Jest and ESLint configs but has no CI, so
regressions surface only when someone runs the tools locally. This
workflow runs three jobs on every push and pull request:

- ESLint over scripts/ (npm run lint)
- the jsdom Jest project (55 tests; the puppeteer project needs a
  localhost:8000 demo server and a headed browser, so it is excluded
  for now)
- a Grunt + Rollup build with an artifact existence check, without
  committing build output (per contributing.md)

Node 22 with npm cache; read-only GITHUB_TOKEN permissions.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
validate.test.cjs previously required a manually started server on
localhost:8000 and a headed browser (headless: false), so npm test
always failed 26 tests out of the box and the suite could not run in CI.

- jest-puppeteer.config.cjs: headless by default; HEADFUL=1 restores a
  visible browser for debugging; pass --no-sandbox in CI containers.
- validate.test.cjs: navigate to an intercepted http://ableplayer.test/
  origin fulfilled from memory instead of localhost:8000. A real http(s)
  origin is still required because isProtocolSafe() resolves relative
  URLs against window.location.origin (opaque on about:blank), but no
  server process is needed.
- ci.yml: new test-browser job runs the puppeteer project (after
  npm run build, which produces build/test/validate.umd.js).

npm test now passes 81/81 locally with zero setup. Build artifacts
intentionally omitted per contributing.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- The jsdom project's webvtt.test.cjs loads build/test/webvtt.umd.js, so
  without a build step the job exercised the committed bundle instead of
  the source under review. Build first, matching the puppeteer job.
- Cancel superseded runs on the same ref so a force-push to an open pull
  request does not leave stale jobs queued.
- Give every job an explicit timeout; the default is six hours, which a
  hung headless page load would otherwise occupy.
- jest-puppeteer: compare HEADFUL explicitly so HEADFUL=0 stays headless
  instead of launching a visible browser (any non-empty string is truthy).
…esting docs

- Only the puppeteer job launches a browser, but puppeteer's postinstall
  downloads a browser during npm ci in every job, and setup-node's npm
  cache does not cover that directory. Set PUPPETEER_SKIP_DOWNLOAD on the
  lint, jsdom and build jobs.
- contributing.md described adjusting the test runner's target URL and a
  default of http://localhost:8000. The suite now intercepts its own
  navigation and needs no server, so document that, and mention HEADFUL=1
  for watching the browser while debugging.
… pages

Adds a CI job that runs @axe-core/playwright WCAG 2.1 A/AA scans over a
representative demo set (video, audio, transcript, sign language,
described video, YouTube shell) plus keyboard operability checks (player
reachable by Tab, play/pause operable with Enter, visible focus
indicator), each at four viewports: 320x568, 768x1024, 1280x800,
3840x2160.

- e2e/serve.mjs: dependency-free static server for the repo root so the
  demos' relative ../build and ../media references resolve as deployed;
  supports single-range requests for media loading.
- playwright.config.js: four viewport projects; boots the server via
  webServer; scans wait for the initialized .able-wrapper so the audited
  DOM is the real player UI.
- Third-party iframes (YouTube) are excluded from the scan subtree --
  cross-origin content cannot be audited from the host page.
- New devDependencies: @playwright/test, @axe-core/playwright.
- npm run test:a11y runs the suite locally (npm run build first).

Automated checks cover only a fraction of WCAG success criteria; a green
run blocks a class of regressions and is not a conformance claim.
- e2e/serve.mjs binds 127.0.0.1 instead of 0.0.0.0, so a local run never
  exposes the repository to the local network.
- The root guard compares with a trailing separator, so a sibling
  directory cannot satisfy the prefix check.
- A suffix range longer than the file clamps to offset 0 rather than
  producing a negative start.
- keyboard.spec.js: describe the transparent-outline check accurately —
  Able Player's :focus rule paints a solid colour, so the check guards
  against a theme overriding it, rather than describing current styling.
- package-lock.json: restore the libc metadata on the pre-existing
  @rollup/rollup-linux-* entries, which a lockfile regeneration had
  dropped; the only intended change is the two new devDependencies.
- The YouTube/Vimeo exclusion is a policy choice, not a technical limit:
  axe can inject into frames, but provider-owned violations are not
  actionable here. Note the trade-off it carries — excluding the element
  also drops frame-title on the embed, which is ours.
- Record that the YouTube demo's embed does not currently initialize on
  develop (initSignLanguage throws when the media element has no <source>
  children), so that entry scans the page shell today.
- Drop the claim that the 320px project catches WCAG 1.4.10 reflow: axe
  has no reflow rule. The value of the four projects is running every
  other rule against each layout.
blogcastAI added a commit to blogcastAI/ableplayer that referenced this pull request Aug 10, 2026
…#773 review fixes into develop

# Conflicts:
#	.github/workflows/ci.yml
#	e2e/a11y.spec.js
#	e2e/keyboard.spec.js
#	e2e/serve.mjs
#	jest-puppeteer.config.cjs
#	playwright.config.js
@joedolson
joedolson merged commit e33d502 into ableplayer:develop Aug 25, 2026
5 checks passed
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.

2 participants