Skip to content
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

Update: change default role for custom elements #2383

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

scottaohara
Copy link
Member

closes #2303

This updates a custom element's default role to none to allow attribute reflection from the custom element parent to its internals.

This change provides additional clarification about how a custom element can be provided a role by authors, and what caveats would change a custom element's default role of none, to another implicit minimum role.

Test, Documentation and Implementation tracking

Once this PR has been reviewed and has consensus from the working group, tests should be written and issues should be opened on browsers. Add N/A and check when not applicable.

  • "author MUST" tests:
  • "user agent MUST" tests:
  • Browser implementations (link to issue or commit):
    • WebKit:
    • Gecko:
    • Blink:
  • Does this need AT implementations?
  • Related APG Issue/PR:
  • MDN Issue/PR:

closes #2303

This updates a custom element's default role to none to allow attribute reflection from the custom element parent to its internals.

This change provides additional clarification about how a custom element can be provided a role by authors, and what caveats would change a custom element's default role of none, to another implicit minimum role.
@scottaohara scottaohara marked this pull request as draft November 22, 2024 20:16
Copy link

netlify bot commented Nov 22, 2024

Deploy Preview for wai-aria ready!

Name Link
🔨 Latest commit 67ce536
🔍 Latest deploy log https://app.netlify.com/sites/wai-aria/deploys/674744d31ec19800083dfbe5
😎 Deploy Preview https://deploy-preview-2383--wai-aria.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@scottaohara
Copy link
Member Author

marking as draft as the form associated custom element mapping table also needs updating - but i want to make sure i got this going in the right direction before i edit that table too

html-aam/index.html Outdated Show resolved Hide resolved
html-aam/index.html Outdated Show resolved Hide resolved
html-aam/index.html Show resolved Hide resolved
html-aam/index.html Outdated Show resolved Hide resolved
html-aam/index.html Outdated Show resolved Hide resolved
html-aam/index.html Outdated Show resolved Hide resolved
html-aam/index.html Outdated Show resolved Hide resolved
pull comments into role table cell
combine custom elements and form associated custom elements into a single mapping table
@scottaohara scottaohara marked this pull request as ready for review November 22, 2024 22:10
Copy link
Contributor

@aleventhal aleventhal left a comment

Choose a reason for hiding this comment

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

Awesome, after many iterations

update wording to be consistent between steps
aarongable pushed a commit to chromium/chromium that referenced this pull request Dec 3, 2024
Update custom role rules and tests based on new text at
w3c/aria#2383

Behind feature flag:
--enable-blink-features=AccessibilityCustomElementRoleNone

Intent to prototype / chromestatus entry:
https://chromestatus.com/feature/5079996916563968?gate=5150348547981312

Bug: w3c/aria#2303
Bug: WICG/webcomponents#1073
Bug: 379674023
Change-Id: If317916b432bc5a627c8ea6fa0654e6c98a10ea6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6042465
Reviewed-by: Chris Harrelson <[email protected]>
Commit-Queue: Aaron Leventhal <[email protected]>
Auto-Submit: Aaron Leventhal <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1391105}
@jnurthen jnurthen requested review from jnurthen and jcsteh December 5, 2024 18:17
@jnurthen jnurthen requested a review from cookiecrook December 5, 2024 18:17
@@ -226,7 +226,8 @@ <h3>HTML Element Role Mappings</h3>
specified which would require a more specific <a>minimum role</a> to be exposed.
</li>
<li>
Where an element is indicated as having &quot;No corresponding (WAI-ARIA) role&quot;, or is mapped to the <a class="core-mapping" href="#role-map-generic">`generic`</a> role, user agents
Where an element is indicated as having &quot;No corresponding (WAI-ARIA) role&quot;, or is mapped to the <a class="core-mapping" href="#role-map-generic">`generic`</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

I conceded excluding aria-roledescription on generic a while back, though I don't fully agree... Adding none to the list of exclusions is probably okay, too.

However, excluding it on all host language native roles is not okay.

