search

alphagov / govuk-frontend

GOV.UK Frontend contains the code you need to start building a user interface for government platforms and services.
https://frontend.design-system.service.gov.uk/
MIT License
1.17k stars 320 forks source link

Error summary list not read out in JAWS (2022) #2657

Closed tvararu closed 2 years ago

tvararu commented 2 years ago

Discussion in cross-gov Slack: https://ukgovernmentdigital.slack.com/archives/C6DMEH5R6/p1654079614172949

Similar issue: https://github.com/alphagov/govuk-frontend/issues/2072 (no longer applicable in my own testing)

Description of the issue

List of error messages within the error summary is not announced when using JAWS 2022 with Edge or Chrome.

NVDA, VoiceOver on iPad were also tested and they correctly recited the content.

Steps to reproduce the issue

Here is an example video on JAWS on another service (https://find-a-lost-trn.education.gov.uk/). The h2 is read out at 00:36, the messages are not. Many thanks to Adam Liptrot on the cross-government Slack who recorded it.

https://user-images.githubusercontent.com/1650875/172218411-a408b8d9-8e03-4810-b498-dab166879537.mov

Actual vs expected behaviour

JAWS is only reading out the heading. It should read out the entire summary.

Environment (where applicable)

querkmachine commented 2 years ago

Hi @tvararu, thanks for the bug report.

Are you able to specify which versions of Chrome and Edge you've reproduced this on? This is useful in case we end up coming back to this later, and there can be differences between browser versions over time.

Thanks!

tvararu commented 2 years ago

@querkmachine updated! :+1:

tvararu commented 2 years ago

Investigation and analysis

@fofr, @adamliptrot-oc, and I have been looking into this for the past couple of days. Here's what we've found so far.

Investigation

The bug can be observed by opening or refreshing a page containing the error summary, while running JAWS 2022.2204.20 (Trial) with Edge 102 on Windows 10:

https://user-images.githubusercontent.com/1650875/172676846-32a4c6b3-6f98-4b5b-8af8-76e0de5596de.mov

In the first video, JAWS is reading out the following (comments added):

Refreshing page, Loading page
GOV.UK Frontend                             // Page title
Heading level 2 There is a problem          // The first element inside the focussed error summary
Page has 1 region, 1 heading, and 2 links

The user is made aware that there is an error, but cannot determine what is wrong (WCAG 3.3.1 Error identification), without further input.

However, if you keep refreshing the page multiple times, something interesting happens (10 seconds in):

https://user-images.githubusercontent.com/1650875/172677052-58613c85-cf76-47cc-b81d-b95507e3cc24.mov

The first 10 seconds in the second video are three attempts that lead to the same output as in the first video. It's being skipped by pressing left ctrl (which silences JAWS) followed by clicking the browser refresh button with the mouse button. Nothing else is changing in the page between refreshes, and no other keys are being pressed.

On the 4th refresh, the JAWS output is:

Refreshi-                               // JAWS cuts itself off mid-word

Alert!                                  // JAWS has encountered an element with role="alert"
There is a problem                      // The content of the aria-labelledby on the alerting element,
                                        // which is targeting the h2

There is a problem                      // The content inside the alerting element,
                                        // being recited one element at a time, first the h2

The date your passport was issued       // The text of the second element, an anchor link,
must be in the past                     // followed by the full href (very tedious to listen to)
http://govuk-frontend-review.herokuapp.com/components/error-summary/preview#example-error-1
The date your passport was issued       // JAWS seems to repeat the repeat the text of an
must be in the past                     // anchor after reading the href

Enter a postcode, like AA1 1AA          // The text and href of the second link, the final element
http://govuk-frontend-review.herokuapp.com/components/error-summary/preview#example-error-2
Enter a postcode, like AA1 1AA          // JAWS repeating the content of the anchor

GOV.UK Frontend                         // Page title
Heading level 2 There is a problem      // The first element inside the focussed error summary
Page has 1 region, 1 heading, and 2 links

JAWS announced an alert, and then read the full contents of the alert element. This behaviour meets WCAG 3.3.1, but it's inconsistent, as not every refresh triggers it.

Here's what (always) happens when we run the page with JavaScript disabled:

https://user-images.githubusercontent.com/1650875/172677277-d962ab16-18c8-4ea2-ab24-2da7860f3bd5.mov

The JAWS output is:

Refreshi pa-                            // JAWS cuts itself off mid-word, slightly later
Alert!
There is a problem
There is a problem

The date your passport was issued must be in the past
http://govuk-frontend-review.herokuapp.com/components/error-summary/preview#example-error-1
The date your passport was issued must be in the past

Enter a postcode, like AA1 1AA
http://govuk-frontend-review.herokuapp.com/components/error-summary/preview#example-error-2
Enter a postcode, like AA1 1AA

GOV.UK Frontend                         // Page title
GOV.UK Frontend                         // The page title, again
Page has 1 region, 1 heading, and 2 links

With JavaScript disabled, the data-module="govuk-summary" element no longer receives focus when the page loads. JAWS reliably picks up and reads the role="alert", and doesn't read out the h2 after, as the error summary is no longer in focus. This behaviour meets WCAG 3.1.1 at the cost of leaving the user's focus at the top of the page.

Analysis

It seems that .focusing the error summary wrapper div, which has role="alert", is leading to a race condition.

The two possible outcomes of the race are:

  1. The .focus comes first, and "cancels out" the alert behaviour (this happens most of the time in testing);
  2. The role="alert" element is "noticed" first, which leads to the alert being read out in full, followed by the focused element.

Other oddities

Solutions thought of so far

1. Multiple aria-labelledby targets (doesn't work)

The aria-labelledby is linking to the h2, but not to the optional description or to the list of errors. The hypothesis was that if we linked to them, JAWS might read them out when it focusses. An example of this approach is here: https://error-summary-labelledby.herokuapp.com/examples/error-summary-with-one-thing-per-page

However, this does not work: when an element gains focus, JAWS reads out the first child and ignores the aria-labelledby.

2. setTimeout (works, but really hacky)

Adding a short timeout before focus triggers does fix the issue, in limited testing:

// src/govuk/components/error-summary/error-summary.js
ErrorSummary.prototype.init = function () {
  var $module = this.$module
  if (!$module) {
    return
  }

+ setTimeout(() => {
   this.setFocus()
+ }, 300)
  $module.addEventListener('click', this.handleClick.bind(this))
}

This works by deferring the focus event to after the point where JAWS has already encountered the role="alert" element.

However, it doesn't work with a timeout of 0, or a call to requestAnimationFrame to move it to execute in next JavaScript tick. The minimum timeout that worked in testing was somewhere between 100ms to 300ms. JAWS needs a variable amount of time to "catch up" and parse the DOM to the point where it finds the alert element. Because we can't accurately determine how long that is going to take, there isn't a way to make this approach demonstrably consistent.

3. Focussing a wrapper element (seems to work!)

@fofr hypothesised that by separating the element that has role="alert" from the element that is receiving focus, that JAWS would choose deterministically:

<!-- src/govuk/components/error-summary/template.njk -->
<!-- Minimally viable code, just for illustration -->
+<div data-module="govuk-error-summary">
<div class="govuk-error-summary
  {%- if params.classes %} {{ params.classes }}{% endif %}" aria-labelledby="error-summary-title" role="alert"
  {%- if params.disableAutoFocus %} data-disable-auto-focus="true"{% endif %}
+ {%- for attribute, value in params.attributes %} {{ attribute }}="{{ value }}"{% endfor %}>
- {%- for attribute, value in params.attributes %} {{ attribute }}="{{ value }}"{% endfor %} data-module="govuk-error-summary">
  <h2 class="govuk-error-summary__title" id="error-summary-title">
    {{ params.titleHtml | safe if params.titleHtml else params.titleText }}
  </h2>
  <div class="govuk-error-summary__body">
    {% if params.descriptionHtml or params.descriptionText %}
    <p>
      {{ params.descriptionHtml | safe if params.descriptionHtml else params.descriptionText }}
    </p>
    {% endif %}
    <ul class="govuk-list govuk-error-summary__list">
    {% for item in params.errorList %}
      <li>
      {% if item.href %}
        <a href="{{ item.href }}"{% for attribute, value in item.attributes %} {{attribute}}="{{value}}"{% endfor %}>{{ item.html | safe if item.html else item.text }}</a>
      {% else %}
        {{ item.html | safe if item.html else item.text }}
      {% endif %}
      </li>
    {% endfor %}
    </ul>
  </div>
</div>
</div>

This seems to work in JAWS, in initial testing:

https://user-images.githubusercontent.com/1650875/172677482-e9c28202-07f2-494d-8a33-a2145f36671b.mov

Here is the JAWS output:

Alert!
There is a problem
There is a problem

The date your passport was issued must be in the past
http://192.168.8.205:3000/components/error-summary/preview#example-error-1
The date your passport was issued must be in the past

Enter a postcode, like AA1 1AA
http://192.168.8.205:3000/components/error-summary/preview#example-error-2
Enter a postcode, like AA1 1AA

Blank                                   // Unsure why JAWS says "Blank", to be investigated
Page has 1 region, 1 heading, and 2 links

There are some more things to iron out with this idea:

4. Removing the .focus JS (unexplored)

The thought here is that in a real service, focus by default should land at the top of the page, near the skip link, which when actioned can move focus to right before the error summary.

I added this in for completeness sake, but I think it would very likely break existing AT, as the .focus was added for a good reason. The role="alert" on its own likely does not work reliably across all AT.

There is also no way to use feature detection, or UA/AT sniffing to conditionally enable it.

TODO

I think the wrapper idea is worth pursuing, so I'll investigate it with multiple AT and see if they work as expected.

JAWS with Internet Explorer / Edge is one of the most commonly used AT combinations. Fixing this issue would likely be beneficial to a lot of users.

owenatgov commented 2 years ago

@tvararu Thanks a lot for this incredibly thorough investigation! We're going to start experimenting with the solutions you proposed. Without investigating this thoroughly myself, I personally think that the wrapper proposal seems the most robust. Please let us know if you make any further progress.

selfthinker commented 2 years ago

Have you considered removing the role="alert"?

It's actually not used according to spec. (It's supposed to be used when something changes via JavaScript not when the page reloads.) That was intentional to fix a specific issue in VoiceOver on iOS and TalkBack on Android. See https://github.com/alphagov/govuk_elements/pull/511 for a further explanation.

Removing it would be the more accurate use of it. It's worth rechecking what that would do in VoiceOver on iOS and TalkBack on Android. If that fix isn't needed anymore, you should just remove it.

owenatgov commented 2 years ago

Initial investigation into removing role="alert"

I had a go at removing role="alert" first as per @selfthinker's suggestion above and found the following results.

Test details

Used the following screen reader/browser combos:

Page used to test: https://govuk-frontend-review.herokuapp.com/full-page-examples/passport-details

Results

Next steps

adamliptrot-oc commented 2 years ago

@owenatgov The JAWS issue might be a case of JAWS examining the page, seeing it is the same/similar and deciding that it should place the cursor at the same point at which you were last on the page. Clearing the buffer (modifier key + Esc) after the page loads consistently places the screen-reader cursor in the error summary box and so might support this.

owenatgov commented 2 years ago

I'm going a little off base from what I said I'd do next, however I had a small hypothesis that this might have been a bug in JAWS 2022 as I was surprised that it didn't respond positively to just programatically focusing without the role="alert" being present. I made a quick codepen to test this theory and in every screen reader/browser combo in the service manual, including JAWS in edge, the screen readers all successfully read out the link that we want them to focus on on load.

This makes me think that there's something else wrong with our error summary implementation, possibly to do with how we're making the containing element focusable programatically. I'm going to spend a little more time exploring this and if I don't get anywhere, I'll return to the wrapper solution.

owenatgov commented 2 years ago

I think I've found a solution 🎉 I can get JAWS and other screen readers to consistently read out the error summary and it's contents on page load by doing the following:

My hypothesis for what happened here is that JAWS 2022 firstly didn't accept us trying to focus a tabindex="-1" element and so refused to read it out, and secondly was only reading one child deep on programatic focus.

I'd like to run this past our team and also assess the consequences of removing that child div where the content of the error summary lives. Overall though I'm feeling positive about this!

owenatgov commented 2 years ago

Some additional musings on the above solution:

Firstly, @36degrees rightly pointed out that it's odd that tabindex="-1" would be causing an issue here as the whole point of -1 is that it's not focusable via the DOM but is still focusable programatically. From MDN's tabindex guidance:

A negative value (usually tabindex="-1") means that the element is not reachable via sequential keyboard navigation, but could be focused with JavaScript or visually by clicking with the mouse. It's mostly useful to create accessible widgets with JavaScript.

This therefore could be a combination of a bug in JAWS and light issues with our markup. @36degrees found this issue which feels related. I think it's a good idea to prompt on the VFO issue with our issue here to ensure it doesn't lose traction.

Secondly, the markup change would mean this is a breaking change and therefore we'll need to wait for the next major release to launch this. I've added the v5 milestone pre-emptively and propose we keep this solution ino ur back pocket until we're ready to publish v5.

I'm very open to further thoughts on this.

tvararu commented 2 years ago

Sounds sensible to me @owenatgov, and thanks for looking into this!

Looking at the VFO issue it looks like Steve Faulkner pointed out that there has been a fix merged in a recent version of JAWS. I'll try downloading the latest trial (June) now and see if it behaves differently.

Secondly, the markup change would mean this is a breaking change and therefore we'll need to wait for the next major release to launch this. I've added the v5 milestone pre-emptively and propose we keep this solution ino ur back pocket until we're ready to publish v5.

In theory, this could be shipped as a non-breaking change, by writing a bit of JavaScript in the error summary initialiser. It could identify the structure of the Error Summary and progressively enhance it to the new and better form, by removing attributes and lifting/shifting content. This could be done in a way that is compatible with both new/old HTML.

In practice, such a solution would need a lot of testing, as it could break things in an unexpected way. And regardless, it's unwarranted in this situation if the issue resolves itself in JAWS.

(However, this sounds like a technique that could be useful for other breaking govuk-frontend HTML markup changes)

tvararu commented 2 years ago

Just tried JAWS 2022.2206.9 ILM, Microsoft Edge 102.0.1245.44.

Our issue persists, but indeed the codepen testcase that Steve is referencing now passes.

I do think our issue is related and I think it's a good idea to bump as you suggested @owenatgov.

This comment in particular resonates:

It's a pretty straight forward issue and when vendors demo their products and JAWS doesn't announce the change customers assume it's an issue in the vendors products - when it's not.

The exact same situation played out with our service; an internal accessibility auditor found this behaviour and marked 3.3.1 as Failed on an audit...

36degrees commented 2 years ago

Secondly, the markup change would mean this is a breaking change and therefore we'll need to wait for the next major release to launch this. I've added the v5 milestone pre-emptively and propose we keep this solution ino ur back pocket until we're ready to publish v5.

I'm very open to further thoughts on this.

If the old markup still works – as in, if users don't make the markup change then the error summary displays the same and functions at least as well as it does now – then we could consider treating it as a 'recommended change', and flagging it in the release notes for the next major version.

There is a slight risk there, as we need to remember to make sure that any changes in 4.x releases work for both the old and new markup, and we don't really have a good way of doing that in the review app at the mo.

We'd want to think it through, but my instinct is that the risk is probably manageable - we're just talking about a wrapper div that would end up doing nothing I think?

owenatgov commented 2 years ago

The plot thickens.

In trying to replicate this using codepen for that JAWS issue, I stumbled upon an alternative solution that I think is even simpler. By removing the aria-labelledby which currently hooks up to the h2 in the error summary and maintaining the removal of the wrapper div around the potential list items and copy, JAWS and other screen readers announce the error summary and its contents on page load consistently 🙌🏻

This solution makes more sense to me as we were applying a labelledby of just the title so my guess was that JAWS wasn't smart enough to continue looking for the other children in the focusable element once it had read out the labelledby. What I don't understand is why applying tabindex="0" would overwrite this behaviour. I was noticing a lot of repetition from JAWS in my testing, consistent with @tvararu's prior testing, so perhaps it was reading out the labelledby then feeling ok about reading out the contents of the focused element. I wonder if the bug here is something to do with how JAWS interprets children of a programatically focusable element.

To summaries, here are the changes I'm going to make to the error summary code:

  1. Remove role="alert"
  2. Remove the aria-labelledby and it's associated attributes
  3. Remove the div that contained the error links and error summary copy

I'm also going to do the following:

  1. Still prompt the JAWS issue linked above noting that we've had some odd behaviour related to tabindex="-1"
  2. Style the child div in such a way that it accounts for old and new markup, as per @36degrees's recommendation. To this effect I've removed the v5 milestone and this can go into the next available 4.X release.

I'm conscious that there may be some deep behaviour around aria-labelledby that I'm unaware of so if this solution is troubling or confusing to anyone please speak up.

owenatgov commented 2 years ago

A quick update as a lot of the discussion on this topic moved to the PR:

In short, my solution doesn't completely solve the problem. I believe it improves the experience for screen reader users but there are still instances of JAWS not reading out the entire contents of the error summary as desired and/or moving focus around the page. See my comment on the PR for details. It's at this stage that I am very confident that it's nothing in our code that's causing this but something at JAWS's end.

Before I raise an issue or prompt on an existing issue, it'd be good to see if the latest JAWS beta, mentioned in the issue we found in the JAWS issue list, works well with the latest iteration of the updated error summary markup as per the PR (review app for change). @tvararu Am I right in thinking that you/your team have access to the JAWS beta? If so, when you have a spare second, it's be super helpful if you could test our work and see where we're at. I don't believe we have access in the team as we're all on macs and our testing environment (AssistivLabs) doesn't let us move versions around easily.

I'm sorry this has been hanging around a while, it's been a tricky one!

tvararu commented 2 years ago

@owenatgov No worries! We've been following intently your updates on the PR :+1:

Am I right in thinking that you/your team have access to the JAWS beta?

I'll ask in wider DfE, I personally don't. I had to use the public JAWS trial in my own videos. Will get back to you on this.

owenatgov commented 2 years ago

Another update:

I misspoke in my previous update; it is possible to update our version of JAWS through a trick I wasn't aware of (thanks again @36degrees). After doing this, I discovered that my present solution had completely broken.

After starting again, I found that the most appropriate solution, after all, was what was suggested originally: move the role="alert" out of the error summary parent into a child container. This is a shame that we weren't able to remove role="alert" but this solution works well. I've also removed aria-labelledby as I've found this reduces unnecessary verbosity across screen readers ie: some of the multiples of announcements of the h2. My other proposed changes (changing tabindex from 0 to -1, removing the div containing the error summary content body) have made no difference from testing.

It's worth noting that my testing was consistent with problems raised in existing issues outside this one with the error summary on VoiceOver (https://github.com/alphagov/govuk-frontend/issues/2072) and NVDA (https://github.com/alphagov/govuk-frontend/issues/2055).

I've updated the PR and will re-prompt the design system team and the DfE folks for another review.