Download & Extend

Improve CKEditor toolbar configuration accessibility

Project:Drupal core
Version:8.x-dev
Component:ckeditor.module
Category:bug report
Priority:critical
Assigned:quicksketch
Status:needs work
Issue tags:accessibility, CKEditor in core, needs accessibility review, Spark

Issue Summary

Prerequisite: You must apply the latest patch here first #1890502: WYSIWYG: Add CKEditor module to core

Problem/Motivation

When configuring the Editor of a text format (such as at the path admin/config/content/formats/full_html), the Toolbar components of the CKEditor can be adjusted through a click, drag-and-drop interface.

text-format-config-with-ckeditor.png

It is not possible to configure the CKEditor toolbar using keyboard navigation.

Turning off JavaScript results in no toolbar configuration.

text-format-config-no-ckeditor-toolbar-config-present-2.png

Proposed resolution

The drag-and-drop click UI should be driven from an HTML form that underlies the more complex UI. The complex UI could be augmented to respond to keyboard input -- this would be accessible. The non-js version form is desirable because it does not leave a site builder completely stranded if their UA cannot produce the complex interaction.

One of the first steps is to wrap the <img> tags that are used to represent CKEditor buttons in <button> tags so they can be accessed via keyboard input.

Screenshot of a text format configuration screen. The Editor option is set to CKEditor. The CKEditor toolbar configuration is shown in a fieldset below. The Chrome Dev tools show that a CKEditor button is represented by a bare img tag.

Once the CKEditor toolbar buttons can be accessed via keyboard input, we'll need to develop a way for users to mark a button as a move candidate, tab to a drop slot and activate a move of the candidate into the slot.

Remaining tasks

Determine a solid implementation through discussion.

User interface changes

The configuration of a toolbar should be achievable through keyboard input alone. Ideally, a non-js fallback will be provided.

API changes

(API changes/additions that would affect module, install profile, and theme developers, including examples of before/after code if appropriate)

AttachmentSizeStatusTest resultOperations
ckeditor-toolbar-config.png37.68 KBIgnored: Check issue status.NoneNone
ckeditor-toolbar-button-as-an-image.png81.32 KBIgnored: Check issue status.NoneNone

Comments

#1

One could of course argue that since JS is an absolute requirement to be able to use CKEditor in the first place, that a no-js version is a nonsensical request.

I assume you're approaching this from the POV of a visually impaired user who is using a no-js browser, and that user should still be able to set up a WYSIWYG editor for his/her visitors?

#2

Once the CKEditor toolbar buttons can be accessed via keyboard input, we'll need to develop a way for users to mark a button as a move candidate, tab to a drop slot and activate a move of the candidate into the slot.

I've thought about how to solve this problem, and my suggested approach would be to simply use the arrow keys on the keyboard to move items around. Up/down arrows activate/deactivate any given button (or move them between rows) and the left/right arrows position the button into the correct location. Does that seem like it would be adequate?

One could of course argue that since JS is an absolute requirement to be able to use CKEditor in the first place, that a no-js version is a nonsensical request.

I agree with this statement. We should focus on making the JS-version accessible. There's no point in making a non-JS version at all.

#3

One followup on my statement here:

There's no point in making a non-JS version at all.

There *is* already a non-JS version (though you have to save the text format first to see it). It's just a plain text area containing JSON configuration of the toolbar. However this textarea is certainly not usable for any normal human and it shouldn't really be considered an alternative at all.

#4

Title:Toolbar configuration for a WYSIWYG editor on a text formatter is not keyboard accessible; a non-JS version is not provided» Toolbar configuration for a WYSIWYG editor is not keyboard accessible; touch suport missing; non-JS version not provided

moshe pointed out in #1833716-82: WYSIWYG: Introduce "Text editors" as part of filter format configuration that touch support is also missing.

I don't think it's an absolute requirement to make this work on smartphones; but it should work on tablets.

#5

Priority:normal» critical

This is going to be a commit-blocker, so raising to critical.

#6

@quicksketch#2: Oh, I would like that things were so easy. If I understand correctly, we're talking about making a drag-n-drop accessible. Technically it should be possible, but practically I think it won't be so easy.

If we decide to work on making this drag-n-drop accessible, though, maybe we should focus on making drupal's drag-n-drop system accessible and then use it for CkEditor. Or am I missing something?

Anyway, does the demo at
quicksketch.org/wysiwyg
allow to test this behavior?

#7

For example, here:
https://www.ssbbartgroup.com/blog/2011/12/12/accessible-drag-and-drop-wh...
there are two links to two accessible drag-n-drop demo: the problem is that the first one works quite well with Voiceover, but the second one does not.

Anyway, tagging for our accessibility team.

#8

If I understand correctly, we're talking about making a drag-n-drop accessible. Technically it should be possible, but practically I think it won't be so easy.

I wrote the table drag functionality that has been present since Drupal 6 (e.g. blocks, menu item, and taxonomy term reordering). It is also keyboard accessible via the arrow keys. I don't think this would be any more difficult. We're not actually making jQuery's drag and drop accessible, we're providing an alternative to drag and drop via the arrow keys.

Regarding mobile/touch support for drag and drop, I think jQuery UI will handle that problem eventually, if it doesn't already. As I understand it, adding touch support is (almost) as simple as adding additional bindings for touchstart and touchend instead of mousedown and mouseup.

#9

@quicksketch: Ok, I am very happy to be wrong regarding the difficulty. However, as a blind user, after dragging an element (e.g. a button) I need to receive feedback from my screen reader to know where I am going to drop it. This will be the hardest part, I think.

However, when things wil start to become "code", I'll review the eventual patch and give my impressions! ;)

#10

Thanks for that feedback @falcon03. I had been thinking that you were referring to accessibility via keyboard manipulation, rather than drag and drop through a screenreader. Do you have any suggestions on how you might expect to handle an administrative situation where you have multiple rows of buttons that need to be put into a specific order?

