Make MD Settings side bar keyboard accessible.

chrome/browser/resources/settings/settings_menu/settings_menu.js

Index: chrome/browser/resources/settings/settings_menu/settings_menu.js
diff --git a/chrome/browser/resources/settings/settings_menu/settings_menu.js b/chrome/browser/resources/settings/settings_menu/settings_menu.js
index 590b33167d96fedbed6013032674a177c7b1d9bc..a0ff69bc913f1b6f2df1810dc4a7831ee6bf20b9 100644
--- a/chrome/browser/resources/settings/settings_menu/settings_menu.js
+++ b/chrome/browser/resources/settings/settings_menu/settings_menu.js
@@ -9,17 +9,12 @@
 Polymer({
   is: 'settings-menu',
 
-  behaviors: [settings.RouteObserverBehavior],
-
   properties: {
     advancedOpened: {
       type: Boolean,
       notify: true,
     },
 
-    /** @private */
-    aboutSelected_: Boolean,
-
     /**
      * Dictionary defining page visibility.
      * @type {!GuestModePageVisibility}
@@ -29,54 +24,47 @@ Polymer({
     },
   },
 
-  ready: function() {
-    // When a <paper-submenu> is created with the [opened] attribute as true,
-    // its _active member isn't correctly initialized. See this bug for more
-    // info: https://github.com/PolymerElements/paper-menu/issues/88. This means
-    // the first tap to close an opened Advanced section does nothing (because
-    // it calls .open() on an opened menu instead of .close(). This is a fix for
-    // that bug without changing that code through its public API.
-    //
-    // TODO(dbeam): we're currently deciding whether <paper-{,sub}menu> are
-    // right for our needs (there have been minor a11y problems). If we decide
-    // to keep <paper-{,sub}menu>, fix this bug with a local Chrome CL (ex:
-    // https://codereview.chromium.org/2412343004) or a Polymer PR (ex:
-    // https://github.com/PolymerElements/paper-menu/pull/107).
-    if (this.advancedOpened)
-      this.$.advancedSubmenu.open();
+  listeners: {
+    'topMenu.tap': 'onLinkTap_',
+    'subMenu.tap': 'onLinkTap_',
+  },
+
+  /** @override */
+  attached: function() {
+    var currentRoute = settings.getCurrentRoute();
+
+    // Focus the initially selected path.
+    Array.prototype.forEach.call(this.root.querySelectorAll('a'),

[Dan Beam, 2017/02/14 05:44:30]

add this https://gist.github.com/danbeam/e17176126d827a8f52f6457c92207cbe and it
should fix compilation

if it doesn't, change to NodeList.prototype.forEach

you might also have to remove some duplicates (if pending_compiler_externs.js is
actually included other places)

[hcarmona, 2017/02/15 02:10:49]

forEach -> for avoids closure weirdness.

+        function(link) {
+      if (link.getAttribute('href') == currentRoute.path)
+        this.$.topMenu.selected = this.$.subMenu.selected = link.href;

[Dan Beam, 2017/02/14 05:44:30]

nit: why are you using .href here instead of getAttribute('href')?  because it
matches how <iron-selector> works?

either way, maybe make a

setSelectedUrl_: function(url) {
  this.$.topMenu.selected = this.$.subMenu.selected = url;
},

and call from both places?

[hcarmona, 2017/02/15 02:10:49]

Done. Added a comment to explain why href is needed instead of
getAttribute('href')

+    }.bind(this));
   },
 
   /**
-   * @param {!settings.Route} newRoute
+   * Prevent clicks on sidebar items from navigating. These are only links for
+   * accessibility purposes, taps are handled separately by <iron-selector>.
+   * @param {!Event} event
+   * @private
    */
-  currentRouteChanged: function(newRoute) {
-    // Make the three menus mutually exclusive.
-    if (settings.Route.ABOUT.contains(newRoute)) {
-      this.aboutSelected_ = true;
-      this.$.advancedMenu.selected = null;
-      this.$.basicMenu.selected = null;
-    } else if (settings.Route.ADVANCED.contains(newRoute)) {
-      this.aboutSelected_ = false;
-      // For routes from URL entry, we need to set selected.
-      this.$.advancedMenu.selected = newRoute.path;
-      this.$.basicMenu.selected = null;
-    } else if (settings.Route.BASIC.contains(newRoute)) {
-      this.aboutSelected_ = false;
-      this.$.advancedMenu.selected = null;
-      // For routes from URL entry, we need to set selected.
-      this.$.basicMenu.selected = newRoute.path;
-    }
+  onLinkTap_: function(event) {
+    if (event.target.href)

[Dan Beam, 2017/02/14 05:44:31]

nit: this is probably fine but event.target.hasAttribute('href') doesn't
actually run the normalization steps

event.target.{webkitM,m}atches('a[href]') is also fairly explanatory (but does
slightly more)

[hcarmona, 2017/02/15 02:10:49]

Done.

+      event.preventDefault();
   },
 
   /**
    * @param {!Event} event
    * @private
    */
-  openPage_: function(event) {
-    var route = settings.getRouteForPath(event.currentTarget.dataset.path);
-    assert(route, 'settings-menu has an an entry with an invalid path');
+  onSelectorActivate_: function(event) {
+    // Keep both menus in sync.
+    this.$.topMenu.selected = this.$.subMenu.selected = event.detail.selected;
+
+    var path = new URL(event.detail.selected).pathname;
+    var route = settings.getRouteForPath(path);
+    assert(route, 'settings-menu has an entry with an invalid route.');
     settings.navigateTo(
-        route, /* dynamicParams */ null, /* removeSearch */ true);
+       route, /* dynamicParams */ null, /* removeSearch */ true);
   },
 
   /**