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

Add ARIA property string reflection on Element #691

Closed
cookiecrook opened this issue Jan 25, 2018 · 54 comments
Closed

Add ARIA property string reflection on Element #691

cookiecrook opened this issue Jan 25, 2018 · 54 comments
Assignees

Comments

@cookiecrook
Copy link
Contributor

cookiecrook commented Jan 25, 2018

Reflection for ARIA has been discussed a number of times over the years.

Summarizing the general concept of attribute-to-property reflection.

In HTML and other specs, you'll see language such as: ~"The tabIndex DOM property reflects the tabindex content attribute." (Note the camel casing difference for this example.)

What that means is that string value of the DOM Property (myDivElement.tabIndex) always remains in sync with the string value of the Content Attribute (<div tabindex="0">). Initial value in the content attribute is also in the DOM property. Any changes to one are reflected in the other, and vice versa.

There are a number of examples in HTML... invalid, required, etc. There are special name change cases like class/className (class is a reserved keyword in most functional programming languages, etc.) and camelCased examples like tabindex/tabIndex.

Relevance to ARIA

The ARIA Specs have only ever defined Content Attributes, but never DOM properties. This biggest impact of this change would mean that in addition to using the standard attribute manipulation:

// setters
el.setAttribute('role', 'button');
el.setAttribute('aria-label', 'Make it so!');

// getters
el.getAttribute('role'); // returns 'button'
el.getAttribute('aria-label'); // returns 'Make it so!'

…a JavaScript author could also use the direct accessor property shorthand.

// setters
el.role = 'button';
el.ariaLabel = 'Make it so!';

// getters
el.role; // returns 'button'
el.ariaLabel; // returns 'Make it so!'

To my knowledge, every time ARIA reflection has been proposed, it's been met with positive response, but for whatever reason, it's never happened. My recollection is that everyone thinks this is a good idea, but no one was sure what spec it should land in.

The ARIA Spec seems like the most logical place to me, and if the WG agrees, I'm prepared to write the spec portions in a Git branch. I'll fill in some slightly less important contextual info in comments below.

@cookiecrook cookiecrook self-assigned this Jan 25, 2018
@cookiecrook
Copy link
Contributor Author

Note: There would need to be a table for the few examples requiring disambiguation or name change exceptions.

Most notably, no one (including the editors over the years) seems to agree if the aria-posinset attribute refers to a two-word concept ("position in set") or a three-word concept ("position inset"). Due to the unfortunately ambiguous name, either could be correct, resulting in Element.ariaPosInSet or perhaps Element.ariaPosInset. The working group would need to pick one, or a new name such as Element.ariaPositionIndex to document the reflection.

The spec would need to call out this and other exceptions to the general camel-case attribute pattern. Hopefully that's a good exercise, which may help avoid ambiguously or inconsistently named attributes in future versions of ARIA.

@cookiecrook
Copy link
Contributor Author

cookiecrook commented Jan 25, 2018

Only one new implementation of ARIA reflection will be required for CR, because an existing implementation exists already in older versions of Internet Explorer. It was eventually disabled in IE because 1) it was never retroactively added to the spec, and 2) developers often assumed IE's non-standard accessors would work in all browsers.

Nevertheless, implementation of string reflection is trivial in most rendering engines, so I don't expect implementation support to be a problem once it's in a working draft.

@cookiecrook
Copy link
Contributor Author

cookiecrook commented Jan 25, 2018

Clarifying term definitions for the issue prose above.

In most specs, the term "property" has a different meaning than ARIA's reference to "states and properties." In most programming languages, a property is a variable that is associated with an object. As an incomplete but brief definition, "DOM property" always refers to accessor use in JavaScript (Element.tabIndex = 0;), and "content attribute" always refers to markup (<element tabindex="0">).

@carmacleod
Copy link
Contributor

This would be really useful. Trying to set element role with el.role is a source of error for many a javascript developer new to aria.

My vote for aria-posinset would be ariaPosInSet, because the way it's used ("1 of 3", "2 of 3", ...) doesn't really have anything to do with "inset", which is usually the number of pixels in a margin.

@cookiecrook
Copy link
Contributor Author

The AOM WICG Incubator group has mostly agreed that AOM could be greatly reduced in scope if ARIA Reflection is accepted, so this is of great interest to that group, too.

@cookiecrook
Copy link
Contributor Author

cookiecrook commented Jan 25, 2018

@carmacleod wrote:

