extensions: fix focus management for all dialogs (not just options).

R=kalman@chromium.org
BUG=438301,438777

Review URL: https://codereview.chromium.org/862403006

Cr-Commit-Position: refs/heads/master@{#315126}

chrome/browser/resources/extensions/extension_focus_manager.js

@@ -3,16 +3,16 @@
 // found in the LICENSE file.
 
 cr.define('extensions', function() {
-  var FocusManager = cr.ui.FocusManager;
-
-  function ExtensionFocusManager() {
-    FocusManager.disableMouseFocusOnButtons();
-  }
+  /**
+   * @constructor
+   * @extends {cr.ui.FocusManager}
+   */
+  function ExtensionFocusManager() {}
 
   cr.addSingletonGetter(ExtensionFocusManager);
 
   ExtensionFocusManager.prototype = {
-    __proto__: FocusManager.prototype,
+    __proto__: cr.ui.FocusManager.prototype,
 
     /** @override */
     getFocusParent: function() {

chrome/browser/resources/extensions/extension_list.js

@@ -133,10 +133,8 @@ cr.define('options', function() {
       if (idToOpenOptions && $(idToOpenOptions))
         this.showEmbeddedExtensionOptions_(idToOpenOptions, true);
 
-      if (this.data_.extensions.length == 0)
-        this.classList.add('empty-extension-list');
-      else
-        this.classList.remove('empty-extension-list');
+      var noExtensions = this.data_.extensions.length == 0;
+      this.classList.toggle('empty-extension-list', noExtensions);
     },
 
     /**
@@ -568,36 +566,31 @@ cr.define('options', function() {
       if (scroll)
         this.scrollToNode_(extensionId);
 
-      document.activeElement.blur();
-
       // Add the options query string. Corner case: the 'options' query string
       // will clobber the 'id' query string if the options link is clicked when
       // 'id' is in the URL, or if both query strings are in the URL.
       uber.replaceState({}, '?options=' + extensionId);
 
+      var overlay = extensions.ExtensionOptionsOverlay.getInstance();
       var shownCallback = function() {
+        // This overlay doesn't get focused automatically as <extensionoptions>
+        // is created after the overlay is shown.
         if (cr.ui.FocusOutlineManager.forDocument(document).visible)
           overlay.setInitialFocus();
       };
-
-      var overlay = extensions.ExtensionOptionsOverlay.getInstance();
       overlay.setExtensionAndShowOverlay(extensionId, extension.name,
                                          extension.icon, shownCallback);
       this.optionsShown_ = true;
 
       var self = this;
       $('overlay').addEventListener('cancelOverlay', function f() {
-        // Restore focus instead of just blurring when this page isn't rebuild
-        // crazy. http://crbug.com/450818
-        document.activeElement.blur();
         self.optionsShown_ = false;
         $('overlay').removeEventListener('cancelOverlay', f);
       });
 
-      // TODO(dbeam): guestview's focus is weird. Only when this is called from
-      // within this event handler *and* after the showing animation completes
-      // does this work.
-      shownCallback();
+      // TODO(dbeam): why do we need to focus <extensionoptions> before and
+      // after its showing animation? Makes very little sense to me.
+      overlay.setInitialFocus();
     },
   };
 

chrome/browser/resources/extensions/extension_options_overlay.js

@@ -44,7 +44,10 @@ cr.define('extensions', function() {
       this.getExtensionOptions_().focus();
     },
 
-    /** @return {?Element} */
+    /**
+     * @return {?Element}
+     * @private
+     */
     getExtensionOptions_: function() {
       return $('extension-options-overlay-guest').querySelector(
           'extensionoptions');
@@ -74,8 +77,8 @@ cr.define('extensions', function() {
      * @param {string} extensionName The name of the extension, which is used
      *     as the header of the overlay.
      * @param {string} extensionIcon The URL of the extension's icon.
-     * @param {function():void} shownCallback A function called when show
-     *     animation completes.
+     * @param {function():void} shownCallback A function called when
+     *     showing completes.
      * @suppress {checkTypes}
      * TODO(vitalyp): remove the suppression after adding
      * chrome/renderer/resources/extensions/extension_options.js

chrome/browser/resources/extensions/extensions.html

@@ -32,6 +32,7 @@
 <script src="chrome://resources/js/cr/ui/drag_wrapper.js"></script>
 <script src="chrome://resources/js/cr/ui/focus_manager.js"></script>
 <script src="chrome://resources/js/cr/ui/focus_outline_manager.js"></script>
+<script src="chrome://resources/js/cr/ui/node_utils.js"></script>
 <script src="chrome://resources/js/cr/ui/overlay.js"></script>
 
 <if expr="chromeos">

chrome/browser/resources/extensions/extensions.js

@@ -374,18 +374,13 @@ cr.define('extensions', function() {
   /**
    * Sets the given overlay to show. This hides whatever overlay is currently
    * showing, if any.
-   * @param {HTMLElement} node The overlay page to show. If falsey, all overlays
+   * @param {HTMLElement} node The overlay page to show. If null, all overlays
    *     are hidden.
    */
   ExtensionSettings.showOverlay = function(node) {
     var pageDiv = $('extension-settings');
-    if (node) {
-      pageDiv.style.width = window.getComputedStyle(pageDiv).width;
-      document.body.classList.add('no-scroll');
-    } else {
-      document.body.classList.remove('no-scroll');
-      pageDiv.style.width = '';
-    }
+    pageDiv.style.width = node ? window.getComputedStyle(pageDiv).width : '';
+    document.body.classList.toggle('no-scroll', node);
 
     var currentlyShowingOverlay = ExtensionSettings.getCurrentOverlay();
     if (currentlyShowingOverlay) {
@@ -394,8 +389,11 @@ cr.define('extensions', function() {
     }
 
     if (node) {
-      if (document.activeElement != document.body)
-        document.activeElement.blur();
+      var lastFocused = document.activeElement;
+      $('overlay').addEventListener('cancelOverlay', function f() {
+        lastFocused.focus();
+        $('overlay').removeEventListener('cancelOverlay', f);
+      });
       node.classList.add('showing');
     }
 
@@ -405,10 +403,27 @@ cr.define('extensions', function() {
     }
 
     $('overlay').hidden = !node;
+
+    if (node)
+      ExtensionSettings.focusOverlay();
+
     uber.invokeMethodOnParent(node ? 'beginInterceptingEvents' :
                                      'stopInterceptingEvents');
   };
 
+  ExtensionSettings.focusOverlay = function() {
+    var currentlyShowingOverlay = ExtensionSettings.getCurrentOverlay();
+    assert(currentlyShowingOverlay);
+
+    if (cr.ui.FocusOutlineManager.forDocument(document).visible)
+      cr.ui.setInitialFocus(currentlyShowingOverlay);
+
+    if (!currentlyShowingOverlay.contains(document.activeElement)) {
+      // Make sure focus isn't stuck behind the overlay.
+      document.activeElement.blur();
+    }
+  };
+
   /**
    * Utility function to find the width of various UI strings and synchronize
    * the width of relevant spans. This is crucial for making sure the

ui/webui/resources/js/cr/ui/node_utils.js

@@ -31,3 +31,22 @@ cr.ui.reverseButtonStrips = function(opt_root) {
     buttonStrip.setAttribute('reversed', '');
   }
 };
+
+/**
+ * Finds a good place to set initial focus. Generally called when UI is shown.
+ * @param {!Element} root Where to start looking for focusable controls.
+ */
+cr.ui.setInitialFocus = function(root) {
+  // Do not change focus if any element in |root| is already focused.
+  if (root.contains(document.activeElement))
+    return;
+
+  var elements = root.querySelectorAll('input, list, select, textarea, button');
+  for (var i = 0; i < elements.length; i++) {
+    var element = elements[i];
+    element.focus();
+    // .focus() isn't guaranteed to work. Continue until it does.
+    if (document.activeElement == element)
+      return;
+  }
+};

ui/webui/resources/js/cr/ui/page_manager/page.js

@@ -79,19 +79,7 @@ cr.define('cr.ui.pageManager', function() {
      * strategy.
      */
     focus: function() {
-      // Do not change focus if any control on this page is already focused.
-      if (this.pageDiv.contains(document.activeElement))
-        return;
-
-      var elements = this.pageDiv.querySelectorAll(
-          'input, list, select, textarea, button');
-      for (var i = 0; i < elements.length; i++) {
-        var element = elements[i];
-        // Try to focus. If fails, then continue.
-        element.focus();
-        if (document.activeElement == element)
-          return;
-      }
+      cr.ui.setInitialFocus(this.pageDiv);
     },
 
     /**

ui/webui/resources/js/cr/ui/page_manager/page_manager.js

@@ -3,8 +3,6 @@
 // found in the LICENSE file.
 
 cr.define('cr.ui.pageManager', function() {
-  /** @const */ var FocusOutlineManager = cr.ui.FocusOutlineManager;
-
   /**
    * PageManager contains a list of root Page and overlay Page objects and
    * handles "navigation" by showing and hiding these pages and overlays. On
@@ -56,7 +54,7 @@ cr.define('cr.ui.pageManager', function() {
     initialize: function(defaultPage) {
       this.defaultPage_ = defaultPage;
 
-      FocusOutlineManager.forDocument(document);
+      cr.ui.FocusOutlineManager.forDocument(document);
       document.addEventListener('scroll', this.handleScroll_.bind(this));
 
       // Trigger the scroll handler manually to set the initial state.
@@ -500,11 +498,10 @@ cr.define('cr.ui.pageManager', function() {
       // Change focus to the overlay if any other control was focused by
       // keyboard before. Otherwise, no one should have focus.
       if (document.activeElement != document.body) {
-        if (FocusOutlineManager.forDocument(document).visible) {
+        if (cr.ui.FocusOutlineManager.forDocument(document).visible)
           overlay.focus();
-        } else if (!overlay.pageDiv.contains(document.activeElement)) {
+        if (!overlay.pageDiv.contains(document.activeElement))
           document.activeElement.blur();
-        }
       }
 
       if ($('search-field') && $('search-field').value == '') {