Basic keyboard access for recently_closed menu on NTP. Allows using up and down arrows to navigate,…

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..535535e6c421726b2978337a1e058f6a09fae291 100644
--- a/chrome/browser/resources/shared/js/cr/ui/menu_button.js
+++ b/chrome/browser/resources/shared/js/cr/ui/menu_button.js
@@ -100,7 +100,6 @@ cr.define('cr.ui', function() {
           break;
 
         case 'activate':
-        case 'blur':
         case 'resize':
           this.hideMenu();
           break;
@@ -120,6 +119,7 @@ cr.define('cr.ui', function() {
         this.menu.hidden = false;
 
         this.setAttribute('menu-shown', '');
+        this.menu.selectedIndex = 0;

[Dan Beam, 2012/09/21 03:44:50]

should the first item always be selected when shown?

[aboxhall, 2012/09/21 06:46:00]

The selectedIndex is set back to -1 below, so it seems reasonable to just go to
the first item each time (unless we want the menu to also keep track of the
previously selected item when it's closed?)

[Dan Beam, 2012/09/21 10:23:48]

Where is it set to -1? Either way I guess this method isn't actually toggling
the visibility of the list or anything (like "showMenu" would you'd think), so I
guess this is fine. You'd think this would start unselected, when their
container was focused the first item (I guess) would be selected, and until
something changed the selected or multiselected + lead would be the same, yes.

[aboxhall, 2012/09/24 00:48:52]

It's set to -1 in the hideMenu() method below.

I agree it would be better to go back to the last selected item; I'll do that.

 
         // when the menu is shown we steal all keyboard events.
         var doc = this.ownerDocument;
@@ -146,6 +146,7 @@ cr.define('cr.ui', function() {
 
       this.showingEvents_.removeAll();
       this.menu.selectedIndex = -1;
+      this.focus();

[Dan Beam, 2012/09/21 03:44:50]

I'm confused by this ^

[aboxhall, 2012/09/21 06:46:00]

It puts the focus back on the menu button when the menu closes, so you can keep
tabbing through the page as before.

[Dan Beam, 2012/09/21 10:23:48]

How does focus()ing a hidden element put the focus back where it was before? I
thought focus()ing a hidden element did nothing in WebKit.

[aboxhall, 2012/09/24 00:48:52]

It's not focusing a hidden element, it's focusing the menu button. |this.menu|
is hidden at this point, but not |this|.

     },
 
     /**
@@ -179,6 +180,7 @@ cr.define('cr.ui', function() {
           e.preventDefault();
           break;
         case 'Esc':
+        case 'U+0009':

[Dan Beam, 2012/09/21 03:44:50]

case 'U+0009': // Whatever the heck this key is.

[Dan Beam, 2012/09/21 10:23:48]

Ping

[aboxhall, 2012/09/24 00:48:52]

Oopsies. It's 'Tab'. Fixed.

         case 'U+001B': // Maybe this is remote desktop playing a prank?
           this.hideMenu();
           break;