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

chrome/browser/resources/extensions/extension_options_overlay.js

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 cr.define('extensions', function() {
   'use strict';
 
   /**
    * The ExtensionOptionsOverlay will show an extension's options page using
    * an <extensionoptions> element.
@@ skipping 26 matching lines @@
       cr.ui.overlay.globalInitialization();
       overlay.addEventListener('cancelOverlay', this.handleDismiss_.bind(this));
 
       this.showOverlay_ = showOverlay;
     },
 
     setInitialFocus: function() {
       this.getExtensionOptions_().focus();
     },
 
-    /** @return {?Element} */
+    /**
+     * @return {?Element}
+     * @private
+     */
     getExtensionOptions_: function() {
       return $('extension-options-overlay-guest').querySelector(
           'extensionoptions');
     },
 
     /**
      * Handles a click on the close button.
      * @param {Event} event The click event.
      * @private
      */
     handleDismiss_: function(event) {
       this.setVisible_(false);
       var extensionoptions = this.getExtensionOptions_();
       if (extensionoptions)
         $('extension-options-overlay-guest').removeChild(extensionoptions);
 
       $('extension-options-overlay-icon').removeAttribute('src');
 
       // Remove the options query string.
       uber.replaceState({}, '');
     },
 
     /**
      * Associate an extension with the overlay and display it.
      * @param {string} extensionId The id of the extension whose options page
      *     should be displayed in the overlay.
      * @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
      * to dependencies.
      */
     setExtensionAndShowOverlay: function(extensionId,
                                          extensionName,
                                          extensionIcon,
                                          shownCallback) {
       var overlay = $('extension-options-overlay');
       var overlayHeader = $('extension-options-overlay-header');
       var overlayGuest = $('extension-options-overlay-guest');
       var overlayStyle = window.getComputedStyle(overlay);
 
       $('extension-options-overlay-title').textContent = extensionName;
       $('extension-options-overlay-icon').src = extensionIcon;
 
-      this.setVisible_(true);
-
       var extensionoptions = new window.ExtensionOptions();
       extensionoptions.extension = extensionId;
 
       // The <extensionoptions> content's size needs to be restricted to the
       // bounds of the overlay window. The overlay gives a minWidth and
       // maxHeight, but the maxHeight does not include our header height (title
       // and close button), so we need to subtract that to get the maxHeight
       // for the extension options.
       var maxHeight = parseInt(overlay.style.maxHeight, 10) -
                       overlayHeader.offsetHeight;
@@ skipping 63 matching lines @@
       // Begin rendering at the default dimensions. This is also necessary to
       // cancel any width/height set on a previous render.
       // TODO(kalman): This causes a visual jag where the overlay guest shows
       // up briefly. It would be better to render this off-screen first, then
       // swap it in place. See crbug.com/408274.
       // This may also solve crbug.com/431001 (width is 0 on initial render).
       overlayGuest.style.width = '';
       overlayGuest.style.height = '';
 
       overlayGuest.appendChild(extensionoptions);
+
+      // Show the dialog after extensionoptions is in the DOM so it gets focus.
+      this.setVisible_(true);

[not at google - send to devlin, 2015/02/06 21:31:38]

Unfortunately showing the overlay early serves a purpose - the extensionoptions
element grows to fit the width of the dialogue, and then the preferred size
logic kicks in, and we resize the dialogue accordingly. Importantly, the width
starts off at the width of the dialogue.

Showing it late means that on construction the width is 0 so the initial
preferred size rendering is like:

This
Is
An
Options
Page

rather than:

This Is An
Options Page

... or whatever. This is similar to crbug.com/431001. The point is, it has a
huge height, so the overlay sizes itself large, but never shrinks. You end up
getting a really tall overlay with a lot of whitespace at the bottom.

If you want to test this, grab
https://github.com/kalman/chrome-extensions/tree/master/browser-clock and change
the manifest from {"options_page": "options.html"} to {"options_ui": {"page":
"options.html"}}.

The real way to fix all of this - which would also address crbug.com/408274 I
think - is to attach the ExtensionOptions element off-screen to something, wait
for it to render (pulling the initial width from somewhere, wherever the
dialogue gets it from) detatch() it (not available yet, see
https://codereview.chromium.org/867393002/), then re-attach it, then set the
dialogue to visible.

I understand if you don't want to do this now.

[Dan Beam, 2015/02/06 21:50:02]

i don't ;), moved back

     },
 
     /**
      * Toggles the visibility of the ExtensionOptionsOverlay.
      * @param {boolean} isVisible Whether the overlay should be visible.
      * @private
      */
     setVisible_: function(isVisible) {
       this.showOverlay_(isVisible ?
           /** @type {HTMLDivElement} */($('extension-options-overlay')) :
           null);
     }
   };
 
   // Export
   return {
     ExtensionOptionsOverlay: ExtensionOptionsOverlay
   };
 });