Add announcement after clicking 'Save and select' in the media library.

Promoting to major bug as per #2834729-200: [META] Roadmap to stabilize Media Library.

There is already an announcement for "Save and Insert" - this is what is read by Voiceover after inserting media with that button (with the announcement in bold). The button that opened the add media dialog is focused, then:

Add media button, group. Added one media item. You are currently on a button, to press this button press control option space.

Of course it can be changed to something different if "Added one media item/Added media items." is not sufficient.

In this patch, I added a similar AnnounceCommand to \Drupal\media_library\Form\AddFormBase::updateLibrary(). The results are very different in Chrome and Safari (the announcement is in bold)
Chrome:

1 item selected, You are currently on a dialog inside of web content. To exit this group, press Control-Option-Shift-Up Arrow

*The announcement is never read, but is present in the drupal-live-announce div. The behavior is the same whether aria-live it set to polite or assertive*

Safari:

", web dialog, with items. Selected. New item added to Media Library. )"

Aside from needing to find a way to get Chrome to acknowledge the announcement, I'm also wondering if the other Chrome/Safari differences are acceptable (particularly since Safari will just read through what is potentially a very long form). If this difference is a problem, it may inform how the Chrome issue is addressed.

Note: One approach already attempted for the missing announcement in Chrome: using a dialog event listener. This did not work as "Save and Select" does not generate a new dialog - it uses an AJAX replace instead.

Thanks @seanB! based on your feedback I added the patch from #3081523: Improve the focus behaviour after clicking 'Save and select' in the media library.. It addresses most of the discrepancy between Chrome and Safari's announcements. Unfortunately, the announcement is still missing from Chrome, but the fact that the announcements are no longer VERY different makes this much easier to deal with. Perhaps the success message will help with that.

I just ran through this with VoiceOver on Chrome and Safari using the patch in comment #3.

While it would be great to get Chrome working correctly with VoiceOver, it is commonly known that VoiceOver is designed to work with Safari and there will be differences. As long as the differences are usable, it isn't a blocker to have them be different.

I think the newest patch solves the issues that are raised by this ticket.

Chrome:
When going through in Chrome, after adding items I am told how many items I've added, and it is understandable.

Safari:
When using Safari, I can tell how many media items I've added, and how many more I can add. This is very nice. While it would be great to have this second piece in Chrome, it is not a blocker.

I'm going to flag this for @cboyden's team to test with Jaws and NVDA during their assistive tech walkthrough on Thursday, as well.

------

I'm curious about the protocol for creating tickets for new items that I found while doing this? I don't want to mess with the current flow, so looking for the right way to do this. Here is what I found for the record (which I'll branch off with feedback):

1. The fact that Chrome reads everything in the form when selecting a media tab is very frustrating when using the screen reader to add media. It would be great if we could create a label to focus on, instead, that would say something like "Active Tab: Add Image Media"
2. When you reach your maximum limit, Chrome does not read the beautiful max items selected text from the dimmed button that Safari users hear. Instead, it just says "You are currently on a button, this item is dimmed" -- perhaps the button text can be dynamically labeled so that Chrome reads the right thing, as well?
3. It would be really nice to completely hide the grid vs. table view options for the media list when using a screen reader, as these are confusing and don't help a screen reader user in any way.

Thanks @rainbreaw, this helps move things forward.
Regarding the filing of new issues, by all means create new ones when you run into problems that haven't been reported. It's very appreciated and not at all disruptive. Very glad you're noticing these things.
A few things to keep in mind when you create the issues

• Leave a comment in the original issue that says something like "while reviewing this I discovered a new problem and filed an issue for it here: {link to the issue}"
• Usually, the original & new issues will reference each other in the "related issues" field. This isn't always necessary as there are also times where they are truly unrelated and you just happened to discover it while reviewing. NBD either way since that's very easy to add later.
• In the issue summary of the new issue, I often find it helpful to reference the issue I was reviewing when I discovered the problem, as it often helps people better understand the issue and how to reproduce it. ("This was discovered while reviewing {link to the issue}...")
• In the case of Media Library, which currently has a roadmap to stable, you may want to add some of the issues you created to the roadmap as well. Something like #6.3 should probably be there since it happens in all browsers and is not good a11y. Ones where it's less clear if they're roadmap-worthy can be discussed in Slack.

Updated the patch to deal with multiple additions and added the patch from #3081523: Improve the focus behaviour after clicking 'Save and select' in the media library. so the screenreader doesn't choke on a large element and should help with the upcoming assistive tech walkthrough.

I also tried a few different approaches to see if I could get Chrome to cooperate with voiceover, but they were unsuccessful. It sounds like this may be acceptable, but if the conclusions of the walkthrough determines something different there are other options to investigate.

Re #10

I'm wondering if we should use the messenger service to add a success message instead of an announcement? The messages also have an announcement (by default a part of messages if I'm not mistaken) and will provide extra visual confirmation.

