- Notifications
You must be signed in to change notification settings - Fork143
ARIA IDL updates: convert eligible attributes to enumerated, new ARIA IDL guidance and examples#2413
ARIA IDL updates: convert eligible attributes to enumerated, new ARIA IDL guidance and examples#2413
Conversation
netlifybot commentedJan 24, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
✅ Deploy Preview forwai-aria ready!
To edit notification comments on pull requests, go to yourNetlify site configuration. |
…ibutes', and remove note referencing true/false
…quirement to not reflect missing/invalid value default
rahimabdi commentedFeb 26, 2025
Adding this PR for discussion at the Feb 27 2025 ARIA WG meeting. |
Uh oh!
There was an error while loading.Please reload this page.
keithamus left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@rahimabdi I've left a couple of review notes which I think apply in a lot of places, but rather than inundating you with review comments hopefully you can take the couple here and pattern match across the rest of the spec.
| </tr> | ||
| </tbody> | ||
| </table> | ||
| <p>The attribute's <a data-cite="html/common-microsyntaxes.html#missing-value-default">missing value default</a> and <a data-cite="html/common-microsyntaxes.html#invalid-value-default">invalid value default</a> are both the Missing state.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
HTML has a concept of states that don't map to a keyword, but each state must have a definition, the definition allows mapping back to a keyword, or null. As an example thepopover attribute has a missing value default ofno-popover state which let's the spec say things like "if the popover attribute is in the no-popover state".
Additionally, traditional enumerated attributes will map an invalid state back to a particular keyword; for examplepopover's invalid attribute state is themanual state, which maps to the keyword"manual". It's important to know what the invalid state maps back to because it determines what value is returned in the IDL for those values.
Note: not all missing values return null (e.g. thekind attribute on<track> elements has a missing value default of the subtitles state, so a track's.kind property returns"subtitles" even if the attribute is not present, another example is thetype attribute on<input> which defaults to"text").
With that said it's possible to be in "no state". In fact, an enumerated attribute without a missing value default will be in "no state", as will invalid. No state, AIUI, will always map to null.
So here, the simplest thing for us to do here, rather than the implied missing state is to remove this, and lean on thelack of a missing/invalid state returning the no state. IOW this line could simply be removed:
| <p>The attribute's <a data-cite="html/common-microsyntaxes.html#missing-value-default">missing value default</a> and <a data-cite="html/common-microsyntaxes.html#invalid-value-default">invalid value default</a> are both the Missing state.</p> |
If you think missing the definitions create more ambiguity, then we can instead do something like:
| <p>The attribute's<adata-cite="html/common-microsyntaxes.html#missing-value-default">missing value default</a>and<adata-cite="html/common-microsyntaxes.html#invalid-value-default">invalid value default</a> are both the Missing state.</p> | |
| <p>The attribute has no<adata-cite="html/common-microsyntaxes.html#missing-value-default">missing value default</a>or<adata-cite="html/common-microsyntaxes.html#invalid-value-default">invalid value default</a>.</p> |
| </tr> | ||
| </tbody> | ||
| </table> | ||
| <p>The attribute's <a data-cite="html/common-microsyntaxes.html#missing-value-default">missing value default</a> and <a data-cite="html/common-microsyntaxes.html#invalid-value-default">invalid value default</a> are both the Undefined state.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The<tr> for the"undefined" keyword is missing a<th> to name its state. In other words there should be a line above L10848 with<th scope="row">Undefined</th>. (I cannot comment on L10848 sadly).
I don't think we want the missing value default here to return the Undefined state, that will mean the.ariaChecked property will return"undefined" whenaria-checked isn't on the element. I presume we want to returnnull instead? Browsers currently return null, so missing should be "no state", i.e. removed:
| <p>The attribute's<adata-cite="html/common-microsyntaxes.html#missing-value-default">missing value default</a> and<adata-cite="html/common-microsyntaxes.html#invalid-value-default">invalid value default</a> are both the Undefined state.</p> | |
| <p>The attribute has no<adata-cite="html/common-microsyntaxes.html#missing-value-default">missing value default</a> and its<adata-cite="html/common-microsyntaxes.html#invalid-value-default">invalid value default</a> are both the Undefined state.</p> |
rahimabdiMar 23, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Partially fixed in new PR (spacing, but not missing value default):https://github.com/w3c/aria/pull/2484/files.
| <tr> | ||
| <th scope="col">Value</th> | ||
| <th scope="col">Value (keyword)</th> | ||
| <th scope="col">State</th> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks like a mix of tabs/spaces here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Fixed in new PR:#2484.
| </tr> | ||
| </tbody> | ||
| </table> | ||
| <p>The attribute's <a data-cite="html/common-microsyntaxes.html#missing-value-default">missing value default</a> is the Undefined state, and its <a data-cite="html/common-microsyntaxes.html#invalid-value-default">invalid value default</a> is the Menu state.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Again, here, the missing value default being theUndefined state means.ariaHasPopup will return"undefined" which I don't think is desirable. Instead:
| <p>The attribute's<adata-cite="html/common-microsyntaxes.html#missing-value-default">missing value default</a> is the Undefined state, and its<adata-cite="html/common-microsyntaxes.html#invalid-value-default">invalid value default</a> is the Menu state.</p> | |
| <p>The attribute has no<adata-cite="html/common-microsyntaxes.html#missing-value-default">missing value default</a>, and its<adata-cite="html/common-microsyntaxes.html#invalid-value-default">invalid value default</a> is the Menu state.</p> |
spectranaut commentedMar 6, 2025
Discussed briefly in today's ARIA meeting:https://www.w3.org/2025/03/06-aria-minutes#cd2e |
pkra commentedMar 7, 2025
@rahimabdi I'm unable to help since this is on your fork. This looks like it will be a bit messy - very sorry! |
rahimabdi commentedMar 7, 2025
@pkra Would it be easier to create a fresh branch from current main? I can re-do all of the updates and copy over the comments. |
pkra commentedMar 10, 2025
@rahimabdi I suspect the answer is yes. Sorry again, also to@keithamus for creating additional review work. |
rahimabdi commentedMar 10, 2025
@pkra No worries, thanks! |
Uh oh!
There was an error while loading.Please reload this page.
Closes#2281
Closes#2279
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.
Preview |Diff