Accessibility issue with the Masquerade Form

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

@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

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.

Thanks for working on this. In an attempt to help the maintainers, I am going through some of the Drupal 7 issues, and closing them, since Drupal 7 is EOL.

Maintainers should grant credit for the great work to the relevant users, even if the patch did not get committed: https://www.drupal.org/docs/develop/issues/issue-procedures-and-etiquett....

Since Drupal 7 is EOL, we can probably close this issue, but feel free to re-open if this is still relevant, and we should consider adding this to the Drupal 11 version?