Skip to content

aria-errormessage is hidden or removed when not pertinent #1588

Merged
jnurthen merged 16 commits intomainfrom
errormessage-hidden-removed
Apr 25, 2023
Merged

aria-errormessage is hidden or removed when not pertinent #1588
jnurthen merged 16 commits intomainfrom
errormessage-hidden-removed

Conversation

@jongund
Copy link
Contributor

@jongund jongund commented Aug 3, 2021

No description provided.

@jongund jongund requested review from jnurthen and joanmarie August 3, 2021 15:44
@jongund jongund linked an issue Aug 3, 2021 that may be closed by this pull request
Author test case: aria-errormessage (ARIA 1.2) #1576
Closed
@joanmarie joanmarie requested a review from spectranaut August 12, 2021 17:19
@spectranaut spectranaut force-pushed the errormessage-hidden-removed branch from b64114e to 8328b20 Compare August 12, 2021 20:47
@spectranaut
Copy link
Contributor

spectranaut commented Aug 12, 2021 •
edited
Loading

I just fixed the conflict in the README and added a json file for the AXE results. It seems to me there is a bug in AXE, but I'm having trouble reporting it...

Here are the results:

  Violation of "aria-valid-attr-value" with 4 occurrences!
    Ensures all ARIA attributes have valid values. Correct invalid elements at:
     - #input-1
     - #input-2
     - #input-3
     - #input-7
    For details, see: https://dequeuniversity.com/rules/axe/4.3/aria-valid-attr-value

According to Jon's examples, inputs 1 - 4 should fail, and 5 - 8 should not, so the AXE results do not match the expectation. The more detailed error says aria-errormessage value id-message-1 must use a technique to announce the message (e.g., aria-live, aria-describedby, role=alert, etc.),. Seems to me this is wrong, and there is an additional bug in that the hiddenness of the element pointed to by aria-errormessage is not reported?

edit: I wrote up a bug: dequelabs/axe-core-npm#328

spectranaut
spectranaut approved these changes Aug 12, 2021
View reviewed changes
Copy link
Contributor

@spectranaut spectranaut left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve this PR but I also added a AXE file, so @joanmarie or @schne324 should take a look too.

jnurthen
jnurthen reviewed Aug 30, 2021
View reviewed changes
@pkra
Copy link
Member

pkra commented Jan 10, 2022 •
edited
Loading

ping @joanmarie @jnurthen

This testcase resolves #1576 (which is technically in the 1.2 milestone).

@pkra pkra added this to the ARIA 1.3 milestone Jan 12, 2022
@spectranaut spectranaut self-assigned this Sep 1, 2022
@jnurthen jnurthen requested review from MelSumner and removed request for joanmarie January 19, 2023 18:30
MelSumner
MelSumner reviewed Jan 31, 2023
View reviewed changes
validator-tests/errormessage-hidden-removed-axe.json
{
"description": "Ensures all ARIA attributes have valid values",
"help": "ARIA attributes must conform to valid values",
"helpUrl": "https://dequeuniversity.com/rules/axe/4.3/aria-valid-attr-value?application=webdriverjs",
Copy link
Contributor

@MelSumner MelSumner Jan 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question, non-blocker: Is the query param needed here? It seems like the URL is valid without it. Just curious.

Copy link
Member

@jnurthen jnurthen Jan 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the file retrieved from aXe. I don't think we should be changing it.

MelSumner
MelSumner reviewed Jan 31, 2023
View reviewed changes
validator-tests/errormessage-hidden-removed.html

<!--
URL: https://www.w3.org/TR/wai-aria-1.2/#aria-errormessage
RULE: " Authors MUST use aria-invalid in conjunction with aria-errormessage and when aria-errormessage is pertinent, authors MUST ensure the content is not hidden so users can navigate to and examine the error message"
Copy link
Contributor

@MelSumner MelSumner Jan 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question, potential blocker: Should an example be included that has no aria-invalid attribute?

From the spec, emphasis mine:

Initially, the object is in a valid state and either has aria-invalid set to false or no aria-invalid attribute, and the element referenced by aria-errormessage is not applicable.

WDYT?

Copy link
Member

@jnurthen jnurthen Jan 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we would if this were a test file for all of aria-errormessage.

However this is only a testfile for the added authors MUST statements in aria 1.2. Without aria-invalid, aria-errormessage is not pertinent so the entire sentence would not apply.

@pkra
Copy link
Member

pkra commented Apr 20, 2023

@jnurthen can we merge this? It's actually closing the last open issue for the 1.2 milestone.

schne324
schne324 approved these changes Apr 21, 2023
View reviewed changes
Copy link
Contributor

@schne324 schne324 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me.

@spectranaut, you probably have already seen this but it looks like the underlying axe-core issue you raised has been resolved: dequelabs/axe-core#3149

@pkra pkra added the Agenda label Apr 22, 2023
@jnurthen jnurthen merged commit 11902fc into main Apr 25, 2023
@pkra pkra deleted the errormessage-hidden-removed branch April 25, 2023 19:36
jnurthen added a commit that referenced this pull request Oct 10, 2023
@jongund @spectranaut @jnurthen
aria-errormessage is hidden or removed when not pertinent (#1588)
b725501 
Co-authored-by: Valerie Young <[email protected]>
Co-authored-by: James Nurthen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jnurthen jnurthen jnurthen left review comments

@MelSumner MelSumner MelSumner left review comments

@spectranaut spectranaut spectranaut approved these changes

@schne324 schne324 schne324 approved these changes

Assignees

@spectranaut spectranaut

Labels

Agenda

Projects

None yet

Milestone

2023

Development

Successfully merging this pull request may close these issues.

Author test case: aria-errormessage (ARIA 1.2)

6 participants

@jongund @spectranaut @pkra @jnurthen @schne324 @MelSumner