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 label on certain roles #985

Merged
merged 17 commits into from Jun 28, 2019

Conversation

Projects
None yet
7 participants
@jnurthen
Copy link
Contributor

commented May 23, 2019

@jnurthen jnurthen added the WIP label May 23, 2019

@carmacleod

This comment has been minimized.

Copy link
Contributor

commented May 23, 2019

  • The Prohibited section for blockquote is before the Inherited section, but for all of the others (caption, code, deletion, emphasis, insertion, note, paragraph, presentation, strong, subscript, superscript), the Prohibited section is after the Inherited section.

  • Consider removing "Name from: N/A" rows from the characteristics table for consistency with other N/A characteristics rows.

@jnurthen

This comment has been minimized.

Copy link
Contributor Author

commented May 23, 2019

* Consider removing "Name from: N/A" rows from the characteristics table for consistency with other N/A characteristics rows.

I considered this but thought it might be worth calling it out explicitly. I'm certainly willing to go either way on it, but I was also thinking of adding a corresponding type in the section https://pr-preview.s3.amazonaws.com/w3c/aria/pull/985.html#namecalculation corresponding with whatever we decide we would call the N/A value.

@carmacleod

This comment has been minimized.

Copy link
Contributor

commented May 24, 2019

I considered this but thought it might be worth calling it out explicitly.

Right. Hard to say. On the one hand, "Prohibited States and Properties: aria-label, aria-labelledby" is pretty explicit and right above where "Name from:" would be, but on the other hand I know what you mean - it is probably worth doing as much as possible to point out this particular change.

I was also thinking of adding a corresponding type in the section https://pr-preview.s3.amazonaws.com/w3c/aria/pull/985.html#namecalculation corresponding with whatever we decide we would call the N/A value.

Right. If we do that, then we need to keep Name From.
So, we would have author, contents, encapsulation, legend, and ... none? prohibited?

@jnurthen jnurthen added the Agenda label Jun 5, 2019

@jnurthen

This comment has been minimized.

Copy link
Contributor Author

commented Jun 5, 2019

@stevefaulkner very interested in your feedback on this

@stevefaulkner

This comment has been minimized.

Copy link
Contributor

commented Jun 6, 2019

@jnurthen from looking at the changes it is unclear to me which roles are effected, is there a list of roles or a rendered view of the branch I can look at?

@carmacleod

This comment has been minimized.

Copy link
Contributor

commented Jun 6, 2019

@stevefaulkner
Here's the list of the roles that prohibit aria-label[ledby] in this PR:

  • blockquote
  • caption
  • code
  • deletion
  • emphasis
  • insertion
  • note
  • paragraph
  • presentation
  • strong
  • subscript
  • superscript

It's mostly new roles (except presentation) that we recently added for HTML role parity, so fairly minimal impact at the moment. This is paving the way for adding the "generic" role, which is intended to be the default role for div and span (and some others). Our current thinking is that generics should be nameless.

Here's the PR Preview. If you look for the word "prohibit" in the preview, then you will hit all of the salient changes.

Show resolved Hide resolved common/script/aria.js Outdated
index.html Outdated
@@ -12325,10 +12439,11 @@ <h3>States and Properties</h3>
</ul>
</li>
</ul>
<p>If the author specfies a WAI-ARIA property on a role where that property is prohibited, the user agent SHOULD treat the property as if it were allowed. </p>

This comment has been minimized.

Copy link
@joanmarie

joanmarie Jun 6, 2019

Contributor

Perhaps we could put an editor's note after this statement to further discourage authors from ignoring the prohibition? Candidate text:

Note: At the present time, user agents have no obligation to ignore prohibited states and properties. However, this may change in a future version of ARIA in order to ensure interoperability.

This comment has been minimized.

Copy link
@jnurthen

jnurthen Jun 6, 2019

Author Contributor

We agreed at the meeting to remove the text from the author errors section I believe. Could this go after line 498 instead?

jnurthen added some commits Jun 6, 2019

@jnurthen

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2019

I will need to split this into 2 (aria-common and aria) - leaving as one until ready for convenience

@carmacleod
Copy link
Contributor

left a comment

Attributes, States, Properties

Show resolved Hide resolved index.html Outdated
Show resolved Hide resolved index.html
Show resolved Hide resolved index.html
Show resolved Hide resolved index.html
@stevefaulkner

This comment has been minimized.

Copy link
Contributor

commented Jun 7, 2019

@jnurthen wondering about the reasons for choosing paragraph/note/blockquote. is there related discussion?

@jnurthen

This comment has been minimized.

Copy link
Contributor Author

commented Jun 7, 2019

@stevefaulkner there is some discussion in #833 - although note looks like a mistake - thanks for catching it. I am removing that - it was intended to be on none - but that doesn't follow the normal role structure.

@scottaohara

This comment has been minimized.

Copy link
Member

commented Jun 7, 2019

