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

prohibit aria-roledescription from role=generic #1142

Merged
merged 6 commits into from Mar 4, 2020
Merged

Conversation

scottaohara
Copy link
Member

@scottaohara scottaohara commented Dec 21, 2019

closes #1140, pending answering comment about note


Preview | Diff

@mtrootyy
Copy link

aria-brailleroledescription also should be prohibited on role generic for the same reason.

@pkra
Copy link
Member

pkra commented Dec 22, 2019

aria-brailleroledescription also should be prohibited on role generic for the same reason.

👍 (But it's less of a problem since the spec says that UAs must not expose aria-brailleroledescription if there's no valid aria-roledescription.)

@scottaohara
Copy link
Member Author

agreed, though still not a bad idea to be explicit :) making a commit to add this now.

@carmacleod carmacleod self-requested a review January 1, 2020 20:05
Copy link
Contributor

@carmacleod carmacleod left a comment

Choose a reason for hiding this comment

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

+1 (with or without note)

@jnurthen
Copy link
Member

@scottaohara Can you please remove the commit prohibiting brailleroledescription on generic from this PR. I can't merge that to the 1.2 branch (which is the desire for this) as we won't have brailleroledescription there. We can ask @pkra to add that to #1097

@scottaohara
Copy link
Member Author

@jnurthen removed it.

pkra added a commit to pkra/aria that referenced this pull request Jan 16, 2020
@jnurthen jnurthen self-requested a review January 16, 2020 23:34
Copy link
Member

@jnurthen jnurthen left a comment

Choose a reason for hiding this comment

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

I have an open question:
Do we want to add a prohibition on user agents exposing aria-roledescription if it is set on a generic? Currently prohibited only says Authors MUST NOT - it has no restriction on user agents (and we want to keep it that way). On the other hand aria-roledescription could have this prohibition in the 2nd bulleted list by adding a 3rd bullet:
"User agents MUST NOT expose the aria-roledescription property if any of the following conditions exist:

  • The element to which aria-roledescription is applied does not have a valid WAI-ARIA role or does not have an implicit WAI-ARIA role semantic.
  • The value of aria-roledescription is empty or contains only whitespace characters.
  • aria-roledescription is prohibited on the explicit or implicit WAI-ARIA role."

(please wordsmith as appropriate)

@carmacleod
Copy link
Contributor

carmacleod commented Jan 17, 2020

Attempted wordsmithing of 3rd point, above:

The element to which aria-roledescription is applied has an explicit or implicit WAI-ARIA role that prohibits aria-roledescription.

...and make it the 2nd point instead?

@mcking65
Copy link
Contributor

I think it is important to have this UA restriction language in roledescription before merging:

The element to which aria-roledescription is applied has an explicit or implicit WAI-ARIA role that prohibits aria-roledescription.

@jnurthen jnurthen self-requested a review January 23, 2020 22:48
@jnurthen
Copy link
Member

@scottaohara pushed a change - can you take a look

@scottaohara
Copy link
Member Author

i think that looks good, thank you @jnurthen, sorry i didn't get to adding it sooner.

Copy link
Contributor

@mcking65 mcking65 left a comment

Choose a reason for hiding this comment

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

Looks good.

@jnurthen jnurthen merged commit f41f707 into master Mar 4, 2020
@scottaohara scottaohara deleted the issue_1140 branch March 4, 2020 03:30
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.

aria-roledescription should not be allowed on role generic
6 participants