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

Change figure & figcaption accName computation #359

Closed
wants to merge 23 commits into from

Conversation

scottaohara
Copy link
Member

@scottaohara scottaohara commented Jan 20, 2022

resolves #325

This PR revises the mappings for figure and figcaption to remove the "labelled by" relations, changing them to "details for".
The accessible name computation for figure removes figcaption, a note is added to further reference this change.

Implementation commitment:


Preview | Diff

resolves #325

This PR revises the mappings for `figure` and `figcaption` to remove the "labelled by" relations, changing them to "details for".
The accessible name computation for `figure` removes `figcaption`, a note is added to further reference this change.

TODO: 
- [ ] need to check on updates to UIA mapping, if any.
- [ ] do we want `figcaption` to map to `caption`? 
- [ ] regardless of above, [`caption`](https://w3c.github.io/aria/#caption) will need to be updated in regards to its referencing `figure` and `figcaption` relationships
- [ ] [`aria-details`](https://w3c.github.io/aria/#aria-details) definition should be updated to reference this change
@scottaohara
Copy link
Member Author

referencing this thread as a very good example of why this change needs to be made: WICG/proposals#45

per what's being discussed there, if an entire video transcript were placed within a figcaption, the flattened text string of the transcript would become the accessible name for the parent figure.... and that would be awful. In contrast, this is a perfectly reasonable thing to use a figure and figcaption for... hence these mapping changes would be beneficial here.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Copy link
Collaborator

@cookiecrook cookiecrook left a comment

Choose a reason for hiding this comment

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

I'll have to stop short of "approving" since I'm not the right person to review ATK or IA2 changes. I see no issues with the other parts of the PR.

@carmacleod
Copy link
Contributor

Aside: As this will be one more element that can only reasonably get its accessible name from aria and not from html (ignoring title attribute), it becomes even more important to have a new html mechanism to give an element an accessible name.

@scottaohara scottaohara closed this Feb 7, 2022
@scottaohara scottaohara reopened this Feb 7, 2022
Copy link
Collaborator

@aleventhal aleventhal left a comment

Choose a reason for hiding this comment

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

Approve for ATK/IA2 parts, but I think we should follow up with Microsoft and Apple before merging.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@aleventhal
Copy link
Collaborator

Ah, I found that Chrome uses NSAccessibilityDetailsElementsAttribute aka AXDetailsElements.
Here's the bug where we landed it: https://bugs.chromium.org/p/chromium/issues/detail?id=1216049
We followed WebKit's lead. There are links in the bug to the WebKit change as well.

scottaohara added a commit to w3c/aria that referenced this pull request Mar 12, 2022
This change in the definition is related to the changes in `figure` and `figcaption` in HTML AAM: w3c/html-aam#359 and introduces the idea that a `caption` may contain structured content - and in this PR - `aria-details` is referenced as a way authors should reference such content if within a `caption`

Additionally, this PR extends the definition to allow `caption` to be used for purposes of naming/describing a `group` or `radiogroup`, which fills some gaps from the dropped `legend` role.

If these updates are accepted, this would help pave the way to resolve #1696 as well.
scottaohara added a commit to w3c/aria that referenced this pull request Mar 12, 2022
this pr updates `aria-details` to mention `figure` and `figcaption`.

This update is related to HTML AAM's update to the `figure` and `figcaption` elements - w3c/html-aam#359.
It is a follow on to my other PR, #1703 which updates the `caption` role and includes information about `aria-details`.
pkra pushed a commit to w3c/aria that referenced this pull request Apr 29, 2022
* revise caption definition

This change in the definition is related to the changes in `figure` and `figcaption` in HTML AAM: w3c/html-aam#359 and introduces the idea that a `caption` may contain structured content - and in this PR - `aria-details` is referenced as a way authors should reference such content if within a `caption`

Additionally, this PR extends the definition to allow `caption` to be used for purposes of naming/describing a `group` or `radiogroup`, which fills some gaps from the dropped `legend` role.

If these updates are accepted, this would help pave the way to resolve #1696 as well.

* add in missing id to first example

* fix spacing issues

* include additional example
pkra pushed a commit to w3c/aria that referenced this pull request Apr 29, 2022
* include mention of figure/caption 

this pr updates `aria-details` to mention `figure` and `figcaption`.

This update is related to HTML AAM's update to the `figure` and `figcaption` elements - w3c/html-aam#359. 

It is a follow on to my other PR, #1703 which updates the `caption` role and includes information about `aria-details`.

Co-authored-by: Sarah Higley <[email protected]>
@scottaohara
Copy link
Member Author

Another thing that has come to my attention is adding a comment to NOT expose the details relationship in the event an author has associated the figure and figcaption via aria-labelledby. In such a case, it would likely be less noisy to drop the details association since the author is specifically referencing the element as the figure's name. e.g.,

<figure aria-labelledby=n>
  <figcaption id=n>this becomes the name</figcaption>
  whatever the content of the figcaption is here.
</figure>

verified the UIA mappings with help from @benbeaudry 
made some of the wording consistent between mappings
added in a SHOULD to indicate that if a figure is explicitly provided an accessible name from its figcaption, then do not expose the details relationship, as that'd add redundancy.
saying it doesn't know what accname is... so, replacing the instances of the links which may be causing the issue?
see https://bugs.chromium.org/p/chromium/issues/detail?id=1426613#c4

aria-details with an empty string could be useful to authors if there was a specific need to _not_ expose a child figcaption - e.g., if the elements need to have remapped roles for whatever reason.
@jnurthen
Copy link
Member

jnurthen commented Oct 5, 2023

@benbeaudry could use your help with the UIA mappings so we can get this merged.

jnurthen pushed a commit to w3c/aria that referenced this pull request Oct 10, 2023
* revise caption definition

This change in the definition is related to the changes in `figure` and `figcaption` in HTML AAM: w3c/html-aam#359 and introduces the idea that a `caption` may contain structured content - and in this PR - `aria-details` is referenced as a way authors should reference such content if within a `caption`

Additionally, this PR extends the definition to allow `caption` to be used for purposes of naming/describing a `group` or `radiogroup`, which fills some gaps from the dropped `legend` role.

If these updates are accepted, this would help pave the way to resolve #1696 as well.

* add in missing id to first example

* fix spacing issues

* include additional example
jnurthen pushed a commit to w3c/aria that referenced this pull request Oct 10, 2023
* include mention of figure/caption 

this pr updates `aria-details` to mention `figure` and `figcaption`.

This update is related to HTML AAM's update to the `figure` and `figcaption` elements - w3c/html-aam#359. 

It is a follow on to my other PR, #1703 which updates the `caption` role and includes information about `aria-details`.

Co-authored-by: Sarah Higley <[email protected]>
@scottaohara
Copy link
Member Author

@benbeaudry, @jnurthen this is ready for re-review and thanks to ben's core aam updates for aria-details i got the info i needed to complete this. so, if everyone can give this one last look, then i think we can merge and get remaining implementation bugs filed for webkit/firefox.

cc @cookiecrook @jcsteh

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
index.html Outdated Show resolved Hide resolved
index.html Outdated 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 Outdated Show resolved Hide resolved
@scottaohara

This comment was marked as outdated.

@cookiecrook
Copy link
Collaborator

Actually I didn't realize this update would require rolling back some WebKit implementation changes wrt figcaption... I think this needs WPT tests to fully qualify the implications of the change, especially the parts about accounting for multiple figcaptions... I think you could easily convert the above examples into a WPT test.

</div>
</td>
</tr>
<!-- <th><a href="#accessible-name-and-description-computation">Naming Algorithm</a></th> -->
<tr>
<th>Comments</th>
<td>
<p>
Only the first child instance of a <a href="#el-figcaption">`figcaption`</a> element that is present in the accessibility tree (e.g., not 'hidden')
Copy link

Choose a reason for hiding this comment

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

For figure, we're explicit that aria-labelledby, aria-describedby or aria-details prevent exposure of the details relation. However, we're not similarly explicit here for the reverse relation. Common sense will hopefully prevail, but I don't love relying on common sense for a spec. Is there something I'm missing that would otherwise make this explicit?

This is a similar situation to https://github.com/w3c/html-aam/pull/481/files#r1424943049. It seems we tend to use ARIA to express roles in a cross-platform manner wherever possible now, but we don't do that for relations, I guess largely because reverse relations and the details-roles attribute complicate things.

@rahimabdi
Copy link
Contributor

Closing, this PR has been migrated to the ARIA monorepo:

@rahimabdi rahimabdi closed this Aug 11, 2024
@scottaohara scottaohara deleted the figure-figcaption-revision branch December 13, 2024 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should figcaption participate in name/desc computation?
8 participants