Using the keyboard arrow keys could probably make it possible for a blind user to move buttons around, but I wonder how we could provide feedback on the current position. Ideally, I'd like to have a screenreader first announce the availability of the action, such as "Bold button. 3rd position, 1st row. Use the arrow keys to reorder this button." And then after the action announce the position such as, "The 'Bold' button is now in the 3rd position, 1st row." Unfortunately I'm not familiar with how to make screen readers announce such a thing.

Moving the button down (and therefor out) of the toolbar would disable the button. Moving a disabled button up would move it into the toolbar and enable it. Perhaps we can describe this simply through description text above toolbar configuration.

#11

#10: Jesse Beach has experience with this and she might be of help here. Unfortunately, she's on medical leave for a few weeks right now. What you describe should definitely be possible. I think you're on a Mac, so: 1) enable VoiceOver, 2) use Safari or Chrome to browse a Drupal 8 site with the Edit module enabled, 3) you should be able to tab to enable Edit mode and then tab through the different editables on the page and edit them.
At appropriate times, you will hear announcements such as the one you're describing.

(I'm only posting this to indicate that this is possible; I'm by no means an expert — please always consider falcon03's feedback over mine :).)

#12

@Wim Leers: don't worry, you're right! :D
Just two facilities for Mac OS X users: (1) if you want to enable Voiceover, simply enter "Command + F5" (entering this hotkey again will turn off it) and (2) If you need a better quality voice (Fred is terrible, don't you think so? ;)) you can get one for free by opening the Voiceover Utility (shortcut: Control + Option + F8), then browse to voices from the settings category list and then select "Ad hoc..." from the "default" listbox.

If anyone needs support with a screen reader, feel free to ping me! :D

@quicksketch: well, currently I have no demo of an usable drag-n-drop for blind users. ARIA should have some roles/attribute to help us to deal with it, but from what I've read ARIA is not enough. Let me find some sources and I'll post them here during this week! :D

#13

Well, the purpose of this comment is to bring up to the table some details on how a blind user could expect a drag-n-drop to work. These are my opinions and they haven't been discussed by the whole accessibility team yet (I'll ensure that this will be discussed soon).

The first thing we need to figure out is the best way to organize draggable items so that they can be dragged and dropped easily and in a logical way. I think we have two possibilities:

  1. Organize the draggable buttons in a table, so that each column will represent a menu. We can add table headings to let blind users understand which menu (column) the cursor is placed in and we can use table footer for disabled buttons. This way, it will be super-easy to adjust the toolbar look to suit your needs, since screen readers allow blind users to navigate a table by rows, columns or cells.
  2. Render the toolbar while configuring it, using CkEditor Aria Roles and adding a "Drag-n-drop" feature to buttons. I think this is not the current approach for the editor, and I think this won't be as intuitive to use for blind users and easy to develop as the first possibility.

Once we figure out the best way to organize draggable items, we need to figure out the effective workflow to drag and drop an item using keyboard and a screen reader. We have two possible implementations:

  1. Use screen readers built-in support for mouse drag and drop emulation. I won't recommend this implementation, since IMO the results can be unpredictable and screen readers don't support mouse drag and drop emulation very well.
  2. Use an anchor to render draggable/droppable items, so that clicking an item (pressing "Enter" for Windows or "Vo + spacebar" for Mac OS X screen reader users) can "fire" the drag or the drop action.

This second implementation needs further details, but I need to think a bit more about them. Also, I mentioned wrapping draggable items into anchors, but I don’t know if we can wrap them into something better.

#14

Note that at #1833716: WYSIWYG: Introduce "Text editors" as part of filter format configuration, for all rerolls of the patch posted last week, it's already the case that when you're using the page without JS, you get a "Configure" button, which when clicked will cause the relevant configuration form for the currently selected text editor to appear. So one (tiny) aspect has already been solved.

#15

Just trying to help out here, but the scope of this issue seems a bit large. And I doubt its all soo critical it needs to be fixed pre-commit of CKEditor. For example, touch support, non-JS version are both not mission critical functions.

#16

I've discussed this with the CKEditor guys (with Piotrek Koszuliński and Wiktor Walc) and asked for feedback.

As per http://docs.ckeditor.com/#!/guide/dev_toolbar, there are two ways of configuring the toolbar: item-by-item or group-based.

We're currently doing item-by-item. But the problem with our current approach is that there are no "names" for each user-defined group which would be translated into "voice labels" for each group, which is better for a11y. We could add this, but this would make the UI too complicated. Of course, there would still be voice labels for each individual button, so it wouldn't be an a11y disaster.

If we'd switch to a group-based toolbar configuration, then the user would have less control, but also less to worry about. There would be a predefined set of groups (the one that CKEditor ships with; all plugins define default locations for their buttons to be placed in a certain position in one of those groups), users would then be able to click/press each button to either enable or disable it and the button would automatically appear in its predefined group, in a predefined location (with some kind of highlighting indication because otherwise the user wouldn't immediately see where the button had appeared). That's also the downside: it's harder to see (for me at least) how to make this accessible

  • Item-by-item configuration advantages: full control.
  • Item-by-item configuration disadvantages: hard to make a simple UI that allows names/voice labels to be defined for the groups, full control forces users to think about which groups and the order of each button within each group.
  • Group-based configuration advantages: the user doesn't have to think about which groups make sense, more consistency on the web/among Drupal sites, names/voice labels are a non-issue (they're predefined), toggling instead of dragging (better for touch devices)
  • Group-based configuration disadvantages: all buttons of all available plugins are enabled by default (we'd have to use removeButtons to exclude buttons the user doesn't want to use), only reordering of groups is allowed, accessibility of the toolbar configuration UI itself may be problematic.

Note: I may have forgotten a few minor points because I had to rewrite this from scratch due to browser data loss :/

#17

Issue tags:+CKEditor in core, +Spark

Tagging; this helps the CKEditor folks track the "CKEditor in core" status on the Drupal side. On the CKEditor side, see http://dev.ckeditor.com/query?keywords=~Drupal.

#18

Assigned to:Anonymous» quicksketch

I'm working on providing a keyboard and screen-reader accessible that I should be able to provide later today.

#19

Woot!

#20

