If the user attempts to upload an incorrect file type through the media library modal, the error message is not read by the screenreader

This relates to WCAG "Error identification" at level A, and "Error suggestion" at level AA. So far we've been treating WCAG level A as major/must-have.

We are SO close to marking Media Library as a stable module, and it was only filed at the eleventh hour prior to alpha week. Treating it as a must-have stable blocker will throw a serious spanner in the works. The release managers have previously indicated that accessibility bugs are eligible for inclusion after marking a core alpha release.

I'll bring this to the release managers' attention in #2834729: [META] Roadmap to stabilize Media Library.

Adding credit for @cboyden, whose team found the bug.

I'm thinking we might be able to just change this (changing polite to assertive):

Before:

          $(this)
            .closest('div.js-form-managed-file')
            .prepend(
              `<div class="messages messages--error file-upload-js-error" aria-live="polite">${error}</div>`,
            );

After:

          $(this)
            .closest('div.js-form-managed-file')
            .prepend(
              `<div class="messages messages--error file-upload-js-error" aria-live="assertive">${error}</div>`,
            );

Although perhaps having a messages element there beforehand is more proper.

Note, this code is in /core/modules/file/file.es6.js, so this bug is more general that media library.

@oknate if the container that has the aria-live role is not present on page load, the content of the error will not be read out by the screenreader. The container should be present and empty, and the error added to it.

I tried the approach in #5 and it did not work, unfortunately.

Patch #4 for this issue #3087305: Form validation error messages within the Media Library widget are not read by the screenreader addresses this issue. The overall issue may be going with a different approach, but it could potentially be cherry picked for a fix on this one. Argh, this is not correct. That patch works for errors with the oembed form, not file upload. Sorry for the noise.

This patch works for me using Safari + VoiceOver.

Note, I tried to do this properly, adding an aria-live region on page load and then appended the error and it never read it out and then I tried again, I switched the wrapper to a role="alert" and did the same thing and it never read it out, but then I simply added Drupal.announce worked like a charm, it announced the error right away.

The "right" way didn't work:

#8 can be tested, but I need to add a dependency on drupal.announce in the library that holds file.js

The patch is currently targeting the File module's JavaScript, so I don't think we can honestly call this a problem with the media system. ;-)

Tagging for accessibility review.

This patch uses Drupal.message instead of a custom rolled output:

-          $(this).closest('div.js-form-managed-file').prepend('<div class="messages messages--error file-upload-js-error" aria-live="polite">' + error + '</div>');
-          Drupal.announce(error, 'assertive');
+          var $messages = new Drupal.Message($messageWrapper[0]);
+          $messages.clear();
+          var $messagesId = $messages.add(error, { 'type': 'error', 'priority': 'assertive' });
+          $('#' + $messagesId).addClass('.file-upload-js-error');

This will allow it to inherit fixes to Drupal.Message and Drupal.announce.

Re: except that we don't hear "error message", this is an issue with Drupal.Message and Drupal.announce. I attempted to add the same visually hidden h2 with the "Error message" text, but I think it's better to handle this in a separate issue, since this is the case with all messages output through the javascript Drupal.Message as well. It doesn't happen with the other issue about alternative text because the message comes from the back end and it pulls all the text including the visually hidden text.

#3089402: Drupal.Message should add 'Error message' prefixed to Drupal.announce error messages

I wasn't able to fix it working in Chrome yet. I think it starts to read the message and then it's interrupted by "close dialog"

I ran patch through 'yarn prettier' that reformats the code.

#15:

I attempted to add the same visually hidden h2 with the "Error message" text

The messages dynamically inserted by Drupal.message intentionally don't have the same H2 as the messages which are present in a full page load. This is by design, and you can find the arguments in the issue which originally added the drupal.message library. The reason is that the library can't know if H2 is an appropriate heading level, at the point where it is inserted. When inserting a heading dynamically, it effectively re-parents all content following it, until another heading of the same or higher priority. That's potentially very disruptive to the document outline for the page as a whole, so drupal.message doesn't add headings.

Instead, it's up to module developers to decide whether a heading is appropriate for their dynamic JS messages, and include the heading in a pre-existing message area.

I don't think a H2 is appropriate for the media library dialog; the messages are just being inserted near the start of the dialog content.

The server-side messages in a full page load have the H2 because it's a Drupal block. These can only be inserted in theme regions, and most themes have an appropriate theme region. We assume the site builder or themer will place this block wisely. (Aside: since layout builder was added, I'm not sure the assumption of H2 for blocks holds true any more.)

1) Fixing the test.
2) Adding the prefix, but with a span instead of an h2. Andrew had suggested using just 'Error' instead of 'Error message' for the prefixed text, but for now I'm leaving it 'Error message', since the backend is using 'Error message' in the H2, and message.es6.js has this:

    /**
     * Provide an object containing the available message types.
     *
     * @return {Object}
     *   An object containing message type strings.
     */
    static getMessageTypeLabels() {
      return {
        status: Drupal.t('Status message'),
        error: Drupal.t('Error message'),
        warning: Drupal.t('Warning message'),
      };
    }

We change the output in this use case to Drupal.t('Error') if there's consensus to shorten it. Since the beta deadline is approaching, perhaps it's best to leave it to this other issue: #3089402: Drupal.Message should add 'Error message' prefixed to Drupal.announce error messages

Looks like that most recent patch has ~200k of extra gunk in it

1. +++ b/core/modules/file/file.es6.js
   @@ -24,6 +24,19 @@
   +            .closest('div.js-form-managed-file')
   
   @@ -156,15 +169,17 @@
   
   

   The div part shouldn't be necessary in these various instances of div.js-form-managed-file. If the div is needed there may be another issue going on that should be addressed.

2. +++ b/core/modules/file/file.es6.js
   @@ -24,6 +24,19 @@
   +          const $wrapper = $('<div data-drupal-messages></div>');
   

   It's preferable to not use a plain markup string

   An explanation of a jQuery alternative - using self closing tags and adding attributes as part of an object literal - can be found at https://api.jquery.com/jQuery/#entry-examples-1
   In this case it would be something like

   $wrapper = $( '<div/>', {
     'data-drupal-messages': '',
   })
   

When the extra unrelated stuff is cleaned out of the patch I'll have a closer look, but it's looking good so far.

Fixing patch noise and #22.2.

For #22.1, I change this and then changed it back. All throughout file.es6.js it's using 'div.js-form-managed-file', so I think we should stay consistent with the rest of the file, and then fix this in a separate issue. What do you think?

Oops, I included a change from #3089402: Drupal.Message should add 'Error message' prefixed to Drupal.announce error messages.

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

Tagging this issue to be worked on at DrupalCon Amsterdam.

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

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

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