Add automated accessibility checks (axe-core + keyboard) for the demo pages - #773
Merged
Merged
Conversation
- 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>
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. ;) |
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
force-pushed
the
upstream/a11y-gate
branch
from
August 10, 2026 04:21
53ab387 to
59335b3
Compare
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
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
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.
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/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.pausedstate plus the accessible-name flip), and a visible focus indicator on the focused control..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— newa11yjob (build →playwright install chromium→npm run test:a11y, Playwright report uploaded as an artifact on failure)playwright.config.js— the four viewport projects + webServer wiringe2e/serve.mjs— dependency-free static server for the repo root, so the demos' relative../buildand../mediareferences resolve exactly as deployed (single-range support for media requests)e2e/a11y.spec.js,e2e/keyboard.spec.js— the specspackage.json— devDependencies@playwright/test+@axe-core/playwright; scripttest:a11ybuild/artifacts committed. The only player-source files in the diff arrive with thestacked 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/playwrightintegration, and trace/HTMLreport artifacts on failure. The two are scoped to separate jobs and separate commands
(
npm testvsnpm run test:a11y), so neither affects the other.Known limitation
On develop the YouTube demo's embed does not initialize —
initSignLanguagethrows when themedia element has no
<source>children, which is the case for YouTube-only embeds — so thatentry 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.