Settings WebUI: focus on section instead of container

The container was given focus so that after the menu (dialog) closed,
the settings would regain focus and to allow arrow keys to update the
scroll position. This introduced the regression that pressing tab
moves focus to the first control (as well as https://crbug.com/708960
and https://crbug.com/709359 which are fixed in subsequent CLs).
This CL adds a condition that if a section was selected, it will gain
focus, otherwise the container gains focus, as was the preexisting fix
for https://crbug.com/707106 and related changes that closely followed.

Bug: 870732
Change-Id: Ic0ae5b44f2778a56999297018427a9fda9f19eb1
Reviewed-on: https://chromium-review.googlesource.com/c/1284064
Commit-Queue: Esmael El-Moslimany <aee@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602990}

chrome/browser/resources/settings/about_page/about_page.js

@@ -638,4 +638,8 @@ Polymer({
     // </if>
     return this.showUpdateStatus_;
   },
+
+  focusSection: function() {
+    this.$$('settings-section[section="about"]').show();
+  },
 });

chrome/browser/resources/settings/basic_page/basic_page.js

@@ -162,6 +162,12 @@ Polymer({
     return visibility !== false;
   },
 
+  focusSection: function() {
+    const section = this.getSection(settings.getCurrentRoute().section);
+    assert(section);
+    section.show();
+  },
+
   /**
    * Queues a task to search the basic sections, then another for the advanced
    * sections.

chrome/browser/resources/settings/settings_main/settings_main.js

@@ -293,4 +293,10 @@ Polymer({
       }, 0);
     });
   },
+
+  focusSection: function() {
+    this.$$(this.showPages_.settings ? 'settings-basic-page' :
+                                       'settings-about-page')
+        .focusSection();
+  },
 });

chrome/browser/resources/settings/settings_page/main_page_behavior.js

@@ -158,7 +158,7 @@ const MainPageBehaviorImpl = {
         else
           promise = this.expandSection_(currentSection);
       } else if (scrollToSection) {
-        currentSection.scrollIntoView();
+        currentSection.show();
       }
     } else if (
         this.tagName == 'SETTINGS-BASIC-PAGE' && settings.routes.ADVANCED &&

chrome/browser/resources/settings/settings_page/settings_section.html

 <link rel="import" href="chrome://resources/html/polymer.html">
 
 <link rel="import" href="chrome://resources/cr_elements/shared_vars_css.html">
+<link rel="import" href="chrome://resources/html/util.html">
 <link rel="import" href="../animation/animation.html">
 
 <dom-module id="settings-section">
@@ -12,6 +13,7 @@
         /* Margin so the box-shadow isn't clipped during animations. */
         margin-inline-end: 3px;
         margin-inline-start: 3px;
+        outline: none;
         position: relative;
       }
 

chrome/browser/resources/settings/settings_page/settings_section.js

@@ -244,6 +244,15 @@ let SettingsSectionElement = Polymer({
     return animation;
   },
 
+  show: function() {
+    this.setAttribute('tabindex', '-1');
+    this.focus();
+    this.scrollIntoView();
+    listenOnce(this, ['blur', 'pointerdown'], () => {
+      this.removeAttribute('tabindex');
+    });
+  },
+
   /**
    * Helper function to animate the card's position and height.
    * @param {string} position CSS position property.

chrome/browser/resources/settings/settings_ui/settings_ui.html

@@ -4,6 +4,7 @@
 <link rel="import" href="chrome://resources/cr_elements/cr_drawer/cr_drawer.html">
 <link rel="import" href="chrome://resources/cr_elements/cr_toolbar/cr_toolbar.html">
 <link rel="import" href="chrome://resources/cr_elements/icons.html">
+<link rel="import" href="chrome://resources/html/util.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-styles/color.html">
 <link rel="import" href="../find_shortcut_behavior.html">
 <link rel="import" href="../global_scroll_target_behavior.html">
@@ -61,8 +62,8 @@
         role="banner"
         show-menu>
     </cr-toolbar>
-    <cr-drawer id="drawer" on-close="onMenuClosed_"
-        heading="$i18n{settings}" align="$i18n{textdirection}">
+    <cr-drawer id="drawer" on-close="onMenuClose_" heading="$i18n{settings}"
+        align="$i18n{textdirection}">
       <div class="drawer-content">
         <template is="dom-if" id="drawerTemplate">
           <settings-menu page-visibility="[[pageVisibility_]]"

chrome/browser/resources/settings/settings_ui/settings_ui.js

@@ -269,12 +269,11 @@ Polymer({
   },
 
   /**
-   * @param {!Event} event
+   * Called when a section is selected.
    * @private
    */
-  onIronActivate_: function(event) {
-    if (event.detail.item.id != 'advancedSubmenu')
-      this.$.drawer.close();
+  onIronActivate_: function() {
+    this.$.drawer.close();
   },
 
   /** @private */
@@ -282,14 +281,25 @@ Polymer({
     this.$.drawer.toggle();
   },
 
-  /** @private */
-  onMenuClosed_: function() {
-    // Add tab index so that the container can be focused.
-    this.$.container.setAttribute('tabindex', '-1');
-    this.$.container.focus();
-
-    listenOnce(this.$.container, ['blur', 'pointerdown'], () => {
-      this.$.container.removeAttribute('tabindex');
-    });
+  /**
+   * When this is called, The drawer animation is finished, and the dialog no
+   * longer has focus. The selected section will gain focus if one was selected.
+   * Otherwise, the drawer was closed due being canceled, and the main settings
+   * container is given focus. That way the arrow keys can be used to scroll
+   * the container, and pressing tab focuses a component in settings.
+   * @private
+   */
+  onMenuClose_: function() {
+    if (this.$.drawer.wasCanceled()) {
+      // Add tab index so that the container can be focused.
+      this.$.container.setAttribute('tabindex', '-1');
+      this.$.container.focus();
+
+      listenOnce(this.$.container, ['blur', 'pointerdown'], () => {
+        this.$.container.removeAttribute('tabindex');
+      });
+    } else {
+      this.$.main.focusSection();
+    }
   },
 });

chrome/test/data/webui/settings/settings_ui_browsertest.js

@@ -69,7 +69,7 @@ TEST_F('SettingsUIBrowserTest', 'MAYBE_All', function() {
       return whenDone
           .then(function() {
             const whenClosed = test_util.eventToPromise('close', drawer);
-            drawer.close();
+            drawer.cancel();
             return whenClosed;
           })
           .then(() => {