This is a good suggestion - and the outcome in this patch is bittersweet.
I created an AJAX Command (with test coverage) for the JavaScript Drupal.Message() functionality, so the message could appear in the dialog. This will be useful to Media Library and core in general. However, I also discovered that the announcement will not make it through (at least with Voiceover, regardless of browser) if the priority isn't set to assertive.This can't be done with message without changing the message type, so AnnounceCommand still needs to be there. A message has still been added to the experience, though.

[Edited to clarify: this is not an issue with the AJAX Command, which works fine with screenreaders overall. Rather, this is an issue specific to how Voiceover (and perhaps other screenreaders -- they have not been tested) behaves after the Media Library Widget form is rebuilt after uploading media. I had this same issue when I was using AnnounceCommand with default priority]

If it turns out other screenreaders do not need the "assertive" priority to be announced, then this potentially means the announcement being read twice. If this is the case, the Message portion of this will need to be discarded, but the MessageCommand should still probably be added in a separate issue.

I asked @bnjmnm what this needs in order to land; he says it could use general accessibility review, especially confirmation (or denial) that non-VoiceOver screen readers don't announce the message multiple times.

Haha I unwittingly implemented a simpler (and probably hence inadequate) variant of this too in #3034242-8: Hide "Save and insert" and "Additional selected media" from users by default, see #3034242-9: Hide "Save and insert" and "Additional selected media" from users by default point 4.

I added a more generic issue for the new ajax command: #3086096: Add a generic Ajax Message command

We need this for this issue too: #3059955: It is possible to overflow the number of items allowed in Media Library

For #15 + #16.

Rerolling on top of #3086096-20: Add a generic Ajax Message command

I've tested this with VoiceOver on MacOSX, with both Chrome and Safari, no eyes, no mouse, and the announcement works well.

1. Selected the Add Media button in a media field to load access to the library
2. Navigated to add an image, uploaded an image through my file system
3. Received screenreader confirmation that the file was added
4. Chose "save and select" which took me out of the modal and back to the node form
5. Received screenreader confirmation that the media was added

Noting here that it looks like #3059955: It is possible to overflow the number of items allowed in Media Library may be moot now, as when testing this it no longer allowed me to add more than the allotted number of items with the screenreader/tab key (though I haven't tested this yet).

Is there something else that needs to be reviewed here, or can this be moved to RTBC?

Thanks @rainbreaw! I think we can move this to RTBC once #3086096: Add a generic Ajax Message command is in, just by reposting the do-not-test patch from #18. Removing the "needs accessibility review" tag.

Yep, once #3086096: Add a generic Ajax Message command lands this is ready :)

Crediting @rainbreaw for accessibility reviews, @seanB for code reviews, myself for code review (duplicate implementation in another issue), @phenaproxima for coordinating.

Uncrediting myself; thank you for the kind thought, but my work did not help shape this patch. :)

Reuploading the patch from #18 now that #3086096: Add a generic Ajax Message command landed 🥳