Okay, here's a reroll that makes this interface as accessible as I know how to make it:

  • ARIA roles and attributes used on each toolbar row, button, and add/remove links
  • Each button now can receive focus, both by the keyboard and by clicking on the button.
  • Clicking on the button will announce the button position, as well as reiterate help text for enabling/disabling buttons, via an alert box.
  • Additional help text is displayed giving the basics of usage, and this text is uses as the aria-description on each button. This has the effect of being read aloud by VoiceOver after a slight delay when any button has received focus.

Because the additional JavaScript needed to take into account right-to-left movements (left and right arrows have flipped behaviors in RTL languages), this patch also includes CSS files for RTL support in the admin interface (we already have RTL CSS files for the rest of the module).

I'll work on getting this patch up on my demo site for front-end testing. @jessebeach and @WimLeers, if you guys would give it a code review that would be great.

Remaining issues I don't think should be directly addressed by this module:

  • Non-JS version of the interface: Like Wim said, it's rather non-sensical to require a non-JS version for administering a JS-only feature. Changes to Edit module already made it possible to change the active editor and switch configuration forms without JS. The toolbar configuration for CKEditor itself shouldn't need a non-JS version (though it is still accessible via a very unfriendly textarea with JSON in it).
  • Touch support: jQuery UI itself is missing touch support for its draggable/droppable/sortable libraries. It's been an long-delayed feature. Here's the official issue. It looks like a common solution right now is to install Touch Punch, a hack that adds global support for touch to jQuery UI. If either jQuery UI adds support or Touch Punch is included on the page, this interface will immediately work on touch-devices. I don't think this is a problem for this interface specifically to solve.
AttachmentSizeStatusTest resultOperations
ckeditor_a11y.patch15.48 KBIdleFAILED: [[SimpleTest]]: [MySQL] Unable to apply patch ckeditor_a11y.patch. Unable to apply patch. See the log in the details link for more information.View details | Re-test

#21

Status:active» needs review

I had uploaded a slightly less polished patch. This one adds a few lines of additional documentation and removes a @todo for touch (since like I said above, this should be handled separately from CKEditor). Patch is relative to Drupal root, since this is written against the core patch version.

AttachmentSizeStatusTest resultOperations
ckeditor_a11y.patch15.48 KBIdleFAILED: [[SimpleTest]]: [MySQL] Unable to apply patch ckeditor_a11y_0.patch. Unable to apply patch. See the log in the details link for more information.View details | Re-test

#22

Very nice — it's already working very well :)

- Are you sure that using alert() is a good idea? (Jesse will probably know this.)
- There's no help yet on the minus and plus buttons (add/remove row).

+++ b/core/modules/ckeditor/css/ckeditor.admin.cssundefined
@@ -29,8 +29,8 @@ ul.ckeditor-buttons {
+  float: left; /* RTL */
+  clear: left; /* RTL */

I think this should be LTR? :)

Same elsewhere.

+++ b/core/modules/ckeditor/js/ckeditor.admin.jsundefined
@@ -20,11 +20,29 @@ Drupal.behaviors.ckeditorAdmin = {
+      $toolbarAdmin.on('keyup.ckeditorMoveButton', '.ckeditor-buttons a', adminToolbarMoveButton);
+      $toolbarAdmin.on('keyup.ckeditorMoveSeparator', '.ckeditor-multiple-buttons a', adminToolbarMoveSeparator);
+
+      // Add click for help.
+      $toolbarAdmin.on('click.ckeditorClickButton', '.ckeditor-buttons a', { type: 'button' }, adminToolbarButtonHelp);
+      $toolbarAdmin.on('click.ckeditorClickSeparator', '.ckeditor-multiple-buttons a', { type: 'separator' }, adminToolbarButtonHelp);

AFAIK it is customary to namespace events not on a "per-action" basis, but on a "per-module" basis. I.e. not "ckeditorSOMETHING", but just "ckeditor".

#23

- Are you sure that using alert() is a good idea? (Jesse will probably know this.)

No I'm not certain, but it works very well with screen readers and it's very clear that it's a popup (the button announces it is a popup on focus and then on click it announces that an alert has opened). I think it's unlikely that users utilizing drag and drop or not using a screen reader will ever know they can click on the buttons directly, since the indication given by the cursor icon (with the "move" icon) is so great. I think we can consider the alert to be seen only by screen reader users, except perhaps on occasion where a user doesn't realize the toolbar is "real" or accidentally clicks on a button. So it's certainly better than doing nothing on click.

- There's no help yet on the minus and plus buttons (add/remove row).

Two reasons: 1) it doesn't need additional help. The primary reason for the help is to announce the row number and position of a button. These buttons never change position and they already have a label announced by screenreaders as to their purpose 2) if you click on them, they need to add/remove a row, not show help. :)

I think this should be LTR? :)

Weird, I could have sworn we used RTL to indicate something had a matching RTL style. But you're right. No idea where I got that impression.

AFAIK it is customary to namespace events not on a "per-action" basis, but on a "per-module" basis. I.e. not "ckeditorSOMETHING", but just "ckeditor".

I'm not sure that custom is something that needs to be enforced, as it'd be perfectly common for a single module to attach two events to the same button if they needed to bind/unbind one (or more) of the events dynamically. In such a case, the module would need to provide more than just the module name. But in this situation, sure, we could easily change this to just "ckeditor", though I personally prefer that the namespace describes the action, as it makes reading the list of bindings on a DOM object clear what each binding does.

So from these, (assuming the alert() is okay), I think the RTL/LTR thing is the only fix that is definitely needed.

#24

RE: alert: yeah, it *might* be okay, but it's far more intrusive than how Jesse did the a11y stuff for Edit.

RE: RE: remove/add remove buttons. HAH! Stupid me! :)

RE: event namespacing: it's definitely possible to do it your way. I'll ask nod_ to chime in on that.

And yes, the RTL/LTR thing is the only "definitely fix" thing from my feedback.

#25

for custom events, yes it's possible that a module might want to have several namespaces, but events can have several namespaces too. might want to combine one or two to make that more digest. Also we talked about that on this issue #674716: We need guidelines on usage of DOM events and jQuery's namespaced events even if it's more custom-event oriented.

