Bug 1241085 - part 3: actually fix about:privatebrowsing clearing the URL bar, r=mconley

MozReview-Commit-ID: JB3GPKsfmTs

--HG--
extra : rebase_source : adf877b86e0a3bc035e3c8bd460b5472997fb7cd

browser/base/content/tabbrowser.xml

@@ -643,8 +643,20 @@
               if (aStateFlags & nsIWebProgressListener.STATE_START &&
                   aStateFlags & nsIWebProgressListener.STATE_IS_NETWORK) {
                 if (aWebProgress.isTopLevel) {
-                  this.mBrowser.urlbarChangeTracker.startedLoad();
-
+                  // Need to use originalLocation rather than location because things
+                  // like about:home and about:privatebrowsing arrive with nsIRequest
+                  // pointing to their resolved jar: or file: URIs.
+                  if (!(originalLocation && gInitialPages.includes(originalLocation.spec) &&
+                        originalLocation != "about:blank" &&
+                        this.mBrowser.currentURI && this.mBrowser.currentURI.spec == "about:blank")) {
+                    // This will trigger clearing the location bar. Don't do it if
+                    // we loaded off a blank browser and this is an initial page load
+                    // (e.g. about:privatebrowsing, about:newtab, etc.) so we avoid
+                    // clearing the location bar in case the user is typing in it.
+                    // loading about:blank shouldn't trigger this, either, because its
+                    // loads are "special".
+                    this.mBrowser.urlbarChangeTracker.startedLoad();
+                  }
                   // If the browser is loading it must not be crashed anymore
                   this.mTab.removeAttribute("crashed");
                 }
@@ -1573,6 +1585,9 @@
             // We'll be creating a new listener, so destroy the old one.
             listener.destroy();
 
+            let oldUserTypedValue = aBrowser.userTypedValue;
+            let hadStartedLoad = aBrowser.didStartLoadSinceLastUserTyping();
+
             // Make sure the browser is destroyed so it unregisters from observer notifications
             aBrowser.destroy();
 
@@ -1585,6 +1600,11 @@
             aBrowser.setAttribute("remote", aShouldBeRemote ? "true" : "false");
             parent.appendChild(aBrowser);
 
+            aBrowser.userTypedValue = oldUserTypedValue;
+            if (hadStartedLoad) {
+              aBrowser.urlbarChangeTracker.startedLoad();
+            }
+
             aBrowser.droppedLinkHandler = droppedLinkHandler;
 
             // Switching a browser's remoteness will create a new frameLoader.

browser/base/content/test/urlbar/browser.ini

@@ -53,6 +53,7 @@ support-files =
 skip-if = os == "linux" # Bug 1073339 - Investigate autocomplete test unreliability on Linux/e10s
 [browser_urlbarHashChangeProxyState.js]
 [browser_urlbarKeepStateAcrossTabSwitches.js]
+[browser_urlbarPrivateBrowsingWindowChange.js]
 [browser_urlbarRevert.js]
 [browser_urlbarSearchSingleWordNotification.js]
 [browser_urlbarSearchSuggestions.js]

browser/base/content/test/urlbar/browser_urlbarPrivateBrowsingWindowChange.js

@@ -0,0 +1,43 @@
+"use strict";
+
+/**
+ * Test that when opening a private browsing window and typing in it before about:privatebrowsing
+ * loads, we don't clear the URL bar.
+ */
+add_task(function*() {
+  let urlbarTestValue = "Mary had a little lamb";
+  let win = OpenBrowserWindow({private: true});
+  let delayedStartupFinished = TestUtils.topicObserved("browser-delayed-startup-finished",
+                                                       subject => subject == win);
+  yield BrowserTestUtils.waitForEvent(win, "load");
+  let urlbar = win.document.getElementById("urlbar");
+  urlbar.value = urlbarTestValue;
+  // Need this so the autocomplete controller attaches:
+  let focusEv = new FocusEvent("focus", {});
+  urlbar.dispatchEvent(focusEv);
+  // And so we know input happened:
+  let inputEv = new InputEvent("input", {data: "", view: win, bubbles: true});
+  urlbar.onInput(inputEv);
+  // Check it worked:
+  is(urlbar.value, urlbarTestValue, "URL bar value should be there");
+  is(win.gBrowser.selectedBrowser.userTypedValue, urlbarTestValue, "browser object should know the url bar value");
+
+  let continueTest;
+  let continuePromise = new Promise(resolve => continueTest = resolve);
+  let wpl = {
+    onLocationChange(aWebProgress, aRequest, aLocation) {
+      if (aLocation && aLocation.spec == "about:privatebrowsing") {
+        continueTest();
+      }
+    },
+  };
+  win.gBrowser.addProgressListener(wpl);
+
+  yield continuePromise;
+  is(urlbar.value, urlbarTestValue,
+     "URL bar value should be the same once about:privatebrowsing has loaded");
+  is(win.gBrowser.selectedBrowser.userTypedValue, urlbarTestValue,
+     "browser object should still know url bar value once about:privatebrowsing has loaded");
+  win.gBrowser.removeProgressListener(wpl);
+  yield BrowserTestUtils.closeWindow(win);
+});

browser/components/sessionstore/SessionStore.jsm

@@ -775,10 +775,12 @@ var SessionStoreInternal = {
         let tabData = TabState.collect(tab);
 
         // wall-paper fix for bug 439675: make sure that the URL to be loaded
-        // is always visible in the address bar
+        // is always visible in the address bar if no other value is present
         let activePageData = tabData.entries[tabData.index - 1] || null;
         let uri = activePageData ? activePageData.url || null : null;
-        browser.userTypedValue = uri;
+        if (!browser.userTypedValue) {
+          browser.userTypedValue = uri;
+        }
 
         // If the page has a title, set it.
         if (activePageData) {
@@ -818,11 +820,8 @@ var SessionStoreInternal = {
           // loading anything. This must happen after the load, as the load will clear
           // userTypedValue.
           let tabData = TabState.collect(tab);
-          if (tabData.userTypedValue && !tabData.userTypedClear) {
+          if (tabData.userTypedValue && !tabData.userTypedClear && !browser.userTypedValue) {
             browser.userTypedValue = tabData.userTypedValue;
-            if (data.didStartLoad) {
-              browser.urlbarChangeTracker.startedLoad();
-            }
             win.URLBarSetURI();
           }