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

SVGs are focusable in IE #15330

Closed
karlgroves opened this issue Apr 30, 2019 · 17 comments
Closed

SVGs are focusable in IE #15330

karlgroves opened this issue Apr 30, 2019 · 17 comments
Assignees
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).

Comments

@karlgroves
Copy link

SVGs are focusable in IE

  • Severity: Low
  • Affected Populations:
    • Blind
    • Low-Vision
    • Motor Impaired
    • Cognitively Impaired
  • Platform(s):
    • Windows - Screen Reader
    • Windows - ZoomText
  • Components affected:
    • Output Markup
    • Publish and Unpublish

Issue description

The next-month and previous-month buttons within the datepicker contain
SVG elements, which are focusable in IE11 because of an anomaly in how
IE handles SVG.

This issue also occurs with the "Previous" and "Next" post
navigation links on published pages.

This causes additional tab stops for keyboard users. These Tab stops are
also not described for assistive technologies, because they have no
accessible names.

The SVG's role ("graphic" or "diagram") may be exposed to screen
reader users, but when they are used as decorative icons they should not
be announced at all.

Issue Code
    <svg xmlns="http://www.w3.org/2000/svg" class="DayPickerNavigation_svg__horizontal DayPickerNavigation_svg__horizontal_1" viewBox="0 0 1000 1000"><path d="..." /></svg>





    <svg xmlns="http://www.w3.org/2000/svg" class="DayPickerNavigation_svg__horizontal DayPickerNavigation_svg__horizontal_1" viewBox="0 0 1000 1000"><path d="...></svg>





    <!-- published page-->


    <a href="...2018/08/13/hello-world/" rel="prev">


        <span class="screen-reader-text">Previous Post</span>


        ...


        <span class="...">


            <span class="...">


                <svg class="icon icon-arrow-left" aria-hidden="true" role="img">... </svg>


            </span>


            Hello world!


        </span>


    </a>


    ...


    <button type="submit" class="search-submit">


        <svg class="icon icon-search" aria-hidden="true" role="img"> ...</svg>


        <span class="screen-reader-text">Search</span>


    </button>

Remediation Guidance

Add focusable="false" and aria-hidden=true to the SVG
elements, so that they're not focusable nor announced by assistive
technologies.

Recommended Code
    <svg focusable="false" aria-hidden="true" xmlns="http://www.w3.org/2000/svg" class="DayPickerNavigation_svg__horizontal DayPickerNavigation_svg__horizontal_1" viewBox="0 0 1000 1000">


       <path d="..." />


    </svg>





    <svg focusable="false" aria-hidden="true" xmlns="http://www.w3.org/2000/svg" class="DayPickerNavigation_svg__horizontal DayPickerNavigation_svg__horizontal_1" viewBox="0 0 1000 1000">


       <path d="...>


    </svg>








    <!-- published page-->


    <a href="...2018/08/13/hello-world/" rel="prev">


        <span class="screen-reader-text">Previous Post</span>


        ...


        <span class="...">


            <span class="...">


                <svg focusable="false" class="icon icon-arrow-left" aria-hidden="true" role="img">... </svg>


            </span>


            Hello world!


        </span>


    </a>


    ...


    <button type="submit" class="search-submit">


        <svg focusable="false" class="icon icon-search" aria-hidden="true" role="img"> ...</svg>


        <span class="screen-reader-text">Search</span>


    </button>

Note: This issue may be a duplicate with other existing accessibility-related bugs in this project. This issue comes from the Gutenberg accessibility audit, performed by Tenon and funded by WP Campus. This issue is GUT-11 in Tenon's report

@gziolo gziolo added the Needs Accessibility Feedback Need input from accessibility label Apr 30, 2019
@aduth aduth added this to To do in Accessibility Audit May 1, 2019
@afercia
Copy link
Contributor

afercia commented May 1, 2019

Accessibility feedback is clear: it's a known issue that all the SVG icons need a focusable="false" attribute for IE11 and that's why the Gutenberg icons have that attribute. The date picker is an external component and I guess it misses this IE11 fix, which is unfortunate.

@afercia afercia added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). and removed Needs Accessibility Feedback Need input from accessibility labels May 1, 2019
@melchoyce
Copy link
Contributor

Screenshot from full report:

image

@nicolad
Copy link
Member

nicolad commented May 14, 2019

As alternative solution arrows might be added via css as background. This can be achieved by overriding: https://github.com/airbnb/react-dates#overriding-styles

@nicolad
Copy link
Member

nicolad commented May 14, 2019

I have opened a PR in react-dates: react-dates/react-dates#1643 but not sure how fast it can be reviewed/ merged.