1. +++ b/core/modules/media_library/src/Form/AddFormBase.php
   @@ -693,12 +695,20 @@ public function updateLibrary(array &$form, FormStateInterface $form_state) {
   +    $announcement = $this->formatPlural(count($media_ids), 'Added one media item.', 'Added @count media items.');
   

   I think there ways of getting entity labels - see \Drupal\Core\Entity\EntityTypeInterface::getCountLabel(). But not entirely sure of how it should be used. I think this means you can do $this->t('Added @label.', ['label' => $entity_type->getCountLabel(count($media_ids))]);

2. +++ b/core/modules/media_library/src/Form/FileUploadForm.php
   @@ -175,6 +175,12 @@ protected function buildInputElement(array $form, FormStateInterface $form_state
   +    // Add container for displaying messages inside the dialog.
   +    $form['messages'] = [
   +      '#type' => 'container',
   +      '#id' => 'media-library-file-messages',
   +    ];
   

   It would be great to have some test coverage of the message appearing etc...

Assigning to myself to work on test coverage.

     $response->addCommand(new InvokeCommand("#media-library-content [value=$media_id_to_focus]", 'focus'));
+    $response->addCommand(new MessageCommand($announcement, '#media-library-file-messages'));
+
+    // An AnnounceCommand must be added as the message is not read unless the
+    // priority is set to assertive. Typically MessageCommand would be
+    // sufficient.
+    $response->addCommand(new AnnounceCommand($announcement, 'assertive'));

This sounds suspicious. The new MessageCommand employs Drupal.Message() in the browser, which in turn uses Drupal.announce(). Can't the assertive priority be passed via MessageCommand?

I see that there is an InvokeCommand for focus, which precedes the MessageCommand and AnnounceCommand in this PHP code. Does the client-side AJAX library process the commands in this order? I'm wondering if this could be a timing issue, rather than the difference between polite and assertive live regions. A focus change is far more important than a polite announcement; so if an polite announcement was made with Drupal.announce(), and then a focus change happens afterwards, then assistive tech could be justified in ignoring (or aborting) a polite announcement. OTOH if a change was made in a polite live region after the focus change, then AT could just defer it until after the name/role/value of the focused control was announced.

aria-live="assertive" has flaky support across browsers and AT. Using assertive instead of polite can make no difference at all in some browers/AT combinations. So it's not be a reliable way to make sure information is conveyed here. My knowledge about this is sketchy and a couple of years out of date, mind you.

Comment #13 hasn't been addressed. Has manual testing been done with any browser and screen reader combinations on Windows? I only see VoiceOver + Chrome/Safari mentioned here. I'm less concerned about repeat announcements, than whether the announcement is made at all. Particularly since this is the first time we're using this pattern with MessageCommand.

Speculation: a polite/assertive live region might be following some user preferences for verbosity and/or delay. For example, VoiceOver (on macOS) has a setting to control the delay before announcing descriptions. A support request we commonly hear in the a11y community comes from developers want to know why their aria-describedby isn't working. It turns out it does work, but they were tabbing to another control quicker than the delay setting. Now I'm wondering if any screen readers have implemented verbosity/delay settings for aria-live announcements; AFAIK they don't, but maybe that's changed.

Addressing #24
1. ✅ Nice catch, updated.
2. ✅ Added test coverage.

Additional changes:

Renaming the selector based on #3059955-22: It is possible to overflow the number of items allowed in Media Library and work done after that, so that the selector matches that patch.

@@ -178,7 +178,9 @@ protected function buildInputElement(array $form, FormStateInterface $form_state
     // Add container for displaying messages inside the dialog.
     $form['messages'] = [
       '#type' => 'container',
-      '#id' => 'media-library-file-messages',
+      '#attributes' => [
+        'class' => ['js-media-library-messages'],
+      ],
     ];

After working on the MessageCommand, I was pretty sure we don't need the extra AnnounceCommand, so I tested it out and you can set it assertive from the new MessageCommand like this:

$response->addCommand(new MessageCommand($message, '.js-media-library-messages', ['priority' => 'assertive']));

It will reach Drupal.Announce with the correct priority. I see that Andrew picked up on this as well in #26.

The new MessageCommand employs Drupal.Message() in the browser, which in turn uses Drupal.announce(). Can't the assertive priority be passed via MessageCommand?

I updated the change record to explain how this is done: https://www.drupal.org/node/3086403

I intentionally left the code coverage here light, because #3055648: Frequent random fail in \Drupal\Tests\media_library\FunctionalJavascript\MediaLibraryTest is RTBC which totally refactors this test. Once that is in, we can add the announce assertion to this method:

+  /**
+   * Clicks "Save and select||insert" button and waits for AJAX completion.
+   *
+   * @param string $operation
+   *   The final word of the button to be clicked.
+   */
+  protected function saveAnd($operation) {
+    $this->assertElementExistsAfterWait('css', '.ui-dialog-buttonpane')->pressButton("Save and $operation");
+
+    // assertWaitOnAjaxRequest() required for input "id" attributes to
+    // consistently match their label's "for" attribute.
+    $this->assertSession()->assertWaitOnAjaxRequest();
+  }

If the $operation is "select", we can assert the announcement happens within that method.

So to keep the changes light for the inevitable refactor, I'm only adding light test coverage for now.

I copied over ::assertAnnounceContains() from MessageCommandTest(). I had copied it into MessageCommandTest(). So I think it's time to create a test trait for this: #3087213: Create a test trait for assertAnnounceContains and assertAnnounceNotContains

Removing unused use statement.

Reroll after this issue committed: #3055648: Frequent random fail in \Drupal\Tests\media_library\FunctionalJavascript\MediaLibraryTest

Manually tested with Voice Over + Safari. Immediately after triggering "Save and select" this indeed announced Added 1 media item 🥳

Was going to RTBC, but then figured @andrewmacpherson would probably want to do so himself.

Tagging this to be worked on at DrupalCon Amsterdam.

Worked 3081526-23.patch for me. After clicking the 'Save and select' buttons of the add form, the outcome is announced on applying the patch. See the screenshot announcement.png

@chaithanya.m, thank you for confirming that the patch in #23 worked.

The patch in #29 no longer applies. This will need a reroll, setting to NW for that. Also to do is the accessibility review and the issue summary is out of date as well.

Patch rerolled.

#36 looks good. Needs code review before moving to RTBC

Applied patch #36 on 9.2.x and it works fine.Now the notice is showing,Adding screenshot after applying this patch.

Adding NorthAmerica2021 and Easy Out of the Box tags for visbility.

DrupalCon NA is April 12-16 with a focus on EOOTB on Wednesday, April 14.
Thanks

Reroll patch for 9.3.x. Attaching a screenshot.

Verified and tested patch #45.
Patch applied successfully and looks good to me.

Testing Steps:
# Goto: Extend
# Install Media Library module
# Goto: node/add/article
# Select Full HTML from text format
# click on the "Insert from media library" icon
# Choose the image
# User is not able to see "Added 1 media item" message once adding the image.

Expected Results:
# User should see the "Added 1 media item" message once adding the image.

Actual Results:
# User is not able to see the "Added 1 media item" message once adding the image.

Looks good to me.
Can be a move to RTBC.

Thanks for testing this! I think we still need it to be manually tested with a screen reader and signed off by a member of the accessibility team, though, since this is primarily an accessibility issue. Also, I'm wondering if we still need to display a visible message, since there is an AnnounceCommand in core.

I'll see if I can get a member of the accessibility team to review this.

i am getting error while applying rerolled-3081526-36.patch for drupal 9.3.x-dev.Adding screen shot for the reference

Patch 'MR !764 mergeable' applied successfully on 9.5.x-dev.

Test Results:
1. Announcement "Added 1 media item" is visible on adding the image.
2. Tried with VoiceOver, it is able to read the announcement once image is added. However, prefer review by accessibility team.

After patch:

RTBC +1

Triaging for bugsmash so tagging. I'm not sure what needs to updated in the issue summary per #35.

Opened a new MR for D10

Adding WCAG 3.1.1.

Still needs an accessibility review

Also the issue summary update was requested in #35 and can't tell if that's happened.

Woops. 3.3.1
https://www.w3.org/WAI/WCAG21/Understanding/error-identification.html

So close, but... It should be 3.3.1 - sorry folks.
https://www.w3.org/WAI/WCAG21/Understanding/error-identification.html