My vote for aria-posinset would be ariaPosInSet, because the way it's used ("1 of 3", "2 of 3", ...) doesn't really have anything to do with "inset", which is usually the number of pixels in a margin.

'inset' can also be used to mean the relative offset inside a collection of items. Regardless if the intention is "InSet" or "Inset", some web authors are going to be confused by this. My vote is to avoid both terms, but I could live with just documenting that it's "ariaPosInSet" not "ariaPosInset".

@carmacleod
Copy link
Contributor

'inset' can also be used to mean the relative offset inside a collection of items

Yes, but on the web, 'inset' is more likely to mean pixels for borders, outlines, shadows.

Another argument for ariaPosInSet is that the spec for aria-posinset says "Authors SHOULD use aria-setsize", which would be reflected as ariaSetSize, which is the "Set" that the author is enumerating. Using "Set" for both attributes semantically ties them together more clearly.

In general, I think the current aria-* attributes should be reflected verbatim, except for camel case and removing the hyphen, so that devs have less to memorize. Unless there's a keyword reason, like, as you mentioned, class/className or for/htmlFor. It's strange enough to have to switch to camel case (I have messed up tabindex and tabIndex several times ;), let alone trying to remember word differences as well. :)

@cookiecrook
Copy link
Contributor Author

@carmacleod wrote:

Yes, but on the web…

We're getting off topic a bit. aria-posinset was just an example. It's likely to be one of many that needs documenting for clarity regardless of how the pattern or IDL is defined.

This [ARIA Reflection] would be really useful. Trying to set element role with el.role is a source of error for many a javascript developer new to aria.

Great, thanks. I also got a preliminary thumbs-up from @joanmarie and @michael-n-cooper so I'll take a stab at this soon.

@carmacleod
Copy link
Contributor

Sorry about going off topic. I tried to tie it back in to the topic with that last paragraph in the comment:

In general, I think the current aria-* attributes should be reflected verbatim ...

@asurkov
Copy link
Contributor

asurkov commented Jan 26, 2018

There's a downside of pouring all aria properties right on Element, which is a hundred of new properties prefixed by 'aria'. It will make all other properties less discoverable, so for example, 'autocomplete' and 'activeElement' for 'a' filter will be lost.

'aria' property on Element encapsulating all aria properties could make it look nicer, for example:

inputEl.aria.invalid = true;
inputEl.aria.labelledBy = labelEl;

@cookiecrook
Copy link
Contributor Author

@asurkov wrote:

There's a downside of pouring all aria properties right on Element, which is a hundred of new properties prefixed by 'aria'.

There are currently 48 aria-* attributes. If you count role, there are 49.

It will make all other properties less discoverable, so for example, 'autocomplete' and 'activeElement' for 'a' filter will be lost.

FWIW, I don't think this is the strongest part of your case. People can scroll before or after the "aria" chunk of the list and not miss anything.

'aria' property on Element encapsulating all aria properties could make it look nicer, for example: inputEl.aria.invalid = true;

I don't have a strong preference for el.ariaLabel over el.aria.label except that there is precedent for the former. If a similar precedent exists for the latter, I don't mind the syntax change. What do you think @rniwa, @minorninth, @alice, and @robdodson?

@alice
Copy link

alice commented Jan 29, 2018

I don't have a strong preference for el.ariaLabel over el.aria.label except that there is precedent for the former. If a similar precedent exists for the latter, I don't mind the syntax change.

I generally agree, but I seem to recall @domenic had strong objections.

The closest we have to a precedent AFAIK is dataset, which I also seem to recall was controversial.

@domenic
Copy link
Contributor

domenic commented Jan 29, 2018

It's quite important to follow the precedent we have for other attributes. This e.g. allows it to work with frameworks like React or Angular which depend on the existing attribute/properly reflection semantics. Introducing a new object whose lifetime is coupled to the element would make it unusable by these frameworks, for no real gain, and additional code complexity of the type we saw with the unfortunate dataset and style properties.

@rniwa
Copy link

rniwa commented Jan 29, 2018

Adding all these properties to both real Element/Node and a virtual AccessibleNode seems a lot worse than a hypothetical need for those libraries to support reflection without any updates to them. In addition, I'm a lot more concerned about backwards compatibility implications on introducing 45+ IDL attributes onto Element itself.

@cookiecrook
Copy link
Contributor Author

@rniwa wrote:

Adding all these properties to both real Element/Node and a virtual AccessibleNode

