#622136 by brandonojc and mgifford: Fixed user filter accessibility.

9 files changed, 57 insertions, 40 deletions

diff --git a/modules/node/node-rtl.css b/modules/node/node-rtl.css
index 7e0dce520..48d62aedc 100644
--- a/modules/node/node-rtl.css
+++ b/modules/node/node-rtl.css
@@ -1,6 +1,6 @@
 /* $Id$ */
 
-#node-admin-content dl.multiselect dd.b .form-item label {
+#node-admin-content dl.multiselect dd .form-item label {
   display: block;
   float: right;
   width: 6em;
diff --git a/modules/node/node.admin.inc b/modules/node/node.admin.inc
index f9c21155d..53a82d350 100644
--- a/modules/node/node.admin.inc
+++ b/modules/node/node.admin.inc
@@ -242,7 +242,7 @@ function theme_node_filters($variables) {
 
   $output .= '<dl class="multiselect">' . (!empty($form['current']) ? '<dt><em>' . t('and') . '</em> ' . t('where') . '</dt>' : '');
 
-  $output .= '<dd class="b">';
+  $output .= '<dd>';
 
   foreach (element_children($form['status']) as $key) {
     $output .= drupal_render($form['status'][$key]);
diff --git a/modules/node/node.css b/modules/node/node.css
index b102203ad..177c574e1 100644
--- a/modules/node/node.css
+++ b/modules/node/node.css
@@ -7,10 +7,10 @@
   background-color: #ffffea;
 }
 /* Override the default multiselect layout in system.css. */
-#node-admin-content dl.multiselect dd.b, dl.multiselect dd.b .form-item {
+#node-admin-content dl.multiselect dd, dl.multiselect dd .form-item {
   width: 20em; /* 6em label + 14em select */
 }
-#node-admin-content dl.multiselect dd.b .form-item label {
+#node-admin-content dl.multiselect dd .form-item label {
   display: block;
   float: left; /* LTR */
   width: 6em;
diff --git a/modules/system/system.css b/modules/system/system.css
index 454edd493..02a6b2657 100644
--- a/modules/system/system.css
+++ b/modules/system/system.css
@@ -193,14 +193,11 @@ tr.merge-up, tr.merge-up td, tr.merge-up th {
   padding-bottom: 0;
   font-size: 0.9em;
 }
-dl.multiselect dd.b, dl.multiselect dd.b .form-item, dl.multiselect dd.b select {
+dl.multiselect dd, dl.multiselect dd .form-item, dl.multiselect dd select {
   font-family: inherit;
   font-size: inherit;
   width: 14em;
 }
-dl.multiselect dd.a, dl.multiselect dd.a .form-item {
-  width: 8em;
-}
 dl.multiselect dt, dl.multiselect dd {
   float: left; /* LTR */
   line-height: 1.75em;
diff --git a/modules/user/user-rtl.css b/modules/user/user-rtl.css
index ed3f23aa8..ca3d9fa87 100644
--- a/modules/user/user-rtl.css
+++ b/modules/user/user-rtl.css
@@ -4,6 +4,11 @@
   padding-left: 0;
   padding-right: 1.5em;
 }
+
+#user-admin-filter dl.multiselect dd .form-item label {
+  float: right;
+}
+
 #user-admin-buttons {
   float: right;
   margin-left: 0;
