Improve CKEditor toolbar authoring accessibility [#2072927]

Problem/Motivation

The default CKEditor toolbar includes numerous semantic markup elements that express the structure of the toolbars to screen reading user agents. For example:

<span id="cke_18" class="cke_toolbar" aria-labelledby="cke_18_label" role="toolbar">
  <span id="cke_18_label" class="cke_voice_label">Links</span>
  <span class="cke_toolbar_start"></span>
  <span class="cke_toolgroup" role="presentation">
    <a id="cke_19" class="cke_button cke_button__link  cke_button_off" "="" href="javascript:void('Link')" title="Link" tabindex="-1" hidefocus="true" role="button" aria-labelledby="cke_19_label" aria-haspopup="false" >
      <span class="cke_button_icon cke_button__link_icon" style="background-image:url(http://ckeditor.com/apps/ckeditor/4.2/plugins/icons.png?t=D6ID);background-position:0 -1200px;background-size:auto;">&nbsp;</span>
      <span id="cke_19_label" class="cke_button_label cke_button__link_label">Link</span>
    </a>
  </span>
  <span class="cke_toolbar_end"></span>
</span>

In the code above, we see that a toolbar is qualified with the role toolbar and label with the aria-labelledby attribute -- aria-labelledby="cke_18_label" -- which refers to the element <span id="cke_18_label" class="cke_voice_label">Links</span>. A screen reading user agent will announce this as the "Links toolbar".

In contrast, the editor markup that Drupal produces for a CKEditor instance, driven by our configuration data that lacks group information, lacks these labels for toolbar groups>

<span id="cke_9" class="cke_toolbar" role="toolbar">
  <span class="cke_toolbar_start"></span>
  <span class="cke_toolgroup" role="presentation">
    <a id="cke_10" class="cke_button cke_button__drupallink  cke_button_off" "="" href="javascript:void('Link')" title="Link" tabindex="-1" hidefocus="true" role="button" aria-labelledby="cke_10_label" aria-haspopup="false" onkeydown="return CKEDITOR.tools.callFunction(10,event);" onfocus="return CKEDITOR.tools.callFunction(11,event);" onmousedown="return CKEDITOR.tools.callFunction(12,event);" onclick="CKEDITOR.tools.callFunction(13,this);return false;">
      <span class="cke_button_icon cke_button__drupallink_icon" style="background-image:url(http://d8.drupal.dev/core/modules/ckeditor/js/plugins/drupallink/link.png?t=D6JD);background-position:0 undefinedpx;background-size:16px;">&nbsp;</span><span id="cke_10_label" class="cke_button_label cke_button__drupallink_label">Link</span>
    </a>
    </span>
  <span class="cke_toolbar_end"></span>
</span>

The Drupal code lacks the aria-labelledby attributes on elements qualified with the role toolbar. There is no information for a screen reading user agent to use that would inform a user what type of toolbar this is.

Because of this, screen reader users must go through each button in order to find the one they want, rather than navigating from toolbar group to toolbar group to find an appropriate set before investigating the individual buttons.

Proposed resolution

Provide the default toolbar group metadata to the theme functions that produce the Drupal CKEditor markup and use that information to provide the correct toolbar group labels.

Remaining tasks

None yet.

User interface changes

Screen reader users will have toolbar label information.

API changes

None that are known now.

#1872206: Improve CKEditor toolbar configuration accessibility

Original report by @jessebeach

Comment	File	Size	Author
#8	ckeditor-authoring-a11y-2072927-8-do-not-test.patch	23.2 KB	jessebeach
#8
#8	interdiff.txt	15.17 KB	jessebeach
#6	ckeditor_authoring_a11y-2072927-6-do-not-test.patch	9.13 KB	Wim Leers
#6
#6	interdiff.txt	5.83 KB	Wim Leers
#4	ckeditor-authoring-a11y-2072927-4-do-not-test.patch	6.83 KB	jessebeach
#4

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

jessebeach CreditAttribution: jessebeach commented 23 August 2013 at 19:42

Upgrading this issue to Critical. Downgrading #1872206: Improve CKEditor toolbar configuration accessibility to Major. See #1872206-92: Improve CKEditor toolbar configuration accessibility for an explanation of why.

Comment #2

jessebeach CreditAttribution: jessebeach commented 23 August 2013 at 19:43

Priority:

Normal

» Critical

Comment #3

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers commented 26 August 2013 at 20:02

Component:	editor.module	» ckeditor.module
Issue tags:	-ckeditor	+CKEditor in core

Fix component and tag.

Comment #4

jessebeach CreditAttribution: jessebeach commented 27 August 2013 at 01:25

File	Size
ckeditor-authoring-a11y-2072927-4-do-not-test.patch	6.83 KB

I've updated the default basic and html formatter CKEditor configurations to a structure that includes group names. So now we get HTML output like this:

<span id="cke_46" class="cke_toolbar" aria-labelledby="cke_46_label" role="toolbar">
  <span id="cke_46_label" class="cke_voice_label">Lists</span>
  <span class="cke_toolbar_start"></span>
  <span class="cke_toolgroup" role="presentation">
    <a id="cke_47" class="cke_button cke_button__bulletedlist cke">&nbsp;</span>
      <span id="cke_47_label" class="cke_button_label cke_button__bulletedlist_label">Insert/Remove Bulleted List</span>
    </a>
    <a id="cke_48" class="cke_button cke_button__numberedlist cke_button_off">
      <span class="cke_button_icon cke_button__numberedlist_icon">&nbsp;</span>
      <span id="cke_48_label" class="cke_button_label cke_button__numberedlist_label">Insert/Remove Numbered List</span>
    </a>
  </span>
  <span class="cke_toolbar_end"></span>
</span>

where we now have proper aria-labelledby attributes with the toolbar group name.

If you install the Config inspector module, you can traverse the new configurations at the following paths on your intall:

/admin/reports/config-inspector/editor.editor.basic_html/tree
/admin/reports/config-inspector/editor.editor.full_html/tree

Tests are not yet passing.

After discussing this approach with Wim Leers, we realized that we may need to recombine this issue and #1872206: Improve CKEditor toolbar configuration accessibility. By changing the structure of the default configurations and how those configurations are parsed, the structure of the CKEditor settings form will no longer produce renderable data.

I think the next step here is to update the settings forms and get basic fields in place to expose and capture group name data. We can worry about how to make the UX pleasant after the form structure is established to produce data that the CKEditor class expects.

Comment #5

jessebeach CreditAttribution: jessebeach commented 27 August 2013 at 01:29

Issue tags:

+sprint, +Spark

Comment #6

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers commented 27 August 2013 at 13:49

File	Size
interdiff.txt	5.83 KB
ckeditor_authoring_a11y-2072927-6-do-not-test.patch	9.13 KB

In this reroll:

Applied translation to group names.
Assigned the "Tools" group name to the button group containing the "Source" button — I think every group should have a name, even if there's a less obvious choice.
The config files now contain button group names, which are language-specific. So I changed the langcode from und to en.
CKEditor was trying to load its default config.js file again (which Drupal does not ship) because the internal plugin didn't load anymore, which happened because this changed the config file structure without updating the code that applies the config/settings (i.e. $editor->settings['toolbar']['buttons'] is now $editor->settings['toolbar']['rows'] *and* has a different structure underneath it).

I just wanted to take away that one problem ("CKEditor is trying to load config.js, WTF?") you'd have to solve that would take you a LOT of digging (because it requires going through many layers to actually solve the problem). It was more efficient for me to solve it because I'm more familiar with that part of the code. Then you can focus on making the UI changes :)

