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

Update: fix inconsistency around aria-setsize=-1 #2341

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

scottaohara
Copy link
Member

@scottaohara scottaohara commented Sep 25, 2024

closes #1759
closes #2346
also closes w3c/core-aam#149

This PR adjusts the requirements for aria-setsize to fix the inconsistency between the ARIA and CORE AAM specs regarding the use of -1 as a value.

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.

  • "author MUST" tests:
  • "user agent MUST" tests:
  • Browser implementations (link to issue or commit):
    • WebKit:
    • Gecko:
    • Blink:
  • Does this need AT implementations?
  • Related APG Issue/PR:
  • MDN Issue/PR:

Preview | Diff

Copy link

netlify bot commented Sep 25, 2024

Deploy Preview for wai-aria ready!

Name Link
🔨 Latest commit 32e0608
🔍 Latest deploy log https://app.netlify.com/sites/wai-aria/deploys/67069cb8cf16df0009437f36
😎 Deploy Preview https://deploy-preview-2341--wai-aria.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

separates out aria-setsize from aria-posinset and aria-level - as the existing requirement of what to do with a 0 or -1 value made sense for the latter two attributes, but is what is causing the inconsistency for aria-setsize.

add new requirements for handling 0/-1 as an unknown/indeterminate size, and indicating user agents must not calculate a size if these values are used for setsize
@scottaohara scottaohara marked this pull request as ready for review September 25, 2024 14:58
core-aam/index.html Outdated Show resolved Hide resolved
Copy link
Contributor

@MelSumner MelSumner left a comment

Choose a reason for hiding this comment

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

This seems to resolve the confusion, thank you!

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

talking with @aleventhal about this some more, we should also add in what to do about if someone uses the empty string, or puts in literal "garbage" text strings.

So this can probably be
positive full numbers works as presently spec'd
0 and any negative number == indeterminate
anything else == undefined and thus ignore.

@aleventhal
Copy link
Contributor

So this can probably be positive full numbers works as presently spec'd 0 and any negative number == indeterminate anything else == undefined and thus ignore.

I know this is going to happen in a follow-up, but it's probably good if it's not too strict, e.g. spaces before and after the number are ok. I was going to do the following:

  1. Empty string or not present = undefined and thus ignore
  2. Pass it to a non-strict int conversion function, and comes back < 1, treat as follows:
    • aria-posinset or aria-level -> 1
    • aria-setsize -> -1 (indeterminate)

index.html Outdated Show resolved Hide resolved
add that aria-setsize/posinset should also calculate value as if the attributes were not set, if they were provided the empty string.

clarify that aria-setsize should return indeterminate for any value set that is not a positive whole number
index.html Outdated Show resolved Hide resolved
simplifying author's should back to just -1 for indeterminate.  core aam / User agents will be where other values are accounted for.
core-aam/index.html Outdated Show resolved Hide resolved
core-aam/index.html Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants