Just looking at my site through http://wave.webaim.org/toolbar

Came obvious that there are accessibility problems with the Masquerade form (see image attached).

All forms need to have a label with them. These don't need to be visible, but they do need to be there to provide semantics.

#3 1502804-accessibility-3.patch3.12 KBandypost
FAILED: [[SimpleTest]]: [MySQL] 16 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#3 masq_acessibility.jpg7.61 KBandypost
#1 1502804-accessibility-1.patch926 bytesandypost
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in sites/default/modules/masquerade/masquerade.module.
[ View ]
Masquerade-accessibility-nolabel.png13.76 KBmgifford


Status:Active» Needs review
new926 bytes
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in sites/default/modules/masquerade/masquerade.module.
[ View ]

Does this patch make a sense? Maybe title should be shorter?

That would work, but would be a bit of a duplication. as the screen reader user would hear the title twice. I do wonder why you added <div class="description"></div>. I'm also not sure why you couldn't just remove $markup_value completely & use the title (without the invisible title of course).

Version:7.x-1.0-rc4» 7.x-1.x-dev
new7.61 KB
new3.12 KB
FAILED: [[SimpleTest]]: [MySQL] 16 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

@mgifford We can't use this long string as title because it related to both input and submit. This ugly fix just tried to implement (mimic) standard element description... but this anyway breaks a form flow... so

here's a new patch, changes
- added title Username, probably we should hide it with '#title_display' => 'invisible'
- reordered a code and added some comments

-also changed a way to display title for quick switches to more accessible

I'd like to get review of @deekayen on this because this changes the form a bit


Patch still seems to work well when I ran it in http://simplytest.me

Status:Needs review» Needs work

The last submitted patch, 1502804-accessibility-3.patch, failed testing.

Traditionally, if you're going to include a description, it's associated with an input field. If you're going to break from that model, I'd think you'd want the instructions to precede the form, not append to it, then again, I'm not blind. If we're really going to consider the impacts of blind people, the Go description on the submit button isn't very helpful either.

+++ b/masquerade.moduleundefined
@@ -555,8 +555,37 @@ function masquerade_block_1() {
+      '#prefix' => '<div class="form-item"><div class="description">',
+      '#markup' => $markup_value,
@@ -578,33 +607,15 @@ function masquerade_block_1() {
-    '#prefix' => '<div class="form-item"><div class="description">',
-    '#markup' => $markup_value,
-    '#suffix' => '</div></div>',

on a one hand this needs better accessible attributes and render

+++ b/masquerade.moduleundefined
@@ -555,8 +555,37 @@ function masquerade_block_1() {
+    if (masquerade_menu_access('autocomplete')) {
+    }
+    // Add instructions for form above. Don't use description for input because
+    // this used for input and submit both.
+    $form['masquerade_desc'] = array(

Anyway this description should use same rules as inputs does

+++ b/masquerade.moduleundefined
@@ -555,8 +555,37 @@ function masquerade_block_1() {
+      $form['masquerade_user_field'] = array(
+        '#prefix' => '<div class="container-inline">',

on a second - this sould be done with proper container wrapper as 8.x-2.x makes.
So description could be attached to container

I'm not highly opinionated on this issue and don't have any background in blind people stuff, nor am I really interested in getting one.

"blind people stuff"

Err, let's just call it Accessibility, since it applies to a wide range of people who use screen readers and other accessibility devices, not just "blind people." Also it is of wide interest because a lot of us work in fields like government or education where we are legally required to produce accessible websites. But that can be a bit hard to work out- should a dev do the bare minimum which is to make it readable in a screen reader and adhere to basic web best practices, or should they be doing usability testing with a screen reader? Probably the former is best for modules like this.

Yeah, I get it. I don't work for the government and I've turned down jobs where I found their portfolio had govt work I might have to participate in. There are too many government departments that have no legitimate constitutional existence and I won't participate willingly in their success.

#8 is simply me letting Andrey take lead on this issue. I don't really care about accessibility, nor would I attempt to maintain its compatibility in the future, but this module has quickly grown to be completely out of my control anyway. Andrey, Mike, and Daniel are good devs and I don't feel the need to clash on this issue.