This is not about AOM or Node... Just Element.

In addition, I'm a lot more concerned about backwards compatibility implications on introducing 45+ IDL attributes onto Element itself.

I understand there is some historical reluctance to adding IDL attributes due to the pervasive misuse of expando attributes. That said, the proposed attributes are all "aria"-prefixed so the risk of backwards incompatibility seems relatively low.

@asurkov
Copy link
Contributor

asurkov commented Jan 29, 2018 via email

@cookiecrook
Copy link
Contributor Author

@domenic
Copy link
Contributor

domenic commented Feb 23, 2018

Good start! However, keep in mind you'll be using https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#reflect, and need to stick to those rules. So for example, boolean? has no valid reflection; only boolean does, and that's for present/not-present attributes. I think you'll want DOMString or DOMString? for those. Similarly, short and float are not valid reflected attribute types.

I also don't see any indication that the attributes which are reflected as DOMString? are enumerated attributes, which is a requirement for them to be reflected in that manner. E.g. https://www.w3.org/TR/wai-aria-1.1/#aria-disabled does not define keywords, states, invalid value defaults, or missing value defaults. See, for example, https://html.spec.whatwg.org/multipage/image-maps.html#attr-area-shape. (This has normative implications; namely, what does aria-disabled="TRUE" do? What about aria-disabled="asdf"? I don't see any spec that governs that right now.)

@cookiecrook
Copy link
Contributor Author

Were you thinking the ARIA boolean-like types and other enumerables could be done this way?

enum AriaTrueFalse { "false", "true" }; // boolean ("false" is default, but not sure how to specify that.)
enum AriaTrueFalseUndefined { "", "false", "true" }; // boolean? (empty string is default)
enum AriaAutoCompleteValues { "none", "inline", "list", "both" }; // "none" is default

interface mixin AriaAttributes {
    attribute DOMString? ariaActiveDescendant;
    attribute AriaTrueFalse ariaAtomic;
    attribute AriaAutoCompleteValues ariaAutoComplete;
    ...
};
Element includes AriaAttributes;

@domenic
Copy link
Contributor

domenic commented Feb 23, 2018

Not quite. Check out the pattern used for, e.g., <area>'s shape, as I linked to. You just use DOMString or DOMString?; no need for an IDL enum. The more important part is laying the foundation by properly defining the keywords, states, invalid value defaults, and missing value defaults for each attribute that gets reflected. Once that foundation is in place, all you have to do is say "The ariaAtomic content attribute reflects the aria-atomic content attribute," and the fact that ariaAtomic is a DOMString and aria-atomic is an enumerated attribute with the appropriate definitions links everything together.

@cookiecrook
Copy link
Contributor Author

cookiecrook commented Feb 23, 2018

@domenic wrote:

Check out the pattern used for, e.g., 's shape, as I linked to. You just use DOMString or DOMString?; no need for an IDL enum. The more important part is laying the foundation by properly defining the keywords, states, invalid value defaults, and missing value defaults for each attribute that gets reflected.

The ARIA spec already defines the value types including token and token list types which are similar to enum. See the characteristics table and token definitions on aria-autocomplete for example.

Once that foundation is in place, all you have to do is say "The ariaAtomic content attribute reflects the aria-atomic content attribute," and the fact that ariaAtomic is a DOMString and aria-atomic is an enumerated attribute with the appropriate definitions links everything together.

I think that's what I've done. Note the normative MUST statements near the Role IDL and just after the AriaProperties IDL. The <sref> and <pref> tags you see in the branch diffs are using a ReSpec conversion to linkify those to their content attribute definitions.

@domenic
Copy link
Contributor

domenic commented Feb 23, 2018

The ARIA spec already defines the value types including token and token list types which are similar to enum. See the characteristics table and token definitions on aria-autocomplete for example.

Sure, but this doesn't meet the requirements for an attribute that's being reflected. For those you need to define states, keywords, invalid value default, and missing value default, and explicitly use the phrase "enumerated attribute".

I think that's what I've done.

Right, those sentences look good. It's the prerequisites that are missing: properly defining enumerated attributes, and using the correct types in the IDL.

@cookiecrook
Copy link
Contributor Author

Sure, but this doesn't meet the requirements for an attribute that's being reflected. For those you need to define states, keywords, invalid value default, and missing value default, and explicitly use the phrase "enumerated attribute".

