-
Notifications
You must be signed in to change notification settings - Fork 28
Add a11y guidance for user agents #238
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
Conversation
This is good, though quite broad and generally applicable to any UI. As we depend on the Credential Management API for credential selection, we need to push this down to the CM API level instead (if not already covered):
https://w3c.github.io/webappsec-credential-management/#user-mediated-selection
Then in our spec we would just say, "See the Credential Management API for the Accessibility Considerations when selecting a credential."
Discussed 11 June minutes
The the holder serves as a "credential chooser", which needs to:
- be keyboard accessible particularly allowing the user to abort an request or issuance operation (e.g., you should be able to hit "esc" to abort the operation)
- present what is being requested should accessible to screen readers.
- Make sure the QR code is labelled
The the holder serves as a "credential chooser", which needs to:
@marcoscaceres
I think we shouldn't mix those concepts.
"credential chooser" aggregates the credentials from multiple "holders".
Matt: also what is the plan with this PR? @MasterKale
I think we should provide the guidance I proposed in #238 (comment) ... those were things that could have been better in Safari's implementation, so this was helpful (even if it applies specifically to holders).
@mohamedamir --
"credential chooser" aggregates the credentials from multiple "holders".
I think, rather --
"credential chooser" aggregates one or more credentials from one or more "holders", from which the user can choose which credential to use for a given purpose.
Matt: also what is the plan with this PR?
My plan has been to incorporate Marcos' suggestions into the next iteration. I'll update this PR by our next B-series meeting on Wednesday.
"credential chooser" aggregates the credentials from multiple "holders".
Can this spec propose guidance for holders? It's been my assumption that Verifiers and now "Credential Choosers" are fair game, but since Holders work with Credential Choosers that's probably the protocol that influences Holder behavior around a11y, no?
Alright, I've finally taken a stab at incorporating the feedback provided thus far. @mohamedamir @marcoscaceres can you please take a look when you have a chance?
@MasterKale, are you going to keep working on this or would like me to take it over?
@marcoscaceres @mohamedamir @timcappalli The accessibility section has been marked class="informative", so Respec says the section is non-normative:
Does the inclusion of SHOULD and MUST in the current version of the text mean this class should be dropped accordingly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"credential choosers" are out of scope for this API, this it is the responsibility of the underlying platform to display them.
Not sure if this actually fits here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, good point. This should really be about user agents, that might put some UI before invoking the credential chooser, making that UI accessible. Could a simple replacement here work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok with keeping [=credential choosers=] here... the API still needs to present a credential chooser because Cred Man explicitly requires it as part of the get a credential steps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dug into the CredMan definition that [=credential choosers=] ends up getting linked to, and it's to a section on User Mediated Selection that talks about credential choosers as something that User Agents implement:
https://www.w3.org/TR/credential-management-1/#user-mediated-selection
When responding to a call to get() on an origin which requires user mediation, user agents MUST ask the user for permission to share credential information. This SHOULD take the form of a credential chooser which presents the user with a list of credentials that are available for use on a site, allowing them to select one which should be provided to the website, or to abort the request entirely.
This must be what @marcoscaceres is referring to above (I'm making this connection more for my benefit.) So perhaps it's okay to keep the mention of credential choosers here. @mohamedamir might this change your mind about this particular section of accessibility guidance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Sorry for being late, fall through the cracks during my vacation)
I think @marcoscaceres is referring this step in the algorithm.
But @marcoscaceres didn't we discuss that the algorithm needs to be updated since at its current state, the algorithm doesn't accurately match the DC flow?
I am sorry, but I still think adding Credential Chooser here is confusing and doesn't full represent the user agent role in DC calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have looked at the algorithm again, and I see how Cred Man actually references a credential chooser (that's different from the credential chooser that Android displays for example). Although effectively, we don't display it in DC, but I am fine keeping it as long as it links to the Cred Man one.
@marcoscaceres @mohamedamir @timcappalli The accessibility section has been marked
class="informative", so Respec says the section is non-normative:
Does the inclusion of SHOULD and MUST in the current version of the text mean this class should be dropped accordingly?
You are right, if we are going to stick with SHOULD and MUST we should drop this class IMHO.
Discussed 3 September minutes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is nearly there... just tightened up the text a little bit.
Thanks for the review @marcoscaceres, I've incorporated those changes. There's only one remaining conversation above, otherwise this PR is ready for @mohamedamir and @timcappalli's review.
I glanced over the gh action failing @MasterKale .
https://github.com/w3c/spec-prod/blob/2f198e5d11d55b1b75f6dd197d8f74d33860fc61/src/validate-links.ts seems to not like a link we have in the spec. No idea what exactly is happening - the error seems pretty weird? But I guess the easiest quick fix would be to disable that in https://github.com/w3c-fedid/digital-credentials/blob/main/.github/workflows/auto-publish.yml since VALIDATE_LINKS is optional & experimental?
I don't have any rights for the gh actions, but could anyone with rights try to trigger a re-run? Maybe some link was briefly down or the gh runners ran into some throttling / triggered DOS protection etc.
Uh oh!
There was an error while loading. Please reload this page.
This PR adds informative guidance to user agents encouraging them to mediate presentation and issuance requests in a way that supports a wide range of accessible inputs and outputs.
The intent is to submit this content in the horizontal review request to the a11y-request repo.
Related to #229
The following tasks have been completed:
(削除) Modified Web platform tests - not testable by WPT. (削除ここまで)Implementation commitment:
Documentation and checks
Preview | Diff