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

Allow setting default accessibility semantics for custom elements #4658

Merged
merged 5 commits into from
Sep 18, 2020

Conversation

domenic
Copy link
Member

@domenic domenic commented May 23, 2019

This WIP pull request drafts a mechanism for using ElementInternals to set a custom element's default accessible role, states, and properties. It builds on w3c/aria#984 and must not be merged before that dependency is ready.

Perhaps a separate issue, but one this PR makes more apparent: I think we should add some text in the custom elements section generally that encourages custom element authors to have a more formalized contract with their consumers, e.g. provide their own recommended content model, strong native accessibility semantics, etc. I touched on this briefly in the example but it should probably be somewhere more normative as well.

Things I could use reviewer feedback on (now resolved):
  • Naming. I sometimes used "default" and sometimes "native". I'm not sure what's more correct. Similarly between "accessibility", "ARIA", and "WAI-ARIA", I tried to use what felt more natural, but might have got it wrong. Sometimes I used "semantics" to mean "roles, states, or properties"; is that good?
  • Placement of spec text. I put the ElementInternals stuff in ElementInternals, but then how that impacts the processing model for AT I put in the "Requirements related to ARIA and to platform accessibility APIs" section. And then I put an example in our intro-to-custom-elements example section. This feels a bit spread out; is it OK?
  • Processing model. I tried to clarify in https://github.com/w3c/aria/issues/982 what the best way to insert the custom elements default values into the "ARIA cascade" was, and this was the best I could come up with. But maybe there is something better or more precise.

/cc @tkent-google @alice @stevefaulkner (your help as HTML-AAM editor would be especially appreciated) @whatwg/a11y @whatwg/components. I'm also happy to open an issue on w3c/webcomponents if folks think that would be helpful.


(See WHATWG Working Mode: Changes for more details.)


/custom-elements.html ( diff )
/dom.html ( diff )
/index.html ( diff )
/infrastructure.html ( diff )

@domenic domenic added addition/proposal New features or enhancements do not merge yet Pull request must not be merged per rationale in comment accessibility Affects accessibility topic: custom elements Relates to custom elements (as defined in DOM and HTML) labels May 23, 2019
@rniwa
Copy link

rniwa commented May 23, 2019

One thing that's strange about this approach is that ARIA roles are typically an intrinsic property of an element, not something that change over time. There is input element but that's more of an anomaly than not. I think it makes more sense to have this be an optional argument to customElements.define instead.

@domenic
Copy link
Member Author

domenic commented May 23, 2019

