fix(aria-valid-attr-value): handle multiple aria-errormessage IDs

[JustasMonkev]

When aria-errormessage contains multiple space-separated IDs, the check now correctly verifies that all IDs are present in aria-describedby instead of failing to match the entire string against tokenized values.

Closes: #4957

[CLAassistant]

All committers have signed the CLA.

[straker]

Thanks for the pr. Unfortunately it isn't an accessibility violation if all the aria-describedby ids are not present. As long as one exists then we want to pass this check.

[JustasMonkev]

Thanks for the pr. Unfortunately it isn't an accessibility violation if all the aria-describedby ids are not present. As long as one exists then we want to pass this check.

Thanks for the comment, updated it

[JustasMonkev]

@straker could you please check again?

[straker]

Sorry for the delay. I've been out of office and so haven't had time to look at this. After discussing this with the team here is what we are thinking.

Testing multiple aria-errormessage ids with various screen readers and browsers, only JAWS/Chrome supports this. NVDA/Firefox only announces the first id, while VoiceOver/Safari doesn't announce any. This means that even if multiple ids are supported by the spec, only JAWS/Chrome actually support it. When cases such as this happen with ARIA attributes, we typically report this as "unsupported" and that it's not widely supported by screen readers.

Taking that into consideration, as well as the various edge cases that would need to be handled to support multiple ids (what happens if the first id isn't on the page but all others are, what happens if the last id is the only one on the page, etc.), we feel it's best to report multiple ids on aria-errormessage as unsupported.

What this would mean is that we would update the aria-errormessage check to provide a new failure message of "Multiple IDs in aria-errormessage is not widely supported in assistive technologies" and in the evaluate function check if the tokenList of aria-errormessage contains more than 2 entires, and if so set the this.data({ message: 'unsupported' }) and return false.

[JustasMonkev]

Sorry for the delay. I've been out of office and so haven't had time to look at this. After discussing this with the team here is what we are thinking.

Testing multiple aria-errormessage ids with various screen readers and browsers, only JAWS/Chrome supports this. NVDA/Firefox only announces the first id, while VoiceOver/Safari doesn't announce any. This means that even if multiple ids are supported by the spec, only JAWS/Chrome actually support it. When cases such as this happen with ARIA attributes, we typically report this as "unsupported" and that it's not widely supported by screen readers.

Taking that into consideration, as well as the various edge cases that would need to be handled to support multiple ids (what happens if the first id isn't on the page but all others are, what happens if the last id is the only one on the page, etc.), we feel it's best to report multiple ids on aria-errormessage as unsupported.

What this would mean is that we would update the aria-errormessage check to provide a new failure message of "Multiple IDs in aria-errormessage is not widely supported in assistive technologies" and in the evaluate function check if the tokenList of aria-errormessage contains more than 2 entires, and if so set the this.data({ message: 'unsupported' }) and return false.

Thanks for the comment, updated it

[straker]

Thanks for the changes. This is almost done but needs a few more tests. There should be an integration test or 2 in aria-valid-attr-value that tests multiple ids on aria-errormessage returns false. To do this add the HTML to the file with a unique id, then add that ID to the corresponding JSON file in the violations section.

[JustasMonkev]

Thanks for the changes. This is almost done but needs a few more tests. There should be an integration test or 2 in aria-valid-attr-value that tests multiple ids on aria-errormessage returns false. To do this add the HTML to the file with a unique id, then add that ID to the corresponding JSON file in the violations section.

Added the tests

[straker]

Awesome, thanks again for all the help!

Reviewed for security