Repository navigation
Conversation
|
Adding reviewers to get some more perspectives on this. The description is missing in the PR, but the relevant discussion is in #1440, and I think the idea is sound. |
|
@smhigley I updated the property description to move this along. Nearly ready IMO. Some comments in the diff. |
mcking65
left a comment
There was a problem hiding this comment.
I think we need a discussion of whether aria-actions should be allowed on elements that are not focusable or referenced by aria-activedescendant. I see potentially big problems with elements that are containers with boundaries that screen readers treat as invisible. I also am concerned about the idea that it could be used on a dialog, which by default, should not be focusable; that is only a fall-back error condition where a dialog would get focus.
I have added several suggestions where I think the language needs more clarity.
|
To what extent is avoiding creating additional vectors for the active finger printing mentioned in section 11 a consideration in the design of this feature? I see some language that appears to be aimed in that direction, such as the requirement for using clicks and default actions. However, as written, it appears to me that it is all author responsibility, and is thus a wide open door. I wonder if it would be possible to have requirements that:
|
|
Matt, why do we need the focusable rule? Would it not be enough for the source to be visible, and the target to be visible & clickable? To activate the action, the browser can send it a click as if a real mouse click occurred. |
I think you may misunderstand. If an author doesn't use a click, it won't reveal anything new about the user. The UI just may not work in some scenarios. We purposefully limited AT's trigger-ability here to a click (rather than a new event or direct API call) to avoid risk of detection. |
Those are all much too rigid/restrictive in my opinion. And possibly too screenreader specific. AT focus or focus-in is an expectation, but not DOM focus. Otherwise we may not be able to make this work well for other AT like Switch Control, Voice Control, etc. Likewise Dragon on Windows, Android's Switch Access, etc. |
|
Are there any open questions with this functionality that needs any help? |
|
What else is needed to get this PR merged? |
|
@mrhbs wrote:
|
The latest spec PR(w3c/aria#1805) requires has-actions to be exposed on every host whose role supports aria-actions and that sets the attribute, regardless of whether any referenced target survives IsValidAriaActionsTarget. Widen the kHasActions gate in AXObject::SerializeUnignoredAttributes so the attribute is added when aria-actions is set on a supporting role, in addition to the existing path that triggers when kActionsIds is non-empty (which covers implicit-actions on menuitem-like roles). The previous gate keyed only on a non-empty kActionsIds, leaving assistive tech unable to distinguish "no aria-actions" from "aria-actions present but all targets filtered." Bug: 408040289, 514751946 Change-Id: Id621008fefd84a8b999565049735ff2c6f47a6a4 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7895414 Reviewed-by: Benjamin Beaudry <benjamin.beaudry@microsoft.com> Commit-Queue: Jacques Newman <janewman@microsoft.com> Cr-Commit-Position: refs/heads/main@{#1641975}
|
🚀 Deployed on https://deploy-preview-1805--wai-aria.netlify.app |
|
🚀 Deployed on https://deploy-preview-1805--wai-aria.netlify.app |
|
🚀 Deployed on https://deploy-preview-1805--wai-aria.netlify.app |
|
🚀 Deployed on https://deploy-preview-1805--wai-aria.netlify.app |
In creating the [aria-actions PR](w3c/aria#1805), it came up that the list of supported value types for a custom UIA property didn't include arrays, even though arrays are supported. This PR updates the docs to explicitly include the respective array types for each of the supported types.
The aria-actions spec PR (w3c/aria#1805) §6.8 includes an Author SHOULD that targets be visible when the host has DOM focus. Today we gave this behavior is chromium due to CanSetFocusAttribute() returning false for the hidden subtree, and IsValidAriaActionsTarget() already rejects targets that are not keyboard-focusable. This change codifies the existing behavior to protect against regression, with no behavioral change. Bug: 408040289 Change-Id: I12e4bc2bd8f2b1cac9f96dbcac1979ea085c3604 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7852706 Commit-Queue: Kurt Catti-Schmidt <kschmi@microsoft.com> Reviewed-by: Kurt Catti-Schmidt <kschmi@microsoft.com> Auto-Submit: Jacques Newman <janewman@microsoft.com> Cr-Commit-Position: refs/heads/main@{#1651177}
…PG test Prohibit aria-actions on the 11 name-prohibited roles per w3c/aria#1805, add an allowEmpty pass case, and bump aria-practices to re-enable the tabs-actions APG example. aria-required-children / nested-interactive don't yet support the aria-actions pattern, so they're disabled per-page pending #5215. Closes #4584
## Summary Adds `aria-actions` to axe-core's known ARIA attributes so it is recognized as valid, allowed, and prohibited on the roles the spec prohibits it on — and re-enables the APG `tabs-actions` example that had been disabled for lack of `aria-actions` support. Per the [spec draft](w3c/aria#1805), `aria-actions`: - **Value type:** ID reference list → `idrefs` - **Global:** yes (like `aria-describedby`) - **Empty allowed:** yes — the spec permits `aria-actions=""` (the deferred-DOM case) → `allowEmpty: true` - **Prohibited roles:** the name-prohibited roles the spec also prohibits it on (all axe `prohibitedAttrs` roles except `none`/`presentation`, which the spec still permits) - **ElementInternals reflection:** `ariaActionsElements` ## Accessibility-supported rationale Following the [Impact on ARIA](https://github.com/dequelabs/axe-core/blob/develop/doc/accessibility-supported.md#impact-on-aria) decision framework: 1. Supported by all platforms? No — shipped in WebKit and Firefox; **Chromium pending**. 2. Does its use negatively impact accessibility? **No** — unsupported browsers simply ignore the attribute (progressive enhancement), and the spec hard-guards exposure. → **allow.** > **Note for reviewers:** the ARIA spec change is still [PR #1805](w3c/aria#1805), not yet merged — this aligns to the two engines shipping ahead of spec approval. We can patch the config later in the unlikely event the spec shifts. ## Changes **Attribute recognition** — `lib/standards/aria-attrs.js`: add the `aria-actions` entry (`idrefs`, global, `allowEmpty`). **Prohibited-on-role** — `lib/standards/aria-roles.js`: add `aria-actions` to `prohibitedAttrs` for `caption`, `code`, `deletion`, `emphasis`, `insertion`, `mark`, `paragraph`, `strong`, `subscript`, `superscript`, `suggestion`. Per [w3c/aria#1805](w3c/aria#1805) these roles prohibit it; `none`/`presentation` do not, so they are left unchanged. **APG test re-enable (Closes #4584)** — bump `aria-practices` to latest `main` (the `tabs-actions` page did not exist at the previously pinned commit) and remove it from `skippedPages`. axe recognizes the attribute but not the authoring *pattern*, so `aria-required-children` (tabs-actions) and `nested-interactive` (listbox-actions) are disabled per-page pending #5215. **Review feedback** — update the stale `wai-aria-1.1` `Source:` comment to the unversioned WAI-ARIA URL; add an `aria-actions=""` pass case exercising `allowEmpty`. ## Testing - `get-global-aria-attrs`, `aria-prohibited-attr` (check + virtual-rule), `aria-valid-attr`, `aria-allowed-attr`, `aria-valid-attr-value` unit + integration tests ✓ - Full APG suite green (76 passing) ✓ - `npm run build` clean; no auto-generated committed files change ## Follow-ups - #5215 — teach `aria-required-children` / `nested-interactive` about the `aria-actions` pattern, then remove the per-page disables in `apg.spec.js` Closes #5199 Closes #4584
| <li>Authors SHOULD ensure that related actions elements are visible and activatable when the current element has DOM focus.</li> | ||
| <li>Authors SHOULD set <code>aria-actions=""</code> on the referencing element when the element is not focused if the related action elements will not exist in the DOM until the referencing element receives focus. This allows assistive technologies to surface the availability of actions when users interact with the element in ways that do not trigger DOM focus.</li> | ||
| <li>User Agents SHOULD use the accessible names of elements referenced by <code>aria-actions</code> to determine the names of actions that are exposed in a platform accessibility API.</li> | ||
| <li>User Agents MUST NOT expose <code>aria-actions</code> if the Author MUST's are not followed.</li> |
There was a problem hiding this comment.
Editorial: MUST's -> MUSTs
Substantive: I don't think this one will hold up as written. Needs to be more explicit.
|
The ARIA Working Group just discussed The full IRC log of that discussion<Zakim> agendum 6 -- -> Merge? - feat: aria-actions addition to the ARIA spec https://github.com//pull/1805 -- taken up [from agendabot]<jugglinmike> github: https://github.com//pull/1805 <jugglinmike> spectranaut_: Can we actually merge this? <jugglinmike> spectranaut_: I believe we have two implementations <jugglinmike> sarah: We have three implementations <jugglinmike> spectranaut_: The main thing is whether or not we can add tests for this. We do have new testing infrastructure... <Jacques> Does this include the accname PR as well? <spectranaut_> q? <jugglinmike> Matt_King: As far as I can tell, I can't get any of the implementations to work as we expect them <jugglinmike> jcraig: I think it's the one we have on the agenda for TPAC about having actions cascade down from the parent element <jugglinmike> jcraig: This has been a major source of confusion for VoiceOver users on our team <sarah> q+ <jugglinmike> jcraig: That's the only thing that I would say might delay things. Maybe not delay moving it to the spec, though <jugglinmike> Matt_King: We spent a couple hours on that issue at TPAC in 2025. We agreed not to do it base on the challenges. We agreed that we want the feature to progress even without that <jugglinmike> jcraig: Maybe I'm misunderstanding what you mean by "I can't get it to work" <jugglinmike> jcraig: We've heard that a lot, though <jugglinmike> jcraig: If you're thinking of a different issue, Matt_King, then I don't want to misrepresent you <Jacques> q+ <jugglinmike> Matt_King: We still need the changes in gh-2845, and then we need to be able to test those <Jacques> q- <jugglinmike> Matt_King: At least on Windows, we don't have an implementation where we can properly test. What Vispero has done so far isn't sufficient <jugglinmike> jcraig: In Safari, you should be able to test it. There's a path in experimental features. It's shipping behind a runtime flag. The same is true on desktop, but I don't recall the state in Chromium <spectranaut_> ack sarah <jugglinmike> sarah: I was going to say the same thing on jcraig's issue. That said, it would be have something in the spec to which we can refer. I don't know if we've ever had a bar where "everything must be perfect" before landing into the spec <jugglinmike> sarah: I think the follow-up is also ready to be merged if jcraig is cool with it. He was the latest person to comment, there <jugglinmike> Matt_King: That follow-up is against gh-1805 <jugglinmike> sarah: I can prepare any of these for any merge sequence <Daniel> q+ <jugglinmike> Matt_King: What does it mean to merge it? I don't understand the consequences of merging, now. What does it become part of ARIA? <Daniel> q{ <spectranaut_> ack Daniel <jugglinmike> ack Daniel <jcraig> q+ <jugglinmike> Daniel: Merging it now means "this is part of the editor's draft". That allows us to eventually include it in the working draft of ARIA. Until we merge it, that's not possible <jugglinmike> Daniel: Because we're so close to 1.3, I would like to have as many tests as possible to understand whether it is viable for inclusion in 1.3 <spectranaut_> q+ <jcraig> ack me <jugglinmike> ack jcraig <jugglinmike> jcraig: I think it's good for us to merge gh-1805. I'm one of the thumbs-up reviewers on this. Merge it into the editor's draft <jugglinmike> jcraig: I think it would be more problematic if gh-1805 landed in 1.3 TR without a couple of these other follow-on issues being resolved. <Matt_King> q+ <Daniel> q+ <Jacques> q+ <jugglinmike> jcraig: To tie that into Daniel's latest comment: if the editor's draft is ready to branch from 1.3 TR (or however you branch these days), then let's do that now. <jugglinmike> jcraig: If it's not ready to branch, then I say to hold off on merging this <spectranaut_> ack Daniel <jugglinmike> Daniel: It was branched some time in June, so it wouldn't be an issue to merge it <jugglinmike> jcraig: Great, then let's merge it straight in. this is good progress--congrats, sarah! <jugglinmike> spectranaut_: I would prefer if they were merged at the same time <jugglinmike> ack spectranaut_ <jugglinmike> spectranaut_: We can and therefore should write tests. We can write tests against the AAMs, now. <jugglinmike> spectranaut_: Is anyone interested in doing that? <jugglinmike> jcraig: You should lead a session on aam tests at TPAC <jugglinmike> spectranaut_: A test-writing session? <jugglinmike> jugglinmike: A workshop? <jugglinmike> cyns: I would love a test-writing workshop <spectranaut_> q? <jugglinmike> ack Matt_King <spectranaut_> ack Matt_King <jugglinmike> Matt_King: I don't think we should merge gh-1805 until we've completed gh-2845. I think 2845 is critical <jugglinmike> Matt_King: We expect the state of a feature to be when it goes from the editor's draft to whatever will be the next step... But what will our criteria be at that point? <jugglinmike> Matt_King: the thing I get concerned about is authors starting to use and rely on it before we have functional implementations and a common understanding <jcraig> q+ <spectranaut_> ack Jacques <jcraig> q+ to agree with matt <jugglinmike> Matt_King: If authors start relying on it too soon, then we could end up with an absolutely awesome feature that receives a really bad rap in the community for some time. I want to avoid that! <jugglinmike> Jacques: What's the status of the third pull request in the accname space? <jugglinmike> spectranaut_: Maybe it was moved in... <jugglinmike> sarah: this pull request was made before the mono-repo. So I think accname should stay separate <jugglinmike> spectranaut_: Can we put it into the other pull request? <jugglinmike> spectranaut_: It hasn't been reviewed <Jacques> q? <jugglinmike> jcraig: Let's keep it separate because the other has so many positive reviews already <jugglinmike> jcraig: I'll put this one on my to-do list <jugglinmike> jcraig: We also need an editor to dismiss the stale review from Erin <jugglinmike> spectranaut_: I've refreshed the review request <spectranaut_> ack jcraig <Zakim> jcraig, you wanted to agree with matt <Daniel> q+ <jugglinmike> spectranaut_: We could use another review on the accname pull request <jugglinmike> bryan: Sure <spectranaut_> ack Daniel <jugglinmike> ack Daniel <jugglinmike> Daniel: Is this already in the monorepo or does it have to be included? <Matt_King> q+ <spectranaut_> ack Matt_King <jugglinmike> spectranaut_: It's in the monorepo. It just shouldn't be merged until aria-actions <jugglinmike> Matt_King: As of a few weeks ago, the implementations had not implemented the accname change, so that part is not ready for testing <jugglinmike> Matt_King: We should flag that <jugglinmike> spectranaut_: Good point. Maybe we should add the "pr tracking" information to this <Rahim> q+ <spectranaut_> ack Rahim <jugglinmike> spectranaut_: Okay, obviously we're not at the point of merging this, yet. We'll revisit it. I hear Matt_King's concern that we don't want to encourage folks to rely on this prematurely, though <jugglinmike> Rahim: I vaguely recall someone (maybe jcraig) encouraged folks not to reference the accname algorithm steps by number <jugglinmike> jcraig: That's right. If the pull request includes that in prose, then we should change it for sure <jugglinmike> jcraig: I agree with Matt_King that it would be a shame to miss out on these features. I've blocked time for two of these. the issue we've been discussing should also be included at TPAC <jugglinmike> spectranaut_: Great. We can't merge this until there are tests, anyway <jugglinmike> zakim, next item |
Co-authored-by: James Craig <cookiecrook@users.noreply.github.com>
🚀 Netlify Preview:
🔄 this PR updates the following sspecs:
Resolves #1440 by adding the
aria-actionsattributeThis has a dependency on the changes in #1454.
PR tracking
Check these when the relevant issue or PR has been made, OR after you have confirmed the
related change is not necessary (add N/A). Leave unchecked if you are unsure. Read the
Process Document or
Test Overview for more information.
Related Core AAM Issue/PR: included in this PRTest, Documentation and Implementation tracking
Once this PR and all related PRs have been been approved by the working group, tests
should be written and issues should be opened on browsers. Add N/A and check when not
applicable.
Preview | Diff