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 details element in firefox nvda #773

Merged
merged 3 commits into from
Sep 20, 2021

Conversation

andymantell
Copy link
Contributor

@andymantell andymantell commented Sep 16, 2021

Description

In order to fix #754, I have stopped polyfilling the details component in browsers that have native support since this is no longer necessary and as evidenced by #754 is actually making the situation worse for screen readers. Removing the polyfil causes Firefox to behave correctly now

Checklist

Assistive tech tested

  • JAWS 2021 Firefox ✔️
  • JAWS 2021 Edge ✔️
  • JAWS 2021 Chrome ✔️
  • JAWS 2021 IE11 ✔️
  • NVDA 2021.2 Firefox ✔️
  • NVDA 2021.2 Edge ✔️
  • NVDA 2021.2 Chrome ✔️
  • NVDA 2021.2 IE11 ✔️
  • Android 10 Talkback ✔️
  • Dragon Naturally Speaking 15 Firefox ❌ Does not work - but as per Fix details element in firefox nvda #773 (comment) - Dragon does not support Firefox
  • Dragon Naturally Speaking 15 Edge ✔️
  • Dragon Naturally Speaking 15 Chrome ✔️
  • Dragon Naturally Speaking 15 IE11 ✔️

@andymantell
Copy link
Contributor Author

@mcheung-nhs or @chrimesdev - do either of you have any apple devices you could check in voiceover for me please? I am due to get an NHS testing iPad soon but probably not for another month yet...

Thanks!

@mcheung-nhs
Copy link
Collaborator

Hi @andymantell - I can check on an iPad, iPhone and Mac.

@andymantell
Copy link
Contributor Author

Thanks @mcheung-nhs, that would be very helpful if you could.

@andymantell
Copy link
Contributor Author

@mcheung-nhs - do you have Dragon too? I can't get Dragon to open the details in Firefox. But I can get it to open it on the current version before my "fix". Can you see if the same happens for you? I fear I may have caused a regression for Firefox / Dragon...

@mcheung-nhs
Copy link
Collaborator

@andymantell - Yes I can check in Dragon as well

@andymantell
Copy link
Contributor Author

andymantell commented Sep 16, 2021

I enquired about this on the X-Gov Slack as the same issue exists in govuk-frontend. @selfthinker very helpfully directed me to https://www.nuance.com/products/help/dragon15/dragon-for-pc/enx/dmpe_sub/Content/Web/about_firefox.htm which indicates that Dragon does not support Firefox any more.

Therefore the fact that the details element does not work in Firefox/Dragon is not something that we need to fix at this point in time.

@mcheung-nhs
Copy link
Collaborator

mcheung-nhs commented Sep 20, 2021

I've tested on VoiceOver and the fix seems to have caused no issues.
One point to note is that removing the polyfill has caused VoiceOver to not announce the summary text as a button due to the role attribute not being set. On MacOS it announces it as a 'Summary' and on iOS it doesn't announce it as anything. However, they both announce that it can be expanded so that should give enough information to the user that you can interact with it.
And one further point whilst testing on iOS is that I found a bug where VoiceOver didn't announce the expanded state when the summary details is expanded. This has been raised as an issue on the govuk-frontend repo and a subsequent bug has been raised with WebKit - https://bugs.webkit.org/show_bug.cgi?id=230408
However, this was not caused by this fix - it was an existing bug - so am happy to approve these changes.

@andymantell
Copy link
Contributor Author

Thanks @mcheung-nhs

@chrimesdev
Copy link
Member

Thanks @andymantell and @mcheung-nhs, looking to do a frontend library release week. Probably on Wednesday, so this will be included.

@chrimesdev chrimesdev merged commit 7fbc85c into master Sep 20, 2021
@chrimesdev chrimesdev deleted the feature/fix-details-element-in-firefox-nvda branch September 20, 2021 12:59
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.

Details component announces its default and expanded state 1 time but not subsequent times using NVDA
3 participants