fix(standards): update aria-errormessage and aria-details to be idrefs#5157
fix(standards): update aria-errormessage and aria-details to be idrefs#5157straker wants to merge 1 commit into
Conversation
chutchins25
left a comment
chutchins25
left a comment
There was a problem hiding this comment.
Nice — clean alignment with the ARIA 1.3 idrefs change, and the aria-errormessage analysis checks out: the aria-valid-attr-value check skips it via skipAttrs, and the dedicated aria-errormessage check handles the multi-id case independently of the standards type. One question inline about test coverage on the aria-details side.
| }, | ||
| 'aria-details': { | ||
| type: 'idref', | ||
| type: 'idrefs', |
There was a problem hiding this comment.
Is there a test case for the aria-details side of this? Unlike aria-errormessage (skipped in aria-valid-attr-value + covered by its own check), aria-details flips behavior here — aria-details="a b" was a violation before and now passes if one id resolves — and I didn't see a multi-id case covering it.
WilcoFiers
left a comment
WilcoFiers
left a comment
There was a problem hiding this comment.
Agreed. I wouldn't mind seeing a test here. Also, can you test these actually work? I think we should still make this change if it doesn't, but we may want to consider opening an issue to add a check with a dedicated issue message for it if people use multiple IDs in aria-errormessage when it doesn't work.
We already have that. It was added in the pr I linked above. As for adding a test for |
Didn't realize that. Fair enough!
Yes I think that'd be good. |
3 participants
This is in the current WAI-ARIA 1.3 draft and both attributes were changed to
idrefstypes. Since the AOM property is an idrefs type (returns an array) this allows us to be in sync with that for the Element Internals work.The
aria-errormessagecheck already accounts the attribute being an idrefs value and will incomplete if the value has more than one id, so no changes should be needed with this pr.