Bug 1399454 - Set initial focus before the first paint. r?florian draft
authorDão Gottwald <dao@mozilla.com>
Wed, 20 Sep 2017 18:46:12 +0200
changeset 667791 6ce61d3f45e1e5d1238ea43be9368eb085393f1b
parent 667790 469eb992a9d166004f2601ce725786f671219054
child 732512 d9546b5c693bb69f540a875624c8388f126bbe08
push id80848
push userdgottwald@mozilla.com
push dateWed, 20 Sep 2017 19:09:33 +0000
reviewersflorian
bugs1399454
milestone57.0a1
Bug 1399454 - Set initial focus before the first paint. r?florian MozReview-Commit-ID: HfFLqU6LcQV
browser/base/content/browser.js
browser/base/content/tabbrowser.xml
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -1391,16 +1391,18 @@ var gBrowserInit = {
 
       try {
         gBrowser.swapBrowsersAndCloseOther(gBrowser.selectedTab, tabToOpen);
       } catch (e) {
         Cu.reportError(e);
       }
     }
 
+    this._setInitialFocus();
+
     // Wait until chrome is painted before executing code not critical to making the window visible
     this._boundDelayedStartup = this._delayedStartup.bind(this);
     window.addEventListener("MozAfterPaint", this._boundDelayedStartup);
 
     this._loadHandled = true;
   },
 
   _cancelDelayedStartup() {
@@ -1598,17 +1600,17 @@ var gBrowserInit = {
       this._schedulePerWindowIdleTasks();
       document.documentElement.setAttribute("sessionrestored", "true");
     });
 
     Services.obs.notifyObservers(window, "browser-delayed-startup-finished");
     TelemetryTimestamps.add("delayedStartupFinished");
   },
 
-  _handleURIToLoad() {
+  _setInitialFocus() {
     let initiallyFocusedElement = document.commandDispatcher.focusedElement;
 
     let firstBrowserPaintDeferred = {};
     firstBrowserPaintDeferred.promise = new Promise(resolve => {
       firstBrowserPaintDeferred.resolve = resolve;
     });
 
     let mm = window.messageManager;
@@ -1620,16 +1622,41 @@ var gBrowserInit = {
     let initialBrowser = gBrowser.selectedBrowser;
     mm.addMessageListener("Browser:FirstNonBlankPaint",
                           function onFirstNonBlankPaint() {
       mm.removeMessageListener("Browser:FirstNonBlankPaint", onFirstNonBlankPaint);
       initialBrowser.removeAttribute("blank");
     });
 
     this._uriToLoadPromise.then(uriToLoad => {
+      if ((isBlankPageURL(uriToLoad) || uriToLoad == "about:privatebrowsing") &&
+          focusAndSelectUrlBar()) {
+        return;
+      }
+
+      if (gBrowser.selectedBrowser.isRemoteBrowser) {
+        // If the initial browser is remote, in order to optimize for first paint,
+        // we'll defer switching focus to that browser until it has painted.
+        firstBrowserPaintDeferred.promise.then(() => {
+          // If focus didn't move while we were waiting for first paint, we're okay
+          // to move to the browser.
+          if (document.commandDispatcher.focusedElement == initiallyFocusedElement) {
+            gBrowser.selectedBrowser.focus();
+          }
+        });
+      } else {
+        // If the initial browser is not remote, we can focus the browser
+        // immediately with no paint performance impact.
+        gBrowser.selectedBrowser.focus();
+      }
+    });
+  },
+
+  _handleURIToLoad() {
+    this._uriToLoadPromise.then(uriToLoad => {
       if (!uriToLoad || uriToLoad == "about:blank") {
         return;
       }
 
       // We don't check if uriToLoad is a XULElement because this case has
       // already been handled before first paint, and the argument cleared.
       if (uriToLoad instanceof Ci.nsIArray) {
         let count = uriToLoad.length;
@@ -1675,39 +1702,16 @@ var gBrowserInit = {
                 window.arguments[7], !!window.arguments[7], window.arguments[8]);
         window.focus();
       } else {
         // Note: loadOneOrMoreURIs *must not* be called if window.arguments.length >= 3.
         // Such callers expect that window.arguments[0] is handled as a single URI.
         loadOneOrMoreURIs(uriToLoad, Services.scriptSecurityManager.getSystemPrincipal());
       }
     });
-
-    this._uriToLoadPromise.then(uriToLoad => {
-      if ((isBlankPageURL(uriToLoad) || uriToLoad == "about:privatebrowsing") &&
-          focusAndSelectUrlBar()) {
-        return;
-      }
-
-      if (gBrowser.selectedBrowser.isRemoteBrowser) {
-        // If the initial browser is remote, in order to optimize for first paint,
-        // we'll defer switching focus to that browser until it has painted.
-        firstBrowserPaintDeferred.promise.then(() => {
-          // If focus didn't move while we were waiting for first paint, we're okay
-          // to move to the browser.
-          if (document.commandDispatcher.focusedElement == initiallyFocusedElement) {
-            gBrowser.selectedBrowser.focus();
-          }
-        });
-      } else {
-        // If the initial browser is not remote, we can focus the browser
-        // immediately with no paint performance impact.
-        gBrowser.selectedBrowser.focus();
-      }
-    });
   },
 
   /**
    * Use this function as an entry point to schedule tasks that
    * need to run once per window after startup, and can be scheduled
    * by using an idle callback.
    *
    * The functions scheduled here will fire from idle callbacks
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -1847,22 +1847,18 @@
               postData: aPostDatas[i],
               userContextId: aUserContextId,
               triggeringPrincipal: aTriggeringPrincipal,
             });
             if (targetTabIndex !== -1)
               this.moveTabTo(tab, ++tabNum);
           }
 
-          if (!aLoadInBackground) {
-            if (firstTabAdded) {
-              // .selectedTab setter focuses the content area
-              this.selectedTab = firstTabAdded;
-            } else
-              this.selectedBrowser.focus();
+          if (firstTabAdded && !aLoadInBackground) {
+            this.selectedTab = firstTabAdded;
           }
         ]]></body>
       </method>
 
       <method name="updateBrowserRemoteness">
         <parameter name="aBrowser"/>
         <parameter name="aShouldBeRemote"/>
         <parameter name="aOptions"/>