Ember.js: [Bug] Attributes and ARIA

Created on 23 Jun 2020  路  7Comments  路  Source: emberjs/ember.js

馃悶 Describe the Bug

Currently, ...attributes does not have a guaranteed merge order, and this is problematic for five ARIA attributes that can have multiple values in the form of an ID reference list and as such, the order of these values matters.

These are the attributes:

馃敩 Minimal Reproduction

To reproduce, follow these steps:

  1. create an addon with a component that uses aria-labelledby or aria-describedby (these will be the simplest ones to demonstrate the behavior). An input field with a label and some associated help text (via aria-describedby) is probably a good simple use case.
  2. give the aria-describedby attribute a value
  3. use ...attributes to ensure that when the component is used, aria-describedby can be modified
  4. use the addon in an app, and put the component in a template
  5. add additional values to aria-describedby
  6. observe the rendered value of aria-describedby in the browser

馃槙 Actual Behavior

The value gets clobbered.

馃 Expected Behavior

I expect that ...attributes provides support for these five ARIA attributes the same way that it does for the class attribute.

a11y

Most helpful comment

To add a bit of technical detail here:

In my view, the rationale for making the change for these attributes is that, according to the spec, the attribute corresponds to a list, rather than a single value.

image

Because the value is specified (and used) as a list, we should apply the rule we used for class (the class attribute is specified as "a set of space separated tokens").

The same logic does not apply cleanly to the style attribute, which is specified to have a special kind of value defined by the style attribute specification. That specification requires custom parsing of the value, which complicates the meaning of merging.


One minor point: while the class attribute is defined as a "set of space-separated tokens", the ARIA spec defines "ID reference list" as a "list of space-separated tokens":

The value of one or more defined聽id attributes聽on other element(s), represented as聽Space-separated tokens

Elsewhere, the ARIA spec is explicit that multiple instances of the same value are allowed:

The list value types鈥擨D reference list and token list鈥攁llow more than one value of the given type to be provided.

This means that merging attributes whose values are defined as "ID reference list" should make sure to use an implementation that does not assume set semantics.


As to whether or not this counts as a bug, I think you can argue it either way (based on the above analysis of the way the values are defined in the spec), but I also think that's a question that doesn't prevent us from continuing to iterate on this proposal.

From my perspective, the next steps is to identify any other browser attributes whose values are specified as lists, and decide what timeframe (and rollout strategy) we should use to fix the issue for those attributes.


TL;DR I think that we should base our merging behavior on an intuitive understanding of what it means to merge the specified definition of the attribute. When you "merge two lists", you concatenate them, not have the second one replace the first.

All 7 comments

@chancancode please let me know if I can provide more context. Hopefully this is enough based on our other discussions on the topic. Thank you for being willing to look into this!

FWIW, I don't think this is a bug. I do absolutely agree that we should add a feature to control ordering / merging behaviors.

Also, please note that the way this issue is resolved today is to duplicate any of the internal values at the callsite, which means that adding automatic merging behavior for these attributes would be a breaking change.

FWIW, I don't think this is a bug. I do absolutely agree that we should add a feature to control ordering / merging behaviors.

@rwjblue It could be useful to dig in a bit more; can you help me understand why you don't see this as a bug?

To add a bit of technical detail here:

In my view, the rationale for making the change for these attributes is that, according to the spec, the attribute corresponds to a list, rather than a single value.

image

Because the value is specified (and used) as a list, we should apply the rule we used for class (the class attribute is specified as "a set of space separated tokens").

The same logic does not apply cleanly to the style attribute, which is specified to have a special kind of value defined by the style attribute specification. That specification requires custom parsing of the value, which complicates the meaning of merging.


One minor point: while the class attribute is defined as a "set of space-separated tokens", the ARIA spec defines "ID reference list" as a "list of space-separated tokens":

The value of one or more defined聽id attributes聽on other element(s), represented as聽Space-separated tokens

Elsewhere, the ARIA spec is explicit that multiple instances of the same value are allowed:

The list value types鈥擨D reference list and token list鈥攁llow more than one value of the given type to be provided.

This means that merging attributes whose values are defined as "ID reference list" should make sure to use an implementation that does not assume set semantics.


As to whether or not this counts as a bug, I think you can argue it either way (based on the above analysis of the way the values are defined in the spec), but I also think that's a question that doesn't prevent us from continuing to iterate on this proposal.

From my perspective, the next steps is to identify any other browser attributes whose values are specified as lists, and decide what timeframe (and rollout strategy) we should use to fix the issue for those attributes.


TL;DR I think that we should base our merging behavior on an intuitive understanding of what it means to merge the specified definition of the attribute. When you "merge two lists", you concatenate them, not have the second one replace the first.

As I mentioned above (and explained in a recent A11Y team call), the simplest fix/change (to make these merge like class) would be a breaking change for anyone that _currently_ uses these attributes. This needs a more concrete proposal in order to mitigate that breakage, and IMHO that should come in the form of an RFC.

That list seems fairly small. If the list of attributes that have space separated tokens is limited (special cases like this can bloat code size) we should be consistent. I wish we had a syntax way of the user specifying the behavior declaratively and we had done that for classes. Like having a placeholder <div ...attributes class="some-class *"> or <div ...attributes class="+some-class -other"> or <div ...attributes +class="some-class"> but I missed the boat on that one and I'm not sure how to do any of these in a backwards compatible way. I'm not even sure if you can remove a class. Would not having the ability to remove one of them ever be a problem here?

Would not having the ability to remove one of them ever be a problem here?

Should we compile some real use cases to make sure it works? Transition path aside, if there are a small list of list-based attributes then I agree we can make it work just fine, but it would be bad if we did it and it ends up being not useful, or even making the problem worse.

Today, the attributes are clobbered, so the finally person that installs the attribute on an element could always have created an argument-based alternative API as an escape value, and combine them however they see fit. If we make this automatically merge, then they would lose that level of control (other than not using ...attributes I guess), so we better make sure it actually does satisfy the use cases before committing to the change.

An RFC sounds good, and the use cases will probably be needed there to show why we are doing it.

I also mostly agree with @krisselden but I also agree that ship has mostly sailed. If we do think there are some plausible path there (or that those features are more needed/important for the ARIA cases) then we perhaps may want to think about that a little more.

Was this page helpful?
0 / 5 - 0 ratings