Make user remove button focusable, and set up the correct tab order.

chrome/browser/resources/chromeos/login/user_pod_row.js

Index: chrome/browser/resources/chromeos/login/user_pod_row.js
diff --git a/chrome/browser/resources/chromeos/login/user_pod_row.js b/chrome/browser/resources/chromeos/login/user_pod_row.js
index 8b9c2774a78c60ae01669abf86e078548ab3413f..4eb7dfaf8931871370e35215f89137142cbd6687 100644
--- a/chrome/browser/resources/chromeos/login/user_pod_row.js
+++ b/chrome/browser/resources/chromeos/login/user_pod_row.js
@@ -49,6 +49,8 @@ cr.define('login', function() {
           this.activate.bind(this));
       this.signinButtonElement.addEventListener('click',
           this.activate.bind(this));
+      this.removeUserButtonElement.addEventListener('click',
+          this.handleRemoveButtonClick_.bind(this));
       this.removeUserButtonElement.addEventListener('mouseout',
           this.handleRemoveButtonMouseOut_.bind(this));
     },
@@ -175,6 +177,9 @@ cr.define('login', function() {
         this.imageElement.title = userDict.emailAddress;
         this.enterButtonElement.hidden = true;
         this.passwordElement.hidden = needSignin;
+        this.removeUserButtonElement.setAttribute(
+            'aria-label', localStrings.getStringF('removeButtonAccessibleName',
+                                                  userDict.emailAddress));
         this.passwordElement.setAttribute('aria-label',
                                           localStrings.getStringF(
                                               'passwordFieldAccessibleName',
@@ -319,10 +324,7 @@ cr.define('login', function() {
       if (!this.parentNode.rowEnabled)
         return;
       var handled = false;
-      if (e.target == this.removeUserButtonElement) {
-        this.handleRemoveButtonClick_(e);
-        handled = true;
-      } else if (!this.signinButtonElement.hidden) {
+      if (!this.signinButtonElement.hidden) {
         this.parentNode.showSigninUI(this.user.emailAddress);
         handled = true;
       }
@@ -403,6 +405,8 @@ cr.define('login', function() {
 
       this.appendChild(userPod);
       userPod.initialize();
+      var index = this.findIndex_(userPod);
+      userPod.removeUserButtonElement.tabIndex = 100 + index;
     },
 
     /**
@@ -470,6 +474,7 @@ cr.define('login', function() {
       // Popoulate the pod row.
       for (var i = 0; i < users.length; ++i) {
         this.addUserPod(users[i], animated);
+        this.pods[i].tabIndex = i + 1;

[dmazzoni, 2011/10/26 16:14:49]

I'm pretty sure everything in the page with the same tabIndex will be focused in
DOM order. I think it might be more robust to just have some fixed tab indexes
for each *group* of related controls - that way it doesn't matter how many of
each there are and there's less to keep track of.

How about defining some constants at the top, with a comment explaining this,
e.g.:

var TOP_CONTROLS_TAB_INDEX = 1;
var PASSWORD_FIELDS_TAB_INDEX = 2;
var REMOVE_BUTTONS_TAB_INDEX = 3;

Optional (not part of this CL): how hard would it be to add a unit test that
ensures that nobody adds a control to this page in the future without setting
the tab index? That'd be a fantastic way to help prevent future regressions.

       }
     },
 
@@ -482,18 +487,21 @@ cr.define('login', function() {
         return;
 
       if (pod) {
+        var focusedTabIndex = 0;
         for (var i = 0; i < this.pods.length; ++i) {
           this.pods[i].activeRemoveButton = false;
           if (this.pods[i] == pod) {
             pod.classList.remove("faded");
             pod.classList.add("focused");
             pod.tabIndex = -1;  // Make it not keyboard focusable.
+            focusedTabIndex = i + 1;
           } else {
             this.pods[i].classList.remove('focused');
             this.pods[i].classList.add('faded');
-            this.pods[i].tabIndex = 0;
+            this.pods[i].tabIndex = i + 1;
           }
         }
+        pod.mainInput.tabIndex = focusedTabIndex;
         pod.focusInput();
 
         this.focusedPod_ = pod;
@@ -503,7 +511,7 @@ cr.define('login', function() {
           this.pods[i].classList.remove('focused');
           this.pods[i].classList.remove('faded');
           this.pods[i].activeRemoveButton = false;
-          this.pods[i].tabIndex = 0;
+          this.pods[i].tabIndex = i + 1;
         }
         this.focusedPod_ = undefined;
       }