Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Option to timeout if target is missing accessibility name #10562

Closed
ckundo opened this issue Nov 28, 2021 · 6 comments
Closed

[Feature] Option to timeout if target is missing accessibility name #10562

ckundo opened this issue Nov 28, 2021 · 6 comments

Comments

@ckundo
Copy link

ckundo commented Nov 28, 2021

Summary

I'd like an option to extend actionability checks to check if the target has an accessible name, so that my tests will time out if they are not accessible to assistive technology.

Background

Actionability checks assert that a number of conditions are true about an element before attempting to fire interactions. For example, if the element is disabled or obscured, the test will timeout on actionability checks. I propose and optional actionability check to assert that the control has an accessible name (and potentially also interactive role), so that test will fail if the control is not accessible to screen readers and other assistive technology.

Accessible name and role are synthesized by the browser into an accessibility tree, and normally (with the exception of experimental AOM in Chrome) aren't available to JS. However, there's work to recreate the browser accessible name and role computation in JS, e.g. https://github.com/eps1lon/dom-accessibility-api, that could be used here.

Example

Current Implementation

const browser = await firefox.launch();
const page = await browser.newPage();
await page.setContent(`<div><input placeholder='First Name' /></div>`); // no name exposed to a11y (missing a label)
await page.fill('input', 'hello'); // works

With this proposed feature enabled, this example would timeout on the last line:

const browser = await firefox.launch();
const page = await browser.newPage({ actionability: { a11y: true }); // turn on the actionability step
await page.setContent(`<div><input placeholder='First Name' /></div>`); // no name exposed to a11y (missing a label)
await page.fill('input', 'hello'); // Times out

I'm proposing extending the actionability checks to assert on the accessible name and roles of the element using something like https://github.com/eps1lon/dom-accessibility-api, so that the above example would timeout, prompting me to update the markup since it doesn't work for screen reader users, dictation technology, and so on:

const browser = await firefox.launch();
const page = await browser.newPage({ actionability: { a11y: true });
await page.setContent(`<div><label>First Name <input placeholder='John Doe' /></label></div>`); // fixed! has an accessible name "First Name"
await page.fill('input', 'hello'); // works

Or using force:

const browser = await firefox.launch();
const page = await browser.newPage({ actionability: { a11y: true }); // turn on the actionability step
await page.setContent(`<div><input placeholder='First Name' /></div>`); // no name exposed to a11y (missing a label)
await page.fill('input', 'hello', { force: true }); // works

Why?

Enabling this feature would allow me to quickly and easily surface basic but critical accessibility failures in my existing suite, without needing to load external libraries or helpers. In fact, I wouldn't change my test examples at all. It would instead require a page or browser level configuration option to enable accessibility in actionability checks. This low touch approach would be a big win for massive codebases trying to shake out accessibility issues in their examples.

Approach

  1. Add an option a11y: boolean; to FrameClickOptions
  2. When the option is passed in as true from page.click('#my-selector', { a11y: true }), run an additional check in the timeout checks.
  3. Add support for global configuration option to default a11y: true, to make it unnecessary to change the test files at all to get the expected timeouts.

What do y'all think?

@ckundo ckundo changed the title Timeout if interactive target is missing accessibility name Option to timeout if interactive target is missing accessibility name Nov 28, 2021
@ckundo ckundo changed the title Option to timeout if interactive target is missing accessibility name [Feature] Option to timeout if interactive target is missing accessibility name Nov 28, 2021
@ckundo ckundo changed the title [Feature] Option to timeout if interactive target is missing accessibility name [Feature] Option to timeout if target is missing accessibility name Nov 28, 2021
@dgozman
Copy link
Contributor

dgozman commented Dec 6, 2021

@JoelEinbinder Could you please review the proposal?

@ckundo
Copy link
Author

ckundo commented Dec 29, 2021

Opened a PoC PR to illustrate an approach, above.

@dgozman
Copy link
Contributor

dgozman commented Jan 11, 2022

@ckundo Thank you for the draft! While I think that this is a good feature, we still need to think about the API shape. Your PR draft makes the check it mandatory, which I assume is not the final suggestion, because that would immediately break many existing users.

a11y: boolean option sounds good, but I'd like us to see more checks if we go for such a generic option. More general, I think it makes sense to include a11y features if we cover a wide range of checks with them. Otherwise, for any real a11y checking you'll have to run some audits anyway, for example axe, and some small helpers in Playwright are not of much use.

Another approach would be to use a special selector that verifies these properties just because you use it. See #11182 for example.

@junyper
Copy link

junyper commented Jan 11, 2022

@dgozman I disagree that this is not of much use. Not providing an accessible name and role for interactive elements is one of the top issues I see in accessibility audits everywhere. It makes a ton of sense to have this baked in to a testing framework so that whenever your tests programmatically interact with an element it at least has a name and role, just like we check for visibility. If it doesn't have an accessible name and role it is "invisible" to screen reader users.

Yes, we need more a11y validations on top of this, but I think it will be harder to have that type of thing built into the framework, because a lot of them will require context specific assertions.

@ckundo
Copy link
Author

ckundo commented Jan 12, 2022

@dgozman the PR is a proof of concept not the final approach. It illustrates the utility I think, in fact capturing what would be accessibility errors in the test cases. The name assertion maps to WCAG 2.1 4.1.2, which accounts for a broad swath of accessibility issues, including many that are canonized in Axe (21 Axe rules relate to 4.1.2). This is a surgical implementation that would give disproportionate utility. Axe is simple too slow and heavy to execute in the core library (I've benchmarked Axe in CircleCI and it takes 90ms to execute on a very simple template, with a payload size of over 500kb. The approach outlined here is more lightweight so that it can live in the core library.

@pavelfeldman
Copy link
Member

Why was this issue closed?

We are prioritizing the features based on the upvotes, recency and activity in the issue thread. It looks like this issue only has a handful of upvotes, has not been touched recently and/or we lack sufficient feedback to act on it. We are closing issues like this one to keep our bug database maintainable. Please feel free to open a new issue and link this one to it if you think this is a mistake.

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

Successfully merging a pull request may close this issue.

5 participants