shouldn't term be changed to prohibited as well, especially since it's no longer related to dt?

definition could be given it's name from the term

<span role="term" id="t">Word</span>
<span role="definition" aria-labelledby="t">
  words to define word.
</span>
@joanmarie

This comment has been minimized.

Copy link
Contributor

commented Jun 7, 2019

definition could be given it's name from the term

<span role="term" id="t">Word</span>
<span role="definition" aria-labelledby="t">
  words to define word.
</span>

When something has a valid name, screen readers might present that name and NOT present the associated text. So if there's no aria-labelledby the term "word" has a presented-by-the-screenreader definition of "words to define word". But with the aria-labelledby present, some screen readers might present the definition of "word" as "word". Unless they have smarts to say definition, like paragraph and friends really shouldn't have a name.

TL;DR: Should we prohibit naming of definition?

@jnurthen

This comment has been minimized.

Copy link
Contributor Author

commented Jun 7, 2019

TL;DR: Should we prohibit naming of definition?

We would have to change definition's definition

Authors SHOULD identify the element being defined by giving that element a role of term and referencing it with the aria-labelledby attribute or by making the element with role term a descendant of the element with role definition.

@scottaohara

This comment has been minimized.

Copy link
Member

commented Jun 7, 2019

@joanmarie I had originally had that thought about definition when doing the review for the revised term and definition work. But I put that aside because I assumed definition would work like group, where it could have a name, and its contents still remain accessible, per the definition of definition as James noted.

For example, a definition that consists of two paragraphs:

<span role="term" id="f">
  Funny
</span>
<div role="definition" aria-labelledby="f">
  <p>Causing laughter or amusement.</p>
  <p>Making or given to making amusing jokes or witticisms.</p>
</div>
@joanmarie

This comment has been minimized.

Copy link
Contributor

commented Jun 7, 2019

It will be interesting to see how screen readers present definition.

@stevefaulkner

This comment has been minimized.

Copy link
Contributor

commented Jun 12, 2019

Of the list provided the only definitely questionable one is blockquote I can envisage scenarios where it would be useful to associate a label/description for a blockquote pertaining to it source (for example). In a quick test I found that both JAWS and NVDA announce the presence of a blockquote and NVDA also announces the accessible name if it has one.

I am on the fence about <p> as i could envisage situation where annotating a paragraph could be useful, although I have not encountered AT support for announcing the presence of <p> elements.

@mcking65

This comment has been minimized.

Copy link
Contributor

commented Jun 13, 2019

@stevefaulkner wrote:

Of the list provided the only definitely questionable one is blockquote I can envisage scenarios where it would be useful to associate a label/description for a blockquote pertaining to it source (for example). In a quick test I found that both JAWS and NVDA announce the presence of a blockquote and NVDA also announces the accessible name if it has one.

I am on the fence about <p> as i could envisage situation where annotating a paragraph could be useful, although I have not encountered AT support for announcing the presence of <p> elements.

I agree that it is feasible for screen readers to support named blockquotes. Most screen readers present the boundaries of blockquotes in reading mode, which provides a reasonable way of supporting names without confusing users. While it doesn't seem like a practice to recommend, prohibiting it may be unnecessary.

However, I strongly believe prohibiting named paragraphs is necessary.

Show resolved Hide resolved index.html
Show resolved Hide resolved index.html Outdated
Show resolved Hide resolved index.html
Show resolved Hide resolved index.html Outdated
@jnurthen

This comment has been minimized.

Copy link
Contributor Author

commented Jun 19, 2019

w3c/accname#53 shows how accname could change to enable this

@jnurthen jnurthen marked this pull request as ready for review Jun 20, 2019

@carmacleod

This comment has been minimized.

Copy link
Contributor

commented Jun 24, 2019

I think this PR is good to go.
The initial set of roles that will prohibit naming makes sense, and more can be added as/if needed.
The wording also allows for other attributes to be prohibited on specific roles in future, as/if needed.

@mcking65
Copy link
Contributor

left a comment

Looks great! Nice work! Let's ship it!

commented "I think this PR is good to go."

@jnurthen jnurthen removed the WIP label Jun 28, 2019

@jnurthen

This comment has been minimized.

Copy link
Contributor Author

commented Jun 28, 2019

I think this is ready to go. @joanmarie do you want to merge or should I?

@joanmarie

This comment has been minimized.

Copy link
Contributor

commented Jun 28, 2019

I think this is ready to go. @joanmarie do you want to merge or should I?

You did all the work. :) Go for it! Do please update the wiki so we can add it to the list of things which will need tests.

@jnurthen jnurthen changed the title [wip] Do not allow label on certain roles Do not allow label on certain roles Jun 28, 2019

@jnurthen jnurthen changed the title Do not allow label on certain roles Prohibit label on certain roles Jun 28, 2019

@jnurthen jnurthen merged commit 834a8ba into master Jun 28, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
ipr PR deemed acceptable.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.