-
Notifications
You must be signed in to change notification settings - Fork 320
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 fallback logo so Chrome will not flag it to screen readers #1724
Fix fallback logo so Chrome will not flag it to screen readers #1724
Conversation
role="none"/"presentational" does not do enough to hide the child fallback image used from browsers. We have found that Chrome's new "Get image descriptions from Google" feature will notice this fallback image. This means when screenreaders such as NVDA visit the logo, it'll announce "Unlabelled graphic, To get missing image descriptions, open the context menu." for this fallback image. By using aria-hidden="true" we can ensure that the SVG and it's children are all hidden from the accessibility tree, which I have confirmed in NVDA + Chrome. - [Get image descriptions on Chrome](https://support.google.com/chrome/answer/9311597?hl=en) - [The Difference Between role=”presentation” and aria-hidden=”true”](https://timwright.org/blog/2016/11/19/difference-rolepresentation-aria-hiddentrue/)
Where possible we should use aria-hidden="true" instead of role="none"/"presentation", this is mainly for consistency with the header component where aria-hidden="true" ensures child elements are hidden as well which is not a problem with the button and footer component. - [The Difference Between role=”presentation” and aria-hidden=”true” ](https://timwright.org/blog/2016/11/19/difference-rolepresentation-aria-hiddentrue/)
Makes sense to me, thanks Nick 👏 How confident are you that this will work consistently everywhere? Do you think we need to test this change across all of the assistive technologies we support? |
@36degrees I have not done a full run through testing with all assistive technologies and browsers as I have confidence that this aria attribute has very broad support and we use it elsewhere (for example the accordion component). I can do some testing soon to double check. |
@36degrees I've tested this in JAWS, NVDA in Edge, Chrome, IE11, Firefox and VoiceOver in Safari and it dont hear any icons being announced when tabbing to the GOV.UK link in the header. |
If you're confident with the changes to the other components as well then I think this is good to merge 👍 |
@36degrees I also tested the start link button |
Needs a changelog entry, will follow up. |
Add CHANGELOG entry for #1724
The markup for the header was changed in alphagov/govuk-frontend#1724: > role="none"/"presentational" does not do enough to hide the child fallback image used in older browsers. > > We have found that Chrome's new "Get image descriptions from Google" feature will notice this fallback image. > > This means when screenreaders such as NVDA visit the logo, it'll announce "Unlabelled graphic, To get missing image descriptions, open the context menu." for this fallback image. > > By using aria-hidden="true" we can ensure that the SVG and it's children are all hidden from the accessibility tree, which I have confirmed in NVDA + Chrome. This change went out as part of GOV.UK Frontend v3.6.0. It was missed when updating the Design System to use v3.6.0 in #1187 (it's a non-breaking change, as the existing markup works fine – this is just an improvement)
role="none"/"presentational" does not do enough to hide the child fallback image used in older browsers.
We have found that Chrome's new "Get image descriptions from Google" feature will notice this fallback image.
This means when screenreaders such as NVDA visit the logo, it'll announce "Unlabelled graphic, To get missing image descriptions, open the context menu." for this fallback image.
By using aria-hidden="true" we can ensure that the SVG and it's children are all hidden from the accessibility tree, which I have confirmed in NVDA + Chrome.
I have also made this change to the other places we're using role="presentation" in this way for consistency.
Closes #1636