but here the click is clearly redundant, it's the event type :) needs some improving.

#26

I went through the patch and made a few changes to the placement of ARIA roles and the some HTML. I tried not to change anything else about the HTML, CSS or JS that doesn't concern accessibility.

The changes facilitate the screen reader to interject with information about the button configuration form. Using ChromeVox will illustrate what I mean. My goal was to make sure a screen reader user knows what the form does and what the current state of the form is. I don't think it's 100% there, but I'd like some feedback on it.

The other major change is I removed the alerts and replaced them with an aria-live region so the state of each button is read without arresting the page with an alert or injecting visual elements.

AttachmentSizeStatusTest resultOperations
1872206_ckeditor-a11y_26.patch16.91 KBIdleFAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1872206_ckeditor-a11y_26.patch. Unable to apply patch. See the log in the details link for more information.View details | Re-test
interdiff.txt9.2 KBIgnored: Check issue status.NoneNone

#27

Is this still in sync with #1890502-41: WYSIWYG: Add CKEditor module to core I had trouble applying it against ckeditor_module-1890502-41.patch

#28

#27: No, it is not; the patches here at #20, #21 and #26 are against #1890502-21: WYSIWYG: Add CKEditor module to core.

#29

Title:Toolbar configuration for a WYSIWYG editor is not keyboard accessible; touch suport missing; non-JS version not provided» Toolbar configuration for a WYSIWYG editor is not keyboard accessible; touch suport missing

Removed "; non-JS version not provided" from the title. A non-JS configuration for a JS-only feature doesn't make sense.

#30

Please provide a demo website, for a11y people to review.

#31

Reroll of #26 against #1890502-41: WYSIWYG: Add CKEditor module to core.

UPDATE: I just noticed that this patch, in combination with #1890502-41: WYSIWYG: Add CKEditor module to core introduces visual glitches. But that's not relevant right now; this issue is about the a11y; the visual glitches we can still fix.

AttachmentSizeStatusTest resultOperations
1872206_ckeditor-a11y_31.patch17.31 KBIdleFAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1872206_ckeditor-a11y_31.patch. Unable to apply patch. See the log in the details link for more information.View details | Re-test

#32

Oh, simply wonderful to see progress here!

Sorry I haven't had time to get on this before. Do I need to apply the CkEditor integration patch (I know the library is already in core) before applying this patch?

Anyway, maybe a demo site could help us getting more eyes on this: given the importance of this patch, I could make some blind (non-developers) screen reader users trying this feature to get more feedback...

#33

#32: Applying #1890502-41: WYSIWYG: Add CKEditor module to core (i.e. CKEditor.module) and #31 (i.e. the latest patch in this issue that makes the toolbar builder more accessible) should do the trick. And: thanks! :)

#34

The patch seemed to apply fine.

I'd like a working public site to apply both patches too though.

When I tab through the edit page, it still misses the toolbar.

When I try to use Opera Mobile to browse it as a Galaxy 2 smartphone I can select the buttons, but not after I I've selected the text. I can create a link but not hit OK to actually add it.

I think something's wonky, but I'm not 100% sure I've got a working environment to test. I just know I've applied both patches, cleared cache & am editing a full html page with ckeditor's toolbar displayed on it.

#35

didn't mean to remove

#36

I had a look at the patch. Things are getting better, but we haven't reached a good point yet, unfortunately.

I tried using the modified interface with Mac OS X and Voiceover, but not with Jaws or NVDA.

After dragging a button and moving it around with arrow keys the only thing I hear is the label of the button that I am dragging and I don't know where I am going to drop it.

Another problem is that I am not able to understand where a row starts and where it finishes, so I am not able to understand which buttons are in a row and which ones are in another one. Could you implement the suggestion in #13 of grouping buttons in a table?

I confirm mgifford's issue: it's not possible to reach the toolbar configuration object by tabbing (btw, the whole toolbar configuration form seems to be wrapped in a particular object that I have to interact with before altering the toolbar configuration) and I have to say that we should improve screen reader feedback when dropping an item. In fact, a blind user needs to explore the new toolbar to understand that the button has been dropped. We could solve this problem by simply making the screen reader say something "X button has been dropped" (don't know the exact string, though).

Finally, I think that the whole mechanism to add/remove a row of buttons is not accessible: when I press one of these buttons I cannot hear anything to understand that something happened!

BTW: it's a bit unrelated (not too much), but is there any chance that we could apply these drag-n-drop improvements to the blocks drag-n-drop interface to make it accessible too?

#37

Thanks everyone for the reviews! I had attempted to get a sandbox up and running, but with our 3 different versions of the module and troubles with D8, I couldn't get it all together easily. The next week we're launching a site, so I personally am going to be too busy to give this significant effort until after that is done.

When I tab through the edit page, it still misses the toolbar.

In Mac OS X, in order for elements other than forms to be selectable you must change the "Full Keyboard Access" setting in the Keyboard preference pane to "All controls". This makes it so that the tab key includes links in web browsers instead of just form elements. Very little of the web is keyboard accessible if you can't click on links with the keyboard.

After dragging a button and moving it around with arrow keys the only thing I hear is the label of the button that I am dragging and I don't know where I am going to drop it.

I haven't tried out the latest version of the patch, but assuming it works in a way similar to the previous iterations, clicking on the button will announce its current position and row. It sounds like this might be too ambiguous of an option however. Perhaps we should include the position and row as part of the ARIA label and update this in all buttons as a button is moved around.

As for the suggestion of using table rows, it sounds like this would alleviate the problem too, since tables typically have built-in support for row/column position. Unfortunately from a technical implementation, drag and drop in between tables presents a significant challenge. Existing drag and drop libraries do not work well with moving table cells, and if we moved things within table cells, you'd have issues trying to dynamically add/remove cells as needed. Keeping them as lists (especially since they're already working as such), will be a lot easier.

BTW: it's a bit unrelated (not too much), but is there any chance that we could apply these drag-n-drop improvements to the blocks drag-n-drop interface to make it accessible too?

The changes we're doing here are specific to this interface, but the approaches are generic. This might serve as a good example to reference when building other accessible drag and drop interfaces, but it's unlikely that any of the code will be directly applicable to other interfaces.