Well, there's actually a lot of cases where things change. You can see https://w3c.github.io/html-aam/#html-element-role-mappings for details. The others are:

  • <a> and <area> (changes based on href attribute)
  • <datalist> (whether or not it's connected to an <input>)
  • <footer> and <header> (depending on their parent)
  • <form> (depending on whether or not it has an accessible name)
  • <img> (depending on its alt="" attribute)
  • <li> (depending on its parent)

... well, you get the idea; I stopped at "L". Most complex controls, or controls that have relationships with other controls, need the ability to dynamically change their role.

@jnurthen
Copy link
Member

I don't know if this influences this decision but the ARIA spec states the following at http://w3c.github.io/aria/#roles

Roles are element types and authors MUST NOT change role values over time or with user actions. Authors wishing to change a role MUST do so by deleting the associated element and its children and replacing it with a new element with the appropriate role. Typically, platform accessibility APIs do not provide a vehicle to notify assistive technologies of a role value change, and consequently, assistive technologies may not update their cache with the new role attribute value.

@domenic
Copy link
Member Author

domenic commented May 23, 2019

Hmm, thanks @jnurthen, that is interesting. The first half of the paragraph, about author behavior, is not as relevant. But the part about platform accessibility APIs not being able to adapt to role changes is quite relevant.

Can someone who works on browser AT confirm that if I write

<a>placeholder</a>
<script>
document.querySelector('a').href = 'https://example.com/';
</script>

then AT users will forever see the <a> as having no role (e.g. UIA Text), instead of the link role (e.g. UIA HyperLink)?

If that's the case, then I guess custom elements aren't going to be able to do any better, and we do need to re-think the design away from a mutable property.

@stevefaulkner
Copy link
Contributor

stevefaulkner commented May 23, 2019

@domenic that's not the best example as despite what the AAM currently says seems like browsers expose the link role regardless of presence of href, bug filed w3c/html-aam#196 :-)

have created a modded test case which tests, i think, what you were asking. no issue for AT.

@domenic
Copy link
Member Author

domenic commented May 23, 2019

Ah, thank you! Well, perhaps I should just start testing those cases I listed myself, but do you know off the top of your head the answer for their analogues? E.g.

<body>
<main></main>
<footer>I have contentinfo role to start with</footer>
<script>
document.querySelector("main").append(document.querySelector("footer"));
// now it should have no role
</script>
</body>

or

<img src="test" alt=""> <!-- presentation role to start with -->
<script>
document.querySelector('img').alt = 'a test image';
// now it should have img role
</script>

?

@jnurthen
Copy link
Member

jnurthen commented May 23, 2019

have created a modded test case which tests, i think, what you were asking. no issue for AT.

@stevefaulkner I'm interested to know how you tested this? Are you looking at the internal browser representation of the tree, the Platform accessibility APIs or what is actually being presented to the user using (for example) a screen reader.
I think the first 2 would be expected to work - where there is a question is whether the screen reader has its own cached copy - I don't know enough about screen reader internals to know if this is still the case or not.

@annevk
Copy link
Member

annevk commented May 24, 2019

If more investigation is required here, perhaps an issue can be used? That'll make reviewing the eventual text a little easier.

@stevefaulkner
Copy link
Contributor

@jnurthen

I'm interested to know how you tested this?

I looked at acc API info, browser acc tree and navigated with JAWS and NVDA.

I also created a second test case with a button to trigger the type change. Same result - SR's tested had no problem with the type change.

@LJWatson
Copy link

@domenic commented:

Naming. I sometimes used "default" and sometimes "native". I'm not sure what's more correct. Similarly between "accessibility", "ARIA", and "WAI-ARIA",
I tried to use what felt more natural, but might have got it wrong. Sometimes I used "semantics" to mean "roles, states, or properties"; is that good?>

The terms "default" and "native" are both correct and interchangeable in this context.

One consideration is that neither "default" nor "native" has an elegant counterpart that describes its opposite. Using "implicit" and "explicit" is one way to make the two different concepts clear, though arguably those terms are harder to comprehend.

The ARIA spec seems to use "default", "native", "implicit", and also "Host language semantics" without particular preference.

Is it worth adding a definition of whichever terms are used, that explains the common alternative terms that are used interchangeably with them?

Most people refer to "ARIA", but AIRC the official "WAI-ARIA" name was necessary because "ARIA" was already trademarked elsewhere. If it's possible to use "ARIA" in the prose but reference "WAI-ARIA" in the biblio, that would be the best of both worlds, if not I think it'll have to be the more awkward formal name.

Using "semantics" or "accessibility semantics" if you want to put particular emphasis on it, to mean role, state and property information, is absolutely fine.

@jnurthen
Copy link
Member

@jnurthen

I'm interested to know how you tested this?

I looked at acc API info, browser acc tree and navigated with JAWS and NVDA.

I also created a second test case with a button to trigger the type change. Same result - SR's tested had no problem with the type change.

My brief testing showed the same but before moving forward with assuming this text in the ARIA spec is now obsolete I'd like to get the opinion of developers of JAWS, NVDA, ORCA, VO (and others if we can find them). I've opened w3c/aria#986 to ask the question if the ARIA spec text is still accurate.

@caridy
Copy link

caridy commented May 24, 2019

There is input element but that's more of an anomaly than not. I think it makes more sense to have this be an optional argument to customElements.define instead.

@rniwa, this option has surfaced multiple times... we are not Ok with forcing the author of the component to register the component, that doesn't work for us, and does not work for many other folks that produce components that can be registered by their consumers with arbitrary tag names.

@rniwa
Copy link

rniwa commented May 24, 2019

There is input element but that's more of an anomaly than not. I think it makes more sense to have this be an optional argument to customElements.define instead.

@rniwa, this option has surfaced multiple times... we are not Ok with forcing the author of the component to register the component, that doesn't work for us, and does not work for many other folks that produce components that can be registered by their consumers with arbitrary tag names.

That's an orthogonal issue. Custom element class can easily define a static method for ARIA role and whoever defines a concrete custom element could use that ARIA role via static method.

source Outdated
this._checked = false;
this.addEventListener('click', this._onClick.bind(this));

<mark> this._internals.role = "checkbox";
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: In this code example, single quotes are used.

source Outdated

<div w-nodev>

<p>Each <span>custom element</span> has a <dfn>native accessibility semantics map</dfn>, which is
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should mention the initial content of native accessibility semantics map. It's empty initially?

@domenic domenic force-pushed the domenic/elementinternals-a11y branch from 6347136 to f138256 Compare June 14, 2019 18:59
@domenic
Copy link
Member Author

domenic commented Jun 14, 2019

@rniwa given the above input from accessibility folks about how native elements often experience role changes, do you think the API here is reasonable to allow custom element authors to achieve the same thing? We could also open an issue with that precise question on the ARIA repository to get more input.

@alice
Copy link
Contributor

alice commented Jun 17, 2019

TL;DR - I think having a way to set a default role on a custom element would be nice, but we absolutely shouldn't exclude it from ElementInternals.

I do see role as typically an intrinsic, static property of an element, but in practice it does change if some property causes the element to be displayed in a different enough way that it is using a different UI metaphor altogether. Another example, beyond input, is <select>, which has totally different appearance and thus role depending on the values of multiple and/or size.

You could argue that custom elements which have a visual appearance should always map to a single UI metaphor, but that seems overly restrictive to me.

Also, given you can change element.role, I would expect authors would have some frustration and confusion at not having a parallel API for ElementInternals.

In Blink, we have code to handle a role changing by destroying and re-creating the associated accessibility node, effectively doing what the ARIA advice suggests authors do themselves. WebKit also has code to handle a role change.

It might be nice to have a way to set a default value for the value of role on ElementInternals via the custom element definition process[1]. However, I don't think we have much to gain by having that be the only way to set role.

--

[1] Perhaps that could also do something clever with other required ARIA attributes associated with that role? Though in practice, ARIA takes care of that by specifying default values where the attribute is not set.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 1, 2019
Adds a way to set default ARIA role, state & properties for custom
elements through ElementInternals. These can be overridden with setting
the ARIA attributes on the element directly.

See whatwg/html#4658 for spec PR.

Change-Id: I0caf6bc302445e48f4e0324513105eba3d6303a6
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 7, 2019
Adds a way to set default ARIA role, state & properties for custom
elements through ElementInternals. These can be overridden with setting
the ARIA attributes on the element directly.

See whatwg/html#4658 for spec PR.

Change-Id: I0caf6bc302445e48f4e0324513105eba3d6303a6
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 7, 2019
Adds a way to set default ARIA role, state & properties for custom
elements through ElementInternals. These can be overridden with setting
the ARIA attributes on the element directly.

See whatwg/html#4658 for spec PR.

Change-Id: I0caf6bc302445e48f4e0324513105eba3d6303a6
@annevk
Copy link
Member

annevk commented Sep 25, 2019

I find it a bit confusing that the definition of the attributes defined at https://w3c.github.io/aria/#dom-ariaattributes use "reflect" whereas here we use a different definition. It seems there should be some kind of abstraction that allows the object that includes the mixin to decide how they work and then the "reflect" definition can be used for elements (maybe, see w3c/aria#1058) and this definition can be used for internals.

@domenic
Copy link
Member Author

domenic commented Sep 25, 2019

That's actually exactly what it does :). See the linked ARIA PR which sets up that infrastructure.

gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 4, 2019
…lementInternals, a=testonly

Automatic update from web-platform-tests
Add ARIA role, state and properties to ElementInternals

Adds a way to set default ARIA role, state & properties for custom
elements through ElementInternals. These can be overridden with setting
the ARIA attributes on the element directly.

See whatwg/html#4658 for spec PR.
Explainer: https://github.com/alice/aom/blob/gh-pages/explainer.md#per-instance-dynamic-semantics-via-the-elementinternals-object
Intent to Implement: https://groups.google.com/a/chromium.org/d/msg/blink-dev/b-cGz9c67pM/0zvBzjhrAAAJ

Change-Id: I0caf6bc302445e48f4e0324513105eba3d6303a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1709950
Commit-Queue: Rakina Zata Amni <rakinachromium.org>
Reviewed-by: Alice Boxhall <aboxhallchromium.org>
Reviewed-by: Kent Tamura <tkentchromium.org>
Cr-Commit-Position: refs/heads/master{#685141}

--

wpt-commits: 5c61f2493fe744dc9f3d08e1f5a80165f574cfa3
wpt-pr: 18223

UltraBlame original commit: a95eec38bb25d830da2c2414356d5904c8e99d6d
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 4, 2019
…lementInternals, a=testonly

Automatic update from web-platform-tests
Add ARIA role, state and properties to ElementInternals

Adds a way to set default ARIA role, state & properties for custom
elements through ElementInternals. These can be overridden with setting
the ARIA attributes on the element directly.

See whatwg/html#4658 for spec PR.
Explainer: https://github.com/alice/aom/blob/gh-pages/explainer.md#per-instance-dynamic-semantics-via-the-elementinternals-object
Intent to Implement: https://groups.google.com/a/chromium.org/d/msg/blink-dev/b-cGz9c67pM/0zvBzjhrAAAJ

Change-Id: I0caf6bc302445e48f4e0324513105eba3d6303a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1709950
Commit-Queue: Rakina Zata Amni <rakinachromium.org>
Reviewed-by: Alice Boxhall <aboxhallchromium.org>
Reviewed-by: Kent Tamura <tkentchromium.org>
Cr-Commit-Position: refs/heads/master{#685141}

--

wpt-commits: 5c61f2493fe744dc9f3d08e1f5a80165f574cfa3
wpt-pr: 18223

UltraBlame original commit: a95eec38bb25d830da2c2414356d5904c8e99d6d
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 4, 2019
…lementInternals, a=testonly

Automatic update from web-platform-tests
Add ARIA role, state and properties to ElementInternals

Adds a way to set default ARIA role, state & properties for custom
elements through ElementInternals. These can be overridden with setting
the ARIA attributes on the element directly.

See whatwg/html#4658 for spec PR.
Explainer: https://github.com/alice/aom/blob/gh-pages/explainer.md#per-instance-dynamic-semantics-via-the-elementinternals-object
Intent to Implement: https://groups.google.com/a/chromium.org/d/msg/blink-dev/b-cGz9c67pM/0zvBzjhrAAAJ

Change-Id: I0caf6bc302445e48f4e0324513105eba3d6303a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1709950
Commit-Queue: Rakina Zata Amni <rakinachromium.org>
Reviewed-by: Alice Boxhall <aboxhallchromium.org>
Reviewed-by: Kent Tamura <tkentchromium.org>
Cr-Commit-Position: refs/heads/master{#685141}

--

wpt-commits: 5c61f2493fe744dc9f3d08e1f5a80165f574cfa3
wpt-pr: 18223

UltraBlame original commit: a95eec38bb25d830da2c2414356d5904c8e99d6d
bors-servo pushed a commit to servo/servo that referenced this pull request Feb 13, 2020
sketching ElementInternals

Form-associated elements are now working.

Remaining test failures in custom-elements/form-associated/ are:
* whatwg/html#4658 hasn't made it into the standard yet, and ElementInternals-accessibility is all about that functionality.
* ElementInternals-setFormValue hits many race conditions with our iframe order of operations. Isolating just the "submit this form with these custom elements, look at the query" part, the payload of each test actually does pass! Running the tests as written, we end up with different tests overwriting each other's elements and getting very confused. One problem is the about:blank double-load-event thing, but fixing that doesn't fix everything.
* ElementInternals-validation doesn't work because we don't have form validation in general; I've commented the relevant stubs.
* ElementInternals-form-disabled-callback fails on #25713.
@hober
Copy link
Contributor

hober commented Mar 11, 2020

Note that Mozilla considers this worth prototyping: mozilla/standards-positions#201

@rniwa
Copy link

rniwa commented Aug 28, 2020

@rniwa given the above input from accessibility folks about how native elements often experience role changes, do you think the API here is reasonable to allow custom element authors to achieve the same thing? We could also open an issue with that precise question on the ARIA repository to get more input.

Sorry for the delay. I had been out on a medical leave for 4 months. We had some extensive internal discussions about this, and we now believe this is a pretty reasonable API. We'd like to see some sugar for defining the default role for a class of custom elements (e.g. maybe a static variable on class) but that could be tackled separately.

@domenic domenic force-pushed the domenic/elementinternals-a11y branch from e32a070 to e95cc26 Compare August 28, 2020 19:41
@domenic
Copy link
Member Author

domenic commented Aug 28, 2020

Awesome, thank you @rniwa! I've rebased this on master. It's still blocked on w3c/aria#984, but otherwise it should be ready to merge; any review is appreciated.

@domenic domenic requested a review from annevk August 28, 2020 19:42
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
@annevk
Copy link
Member

annevk commented Sep 7, 2020

cc @whatwg/a11y @whatwg/components (unclear if the OP notification worked as the bot that modifies OP isn't able to ping these groups)

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

Found two possible nits and pushed a fixup for the remainder.

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility Affects accessibility addition/proposal New features or enhancements do not merge yet Pull request must not be merged per rationale in comment topic: custom elements Relates to custom elements (as defined in DOM and HTML)
Development

Successfully merging this pull request may close these issues.

10 participants