Bug 1500486 - QuantumBar: Handle loading of non-urls entered in the new address bar. r=dao
authorMark Banner <standard8@mozilla.com>
Fri, 21 Dec 2018 16:58:47 +0000
changeset 451691 007bbc7c20efd8495632bcbc05e46d50fbced85f
parent 451690 2f98a1c651b314896f199db5083f01284d89889a
child 451692 72a39ded10f87d08b67b84c45030934510d926fd
push id35251
push userccoroiu@mozilla.com
push dateFri, 21 Dec 2018 21:54:30 +0000
treeherdermozilla-central@74101900e7d4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdao
bugs1500486
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1500486 - QuantumBar: Handle loading of non-urls entered in the new address bar. r=dao Differential Revision: https://phabricator.services.mozilla.com/D10346
browser/components/urlbar/UrlbarInput.jsm
browser/components/urlbar/tests/browser/browser.ini
browser/components/urlbar/tests/browser/browser_UrlbarLoadRace.js
browser/components/urlbar/tests/browser/head.js
--- a/browser/components/urlbar/UrlbarInput.jsm
+++ b/browser/components/urlbar/UrlbarInput.jsm
@@ -229,33 +229,31 @@ class UrlbarInput {
     // BrowserUsageTelemetry.recordUrlbarSelectedResultMethod(
     //   event, this.userSelectionBehavior);
 
     url = url.trim();
 
     try {
       new URL(url);
     } catch (ex) {
-      // TODO: Figure out why we need lastLocationChange here.
-      // let lastLocationChange = browser.lastLocationChange;
-      // UrlbarUtils.getShortcutOrURIAndPostData(text).then(data => {
-      //   if (where != "current" ||
-      //       browser.lastLocationChange == lastLocationChange) {
-      //     params.postData = data.postData;
-      //     params.allowInheritPrincipal = data.mayInheritPrincipal;
-      //     this._loadURL(data.url, browser, where,
-      //                   openUILinkParams);
-      //   }
-      // });
+      let browser = this.window.gBrowser.selectedBrowser;
+      let lastLocationChange = browser.lastLocationChange;
+
+      UrlbarUtils.getShortcutOrURIAndPostData(url).then(data => {
+        if (where != "current" ||
+            browser.lastLocationChange == lastLocationChange) {
+          openParams.postData = data.postData;
+          openParams.allowInheritPrincipal = data.mayInheritPrincipal;
+          this._loadURL(data.url, where, openParams);
+        }
+      });
       return;
     }
 
     this._loadURL(url, where, openParams);
-
-    this.view.close();
   }
 
   /**
    * Called by the view when a result is picked.
    *
    * @param {Event} event The event that picked the result.
    * @param {UrlbarMatch} result The result that was picked.
    */
@@ -555,17 +553,17 @@ class UrlbarInput {
 
     // TODO: These should probably be set by the input field.
     // this.value = url;
     // browser.userTypedValue = url;
     if (this.window.gInitialPages.includes(url)) {
       browser.initialPageLoadedFromURLBar = url;
     }
     try {
-      UrlbarUtils.addToUrlbarHistory(url);
+      UrlbarUtils.addToUrlbarHistory(url, this.window);
     } catch (ex) {
       // Things may go wrong when adding url to session history,
       // but don't let that interfere with the loading of the url.
       Cu.reportError(ex);
     }
 
     params.allowThirdPartyFixup = true;
 
@@ -597,16 +595,18 @@ class UrlbarInput {
         // TODO: Implement handleRevert or equivalent on the input.
         // this.input.handleRevert();
       }
     }
 
     // TODO This should probably be handed via input.
     // Ensure the start of the URL is visible for usability reasons.
     // this.selectionStart = this.selectionEnd = 0;
+
+    this.closePopup();
   }
 
   /**
    * Determines where a URL/page should be opened.
    *
    * @param {Event} event the event triggering the opening.
    * @returns {"current" | "tabshifted" | "tab" | "save" | "window"}
    */
--- a/browser/components/urlbar/tests/browser/browser.ini
+++ b/browser/components/urlbar/tests/browser/browser.ini
@@ -8,8 +8,9 @@ support-files =
 
 [browser_UrlbarInput_formatValue.js]
 [browser_UrlbarInput_overflow.js]
 [browser_UrlbarInput_tooltip.js]
 [browser_UrlbarInput_trimURLs.js]
 subsuite = clipboard
 [browser_UrlbarInput_unit.js]
 support-files = empty.xul
+[browser_UrlbarLoadRace.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/urlbar/tests/browser/browser_UrlbarLoadRace.js
@@ -0,0 +1,72 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+// This test is for testing races of loading the Urlbar when loading shortcuts.
+// For example, ensuring that if a search query is entered, but something causes
+// a page load whilst we're getting the search url, then we don't handle the
+// original search query.
+
+add_task(async function setup() {
+  sandbox = sinon.sandbox.create();
+
+  registerCleanupFunction(async () => {
+    sandbox.restore();
+  });
+});
+
+async function checkShortcutLoading(modifierKeys) {
+  let deferred = PromiseUtils.defer();
+  let tab = await BrowserTestUtils.openNewForegroundTab({
+    gBrowser,
+    opening: "about:robots",
+  });
+
+  // We stub getShortcutOrURIAndPostData so that we can guarentee it doesn't resolve
+  // until after we've loaded a new page.
+  sandbox.stub(UrlbarUtils, "getShortcutOrURIAndPostData").callsFake(() => deferred.promise);
+
+  gURLBar.focus();
+  gURLBar.value = "search";
+  gURLBar.userTypedValue = true;
+  EventUtils.synthesizeKey("KEY_Enter", modifierKeys);
+
+  Assert.ok(UrlbarUtils.getShortcutOrURIAndPostData.calledOnce,
+    "should have called getShortcutOrURIAndPostData");
+
+  BrowserTestUtils.loadURI(tab.linkedBrowser, "about:license");
+  await BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+
+  let openedTab;
+  function listener(event) {
+    openedTab = event.target;
+  }
+  window.addEventListener("TabOpen", listener);
+
+  deferred.resolve({
+    url: "https://example.com/1/",
+    postData: {},
+    mayInheritPrincipal: true,
+  });
+
+  await deferred.promise;
+
+  if (modifierKeys) {
+    Assert.ok(openedTab, "Should have attempted to open the shortcut page");
+    BrowserTestUtils.removeTab(openedTab);
+  } else {
+    Assert.ok(!openedTab, "Should have not attempted to open the shortcut page");
+  }
+
+  window.removeEventListener("TabOpen", listener);
+  BrowserTestUtils.removeTab(tab);
+  sandbox.restore();
+}
+
+add_task(async function test_location_change_stops_load() {
+  await checkShortcutLoading();
+});
+
+add_task(async function test_opening_different_tab_with_location_change() {
+  await checkShortcutLoading({altKey: true});
+});
--- a/browser/components/urlbar/tests/browser/head.js
+++ b/browser/components/urlbar/tests/browser/head.js
@@ -6,16 +6,17 @@
  */
 
 "use strict";
 
 let sandbox;
 
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 XPCOMUtils.defineLazyModuleGetters(this, {
+  PromiseUtils: "resource://gre/modules/PromiseUtils.jsm",
   Services: "resource://gre/modules/Services.jsm",
   QueryContext: "resource:///modules/UrlbarUtils.jsm",
   UrlbarController: "resource:///modules/UrlbarController.jsm",
   UrlbarMatch: "resource:///modules/UrlbarMatch.jsm",
   UrlbarUtils: "resource:///modules/UrlbarUtils.jsm",
 });
 
 /* global sinon */