#38

This patch is a reroll of #31 against #1890502-66: WYSIWYG: Add CKEditor module to core.

AttachmentSizeStatusTest resultOperations
1872206_ckeditor-a11y_38.patch22.99 KBIdleFAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1872206_ckeditor-a11y_38.patch. Unable to apply patch. See the log in the details link for more information.View details | Re-test

#39

falcon03, thank you for the feedback. I hope this update gets closer to the UX behavior that you described in #36. I updated the patch with the following behaviors:

  1. Added tab-indexes to the toolbar buttons in the configuration fieldset. I hope that makes them focusable regardless of keyboard settings.
  2. When tabbing into a toolbar, the state of the toolbar buttons (active or available) and the row number are announced.
  3. When moving a toolbar button, the state of the toolbar the button is moved into (active or available), the row number and the position of the button are announced.
  4. When adding or removing a toolbar, the number of remaining toolbars is announced.

I will set up a testing site in a moment so these changes can be reviewed quickly. Check the following comment for the location and credentials of the testing site.

AttachmentSizeStatusTest resultOperations
1872206_ckeditor-a11y_39.patch25.85 KBIdleFAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1872206_ckeditor-a11y_39.patch. Unable to apply patch. See the log in the details link for more information.View details | Re-test
interdiff_38-39.txt7.58 KBIgnored: Check issue status.NoneNone

#40

Found a small issue while setting up the demo site. I need to attach a new patch.

The demo site is available at the following address with credentials

URL: http://ckeditor.qemistry.us
user: tester
password: tester
page: http://ckeditor.qemistry.us/admin/config/content/formats/full_html

This site will be available for a few days, then I will take it down.

AttachmentSizeStatusTest resultOperations
1872206_ckeditor-a11y_40.patch25.85 KBIdleFAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1872206_ckeditor-a11y_40.patch. Unable to apply patch. See the log in the details link for more information.View details | Re-test

#41

I tried doing this locally with these patches:
ckeditor_module-1890502-66.patch
1872206_ckeditor-a11y_40.patch

And also on the demo site (thanks!), but I couldn't tab to CKEditor let alone within it.

I might be missing something, but at this point I don't see how to access the WYSIWYG without a mouse.

#42

@mgifford, I was able to use the keyboard arrows to customize the editor....

Toolbar buttons may be moved by drag and drop or with the keyboard arrow keys. Move a button up into the active toolbar to enable it, or down into the available buttons list to disable it. Dividers are available to create logical button groups.

#43

Hi @animelion - thanks for reaching out. I do think that more data is required though. I've tried on a Mac with FF & Safari. In our tests you can't TAB onto the editor toolbar, getting into the text area is fine, but from there it jumps to the next form elements, not into the toolbar. Now if there were an alert that pops up to remind folks that they can use the WYSIWYG that would be useful.

I brought this up in the Accessibility Skype chat and several others were having trouble using it, but we're now much further ahead.

First there are here are keyboard shortcuts e.g. CTRL+b for bold CTRL+i for italic and CTRL+L does the link. The title on the the link icons can also remind folks about the keyboard shortcuts.

ALT+0 provides accessibility help
ALT+F10 to access the editor & navigate to tab-list in the dialogue

I had to dig for this, but on the Mac it's: FN+ALT+F10 (or fn+option+F10). This probably needs to be updated in the help as otherwise it's just going to be annoying for Mac users.

It seems to work, but this needs to be more discoverable. If you've got 3 Drupal accessibility experts who have no idea how to get into the admin menu, then the general content admin will have no idea.

EDIT: Lots of info on this page here (though seems to be 2 years out of date) - http://ckeditor.com/blog/CKEditor-WAI-ARIA-Usable-Accessibility

#44

Ok, I think I understand now mgifford. You're talking about the usability of the CKEditor wysiwyg editor itself. That's a fair point. My experience with it was the same in that I tabbed into the edit area and then right out. I had to go read the documentation to learn the ALT + F10 trick to get into the toolbars.

But this issue is about configuring a CKEditor toolbar, not using the CKEditor wysiwyg.

This is a demonstration video of me tabbing through the configuration UI: http://screencast.com/t/16fnfbcpH

We want to pass judgement on the configuration experience in this issue, not using the editor. Using the editor is also an issue, but one we should kick over to the CKEditor team.

#45

There are things that can be done to improve the accessibility of the CKEditor integration and how it is presented to Drupal users, but we can open those up as a new issue. These wouldn't be big things, but more polish.

I think we can say the known accessibility issues are sufficiently addressed for this issue.

Now what needs to be done to make this RTBC?

EDIT: Also, I did do some testing of the editor from my iPhone. Worked pretty well there too.

EDIT2: Actually I was wrong on that. Or maybe I just haven't figured out how to use it with my iPhone.

#47

I would like to test a bit deeper the patch before marking it RTBC.

Firstly, given the importance of the patch, I think we should test it using Windows + Jaws and NVDA. I'll do this ASAP.

Also, I have a tabbing issue in the toolbar configuration form (the wrapper for the toolbar configuration is not focused by tabbing), so IMO the issue needs a bit more work.

I agree with jessebeach: we should avoid talking about two different aspects of the toolbar into the same issue. The problem outlined by mgifford exists, and we must solve it, but in a follow up (IMO).

#48

The patch at #40 has now been merged into the main CKEditor module patch at #1890502-78: WYSIWYG: Add CKEditor module to core, as per mgifford's comment at #45.

mgifford, please remove the "needs accessibility review" tag at #1890502: WYSIWYG: Add CKEditor module to core when you deem it ready.

#49

Getting a "good but not perfect" version of CKEditor into core will make the development, testing and review process much much easier. We will no longer need to build the patch in this queue off the patch in another queue and reroll as the dependency patch changes.

We will continue to improve the UI, visual and aural, after feature freeze. That is not in question. Right now we want to get to a place where we know the limitations and we decide we can live with them for a short time because the resolutions are documented or the problems are at least defined.

Having CKEditor in core will accelerate the integration and bug finding/fixing efforts. We should balance perfection with progress.

#50

My discovery issue is listed here - #1904986: Make Keyboard Controls Discoverable in CKEditor related to my post in #43 above.

#51

Status:needs review» needs work

Sorry, but I think I have to change status to "NW".

Using Firefox 18.0.1 and NVDA 2012.3.1 on a Windows XP SP2 machine I cannot drag any button using the arrow keys.

But unfortunately this is not my abitual windows testing environment, so I'll give more details about the problem later.

BTW, I noticed that this patch has been merged into the CKEditor module addition one.
But I wonder if we'd not better keeping this issue separate from the CKEditor module addition one. This way this issue won't block the CKEditor module addition one and, at the same time, will make this patch easier to test/reroll/work on (IMO).

#52

Oh, and I forgot: can we disable the overlay module from the demo site for this issue? Overlay is not so screen reader friendly.

#53

I wonder if this is a windows problem in general? I haven't tested this in windows yet for keyboard only users.

Does JAWS give you any errors? How does it differ from the experience with VoiceOver?

Someone disabled the overlay which is good.

#54

OT: I thought Overlay was pretty good or at least better than dialogs in terms of a11y? You may want to chime in at #1889150: Simplify overlay look, *only use for contextual operations* regarding Overlay.module versus jQuery Dialog in terms of a11y.

#55

@WimLeers Its both crap atm :) So there is little to discuss, untill jQuery releases their improvements to jQuery dialog a11y which overlay needs to adapt.

#56

Project:CKEditor for WYSIWYG Module» Drupal core
Version:8.x-1.x-dev» 8.x-dev
Component:Code» ckeditor.module

#57

As Falcon03 mentioned in #51, much of the code in this thread was incorporated into the ckeditor patch that went into core.

I've rerolled this patch, mainly just replacing the custom ARIA live implementation with Drupal.announce. We can continue improving the keyboard/screen reader support from here.

AttachmentSizeStatusTest resultOperations
ckeditor-a11y-1872206-57.patch4.53 KBIdlePASSED: [[SimpleTest]]: [MySQL] 53,667 pass(es).View details | Re-test

#58

Status:needs work» needs review

Setting as "needs review" for testbot. I'll try to give a detailed feedback soon.

#59

Assigned to:quicksketch» Anonymous

This looks cool and I'll review it when I get the chance, though I probably shouldn't be assigned the issue.

#60

Definitely wanting to get @falcon03's & @quicksketch's view on this.

In my testing it seems to be much better. Unfortunately in moving a wysiwyg button with VoiceOver all I get is the name of the block repeated. There's no way of knowing what has been done or where that button is sitting now. It simply repeats the name of the button. Simply repeating "Outdent button" is no where near as useful as "Outdent button moved 2 down list"

I was testing here:
simplytest.me/admin/config/content/formats/full_html

#61

Category:task» bug report

#62

#57: ckeditor-a11y-1872206-57.patch queued for re-testing.

#63

#57: ckeditor-a11y-1872206-57.patch queued for re-testing.

#64

Title:Toolbar configuration for a WYSIWYG editor is not keyboard accessible; touch suport missing» Improve CKEditor toolbar configuration accessibility
Priority:critical» normal

Here's a reroll with the following changes:

  • I'd found that keeping track of the current button focus was difficult because the outline around the button disappeared when hovering the mouse on a button. normalize.css apparently removes outlines from links on hover, so I added specific CSS to keep the outline in place all the time on the current button.
  • There were multiple announcements happening on the same events, resulting in only the last announcement winning. In response to moving a button into a new row, the page would only announce "Active button row 2" instead of the more helpful, "Moved to active button row 2, position 1 of 10". To fix this I repurposed grantRowFocus() into announceButtonPosition() so its responsible for announcing all button positions, rather than just the row number. It's only triggered on button focus now, instead of row focus.
  • The adminToolbarButtonHelp() function is largely redundant with the new ARIA-based messages. There's no need to announce the button position on click when the button position is already announced on focus. So the click event no longer serves any usefulness for accessibility. To prevent confusion for all users, I converted the click event *back* to using alert(), so that the button does something when it's clicked (as opposed to doing nothing at all). If we decide that we don't want an alert on clicking, we should remove adminToolbarButtonHelp() entirely.
  • Fixed up a few places where we were using Drupal.t() inappropriately, switching us to use Drupal.formatPlural().

I'm also downgrading this issue to "normal" because the current system is already accessible in some way. A lot of our improvments were integrated into the original patch that got committed. This new patch is only improving upon the current work.

AttachmentSizeStatusTest resultOperations
ckeditor_toolbar-a11y-1872206-63.patch9.82 KBIdlePASSED: [[SimpleTest]]: [MySQL] 55,352 pass(es).View details | Re-test

#65

#66

Yay, quicksketch++ :)

@mgifford: curious what your a11y feedback to #64 is! :)

#67

Priority:normal» critical
Status:needs review» needs work

I don't know the reason why, but the latest patch brings the toolbar configuration form to the point where no feedback about dragging and dropping was given by screen readers.

Anyway, from my blind-user perspective I find really hard to use this configuration form. The main problem is that I find really, really hard to understand which group a button belongs to. So it's hard to understand how the toolbar is going to look like after dragging and dropping some buttons (including dividers). Unfortunately hearing row and column position of a button doesn't give me the context I need to know exactly what I'm doing.

When CKEditor show its toolbar with a rich Textarea, it does something great. I don't know what markup it uses, but it shows a list of groups (toolbar menus) and I can easily surf through them. Once I find the group that I am interested in, I can easily surf its buttons (and not buttons that belong to other groups if I do not want to). I think that using the same approach in this configuration form would give me the context I need.

Long story short, I think the configuration form should work like this: the CKEditor toolbar should be rendered the same way it is rendered when it appears next to a rich textarea. Clicking a button should activate the "drag this element" event. After dragging this event, I should be able to surf the toolbar jumping from a group to another and/browsing buttons that belong to a group. When I do this, I would like to hear the name of the group or the label of the button that the cursor is placed on and, if I am dragging a button, something like "you're dragging x button" (where "x" is the label of the button that I am dragging). When I am dragging an element and I click on a button it should be dropped into the cursor position and I would like to hear something like "X button has been dropped".