Okay thanks. I understand now. @joanmarie @michael-n-cooper would you like @domenic's remarks tracked as separate issues, or would you like me to attempt completing this in the same branch? At a minimum, it would require edits to the value types section and any of the attributes using Token/TokenList. I'd rather approach them as individual issues/edits once the reflection branch is in.

I don't foresee any harm in resolving those after the IDL edit is added, since the implementations treat most of these as ENUMs anyway. If anything, adding the IDL may help us flesh out where/if we have remaining spec ambiguities.

@domenic
Copy link
Contributor

domenic commented Feb 23, 2018

Well, the issue is that this is actually a prerequisite for the IDL work, because without it, the IDL attributes don't have coherent definitions. That is, it would be impossible to implement the reflection, since the spec would say (per the sentences you point out) "reflect X as Y", but there are no rules for reflecting X as a Y (since X is not an enumerated attribute).

@asurkov
Copy link
Contributor

asurkov commented Feb 23, 2018

@cookiecrook Have you considered to extend a number of supported types for attributes to keep them more flexible? For example, ariaLabelledBy would benefit if it returned a list of IDRef strings instead a DOMString. Or if ariaLabelledBy accepted an element instead string, that the user could avoid of putting a lot of IDs into a document to define relationships.

@domenic
Copy link
Contributor

domenic commented Feb 23, 2018

For example, ariaLabelledBy would benefit if it returned a list of IDRef strings instead a DOMString.

I don't understand this. DOMString is already the most general type of string...

@asurkov
Copy link
Contributor

asurkov commented Feb 23, 2018

Current proposal is <div aria-labelledby="id1 id2">.ariaLabelledBy should return "id1 id2" string. My point is it should be nicer to have DOMStringList or DOMTokenList instead a generic DOMString, so the user doesn't have to to parse strings to change IDs.

@alice
Copy link

alice commented Feb 27, 2018

The current proposal is that the IDREF and element reference properties live side-by-side without interfering with each other:

// Reflects to "aria-labelledby" DOM attribute
button.ariaLabelledBy = "id1";

// DOM result: <button aria-labelledby="id1">

// Takes precedence; does not affect reflected attribute value
el.ariaLabelledByElements = [el2, el3];

// DOM remains unchanged: <button aria-labelledby="id1">

@cookiecrook
Copy link
Contributor Author

@domenic wrote:

DOMTokenList is ordered

If that's the case, DOMTokenList.add() should probably have an optional index parameter, or DOMTokenList should have a logical way to do splicing. Regardless, it seems like DOMTokenList will probably work for role and IDREFS values until a more specific type comes along. Thanks all.

@asurkov
Copy link
Contributor

asurkov commented Feb 27, 2018 via email

@alice
Copy link

alice commented Feb 27, 2018

@asurkov Having htmlForElements would allow label relationships to go across shadow DOM boundaries, for one.

@asurkov
Copy link
Contributor

asurkov commented Feb 27, 2018 via email

@alice
Copy link

alice commented Feb 27, 2018

Got it.

I think we discussed this at length previously and decided that trying to handle IDREF<->Element reflection had too many issues, and that it was simpler not to try and reflect element references out to string attributes but instead have the parallel properties instead.

@carmacleod
Copy link
Contributor

@alice Do you have a pointer to that discussion, by chance? Just want to try to understand the issues.

@alice
Copy link

alice commented Feb 28, 2018

@carmacleod It happened in face to face discussions, but I think it boiled down to issues of fidelity - an IDREF may be (temporarily or permanently) invalid which is not the same thing as not having been set at all, and an element may not have an ID.

@asurkov is right, if we were reflecting to/from fooElements we would certainly need to come up with an answer for what would happen if one or more of the elements didn't have an ID. I had a go at writing up some rules a while ago: https://gist.github.com/alice/c684bd031adcd0483d0306d95c32d0e9/e200d83589cc17eb9a4118622b2b635393030bbc#relationship-reflection

@cookiecrook
Copy link
Contributor Author

PR #708

@carmacleod
Copy link
Contributor

carmacleod commented Apr 2, 2018

@cookiecrook Where did the DOMTokenList changes go? They're missing from PR #708. I assume we still want DOMTokenList for role, ariaControls, ariaDescribedBy, ariaFlowTo, ariaLabelledBy, ariaOwns, ariaRelevant, and ariaSort?

It's definitely ordered. W3C spec for DOMTokenList says:

A DOMTokenList object has an associated ordered set of tokens, which is initially empty.

