w3c / accname

Accessible Name and Description Computation
https://w3c.github.io/accname/
62 stars 22 forks source link

WPT test fails if I enforce name prohibited #240

Open aleventhal opened 4 months ago

aleventhal commented 4 months ago

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 commented 4 months ago

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

aleventhal commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

Referenced subtests starting here: https://github.com/web-platform-tests/wpt/blob/master/accname/name/comp_label.html#L28

aleventhal commented 4 months ago

@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 #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 commented 4 months ago

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 commented 4 months ago

"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 commented 4 months ago

"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 commented 4 months ago

@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 commented 4 months ago

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

cookiecrook commented 4 months ago

@aleventhal can you help me recall if these WPT tests need to be removed?

zcorpan commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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

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

cookiecrook commented 3 months ago

@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 commented 3 months ago

@aardrian wrote:

AIUI that means the test actually have the wrong expectations.

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

cookiecrook commented 3 months ago

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 commented 3 months ago

@cookiecrook

@aardrian wrote:

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