Last (but not least), we shouldn't bind dragging and dropping buttons to using arrow keys.

Finally, I have to say that I am not sure this text is clear. Let me know if you need further details and/or have some questions!

#68

Thanks @falcon03 for your feedback. Could you describe what method you use to drag and drop the buttons? I've been putting effort into ensuring that the interface is navigable via the keyboard (that should give you feedback after every button movement at least), but it sounds like you can organize a toolbar via a mouse even without seeing the position of cursor or the buttons. Do you use an equivalent of holding down the mouse button, moving the button to the new position, and then releasing the mouse button?

Matching the markup CKEditor provides normally is technically challenging because the groups don't exist yet while you are creating the toolbar. The way we're assembling groups currently is by letting the user move the group dividers to any position they want, and it will make the buttons on the left into one group and the buttons on the right into a second group. This makes for a simple implementation where everything is all in one list together, rather than having nested draggable areas.

#69

Last (but not least), we shouldn't bind dragging and dropping buttons to using arrow keys.

On this last point, we don't think we can remove the keyboard support. It's fine if you don't use the keyboard support, but navigation via the keyboard is still a large factor in overall accessibility. Why would we want to remove it entirely?

#70

@quicksketch: My reply is between your questions.

You wrote:
Thanks @falcon03 for your feedback. Could you describe what method you use to drag and drop the buttons? I've been putting effort into ensuring that the interface is navigable via the keyboard (that should give you feedback after every button movement at least), but it sounds like you can organize a toolbar via a mouse even without seeing the position of cursor or the buttons. Do you use an equivalent of holding down the mouse button, moving the button to the new position, and then releasing the mouse button?

Technically emulating a mouse with a keyboard and a screen reader is possible, but it does not work well. When I said "we shouldn't bind the dragging and dropping to arrow keys" I meant that e.g. on Mac OS X i can browse the page using arrow keys, but also using some other shortcut provided by Voiceover (e.g. "Control + Option + Right arrow" to go to the next page element or "Control + Option + Left arrow" to go to the previous page element). I can also browse the page with a touchscreen mobile device (e.g. an iPhone) or with the Macbook trackpad, as well as tabbing through various page elements. After dragging a button, I would like to move it around with these other shortcuts, not only using arrow keys. But talking to @jessebeach in the Skype Accessibility group about another issue I found out that can be hard, if not impossible, to manage this kind of navigation. What do you think about?

You wrote:
Matching the markup CKEditor provides normally is technically challenging because the groups don't exist yet while you are creating the toolbar. The way we're assembling groups currently is by letting the user move the group dividers to any position they want, and it will make the buttons on the left into one group and the buttons on the right into a second group. This makes for a simple implementation where everything is all in one list together, rather than having nested draggable areas.

Ok, I can understand. But this is the most important issue I found: while for sighted user this mechanism can be obvious, I'm afraid that for blind users it cannot. Additionally, this mechanism has another issue: in CKEditor (but not when it is used by Drupal, see #1900246: Menu item labels have been lost and comment #3 of that issue for steps to reproduce), each group has a "title" (or "name" or "label", I don't know how it is called). When browsing between groups or between buttons that belong to different groups, I can hear the name/title/label of the group. Let's say that I want to add a new group: how can I set a label/title/name for the new group with our configuration form?

#71

The most important aspect you found, falcon03, is #1900246: Menu item labels have been lost if I understand you correctly.

The answer is simple: http://docs.ckeditor.com/#!/guide/dev_toolbar. There are two types of configuration:

  1. groups configuration. Strongest advantage: better a11y (because each group has a name that is pronounced by screen readers). (Also advantage: newly installed plugins' buttons automatically appear, because they come with predefined button-group associations, and this is the real reason why this exists.) Strongest disadvantage: impossible to control precisely where each button appears in the CKEditor toolbar. (Also strong disadvantage: much, much harder to make an easy to use UI for this.)
  2. item by item configuration. Strongest advantage: precisely control button placement (and easier to make this UI). Strongest disadvantage: worse a11y.

Now, it is possible to define names for any set of subsequent buttons when using item by item configuration (so: user-defined groups) — see the first example at http://docs.ckeditor.com/#!/guide/dev_toolbar-section-2. It's also possible to implement this. But it will make the UI inherently much more difficult to use, because the person that configures the CKEditor toolbar will have to think about assigning names, whereas now that's not necessary. Worse, the current CKEditor toolbar configuration UI (which is as minimal as possible) would have to be extended to somehow somewhere allow these names to be entered.

(I discussed this with the CKEditor developers and our current approach seemed to be the best compromise, at least initially: CKEditor is still fully accessible, only without the button groups.)

I hope this explanation was mostly clear.

#72

As per #1900246-6: Menu item labels have been lost, marked #1900246 as a duplicate of this one. That issue is about the symptoms/consequences of the choices we make on the CKEditor toolbar configuration side.

#73

@Wim Leers: thank you very much for this explanation. It was really clear and I can understand perfectly the situation now.

Unfortunately I have to say that group names are really important. As I have already said, group names allow me to skip easily buttons that I am not interested in, since I can browse through groups before browsing through buttons that belong to a group.

You can say that there are hotkeys to do quickly what I want, and you're definitely right. But people don't always remember hotkeys or know about their existence.

So I hope that we can refactor this configuration form to use group names (or maybe delete it at all, providing just a default toolbar configuration).

#74

Assigned to:Anonymous» quicksketch

So I hope that we can refactor this configuration form to use group names

This is not a simple refactoring. That would require a rewrite (mostly). The UI will have to work completely differently, because of this entirely new concept of groups.

(or maybe delete it at all, providing just a default toolbar configuration).

This is absolutely unacceptable, because it'd make it impossible to customize the CKEditor toolbar configuration.

The only choices we have are: 1) rewrite, 2) to enter groups in the current UI: disable JS and enter JSON configuration for the CKEditor toolbar in the format that CKEditor expects (i.e. the current UI is JS-only), 3) live with slightly lesser a11y.