@backwardok
Copy link

@nicolad @karlgroves does there need to be also the aria-hidden="true" on the SVGs as well? react-dates has focusable="false" already to address the tab focus issue in IE, but I'm curious why the entire svg needs to be aria-hidden as well.

@nicolad
Copy link
Member

nicolad commented May 14, 2019

@backwardok AFAIK aria-hidden removes element altogether from accessibility tree, not just make it unfocusable. If IE would not have any quirks I think there will be no need for this kind of double guarding.

@backwardok
Copy link

@nicolad from the description in the issue, this is about keyboard tab behavior, which is unrelated to aria-hidden. Using aria-hidden affects more than just IE, so I want to understand what the reasoning is behind needing it.

@karlgroves is there an issue with screen readers reading out svgs that have no textual content that's prompting the need to also have aria-hidden="true"?

@afercia
Copy link
Contributor

afercia commented May 14, 2019

react-dates has focusable="false" already

That's possible, but the icons rendered in the Gutenberg calendar don't have focusable="false". Snippet of the rendered markup:

<svg class="DayPickerNavigation_svg__horizontal DayPickerNavigation_svg__horizontal_1"
    viewBox="0 0 1000 1000"><path ...

@backwardok SVG icons in Gutenberg are purely decorative and the necessary information is provided via the control (generally a button) that wraps the icon. To make sure the SVG icons are actually perceived as images and also ignored by all the various browser/screen reader combos, we're using role="img" aria-hidden="true". Do you see any specific concern with these attributes?

@backwardok
Copy link

@afercia what version of react-dates do you have? I know that in previous versions, the focusable attributes were getting stripped out in optimization, so we removed that step (react-dates/react-dates#1350) in 18.0.2.

Using role="img" isn't necessary if you're going to also apply aria-hidden="true". Setting a role allows a node to take on a separate role from its default role in the accessibility tree, but if you're hiding that node from the tree altogether, then the role doesn't matter.

@afercia
Copy link
Contributor

afercia commented May 15, 2019

Version:
in the package.json I see "^17.1.1", I think the actual used version is 17.2.0 though this is a question the packages experts here can answer better than me.

Using role="img" isn't necessary if you're going to also apply aria-hidden="true". Setting a role allows a node to take on a separate role from its default role in the accessibility tree

Yep, I'm pretty familiar with this 🙂 Based on testing and research (admittedly ran a while ago) not all browsers expose a native role for SVGs. Using role="img" just makes sure browsers behave the same. aria-hidden="true" makes sure the SVGs are ignored. I don't think this harms anything, instead it makes the behavior consistent across browsers. If you think this can harm users in any way I'd encourage you to create a separate issue. Any feedback is very welcome. Please let's try to keep this issue focused on the missing focusable="false" 🙂

@backwardok
Copy link

Since you're using a version of react-dates that stripped out the focusable="false" attribute when it was set on the svg, the proper way to address the issue of that attribute missing is to upgrade to a newer version of react-dates.

@gziolo gziolo added the [Status] In Progress Tracking issues with work in progress label Aug 8, 2019
@tellthemachines
Copy link
Contributor

Seems like upgrading to version 18.0.2 of react-dates could fix this issue. According to their changelog the breaking changes shouldn't affect us. Shall we give it a go?

@nicolad
Copy link
Member

nicolad commented Mar 30, 2020

Seems like upgrading to version 18.0.2 of react-dates could fix this issue. According to their changelog the breaking changes shouldn't affect us. Shall we give it a go?

Updated and tested here: #21256 (comment)

@afercia
Copy link
Contributor

afercia commented Mar 30, 2020

Note: the SVG icons that need to be tested are the one within the date-picker (the "calendar") as shown in the screenshot in a previous comment.

@nicolad
Copy link
Member

nicolad commented Mar 30, 2020

Note: the SVG icons that need to be tested are the one within the date-picker (the "calendar") as shown in the screenshot in a previous comment.

Good observation.
Indeed. aria-hidden is not applied via upgrade to v 18.0.2:

image

@backwardok
Copy link

Adding comment here as well as that PR, but aria-hidden was never necessary. The upgrade is to make sure that the focusable attribute is present, which it looks like from your screenshot that this is true.

@skorasaurus skorasaurus removed the [Status] In Progress Tracking issues with work in progress label Feb 9, 2021
@youknowriad
Copy link
Contributor

IE11 support is being dropped, context:

I'm closing this issue as it's not something we'll be investing in.

Accessibility Audit automation moved this from In progress to Closed Apr 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).
Projects
Development

Successfully merging a pull request may close this issue.

10 participants