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

fix(aria-allowed-attr): allow figure role #1558

Merged
merged 2 commits into from
May 24, 2019
Merged

fix(aria-allowed-attr): allow figure role #1558

merged 2 commits into from
May 24, 2019

Conversation

straker
Copy link
Contributor

@straker straker commented May 9, 2019

The figure role is needed to properly read the figcaption element in some browser and AT combinations. See https://www.scottohara.me/blog/2019/01/21/how-do-you-figure.html.

Linked issue: #1460

Reviewer checks

Required fields, to be filled out by PR reviewer(s)

  • Follows the commit message policy, appropriate for next version
  • Has documentation updated, a DU ticket, or requires no documentation change
  • Includes new tests, or was unnecessary
  • Code is reviewed for security by: << Name here >>

@WilcoFiers
Copy link
Contributor

@straker did you test this in JAWS? According to Steve Faulkner's docs this doesn't work:
https://freedomscientific.github.io/VFO-standards-support/aria.html#index-aria-figure

@straker
Copy link
Contributor Author

straker commented May 10, 2019

I did not. I relied on the testing of Scotts article which showed JAWS needing it in IE11.

IE11 requires the use of role="figure" and an aria-label or aria-labelledby pointing to the figcaption to mimic native announcements. It’s not surprising that IE11 doesn’t support the native element, as HTML5 Accessibility’s IE11 browser rating will never improve. But at least ARIA can provide the semantics.

I think there are two different use cases that we are trying to cover. role=figure on any element may not be supported (Steve Faulkner's test), but role=figure on a <figure> element is needed in some browsers/AT combinations in order to read the figcaption (Scott's test).

I agree with Scott that we shouldn't just straight out flag it as unsupported, but maybe we just need to do what we did with aria-roledescription and say where it is supported.

I can do some testing for all cases and come up with a support matrix for when/where it is supported.

@WilcoFiers
Copy link
Contributor

My main question, which wasn't obvious from Scott's article, was whether or not that figure element is announced as a figure. I'm sure if you put an aria-label(ledby) on it, it'll be announced, but is it announced as a "figure" or "graphic" or whatever.

@straker
Copy link
Contributor Author

straker commented May 14, 2019

Here's the results of my testing with role=figure https://codepen.io/straker/pen/zQoQYr. Any support element/browser/AT combination the browser announced the element as figure role (or graphic for Talkback). In terms of who announced the caption of the 4 different figures, VO/Safari announced it natively while JAWS/IE11 only announced the caption with aria-labelledby. These results are consistent with Scotts findings.

So in terms of what we want to allow. VO/Safari and JAWS/IE11 (with role=figure and aria-lablledby) are the only combinations that announce the figure role and the caption, while Talkback/Android announces graphic for most applications. So support isn't great, but there is a benefit for IE11 to add value even if no other browsers get the same value.

Copy link
Contributor

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

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

Slight scope creep, but we should fix this.

doc/aria-supported.md Outdated Show resolved Hide resolved
@straker straker self-assigned this May 24, 2019
@straker straker merged commit a4b5240 into develop May 24, 2019
@straker straker deleted the allowFigureRole branch May 24, 2019 15:24
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.

2 participants