Suggested change
Where an element is indicated as having &quot;No corresponding (WAI-ARIA) role&quot;, or is mapped to the <a class="core-mapping" href="#role-map-generic">`generic`</a>
Where an element is mapped to the <a class="core-mapping" href="#role-map-generic">`generic`</a>

<td>If the author assigned a conforming ARIA role using the `role` attribute, map to that role. Otherwise, the <a class="core-mapping" href="#role-map-generic">`generic`</a> role.</td>
<td>
<ul>
<li>If the author assigned a conforming ARIA role using the `role` attribute, or by the custom element's internals: map to the specified role.</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

Removing ARIA because some readers won't consider all conforming roles to be "ARIA", strictly speaking. For example, EPUB publishers may not understand that doc-chapter is technically an "ARIA" role, but they should understand it's a conforming role. The extra adjective here is irrelevant IMO.

Suggested change
<li>If the author assigned a conforming ARIA role using the `role` attribute, or by the custom element's internals: map to the specified role.</li>
<li>If the author assigned a conforming role using the `role` attribute, or by the custom element's internals: map to the specified role.</li>

<ul>
<li>If the author assigned a conforming ARIA role using the `role` attribute, or by the custom element's internals: map to the specified role.</li>
<li>else if the author assigned HTML attributes that result in a <a>minimum role</a>: map to the minimum role.</li>
<li>else if the custom element is focusable: map to the <a class="core-mapping" href="#role-map-generic">`group`</a> role</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why group? Could we map to unknown or something and log a warning?

Copy link
Contributor

Choose a reason for hiding this comment

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

also this links to generic, not group

Copy link
Contributor

Choose a reason for hiding this comment

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

that typo maight be easier to avoid using the respective <rref> tag...

<li>If the author assigned a conforming ARIA role using the `role` attribute, or by the custom element's internals: map to the specified role.</li>
<li>else if the author assigned HTML attributes that result in a <a>minimum role</a>: map to the minimum role.</li>
<li>else if the custom element is focusable: map to the <a class="core-mapping" href="#role-map-generic">`group`</a> role</li>
<li>else if an author specifies a global ARIA attribute on the custom element that creates a relation with another element: map to the <a class="core-mapping" href="#role-map-generic">`group`</a> role</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming you meant group for both? Not generic for both?

Suggested change
<li>else if an author specifies a global ARIA attribute on the custom element that creates a relation with another element: map to the <a class="core-mapping" href="#role-map-generic">`group`</a> role</li>
<li>else if an author specifies a global ARIA attribute on the custom element that creates a relation with another element: map to the <a class="core-mapping" href="#role-map-group">`group`</a> role</li>

Copy link
Contributor

Choose a reason for hiding this comment

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

or <rref>

<li>else if the custom element is focusable: map to the <a class="core-mapping" href="#role-map-generic">`group`</a> role</li>
<li>else if an author specifies a global ARIA attribute on the custom element that creates a relation with another element: map to the <a class="core-mapping" href="#role-map-generic">`group`</a> role</li>
<li>else if the custom element has no attached shadow root: map to the <a class="core-mapping" href="#role-map-generic">`generic`</a> role</li>
<li>else if the custom element has an `aria-live` attribute: map to the <a class="core-mapping" href="#role-map-generic">`generic`</a> role</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why generic? Could we map to unknown or something and log a warning? Generic-izing this element will have the side effect of making some other features not work unexpectedly.

<li>else if the author assigned HTML attributes that result in a <a>minimum role</a>: map to the minimum role.</li>
<li>else if the custom element is focusable: map to the <a class="core-mapping" href="#role-map-generic">`group`</a> role</li>
<li>else if an author specifies a global ARIA attribute on the custom element that creates a relation with another element: map to the <a class="core-mapping" href="#role-map-generic">`group`</a> role</li>
<li>else if the custom element has no attached shadow root: map to the <a class="core-mapping" href="#role-map-generic">`generic`</a> role</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the generic minimum role close this off for updates later? Maybe an author intended to add an aria-label at some point later, but this mapping now invalidates that later update?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9.3 Presentational Roles Conflict Resolution does not consider custom element use cases
3 participants