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..affd8c1734b4b7603bc1c9d09a26b09b2938c86c 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,57 @@ 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();

[Dan Beam, 2017/02/15 23:14:32]

nit: var currentPath = settings.getCurrentRoute().path;

[hcarmona, 2017/02/15 23:35:31]

Done.

+
+    // Focus the initially selected path.
+    var anchors = this.root.querySelectorAll('a');
+    for (var i = 0; i < anchors.length; ++i) {
+      if (anchors[i].getAttribute('href') == currentRoute.path) {
+        this.setSelectedUrl_(anchors[i].href);
+        break;
+      }
+    }
+  },
+
+  /**
+   * 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
+   */
+  onLinkTap_: function(event) {
+    if (event.target.hasAttribute('href'))
+      event.preventDefault();
   },
 
   /**
-   * @param {!settings.Route} newRoute
+   * Keeps both menus in sync. |url| needs to come from |element.href| because
+   * |iron-list| uses the entire url. Using |getAttribute| will not work.
+   * @param {string} url
    */
-  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;
-    }
+  setSelectedUrl_: function(url) {
+    this.$.topMenu.selected = this.$.subMenu.selected = url;
   },
 
   /**
    * @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) {
+    this.setSelectedUrl_(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);

[dschuyler, 2017/02/15 21:45:23]

It looks like a space got mistakenly removed.

[hcarmona, 2017/02/15 23:35:31]

Good eye. Fixed.

   },
 
   /**