diff --git a/modules/user/user.admin.inc b/modules/user/user.admin.inc
index bbc633436..f317a1bbf 100644
--- a/modules/user/user.admin.inc
+++ b/modules/user/user.admin.inc
@@ -44,10 +44,17 @@ function user_filter_form() {
   );
   foreach ($session as $filter) {
     list($type, $value) = $filter;
-    // Merge an array of arrays into one if necessary.
-    $options = $type == 'permission' ? call_user_func_array('array_merge', $filters[$type]['options']) : $filters[$type]['options'];
-    $params = array('%property' => $filters[$type]['title'] , '%value' => $options[$value]);
-    if ($i++ > 0) {
+    if ($type == 'permission') {
+      // Merge arrays of module permissions into one.
+      // Slice past the first element '[any]' whose value is not an array.
+      $options = call_user_func_array('array_merge', array_slice($filters[$type]['options'], 1));
+      $value = $options[$value];
+    }
+    else {
+      $value = $filters[$type]['options'][$value];
+    }
+    $params = array('%property' => $filters[$type]['title'] , '%value' => $value);
+    if ($i++) {
       $form['filters']['current'][] = array('#markup' => t('<em>and</em> where <strong>%property</strong> is <strong>%value</strong>', $params));
     }
     else {
@@ -60,13 +67,11 @@ function user_filter_form() {
     $form['filters']['status'][$key] = array(
       '#type' => 'select',
       '#options' => $filter['options'],
+      '#title' => $filter['title'],
+      '#default_value' => '[any]',
     );
   }
 
-  $form['filters']['filter'] = array(
-    '#type' => 'radios',
-    '#options' => $names,
-  );
   $form['filters']['buttons']['submit'] = array(
     '#type' => 'submit',
     '#value' => (count($session) ? t('Refine') : t('Filter')),
@@ -94,13 +99,17 @@ function user_filter_form_submit($form, &$form_state) {
   $op = $form_state['values']['op'];
   $filters = user_filters();
   switch ($op) {
-    case t('Filter'): case t('Refine'):
-      if (isset($form_state['values']['filter'])) {
-        $filter = $form_state['values']['filter'];
-        // Merge an array of arrays into one if necessary.
-        $options = $filter == 'permission' ? call_user_func_array('array_merge', $filters[$filter]['options']) : $filters[$filter]['options'];
-        if (isset($options[$form_state['values'][$filter]])) {
-          $_SESSION['user_overview_filter'][] = array($filter, $form_state['values'][$filter]);
+    case t('Filter'):
+    case t('Refine'):
+      // Apply every filter that has a choice selected other than 'any'.
+      foreach ($filters as $filter => $options) {
+        if (isset($form_state['values'][$filter]) && $form_state['values'][$filter] != '[any]') {
+          // Merge an array of arrays into one if necessary.
+          $options = ($filter == 'permission') ? form_options_flatten($filters[$filter]['options']) : $filters[$filter]['options'];
+          // Only accept valid selections offered on the dropdown, block bad input.
+          if (isset($options[$form_state['values'][$filter]])) {
+            $_SESSION['user_overview_filter'][] = array($filter, $form_state['values'][$filter]);
+          }
         }
       }
       break;
@@ -886,14 +895,11 @@ function theme_user_filters($variables) {
       $output .= '<li>' . drupal_render($form['current'][$key]) . '</li>';
     }
   }
+  $output .= '</ul>';
 
-  $output .= '<li><dl class="multiselect">' . (!empty($form['current']) ? '<dt><em>' . t('and') . '</em> ' . t('where') . '</dt>' : '') . '<dd class="a">';
-  foreach (element_children($form['filter']) as $key) {
-    $output .= drupal_render($form['filter'][$key]);
-  }
-  $output .= '</dd>';
+  $output .= '<dl class="multiselect">' . (!empty($form['current']) ? '<dt><em>' . t('and') . '</em> ' . t('where') . '</dt>' : '');
 
-  $output .= '<dt>' . t('is') . '</dt><dd class="b">';
+  $output .= '<dd>';
 
   foreach (element_children($form['status']) as $key) {
     $output .= drupal_render($form['status'][$key]);
@@ -902,7 +908,6 @@ function theme_user_filters($variables) {
 
   $output .= '</dl>';
   $output .= '<div class="container-inline" id="user-admin-buttons">' . drupal_render($form['buttons']) . '</div>';
-  $output .= '</li></ul>';
 
   return $output;
 }
diff --git a/modules/user/user.css b/modules/user/user.css
index f878b31b2..ceb1562d1 100644
--- a/modules/user/user.css
+++ b/modules/user/user.css
@@ -9,12 +9,15 @@
 #permissions tr.odd .form-item, #permissions tr.even .form-item {
   white-space: normal;
 }
-#user-admin-filter ul {
-  list-style-type: none;
-  list-style-image: none;
-  padding: 0;
-  margin: 0;
-  width: 100%;
+/* Override the default multiselect layout in system.css. */
+#user-admin-filter dl.multiselect dd, dl.multiselect dd .form-item {
+  width: 20em; /* 6em label + 14em select */
+}
+#user-admin-filter dl.multiselect dd .form-item label {
+  display: block;
+  float: left; /* LTR */
+  width: 6em;
+  font-weight: normal;
 }
 #user-admin-buttons {
   float: left; /* LTR */
diff --git a/modules/user/user.module b/modules/user/user.module
index a136dbde9..a9cca9849 100644
--- a/modules/user/user.module
+++ b/modules/user/user.module
@@ -2783,7 +2783,9 @@ function user_filters() {
     $filters['role'] = array(
       'title' => t('role'),
       'field' => 'ur.rid',
-      'options' => $roles,
+      'options' => array(
+        '[any]' => t('any'),
+      ) + $roles,
     );
   }
 
@@ -2800,13 +2802,19 @@ function user_filters() {
   ksort($options);
   $filters['permission'] = array(
     'title' => t('permission'),
-    'options' => $options,
+    'options' => array(
+      '[any]' => t('any'),
+    ) + $options,
   );
 
   $filters['status'] = array(
     'title' => t('status'),
     'field' => 'u.status',
-    'options' => array(1 => t('active'), 0 => t('blocked')),
+    'options' => array(
+      '[any]' => t('any'),
+      1 => t('active'),
+      0 => t('blocked'),
+    ),
   );
   return $filters;
 }
diff --git a/modules/user/user.test b/modules/user/user.test
index ec24035a8..23ed393c6 100644
--- a/modules/user/user.test
+++ b/modules/user/user.test
@@ -984,7 +984,6 @@ class UserAdminTestCase extends DrupalWebTestCase {
 
     // Filter the users by permission 'administer taxonomy'.
     $edit = array();
-    $edit['filter'] = 'permission';
     $edit['permission'] = 'administer taxonomy';
     $this->drupalPost('admin/people', $edit, t('Filter'));