CKEditor 5 text area doesn't have appropriate aria-label for screen reader

Have not tested but bugs will need a failing test assertion to show the issue.

The JS changes look very sensible and simple — that's a great start! 😄👍

Interestingly, this made one Functional JavaScript test fail:

Drupal\Tests\ckeditor5\FunctionalJavascript\CKEditor5OffCanvasTest::testOffCanvasStyles
Behat\Mink\Exception\ElementNotFoundException: Element matching css "style#ckeditor5-off-canvas-reset" not found.

🤔

I'm just looking at the source here.

// Because CKEditor5 re-renders aria-label every time on focus/blur,
// overwrite the label to appropriate field label value.

This sounds like an upstream bug. Shouldn't CKEditor just do this:

// Update aria-label with the value from default textarea label.

Not that we can't too, but surely, if there is a label in the text area, CKEditor should respect that, right? Or am I missing something?

Good question. We should identify whether it's an upstream bug and report it upstream if it hasn't been already.

If it is an upstream bug, we should still work around it to help hundreds of thousands of Drupal 9/10 users. But we should add a @todo on our end to ensure we remove the work-around when it is fixed upstream.

Perfect! Thanks for that research! 🙏👍

So let's fix it here with a // @todo … as I mentioned :)

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.

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 was very happy to see this issue posted as this has come up in our testing as well and was planning to create this issue myself.

I tested that latest MR indeed fixes the label issue (with VoiceOver) but I noted that there seems to be also one degradation. It seems that the info about CKEditor accessibility instructions have dropped away. In our accessibility testing we considered that piece information (alt-0 / option-0 for help) to be important for the user.

I think we should keep that piece of information announced as well.

Without the patch:

With the patch:

(I was testing with several different types of fields - named Body, Corpse, Cadaver and Carcass. Corpse-field was formatted long text without summary.)

This issue is somewhat related to #1562776: Edit Summary label is unclear to users as the summary/main text labelling is a special case of the problem.

Sorry for having to wait for a reply.
I just had the at that moment latest 11.x dev branch of core, nothing else.
And the other one was the this issues branch.

i've tried to apply MR7170 but i somehow run into an assertion error when i try to run drush cr after switching to that issue fork. since that error happens consistently (spun up a new instance where i ran into the exact same error) is it possible that the MR might need a rebase? not sure if that would fix it, but the last commit was four months ago, and the assertion error i get ( a non e existent navigation.info.yaml ) is totally unrelated to the changes in the MR.

$> ddev drush cr
AssertionError: The file specified by the given app root, relative path and file name (/var/www/html/web/core/modules/navigation/navigation.info.yml) do not exist. in /var/www/html/repos/drupal/core/lib/Drupal/Core/Extension/Extension.php on line 73 #0 /var/www/html/repos/drupal/core/lib/Drupal/Core/Extension/Extension.php(73): assert()
#1 /var/www/html/repos/drupal/core/lib/Drupal/Core/Extension/ModuleHandler.php(114): Drupal\Core\Extension\Extension->__construct()
#2 /var/www/html/repos/drupal/core/lib/Drupal/Component/DependencyInjection/Container.php(258): Drupal\Core\Extension\ModuleHandler->__construct()
#3 /var/www/html/repos/drupal/core/lib/Drupal/Component/DependencyInjection/Container.php(176): Drupal\Component\DependencyInjection\Container->createService()
#4 /var/www/html/repos/drupal/core/lib/Drupal/Core/DrupalKernel.php(585): Drupal\Component\DependencyInjection\Container->get()
#5 /var/www/html/vendor/drush/drush/src/Boot/DrupalBoot8.php(210): Drupal\Core\DrupalKernel->preHandle()
#6 /var/www/html/vendor/drush/drush/src/Boot/BootstrapManager.php(211): Drush\Boot\DrupalBoot8->bootstrapDrupalFull()
#7 /var/www/html/vendor/drush/drush/src/Boot/BootstrapManager.php(397): Drush\Boot\BootstrapManager->doBootstrap()
#8 /var/www/html/vendor/drush/drush/src/Application.php(214): Drush\Boot\BootstrapManager->bootstrapMax()
#9 /var/www/html/vendor/drush/drush/src/Application.php(180): Drush\Application->bootstrapAndFind()
#10 /var/www/html/vendor/symfony/console/Application.php(258): Drush\Application->find()
#11 /var/www/html/vendor/symfony/console/Application.php(167): Symfony\Component\Console\Application->doRun()
#12 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(110): Symfony\Component\Console\Application->run()
#13 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(40): Drush\Runtime\Runtime->doRun()
#14 /var/www/html/vendor/drush/drush/drush.php(139): Drush\Runtime\Runtime->run()
#15 /var/www/html/vendor/drush/drush/drush(4): require('...')
#16 /var/www/html/vendor/bin/drush(119): include('...')
#17 {main}
AssertionError: The file specified by the given app root, relative path and file name (/var/www/html/web/core/modules/navigation/navigation.info.yml) do not exist. in assert() (line 73 of /var/www/html/repos/drupal/core/lib/Drupal/Core/Extension/Extension.php).

Question will this fix an issue where if the text field is required the aria-label isn't announcing?

Can I suggest an approach that addresses both the concerns about required and the help text, and avoids fighting with CKEditor over the aria-label?

Rather than adding an aria-label to the CKEditor frame, add it to the field wrapper, with another attribute of role="group":
<div class="form-item" role="group" aria-label="[original field label] [, required (if applicable)]"...

eg
<div class="form-item" role="group" aria-label="Body, Required"...

Then the screen reader will speak the various inner labels (there may be several: Summary, Text Area, etc), but you would hear the field label itself and whether the fieldset is required when the cursor enters the region.

Technically this converts the field label into a legend, but seeing as CKEditor is labelling an application...having both labels might make a lot of sense.

...although ideally the required attribute WOULD go on the textarea itself and not the outer group, since it is not the group that is required. So maybe the event listener code in the MR would still be needed to sync the required attribute, if CKEditor is messing with that too.

I agree with #22, it is helpful to keep the alt-0 / option-0 for help information in the aria-label. I updated the existing merge request so that the field label is placed before the aria label instead of overwriting the value.

Attached is a snapshot of the MR so it can be safely used with composer patches.

#26 still needs to be addressed.