Update: Sorry - never mind. For whatever reason, when I looked at #708 earlier the DOMTokenList changes were missing. They're good now. Not sure what happened - may have been some caching thing - anyhow, disregard this comment.

@cookiecrook
Copy link
Contributor Author

If you notice one you think should be changed, click on the pull request line number in the diff and comment directly.

@carmacleod
Copy link
Contributor

I would, but they're all good. When I initially clicked on that first commit ("IDL attribute reflection branch") those 8 attributes were all DOMString for me. But when I looked again several hours later (after the other 2 commits) they all magically went back to being DOMTokenList attributes. It was sort of "twilight zone". It's all good now. :)

@cookiecrook
Copy link
Contributor Author

cookiecrook commented May 2, 2018

I updated PR #708 based on some new findings. Summarizing here.

  • Minor prose updates.
  • Moved the [Reflect] definitions back into the IDL.
  • Changed all long values to DOMString, since long cannot be nullable.
  • Changed all double values to DOMString, since double-to-string conversion is lossy and imprecise.
  • Changed all DOMTokenList values to DOMString (long justification below).

DOMTokenList values changed to DOMString because there is no IDL-only way to reflect DOMTokenList into a string content attribute. Options seem to be:

  1. Leave all as reflected DOMString values (as the PR is currently).
  2. Make some of them (including 'role' and 'aria-labelledby') non-reflected DOMTokenList values.
  3. Make some of them (including 'role' and 'aria-labelledby') readonly DOMTokenList values, with custom getters and setters so they behave like reflected strings attributes. In other words, you could use the add() and remove() methods, but still get and set the string value directly, but there'd be no getter for the actual DOMTokenList.

Of the three, it's likely 1 and 3 are the only options authors would be happy with, and the editor's discussion is leaning toward option 1 (DOMString only, no DOMTokenList) b/c The implementation is more simple and DOMTokenList does not provide much value for ARIA attrs:

  • DOMTokenList has no ordered splice() or insertBefore() method on DOMTokenList. This would provide minor value for ARIA but seems irrelevant for classList.
  • DOMTokenList's add(), remove(), toggle(), and contains() methods provide value for classList but don’t provide much for ARIA-related usage.

Discussed with the AOM editors. @minorninth replied:

I don't think DOMTokenList gains us much. In fact I think it'd be far less intuitive for role, since 99% of the time people just pass a single string. So my vote is for reflected DOMString.

So we're back to something closer to the original PR: All attributes optional (nullable), non-readonly, reflected DOMString values.

@domenic
Copy link
Contributor

domenic commented May 2, 2018

What does [Reflect] mean? It's not defined in any specification.

I'm surprised you're departing from the pattern used in the HTML spec and other specs that define content attributes.

@domenic
Copy link
Contributor

domenic commented May 2, 2018

I don't understand the DOMTokenList options. In particular this "IDL-only way" constraint doesn't make sense to me. Why not reflect these as you would for every other DOMTokenList?

Note also that DOMTokenList properties can be treated as strings:

el.classList = "foo"; // works fine
const string = el.classList + " bar"; // works fine; implicit conversion to string
const string2 = el.classList.toString(); // works fine; explicit conversion to string

@cookiecrook
Copy link
Contributor Author

cookiecrook commented May 2, 2018

What does [Reflect] mean?

Is this a rhetorical question? I can find a number of places you reference this feature, including:
domenic/html-as-custom-elements#31

It's also used and documented by a number of vendors, including Blink. If you're trying to make a point that the spec hasn't caught up yet, or that it should not be used for some reason, please be more direct. If there's another point I'm missing, please explain. Thanks.

@domenic
Copy link
Contributor

domenic commented May 2, 2018

Right, it's an implementer-specific technology, not part of standards. I am genuinely curious what definition you are using, because each implementer has their own version, and none of them are standardized. (That includes my own toy "implementation" in html-as-custom-elements.)

I do anticipate there is work to do here if you plan to use them in a standard; e.g. we've debated for a long time some potential semantics in whatwg/html#3238, and if you're willing to tackle that issue with all the complexity noted therein as a prerequisite to moving forward on this project, that would be great indeed. Was that your plan?

@cookiecrook
Copy link
Contributor Author

Thanks for being direct in the last comment. I'll pull those from the IDL. The reflection requirements remain in prose so no further modification will be needed. DOMTokenList discussion remains ongoing, too.

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

No branches or pull requests

6 participants