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

WPT test fails if I enforce name prohibited #240

Open
aleventhal opened this issue Jun 27, 2024 · 19 comments
Open

WPT test fails if I enforce name prohibited #240

aleventhal opened this issue Jun 27, 2024 · 19 comments

Comments

@aleventhal
Copy link
Contributor

I'm working to update the Blink accessibility code to not expose a name on roles where the name is prohibited. There is a good reason for these rules, as screen readers cannot always cope with having text both in a label and in descendant content.
(Note: I want to experiment with moving the prohibited name to a description, and logging an a11y error in developer tools

However, this causes errors in external/wpt/accname/name/comp_label.html, which seem to assume that aria-label will produce an accname on these prohibited roles. Shouldn't the test be using valid ARIA?

---- Output of test ----

This is a testharness.js-based test.
Found 5 FAIL, 0 TIMEOUT, 0 NOTRUN.
[FAIL] label valid on div with associationlist role
assert_equals:

x
expected "label" but got ""
[FAIL] label valid on div with associationlistitemkey role
assert_equals:
x
expected "label" but got ""
[FAIL] label valid on div with associationlistitemvalue role
assert_equals:
x
expected "label" but got ""
[FAIL] label valid on dd element
assert_equals:
x
expected "label" but got ""
[FAIL] label valid on dt element
assert_equals:
x
expected "label" but got ""
Harness: the test ran to completion.


@aleventhal
Copy link
Contributor Author

Related: issue 241, which is for ACCNAME testable statements.

@aleventhal
Copy link
Contributor Author

This WPT test currently prevents Chrome from experimentally repairing prohibited names in content, e.g. by moving the name to the description of the element.

@accdc @cookiecrook @scottaohara WDYT?

@scottaohara
Copy link
Member

maybe any tests for prohibited name elements/roles should be tentative? i mean, one of the reasons naming is prohibited is based on the fact that the support was spotty / the fact that screen readers behaved differently even if the element did expose a name.

but for these elements/roles specifically, these roles are in flux / not actually in the aria spec. so maybe these specific tests need to be rethought?

@cookiecrook
Copy link
Contributor

cookiecrook commented Jul 1, 2024

If I understand correctly, @aleventhal is suggesting that the current label test is invalid because it prevents adding prohibited name tests... If that's correct Aaron, then I support removing specific label tests for elements or roles with name prohibited. If so, this issue should be moved to web-platform-tests/interop-accessibility, rather than w3c/accname @spectranaut or @jnurthen may be able to move it.

Update: Looking more closely, the author error misuse does not negate the need for the UA to pass the information through if it exists, right? I think this should remain an AccName issue until the ARIA WG decides what should be done. If the WG decides Aaron is correct and the tests should be removed, there needs to be a spec change (I think?) and tentative name-prohibited tests (as Scott suggests above).

@cookiecrook
Copy link
Contributor

@aleventhal
Copy link
Contributor Author

@cookiecrook Ok. Let's do this. I'll create a prototype of my idea so that we can play with it and then decide whether to standardize it.

Assumption: prohibited name cases cause text to variably be announced by some screen readers and not others
Mitigations:

  1. User agent may move the accessible name to the description
  2. If user agent does Need clarity on computation involving first traversal #1, it should also log an an error to the developer console or otherwise provide the error in its developer tools, with information about how to address it.

@cookiecrook
Copy link
Contributor

cookiecrook commented Jul 2, 2024

From an offline convo with @aleventhal:

Aaron: Darn, those WPT tests mean I can’t easily prototype prohibited name repair, even behind a feature flag. I want to collect community feedback.

James: Can you attend next week’s ARIA meeting (July 11)? We can probably resolve it then if we should pul those tests in the meantime…

Aaron: Ok yeah. I don’t like WPT tests enforcing non-repair of bad authoring. At least I want to experiment / prototype. but i do understand both sides of it.

James: The distinction I’m trying to clarify is that it’s an author requirement, not a UA requirement, and the UAs are subject to “prioritize users ofter authors over implementors.” So a validator should flag it for sure, but if the author makes a mistake, that mistake may be the thing that actually helps a user in that case. But I also acknowledge your point about permissiveness in repair.

Aaron: Right, I’m trying to improve on what happens if the author makes a mistake, as an experiment behind a flag. Maybe it’s an issue on our end that we can’t turn off the experiment for WPT tests. Hmm, yeah no way to do that rn apparently.

James: I think AccName should add an exit clause for roles with name prohibited… It doesn’t yet, as I understand it… If we agree it should, we can pull those tests (quickly, before the spec change), and reference the AccName issue from the WPT test file comment.

FYI @spectranaut @jnurthen re: July 11 meeting scheduling

@accdc
Copy link
Contributor

accdc commented Jul 2, 2024

"I think AccName should add an exit clause for roles with name prohibited…"

What would that look like do you think? I'll need a proposal since you are more familiar with the desired outcome for implementors than I.

@MelSumner
Copy link
Contributor

"I think AccName should add an exit clause for roles with name prohibited…"

I agree that we could use some more actionable guidance on what kind of outcome is desired here.

@aleventhal
Copy link
Contributor Author

@MelSumner In the meantime I'm going to build some experiments in Chrome so that you can pass a command line argument to enable different behavior, and we can try some things.

@spectranaut
Copy link
Contributor

Discussed in today's ARIA meeting: https://www.w3.org/2024/07/11-aria-minutes.html#t07

@zcorpan
Copy link
Member

zcorpan commented Aug 6, 2024

Shouldn't the test be using valid ARIA?

Tests should exercise UA requirements and verify that the requirements are followed, irrespective from any author requirements.

In https://w3c.github.io/accname/#computation-steps I see

If the root node's role prohibits naming, return the empty string ("").

@aleventhal
Copy link
Contributor Author

Thanks @zcorpan. AIUI that means the test actually have the wrong expectations.
E.g.

expected "label" but got ""
[FAIL] label valid on dd element

@MelSumner
Copy link
Contributor

Could I get someone to explain how failing WPT tests are preventing experimentation?

I think I might be missing a beat here, apologies.

@cookiecrook
Copy link
Contributor

cookiecrook commented Aug 15, 2024

@MelSumner Generally the automation commit queues of the browser engines have a CI check that expects no new failures in either the engine-internal automation, or the shared WPT automation project, which is incorporated into all major engines. Since Aaron's change triggers a new failure in this (now challenged) test expectation, the solution would be to either:

  1. Change the patch so that the new failure no longer fails. (Fixing a regression introduced by the patch.)
  2. Remove the failing test if it's determined to be an invalid expectation.
  3. Add a test failure exception expectation to the patch. (Different implementations have different rules about when this patch is okay to use, but I don't think it'd be expected in this particular case.)

@cookiecrook
Copy link
Contributor

@aardrian wrote:

AIUI that means the test actually have the wrong expectations.

Okay, @aleventhal please review the WPT PR. Thanks.

@cookiecrook
Copy link
Contributor

I removed that test rather than changing it. I could not change the expectation to an empty string, since WebKit and Gecko are not disallowed from passing the label through, but Chromium also isn't required to pass the authoring error provided label though to the user trying to access their 401k. 😉

@aardrian
Copy link

@cookiecrook

@aardrian wrote:

That was Aaron, but I am flattered you think I am that hip with my initialisms.

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

No branches or pull requests

8 participants