Suggestions:

The "name" key in config files should be renamed to "label", or maybe even "button_group_label".
The "items" key in config files should be renamed to "buttons".

+++ b/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/Editor/CKEditor.php
@@ -259,22 +274,12 @@ public function getLibraries(EditorEntity $editor) {
+        $toolbar[] = (object) $group;

Why this cast?

Comment #7

jessebeach CreditAttribution: jessebeach commented 27 August 2013 at 18:03

The "name" key in config files should be renamed to "label", or maybe even "button_group_label".
The "items" key in config files should be renamed to "buttons".

I had had something like this originally, but then I just went with the same terms that CKEditor uses, so we don't need translate the keys when we create the configuration that gets passed to it e.g.

config.toolbar = [
    { name: 'document', items: [ 'Source', '-', 'NewPage', 'Preview', '-', 'Templates' ] },
    { name: 'clipboard', items: [ 'Cut', 'Copy', 'Paste', 'PasteText', 'PasteFromWord', '-', 'Undo', 'Redo' ] },
    '/',
    { name: 'basicstyles', items: [ 'Bold', 'Italic' ] }
];

http://docs.ckeditor.com/#!/guide/dev_toolbar-section-2

Comment #8

jessebeach CreditAttribution: jessebeach commented 27 August 2013 at 22:10

File	Size
interdiff.txt	15.17 KB
ckeditor-authoring-a11y-2072927-8-do-not-test.patch	23.2 KB

I've fixed all the failing tests that went sideways after changing the CKEditor configuration structure.

I started fixing the tests in CKEditorAdminTest.php and realized that we need to fix the configuration piece of CKEditor at the same time as fix the authoring piece.

So I'm going to take the patch here and merge it over to #1872206: Improve CKEditor toolbar configuration accessibility and continue the work there.