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

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 cr.define('cr.ui', function() {
   /** @const */
   var Menu = cr.ui.Menu;
   /** @const */
   var positionPopupAroundElement = cr.ui.positionPopupAroundElement;
 
@@ skipping 65 matching lines @@
           if (e.currentTarget == this.ownerDocument) {
             if (!this.contains(e.target) && !this.menu.contains(e.target))
               this.hideMenu();
             else
               e.preventDefault();
           } else {
             if (this.isMenuShown()) {
               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();
             }
           }
           break;
         case 'keydown':
           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;
       }
     },
 
     /**
      * 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');
       event.initUIEvent('menushow', true, true, window, null);
 
       if (this.dispatchEvent(event)) {
         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_();
       }
     },
 
     /**
      * Hides the menu. If your menu can go out of scope, make sure to call this
      * first.
      */
     hideMenu: function() {
       if (!this.isMenuShown())
         return;
 
       this.removeAttribute('menu-shown');
       this.menu.hidden = true;
 
       this.showingEvents_.removeAll();
-      this.menu.selectedIndex = -1;
+      this.focus();
     },
 
     /**
      * Whether the menu is shown.
      */
     isMenuShown: function() {
       return this.hasAttribute('menu-shown');
     },
 
     /**
      * Positions the menu below the menu button. At this point we do not use any
      * advanced positioning logic to ensure the menu fits in the viewport.
      * @private
      */
     positionMenu_: function() {
       positionPopupAroundElement(this, this.menu, this.anchorType,
                                  this.invertLeftRight);
     },
 
     /**
      * Handles the keydown event for the menu button.
      */
     handleKeyDown: function(e) {
       switch (e.keyIdentifier) {
         case 'Down':
         case 'Up':
         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;
       }
     }
   };
 
   /**
    * Helper for styling a menu button with a drop-down arrow indicator.
    * Creates a new 2D canvas context and draws a downward-facing arrow into it.
    * @param {string} canvasName The name of the canvas. The canvas can be
@@ skipping 37 matching lines @@
         'drop-down-arrow-hover', ARROW_WIDTH, ARROW_HEIGHT, hoverColor);
     createDropDownArrowCanvas(
         'drop-down-arrow-active', ARROW_WIDTH, ARROW_HEIGHT, activeColor);
   };
 
   // Export
   return {
     MenuButton: MenuButton
   };
 });