There has been some accessibility testing of BUEditor on Drupal.org on this issue:
#1306752: Accessibility problems with the BUEditor

In the latest tests (comments #58-62 on that issue), the sandbox on http://t1.ufku.com/contact was tested by mgifford. He found that, contrary to previous versions of BUEditor, a non-sighted user has no way to discover the control keys that they can use to access the buttons. He suggested adding some text that says:

Access the BUEditor help by pressing Ctrl + H

He also said: "I think an earlier version of code (no longer available as a sandbox) had the first tab element in the string of icons display some help text to inform you that you could press Ctrl + I to produce <em>."

Is it possible for the BUEditor to put this back in? The only other alternative right now (I think) is for drupal.org to put help text under the fields using the BUEditor with that information, but I'm not really sure that's possible...

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

ufku’s picture

Category: bug » support
Priority: Major » Normal

Button titles include the shortcut info. There are also accesskey and alt attributes that give info about the accesskey. Note that Ctrl-shortcuts is not available by default. It is activated by an additional library if it's included in editor settings, and it does not affect the default accesskey functionality.

There has been no regression considering ctrl keys.

jhodgdon’s picture

I'll have to get a second opinion on this from the Accessibility team. I was just reporting what they said in their review.

Ade H’s picture

Title attribute content cannot be regarded as accessible. Many aural ATs ignore it by default (for brevity), keyboarders will never see it in any browser that I know of, and low-vision users cannot read them.

mgifford’s picture

Issue tags: +accessibility

tagging. Also verifying @Ade H's comment about title's. Alt Text is definitely the way to go.

mgifford’s picture

This is still a problem.

mgifford’s picture

Version: 6.x-2.x-dev » 7.x-1.x-dev
Issue summary: View changes
Issue tags: +affects Drupal.org

This is still an issue on d.o

jhodgdon’s picture

http://webaim.org/techniques/forms/controls#image_button says: "If you use an image button ( rather than a standard button, the input must have appropriate alt text." and suggests markup like:

<input type="image" name="submitbutton" alt="search" src="submit.png">

http://www.w3.org/WAI/GL/techniques.htm#A.1.4 says the same thing (more authoritative source).

But... The individual buttons already do have an "alt" attribute on them. For instance, the first button I am seeing here has alt="Bold(B)".

So the alt text is there. Are the browsers you are using not seeing that alt text, or do you just think the module should be putting in better alt text (maybe the same as the title attribute)?

By the way, the code that generates the alt/title text is around line 138 in
http://cgit.drupalcode.org/bueditor/tree/bueditor.js

mgifford’s picture

Status: Active » Needs review
FileSize
781 bytes

So, there is an alt text, but it isn't very descriptive. It doesn't actually provide as much information as the title.

Often screen readers simply ignore the title as it is usually useless, so the important tag is the alt text. I've fleshed that out a bit in the patch I've provided. It would be fine if it were as extensive as the title, but the alt text shouldn't be less informative that the title.

Also, for keyboard only users don't access alt text and can't benefit from the hovering over the image and seeing the title tag be displayed. For keyboard only users there is really no way to access what are the shortcuts other than looking at the source.

For those users it would be useful to have something like the Skip Links that simply shows up when you tab over the interface. The text obviously needs work, but it could look something like:

<a class="element-invisible element-focusable" href="#">Using (Shift + Alt + COMMAND CHARACTER) you can leverage BUEditor. [Bold(B), Italics(I), Strikethrough(S)]</a>

It just has to appear before the list of icons so that it is the first think someone tabs to.

jhodgdon’s picture

Status: Needs review » Needs work

Good explanation!

This patch doesn't look right to me:

-    alt = title + (key ? '('+ key +')' : '');
+    alt = access && key ? title + ' (Access Key: '+ access +' + '+ key +')' : title + (key ? '('+ key +')' : '');
+    title += access && key ? ' ('+ access +' + '+ key +')' : '';
     title += access && key ? ' ('+ access +' + '+ key +')' : '';

- In the alt line, it looks like it is using the key variable even if it is possibly not defined?
- The title line looks like it is now in the file twice?

Regarding your suggestion for additional text, it seems like that could be added to the loop inside

BUE.theme = function (tplid) {

in http://cgit.drupalcode.org/bueditor/tree/bueditor.js

The way I would do it would be to build up a variable called "explanation", and initialize it to

'<a class="element-invisible element-focusable" href="#">BUEditor commands: Use (Shift + Alt + COMMAND CHARACTER). Available commands: '

initially.

Then each time through the loop, in the area of the lines you are adding to the file in this patch, you should be able to do something like:

if (key) {
  explanation += title + '(' + key + '),'
}

although you'd probably want to remove the final , at the end.

mgifford’s picture

Status: Needs work » Needs review
FileSize
1.39 KB

Good suggestions. I think the key variable is checked properly. Maybe not, let me know what code is missing.

How about this.

jhodgdon’s picture

Status: Needs review » Needs work

Ummm...

+    if (key) {
+      if (explanation_count == 0) {
+        explanation += title + '(' + key + ')';
+      }
+      else {
+        explanation += ', ' + title + '(' + key + ')';
+      }
+    }

After adding something to explanation, you need to set explanation_count to a non-zero value so you get the comma next time?

And then here:

+    alt = access && key ? title + ' (Access Key: '+ access +' + '+ key +')' : title + (key ? '('+ key +')' : '');

This is really convoluted, with two ? : if statements. I think the code would be a lot clearer if it was a regular if() statement.

Also I see here the button is only printed out if the 'access' variable is also TRUE, but up above in explanation, you are only checking if 'key' evaluates to TRUE? Probably the whole thing could be put into one if() statement, like:

if (access) {
   if (key) {
      // do explanation, alt, title with key
   } else {
     // do explanation, alt, title without key
  }
  // add alt/title to the html variable
}

Although this would change the output slightly, since for whatever reason, the buttons are not currently being output if they have no key defined, which seems a bit odd to me.

One more thing: I forgot to mention it before but at the end of the function you need to close the A tag in explanation, plus actually output the explanation. As it is now, you're building it up but it never gets finished or printed out.

mgifford’s picture

Status: Needs work » Needs review
FileSize
1.63 KB
mgifford’s picture

forgot to export the explanation.

mgifford’s picture

sorry for the noise...

mgifford’s picture

Now it just has to be pretty.
On Hover Discoverability.

jhodgdon’s picture

For readability, I think there should be a space between the title and the (key)? Also, it's not really good form to put a <br /> at the end of an A tag, and shouldn't be necessary. Why did you do that? Maybe needs a CSS update instead to put it on its own line and/or make it actually invisible?

mgifford’s picture

Agreed on the space & <br />. It should have totally been done with CSS. I'm just doing this very incrementally as you can see. Not always the most effective way.

It's definitely looking better now:
BUEditor discoverability.

jhodgdon’s picture

Category: Support request » Bug report
Status: Needs review » Needs work

Oh, I just assumed if you uploaded a patch and marked it "needs review" that you actually wanted a review. How about if we leave this issue at "needs work" until you have what you think is a viable patch then?

And by the way... Why is this explanatory text a link? It doesn't link to anything.

mgifford’s picture

I do really appreciate your speedy reviews.. You've responded way faster than the average Drupal person. It's been great, and always with stuff to help nudge this along.

The link is needed. It doesn't need to go anywhere, it should be styled so it doesn't look like a link (even on focus/hover), but without a link there's nothing to tab to. I guess we could make it an input button but that doesn't make any sense either.

Maybe there's a way to do this with JS, but....

If that works for you I'll add the styles, otherwise we should talk about what is possible.

jhodgdon’s picture

I'm not much of an accessibility expert, but I do remember about links being tabbable (if that's a word), so that makes sense.

Just so we're clear... I'm not one of the maintainers of this module. I just want to make sure the tools we have on Drupal.org are usable by everyone, even if they're tools I personally don't use. :)

Anyway, I think the text looks good now, and I don't see any issues in the patch (aside from needing to hide the link from sighted users presumably).

At some point, applying this patch to a d.o test site so that both regular-browser-sighted and various adaptive-browser users can try it out might be a good idea? It might also be good to test it out with a non-sighted user to find out if the text makes sense to them too?

mgifford’s picture

Status: Needs work » Needs review
FileSize
2.67 KB

Tabbable.. Totally makes sense. And it's a big deal for keyboard only users.

Really appreciate your efforts to help make Drupal more inclusive. It's always a bit of a struggle to get these things in. However, it's a pretty straight forward change that should really be invisible to almost everyone (but quite informative to others).

Totally makes sense to look at applying this patch to a d.o test site so that both regular-browser-sighted and various adaptive-browser users can try it out. However, it's not like drupal.org is going to be adding anything other than stable releases unless there's a strong reason....

Also agreed about calling for some user testing with screen reader users. I am confident that it would be read by a screen reader, but it's something I understand the context for, so it's not really a clear representation if it is understandable or not for a blind user.

jhodgdon’s picture

"Some people cannot use bueditor" seems like a strong reason. The Drupal community does have accessibility as a core value, right?

mgifford’s picture

Yup. :)