Convey AJAX progress messages to assistive technology.

Looking at where the throbber is created in JavaScript `/core/misc/ajax.es6.js:817`:

Drupal.Ajax.prototype.setProgressIndicatorThrobber = function () {
    this.progress.element = $('<div class="ajax-progress ajax-progress-throbber"><div class="throbber">&nbsp;</div></div>');
    if (this.progress.message) {
      this.progress.element.find('.throbber').after('<div class="message">' + this.progress.message + '</div>');
      console.log(Drupal);
    }
    $(this.element).after(this.progress.element);
  };

I wonder if it would be sufficient to add `role="alert"` to the message div. I am testing this locally on my Mac with VoiceOver and it seems to work, but I was thinking the alert role needed to be present on initial page load to be announced to AT.

This is announcing the message to me with VoiceOver on macOS:

Drupal.Ajax.prototype.setProgressIndicatorThrobber = function () {
    this.progress.element = $('<div class="ajax-progress ajax-progress-throbber"><div class="throbber">&nbsp;</div></div>');
    if (this.progress.message) {
      this.progress.element.find('.throbber').after('<div role="alert" class="message">' + this.progress.message + '</div>');
      console.log(Drupal);
    }
    $(this.element).after(this.progress.element);
  };

Here is a patch making the change I have described above.

@andrewmacpherson, yes it's my first core patch :) Glad I finally was able to submit one!

I think Everett brings this pattern up (in a positive way) in his DrupalCon talk about Javascript https://www.youtube.com/watch?v=UKkBBSDA3TQ

Great initiative!

Should we also add support for an announcement when the AJAX command is finished?

Hi!
Added patch which announcing:

1. 'please wait ellipsis' on throbber/bar/fullscreen starts (maybe remove '...' ?)
2. 'success' on ajax success
3. 'custom message' on error if it setted.

seems for FunctionalJavascript tests passing Dupal.announce should be added as dependency.

This builds on the approach in #27. This addresses the test failures and adds settings to determine if progress should be announced (defaults to true) and how long to wait for an Ajax request to be in progress before an announcement is generated (defaults to 1000ms).

Per @andrewmacpherson's guidance, we now have working implementations (patches passing tests) for both of the proposed approaches:

1. making the existing "AJAX progress" markup not just provide a visual and textual indication like it does today, but also make it trigger an announcement: #6
2. using the Drupal.announce() API: #33

I defer to the accessibility maintainers, but there are a few things that @andrewmacpherson wrote in #29 that I wanted to chime in on:

We'd also want to avoid excess verbosity; no need to announce "please wait" if an AJAX request is completed quickly.

This is impossible to know. It depends on both network conditions and server-side capacity and business logic complexity. With varying network conditions, this is literally impossible. Assuming constant network conditions, it'd still depend on the particular Drupal instance, because for example an autocomplete would be super fast on a small site but might take seconds on a site with an enormous amount of content. Even in #33's automatic logic to only announce things after one second, that may still result in an unpleasant experience because if the announcement implies waiting and then there is no need for waiting … that still yields an odd experience.

Overall, I think managing separate live regions in the same location as the visual throbber is the most desirable approach, but it seems fiddlier.

This looks pretty good to me, overall. A few changes suggest themselves:

1. +++ b/core/lib/Drupal/Core/Render/Element/RenderElement.php
   @@ -405,6 +405,14 @@ public static function preRenderAjaxForm($element) {
   +      // Set default value for progress screenreader enabled status if needed.
   +      if (isset($settings['progress']) && !isset($settings['progress']['announce'])) {
   +        $settings['progress']['announce'] = TRUE;
   +      }
   +      // Set default value for progress screenreader delay if needed.
   +      if (isset($settings['progress']) && !isset($settings['progress']['announceDelay'])) {
   +        $settings['progress']['announceDelay'] = 1000;
   +      }
   

   This could just be slightly less verbose, I think; something like:

   if (isset($settings['progress'])) {
     $settings['progress'] += [
       'announce' => TRUE,
       'announceDelay' => 1000,
     ];
   }
   

   Also, I think we need to document these new properties. I'm not sure where the contents of #ajax are documented, though.

2. +++ b/core/misc/ajax.es6.js
   @@ -861,6 +865,16 @@
   +      // Wait 1 second before announcing progress to screenreader so fast ajax
   +      // operations are not announced.
   

   This is not exactly accurate, and could use a bit of grammatical polish. I would suggest something like this: "To avoid excessive verbosity, wait a bit (1 second by default) before announcing progress to screen readers, so that very fast AJAX operations are not announced."

3. +++ b/core/misc/ajax.es6.js
   @@ -861,6 +865,16 @@
   +        // The scope of {this} does not have access to when this.progress is
   +        // altered on success, so the check is to see if $(this.element) is
   +        // still disabled. Ideally there is a cleaner way to do this.
   +        if ($(this.element).prop('disabled') && this.progress.announce) {
   

   This could have accessibility implications of which I'm unaware. I have definitely heard some concerns from @andrewmacpherson about disabling elements in general, so I'm not sure this is what we'd want to do. If push comes to shove, surely we could do something like this?:

   const self = this;
   setTimeout(() => {
     // Do stuff with self
   }, this.progress.announceDelay);
   

4. +++ b/core/tests/Drupal/FunctionalJavascriptTests/Ajax/CommandsTest.php
   @@ -54,19 +55,20 @@ public function testAjaxCommands() {
   -    $this->assertWaitPageContains('<div id="drupal-live-announce" class="visually-hidden" aria-live="polite" aria-busy="false">Default announcement.</div>');
   +    $this->assertNotNull($assert_session->waitForElementVisible('css', 'div#drupal-live-announce.visually-hidden[aria-live="polite"][aria-busy="false"]:contains("Default announcement.")'));
   

   Why did this change? Is assertWaitPageContains() insufficient for those purposes?

Re: #35.3

This could have accessibility implications of which I'm unaware. I have definitely heard some concerns from @andrewmacpherson about disabling elements in general, so I'm not sure this is what we'd want to do.

The disabling of $(this.element) is existing behavior in ajax.es6.js, the patch just adds a conditional that is based on behavior that already exists.

This conditional is added so the progress announcement is not triggered if the AJAX request has already completed after the announceDelay. I debugged this to see if there were any other means of determining if the AJAX request was complete and couldn't find anything (though I suspect I may have missed something). The best way I could find to do it was checking if $(this.element) is disabled -- something that happens a few lines prior to adding the progress indicator and is reverted after the request completes. Very open to other ways of checking for completion in this scope!

At the very least, the comment can be rewritten to make this clearer.

#35.4

Why did this change? Is assertWaitPageContains() insufficient for those purposes?

Honestly I found something that worked while troubleshooting and just stuck with that. This deserves more explanation so I'll either get to the bottom of it or at worst provide a failing test patch to demonstrate it is necessary.

The disabling of $(this.element) is existing behavior in ajax.es6.js, the patch just adds a conditional that is based on behavior that already exists.

Fair enough. I think that can be considered a pre-existing accessibility sadness in Drupal's AJAX system, and sometimes small, unfortunate hacks are necessary in order to reach a greater good. As long as the comment is updated for clarity, I'm okay with that.

#35.1 Simplified the array and added documented the properties.

#35.2 Comment from a previous approach updated to reflect the current approach.

#35.3 Updated comment to hopefully make things more straightforward.

#35.4 The original test works fine and those changes were reverted. The changes were to address something in patch #26 that has since been changed.

+++ b/core/misc/ajax.es6.js
@@ -861,6 +865,20 @@
+      }, this.progress.announceDelay);

Discussed with @bnjmnm, and we realized that this.progress.announceDelay might be undefined. However, it seems that we can't change this in Drupal.Ajax's defaults and have it enter this scope (or at least, if we can, it's not clear how we would do that), so this line should probably be changed to the safer this.progress.announceDelay || 1000.

Beyond that, this looks okay and should go in front of the accessibility maintainers. We brainstormed a couple of ways that we might add automated tests for this, but we concluded that testing a "delay" might be a bit harder than it's worth (i.e., prone to random timing failures). So manual testing is probably sufficient for this (it'll need that anyway for accessibility sign-off).

Good call on #39, this makes the change.

Can someone help me with steps to reproduce so that I can test? Here's what I did:

1. Set up a fresh d8 install.
2. Applied patch #40
3. Added a file upload field to the article content type
4. Adjusted php.ini to allow super large uploads so I could get the throbber to run for a decent length of time
5. Created a new article, turning VoiceOver on when uploading a 500+ meg file

I did not hear any progress updates for the throbber. Am I doing something wrong? I'd be happy to test on NVDA too if I can confirm I'm not doing something wrong on VO.

Video of the testing with VO: https://www.youtube.com/watch?v=vDVJIpvvJdQ

Like @micnap, I'm having a very difficult time getting any announcement to come through, and haven't been successful in testing this.

I even set my browser speed to slow 3G, which didn't succeed in getting to any announcement.

Update the success message and fixed the repeated announcement issue.

@chaitanyamaili it's not clear what the context of your patch is. Is this built on top of earlier patches such as #40. If so, your patch should be the changes from #40 with your additional changes, diffed against the dev branch (currently 8.9.x). Making a drupal patch with git.

When changes are made to an earlier patch, it should also be accompanied by an interdiff, which shows what those changes are. Creating an interdiff

In other words:

• The .patch file should include ALL of the changes of the current solution
• The interdiff should be the difference between the current patch and the one prior to it

If your patch was not built on earlier versions and is a new solution, that needs to be made very clear in your comments. If that's the the case then the approach should probably include the additional functionality requested in the issue such as making the announcements optional and providing a delay.

Although I'm not 100% of the context, I can provide some feedback on what is there:

1. +++ b/core/misc/ajax.es6.js
   @@ -967,6 +968,11 @@
   +    if (status == 'success') {
   

   This should be a triple equals

2. +++ b/core/misc/ajax.es6.js
   @@ -967,6 +968,11 @@
   +    if (status == 'success') {
   +      Drupal.announce('Content has been updated.');
   +    } else {
   +      Drupal.announce('Not able to load the content, please try again.');
   +    }
   

   These messages would need to be more generic, "Content has been updated" is not applicable to all situations.

   Furthermore, strings need to be wrapped in Drupal.t() to facilitate translatability.

3. +++ b/core/misc/announce.es6.js
   @@ -55,13 +55,21 @@
   +      // To prevent same repeated announcement.
   +      if (announcement.text != previousText) {
   +        previousText = announcement.text;
   +        text.unshift(announcement.text);
   +        // If any of the announcements has a priority of assertive then the group
   +        // of joined announcements will have this priority.
   +        if (announcement.priority === 'assertive') {
   +          priority = 'assertive';
   +        }
   +      }
   +      else {
   +        previousText = '';
          }
   

   I don't believe this is in scope for this issue.

The feedback in #42 and #44 was very helpful -- I realized that the announcements were only added to elements rendered with $element['#ajax']['event'] present and with a progress message defined. Among other things, this can't depend on a progress message being defined as in many cases this is needed only the spinner is displayed.

This winds up expanding where this occurs significantly, so testing is even more important than before, and should actually provide results...

Review of #48:

1. I don't know what you mean here:

To avoid excessive verbosity,

Can you be more specific? What excessive verbosity would happen?

Ah I see: "so that very fast AJAX operations are not announced." Perhaps we can get that in there earlier. How about "To prevent very fast AJAX operations from being announced, wait before announcing progress to screenreaders for the duration of the announceDelay property (1 second by default)."

2. I'm having trouble getting this working. Using /media/add/video with a 70 MB video, It was uploading too fast. So changed the wait time to 10 milliseconds, and it still wasn't working. It turns out the _this.progress.message was empty:

Due to this, this code was failing:

      setTimeout(function () {
        if ($(_this.element).prop('disabled') && _this.progress.announce) {
          Drupal.announce(_this.progress.message);
        }
      }, 1000);

There's no check that _this.progress.message is set.

If I change it to Drupal.announce(_this.progress.announceMessage);it work, although it was read at the end.

3. Is this being used? Drupal.announce(customMessage); Andrew brought this up too in #41.

#49.1: rephrased that and hope its a little clearer.

#49.2: Looks like there were a few problems here that should be cleared up.

#49.3 It looks like Drupal.announce(customMessage); came from an earlier iteration of the patch that included error messages in the announcements. This is outside the scope of conveying progress messages, but should probably get a followup issue (particularly since the method of conveying these errors may require some opinionated decisions).

Some additional changes were made based on testing aided by Charles Proxy, which allowed me to throttle my network connection to very slow speeds. This opened up the scenarios I could test and it exposed the need to have a repeating message for two reasons:

• In some scenarios, the initial progress message was never announced due to being crowded out by other announcements that immediately followed the AJAX request (such as new element being in focus)
• Simulating a very slow connection made it apparent that during a very slow operation, the user needs a way to distinguish something in progress to something that ceased working.

In order to make announcements repeatable, a non-spoken unicode character is added in different amounts each time so it's interpreted as a new string that should be announced, but the content read is the same.

The times chosen for the announceDelay and announceIntervalTime properties are very rough guesses based on nothing more than they seem like good starting points for necessary research and discussion.

Tagging this very important issue to be worked on at DrupalCon Amsterdam.

Re-roll #50 for 9.3.x

Resolved #55command failed issue.

I didn’t test all the cases, but it looks good for me.

RE #59 if the most recent patch is fails testing or can't apply, the issue can't be marked RTBC, that's. Those are concrete requirements of having something be "Reviewed and Tested by the Community".

You also mention that

I didn’t test all the cases,

, so that precludes it from being set to RTBC as well. What is VERY helpful is to describe the test cases you did evaluate (plus any relevant screenshots) so another reviewer is aware what parts have already been covered. In general a review needs additional detail beyond "looks good for me.". It needs to be documented so the committer is aware what evaluation took place.

This is also tagged "needs accessibility review", so that needs to happen before RTBC too.

Rerolled the patch in #56, thanks!

The most recent patches were failing so I rerolled the patch presented in #61 against the 9.5.x branch and was able to get it to apply cleanly. Hopefully we can get some additional accessibility review around the effectiveness of these changes.

I would be happy to help with additional test cases & further edits to get this moving along. Should we also start planning for this against the 10.1.x branch as well to be proactive?

Looks like the spelling of "screenreader" is causing a build error so I have adjusted the two places called out to "screen reader".

Updated build command was missed during reroll, this should account for those changes.

Fixed spelling regression after updating ES6

It looks like passing more than one argument in WebAssert::responseContains was deprecated in Drupal 9.1. I've removed the secondary argument as it is also listed in the comments directly above the line.

Can the issue summary be updated. Number of "tbd" in there.

Looking at the test coverage can we also add a test for the message in the announce. Make sure that is getting the right message.

Adding tagging for 4.1.3.

Rebased on 11.x

Adding per #70

I rebased the merge request to fix the merge conflicts in #76 and updated the issue summary.

Left some comments on the MR.

I have created kernel and Nightwatch tests for this merge request.

I suspect the pipeline failures are related to the changes. I put details in comments on the MR.

Back to green!

The issue summary needs steps to reproduce to help reviewers.

I've added 'Steps to reproduce' and Testing the MR' to the issue.

Just to be sure: I tested this again (with 5000ms like in the description) and I can confirm it work based on the steps.

I tested this on Mac OS 27.0.1 with Voice Over on and in Chrome with Network throttling on.

I've tested MR4913 based on the instructions provided in the issue summary by @f0ns. In the content view (progress_view.mp4) and the test module (progress_test.mp4) were tested on macOS 26.7.1 with VoiceOver active in the latest version of Edge (155.0.4283.45) - trying to throttle Safari seemed too hackish https://www.browserstack.com/guide/how-to-perform-network-throttling-in-.... In both tests only the disabled button got announced but the actual progress was not unfortunately. :/ And I will also try to test the MR in Orca in a virtualized Fedora later tonight.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

I couldn't reproduce this (Safari and Chrome) but browser compatibility can be tricky sometimes (understatement).

I've altered the code, the first announcement is now assertive and the repeats stay polite.

This should fix it imo.

@rkoller Would be great if you can give this another go in Edge when you find the time. Thanks for the help!