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

updates per discussion in #1598 #1633

Merged
merged 4 commits into from
Nov 12, 2021
Merged

updates per discussion in #1598 #1633

merged 4 commits into from
Nov 12, 2021

Conversation

jnurthen
Copy link
Member

@jnurthen jnurthen commented Nov 2, 2021

resolves #1598


Preview | Diff

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@jnurthen
Copy link
Member Author

jnurthen commented Nov 9, 2021

@cookiecrook @aleventhal @cyns @domenic @annevk
Can folks please review this and let us know if this will work.

@domenic
Copy link
Contributor

domenic commented Nov 10, 2021

So I think the reflection text looks good. However it exposes a confusion that makes the spec kind of hard to understand, which is that you talk a lot about "ARIA enumerated attributes". But these do not match HTML's notion of enumerated attribute.

This is most stark in how the new reflection text says

If a reflecting IDL attribute is a nullable DOMString attribute whose content attribute is not an enumerated attribute

but then apparently the goal is to apply this reflection algorithm to the various ARIA attributes, per

When the ARIA attribute definition includes a table enumerating the attribute's allowed values, that attribute is a nullable enumerated attribute

Another confusing issue is that "nullable enumerated attribute" is confusing content attributes and IDL attributes. IDL attributes can be nullable, and content attributes can be enumerated, but which does "nullable enumerated" apply to?

It would be best to remove all mention of "enumerated attribute" and "nullable enumerated attribute" from this document, I think. Maybe replace it with a new term like "multi-value ARIA attribute"?

@domenic
Copy link
Contributor

domenic commented Nov 10, 2021

Stated from another perspective: HTML already defines reflection algorithms for nullable IDL attributes that reflect enumerated content attributes. They perform validation/canonicalization.

You want to use a different algorithm that does not perform validation/canonicalization. And you want to use a nullable IDL attribute. Thus, your content attributes must not be enumerated attributes; they must be some new category of thing (which I proposed calling "multi-value ARIA attribute").

@domenic
Copy link
Contributor

domenic commented Nov 11, 2021

If I search https://pr-preview.s3.amazonaws.com/w3c/aria/pull/1633.html for "enumerated attribute" 18 instances remain. I think all of them need to be changed...

Copy link
Contributor

@domenic domenic left a comment

Choose a reason for hiding this comment

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

Love it!

<p>When the ARIA attribute definition includes a table enumerating the attribute's allowed <span>values</span>,
that attribute is a nullable <a data-cite="html/common-microsyntaxes.html#enumerated-attribute">enumerated attribute</a>.
Each value in the table is a keyword for the attribute, mapping to a state of the same name. </p>
<section>
Copy link
Contributor

Choose a reason for hiding this comment

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

I see why you moved the "Multi-Value" section down to an H3 sub-section, but should this higher-level section get a new IDREF? (No strong preference. Approving as-is.)

@jnurthen jnurthen merged commit bd03476 into main Nov 12, 2021
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

Successfully merging this pull request may close these issues.

Updating ARIA 1.2 due to IDL implementations (exit and re-enter CR?)
4 participants