I think the only sane choice is #1 (rewrite), but there's of course only so much time and we want to move forward on other fronts too.

I sincerely hope quicksketch thinks I'm being silly and that the ability to set group names isn't synonymous to an (almost complete) rewrite. He's also the person who wrote the current drag-and-drop UI so he's definitely in a better position to judge this.

#75

I agree with Wim that I don't think a default-only toolbar configuration would be a good fit for Drupal. We're now hinging (or hoping to hinge) other functionality on the toolbar, such as the allowed tag list, since it's easier for an end-user to place a button for bold than it is to enable a <strong> tag.

I can't immediately think of a good solution for naming individual groups. If you named a group, you'd also probably need to be able to move that entire group to a new position at the same time. Say if you named the first group "Formatting" that contained bold, italics, and strikethrough, but then you decided you wanted the first group to be "Media" instead, which contained the Image button. You'd need to be able to swap the positions of the entire groups at the same time. If you just moved the buttons, the group labels would now be mismatched with the buttons inside of it. At the same time, you still need to be able to move the individual buttons within groups. Like I said in #68, I don't think nested draggable objects is going to be a good solution.

Regarding places to enter a group label, here are a couple of ideas:

  • We could make some kind of inline-presentation of group labels. This would increase the noise on the configuration form quite a bit and it would make a new set of accessibility problems. I also don't know how we would fit inline fields in a mobile situation. Despite these problems, inline fields for group labels seems like it'd be the best option to me, assuming we could figure out the UX challenges.
  • We could put group configuration in a modal. Some kind of trigger would need to be added to open the modal, for which I don't have good ideas either. Something like a tab that extends from a group is a possibility, or maybe just clicking on any button in the group(?)
  • The simplest solution: we just but a bunch of textfields after the toolbar configuration. :P
  • We could add a toggle to the entire toolbar that replaces the display of the buttons with the display of the group names, with textfields in the place of the individual buttons. This would disable drag and drop and only allow naming of groups with the current button positions.

#76

We could add a toggle to the entire toolbar that replaces the display of the buttons with the display of the group names, with textfields in the place of the individual buttons. This would disable drag and drop and only allow naming of groups with the current button positions.

I think this is the most elegant solution. It's still a new set of accessibility problems though.

#77

Status:needs work» needs review

#64: ckeditor_toolbar-a11y-1872206-63.patch queued for re-testing.

#78

Status:needs review» needs work

I like that the patch in #64 keeps the CSS outline. I do think we should re-roll the patch to try it with the toggle as recommended by @Wim & @quicksketch

#79

This issue has me worn down. The current patch is an improvement on the accessibility (though apparently not good enough yet). Adding support for named groups of buttons sounds like a feature enhancement and should probably be a followup. The end result of this issue is making me feel like writing a friendly UI for 99.9% of Drupal audiences isn't worth the disproportionate amount of effort to support the 0.1%. We've literally spend 5 times as long making the UI more accessible as it took to write it in the first place. As far as an effort/return investment goes, spending more time increasing the usability of a UI that administrators will need to configure once per site is not worth it.

#80

@quicksketch I think thats understandable, a11y issues have worn me down in the past as well. But its likely there are some unique scenario's here at play, that make it hard for this to be easily accessible. I think its common when we invent new interaction patterns, that it comes with a significant a11y burdon in both figuring out the best approach and implementing it, sadly there is often no clear roadmap to fixing a11y issues. In the end we are doing this for the cause, there is no return on investment calculation you can make, I think thats important to remember even when you are demotivated by the substantial time investment in issues like these.

I think it makes sense for you and WimLeers to perhaps skype/chat with falcon03, because the propositions here seem to indeed carry way too much overhead that will negatively impact ux and code. I think we need to take a step back and revaluate our direction here. I don't think that manually assigning names to groups of buttons is an feasible option, you cannot introduce such overhead on the sitebuilder for any purpose.

Is it possible to do this should be done automatically - maybe those groups wil be non-sensical but I am quite sure for the majority of buttons this approach would be fine. In the cases where one wants very specific group names, because maybe there are dozens and dozens of special buttons - then contrib could provide this solution.

#81

@quicksketch: When I reported these issues I didn't want to demotivate you... I know that fixing them is hard, but unfortunately these issues exist and can give blind people a hard time.

As @Bojhan said, maybe we'd better chat to discuss these issues in detail. Let me know if you, @Wim Leers and anyone else are interested in this proposal, so that we can establish a good time for anyone (I'm from Italy). There is a Skype accessibility group, but we could also have a private chat on Skype or IRC.

We should figure out a solution as soon as possible, maybe before Drupalcon Portland; otherwise, as @mgifford suggested in the Skype accessibility group, we can sit at a table at Drupalcon Portland and figure out the best solution! :-)

#82

@quicksketch accessibility is a definitely a huge challenge for any evolving software. It's often a matter of madly rushing around to try to keep up with the really wonderful stuff that folks are developing (but that often introduce barriers for participation).

I was hoping I could find the relative growth of JS in D8 from http://www.ohloh.net/p/drupal but it's not nicely broken up at this point. The front end has gotten way nicer, but also substantially more complicated from a code perspective.

Picking an editor for D8 is a huge step. Sadly all of the editors have had accessibility challenges with them. CKEditor had some good ground initially on making improvements, but there are still lots of known issues:
http://dev.ckeditor.com/query?status=!closed&component=Accessibility

Not that it will be addressed quickly by CKEditor, but is this ultimately something that should be solved upstream in their issue queue? I didn't see it listed there. I am much more certain that this community can fix the problem. It isn't something right now that is even on their radar as far as I could tell.

#83

The toolbar itself for CKEditor is already quite accessible. The original problem brought up here is that the task of configuring the toolbar is not accessible enough.

A second issue was brought up in #67. We're currently not naming our groups when they are configured. So in addition to making the task of configuring a toolbar accessible, we need to make sure that the resulting configuration is also accessible. (Basically, we need an accessible UI for creating an accessible UI). There aren't any problems with CKEditor that need to be solved here, we just need to figure out how to make a UI for configuring it.

nobody click here