Basic keyboard access for recently_closed menu on NTP (re-work).

chrome/browser/resources/shared/js/cr/ui/menu_button.js

Index: chrome/browser/resources/shared/js/cr/ui/menu_button.js
diff --git a/chrome/browser/resources/shared/js/cr/ui/menu_button.js b/chrome/browser/resources/shared/js/cr/ui/menu_button.js
index 3363babb65b5bd3f2ffa490243c1da3f9aed5566..f6c76ede20c415f5266cfe8389dfbd27c0cfc806 100644
--- a/chrome/browser/resources/shared/js/cr/ui/menu_button.js
+++ b/chrome/browser/resources/shared/js/cr/ui/menu_button.js
@@ -83,7 +83,7 @@ cr.define('cr.ui', function() {
               this.hideMenu();
             } else if (e.button == 0) {  // Only show the menu when using left
                                          // mouse button.
-              this.showMenu();
+              this.showMenu(false);
               // Prevent the button from stealing focus on mousedown.
               e.preventDefault();
             }
@@ -93,14 +93,19 @@ cr.define('cr.ui', function() {
           this.handleKeyDown(e);
           // If the menu is visible we let it handle all the keyboard events.
           if (this.isMenuShown() && e.currentTarget == this.ownerDocument) {
-            this.menu.handleKeyDown(e);
-            e.preventDefault();
-            e.stopPropagation();
+            if (this.menu.handleKeyDown(e)) {
+              e.preventDefault();
+              e.stopPropagation();
+            }
           }
           break;
 
+        case 'focus':

[Dan Beam, 2012/11/12 22:55:03]

is there a functional difference between "focus on a different element" and
"blur on this element"? can there be no current focus (i.e. no
document.activeElement)?

[aboxhall, 2012/11/13 00:49:52]

Yep - at the time of blur, the new document.activeElement hasn't yet been set,
so this doesn't work.

+          if (!this.contains(e.target) && !this.menu.contains(e.target))
+            this.hideMenu();
+          break;
+
         case 'activate':
-        case 'blur':
         case 'resize':
           this.hideMenu();
           break;
@@ -109,8 +114,10 @@ cr.define('cr.ui', function() {
 
     /**
      * Shows the menu.
+     * @param {boolean} shouldSetFocus Whether to set focus on the

[Dan Beam, 2012/11/12 22:55:03]

nit: is this boolean optional to pass? i.e. is there code you want to keep just
as:

  this.showMenu();

? if so, you should make this

  @param {boolean=} opt_shouldSetFocus

but I'd recommend just making all places pass a boolean isntead.

[aboxhall, 2012/11/13 00:49:52]

I agree, and I'm pretty sure I changed all call sites.

+     *     selected menu item.
      */
-    showMenu: function() {
+    showMenu: function(shouldSetFocus) {
       this.hideMenu();
 
       var event = document.createEvent('UIEvents');
@@ -120,13 +127,15 @@ cr.define('cr.ui', function() {
         this.menu.hidden = false;
 
         this.setAttribute('menu-shown', '');
+        if (shouldSetFocus)
+          this.menu.focusSelectedItem();
 
         // when the menu is shown we steal all keyboard events.
         var doc = this.ownerDocument;
         var win = doc.defaultView;
         this.showingEvents_.add(doc, 'keydown', this, true);
         this.showingEvents_.add(doc, 'mousedown', this, true);
-        this.showingEvents_.add(doc, 'blur', this, true);
+        this.showingEvents_.add(doc, 'focus', this, true);
         this.showingEvents_.add(win, 'resize', this);
         this.showingEvents_.add(this.menu, 'activate', this);
         this.positionMenu_();
@@ -145,7 +154,7 @@ cr.define('cr.ui', function() {
       this.menu.hidden = true;
 
       this.showingEvents_.removeAll();
-      this.menu.selectedIndex = -1;
+      this.focus();
     },
 
     /**
@@ -175,11 +184,12 @@ cr.define('cr.ui', function() {
         case 'Enter':
         case 'U+0020': // Space
           if (!this.isMenuShown())
-            this.showMenu();
+            this.showMenu(true);
           e.preventDefault();
           break;
         case 'Esc':
         case 'U+001B': // Maybe this is remote desktop playing a prank?
+        case 'U+0009': // Tab
           this.hideMenu();
           break;
       }