Backed out 3 changesets (bug 1636583) for Browser-chrome failures in browser/components/urlbar/tests/browser/browser_handleCommand_fallback.js. CLOSED TREE

Backed out changeset a2e636ff03c2 (bug 1636583)
Backed out changeset 660b7de89215 (bug 1636583)
Backed out changeset 9fd142817919 (bug 1636583)

browser/components/urlbar/UrlbarInput.jsm

@@ -476,47 +476,36 @@ class UrlbarInput {
       selType,
     });
 
-    let isValidUrl = false;
     try {
       new URL(url);
-      isValidUrl = true;
-    } catch (ex) {}
-    if (isValidUrl) {
-      this._loadURL(url, where, openParams);
+    } catch (ex) {
+      // This is not a URL, so it must be a search or a keyword.
+
+      // TODO (Bug 1604927): If the urlbar results are restricted to a specific
+      // engine, here we must search with that specific engine; indeed the
+      // docshell wouldn't know about our engine restriction.
+      // Also remember to invoke this._recordSearch, after replacing url with
+      // the appropriate engine submission url.
+
+      let browser = this.window.gBrowser.selectedBrowser;
+      let lastLocationChange = browser.lastLocationChange;
+      UrlbarUtils.getShortcutOrURIAndPostData(url).then(data => {
+        // Because this happens asynchronously, we must verify that the browser
+        // location did not change in the meanwhile.
+        if (
+          where != "current" ||
+          browser.lastLocationChange == lastLocationChange
+        ) {
+          openParams.postData = data.postData;
+          openParams.allowInheritPrincipal = data.mayInheritPrincipal;
+          this._loadURL(data.url, where, openParams, null, browser);
+        }
+      });
+      // Bail out, because we will handle the _loadURL call asynchronously.
       return;
     }
 
-    // This is not a URL and there's no selected element, because likely the
-    // view is closed, or paste&go was used.
-    // We must act consistently here, having or not an open view should not
-    // make a difference if the search string is the same.
-
-    // If we have a result for the current value, we can just use it.
-    if (this._resultForCurrentValue) {
-      this.pickResult(this._resultForCurrentValue, event);
-      return;
-    }
-
-    // Otherwise, we must fetch the heuristic result for the current value.
-    // TODO (Bug 1604927): If the urlbar results are restricted to a specific
-    // engine, here we must search with that specific engine; indeed the
-    // docshell wouldn't know about our engine restriction.
-    // Also remember to invoke this._recordSearch, after replacing url with
-    // the appropriate engine submission url.
-    let browser = this.window.gBrowser.selectedBrowser;
-    let lastLocationChange = browser.lastLocationChange;
-    UrlbarUtils.getHeuristicResultFor(url).then(newResult => {
-      // Because this happens asynchronously, we must verify that the browser
-      // location did not change in the meanwhile.
-      if (
-        where != "current" ||
-        browser.lastLocationChange == lastLocationChange
-      ) {
-        this.pickResult(newResult, event, null, browser);
-      }
-    });
-    // Don't add further handling here, the getHeuristicResultFor call above is
-    // our last resort.
+    this._loadURL(url, where, openParams);
   }
 
   handleRevert() {
@@ -528,34 +517,17 @@ class UrlbarInput {
   }
 
   /**
-   * Called when an element of the view is picked.
+   * Called by the view when an element is picked.
    *
    * @param {Element} element The element that was picked.
    * @param {Event} event The event that picked the element.
    */
   pickElement(element, event) {
+    let originalUntrimmedValue = this.untrimmedValue;
     let result = this.view.getResultFromElement(element);
     if (!result) {
       return;
     }
-    this.pickResult(result, event, element);
-  }
-
-  /**
-   * Called when a result is picked.
-   *
-   * @param {UrlbarResult} result The result that was picked.
-   * @param {Event} event The event that picked the result.
-   * @param {DOMElement} element the picked view element, if available.
-   * @param {object} browser The browser to use for the load.
-   */
-  pickResult(
-    result,
-    event,
-    element = null,
-    browser = this.window.gBrowser.selectedBrowser
-  ) {
-    let originalUntrimmedValue = this.untrimmedValue;
     let isCanonized = this.setValueFromResult(result, event);
     let where = this._whereToOpen(event);
     let openParams = {
@@ -575,7 +547,7 @@ class UrlbarInput {
         selIndex,
         selType: "canonized",
       });
-      this._loadURL(this.value, where, openParams, browser);
+      this._loadURL(this.value, where, openParams);
       return;
     }
 
@@ -807,16 +779,10 @@ class UrlbarInput {
       selType: this.controller.engagementEvent.typeFromElement(element),
     });
 
-    this._loadURL(
-      url,
-      where,
-      openParams,
-      {
-        source: result.source,
-        type: result.type,
-      },
-      browser
-    );
+    this._loadURL(url, where, openParams, {
+      source: result.source,
+      type: result.type,
+    });
   }
 
   /**
@@ -915,13 +881,13 @@ class UrlbarInput {
 
   /**
    * Invoked by the view when the first result is received.
+   * To prevent selection flickering, we apply autofill on input through a
+   * placeholder, without waiting for results.
+   * But, if the first result is not an autofill one, the autofill prediction
+   * was wrong and we should restore the original user typed string.
    * @param {UrlbarResult} firstResult The first result received.
    */
-  onFirstResult(firstResult) {
-    // To prevent selection flickering, we apply autofill on input through a
-    // placeholder, without waiting for results. But, if the first result is
-    // not an autofill one, the autofill prediction was wrong and we should
-    // restore the original user typed string.
+  maybeClearAutofillPlaceholder(firstResult) {
     if (
       this._autofillPlaceholder &&
       !firstResult.autofill &&
@@ -930,10 +896,6 @@ class UrlbarInput {
     ) {
       this._setValue(this.window.gBrowser.userTypedValue, false);
     }
-
-    if (firstResult.heuristic) {
-      this._resultForCurrentValue = firstResult;
-    }
   }
 
   /**

browser/components/urlbar/UrlbarProvidersManager.jsm

@@ -161,9 +161,9 @@ class ProvidersManager {
   /**
    * Starts querying.
    * @param {object} queryContext The query context object
-   * @param {object} [controller] a UrlbarController instance
+   * @param {object} controller a UrlbarController instance
    */
-  async startQuery(queryContext, controller = null) {
+  async startQuery(queryContext, controller) {
     logger.info(`Query start ${queryContext.searchString}`);
 
     // Define the muxer to use.
@@ -487,9 +487,7 @@ class Query {
       }
     }
 
-    if (this.controller) {
-      this.controller.receiveResults(this.context);
-    }
+    this.controller.receiveResults(this.context);
   }
 }
 

browser/components/urlbar/UrlbarUtils.jsm

@@ -22,13 +22,11 @@ const { XPCOMUtils } = ChromeUtils.import(
 );
 XPCOMUtils.defineLazyModuleGetters(this, {
   BrowserUtils: "resource://gre/modules/BrowserUtils.jsm",
-  BrowserWindowTracker: "resource:///modules/BrowserWindowTracker.jsm",
   PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.jsm",
   PlacesUIUtils: "resource:///modules/PlacesUIUtils.jsm",
   PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",
   Services: "resource://gre/modules/Services.jsm",
   UrlbarPrefs: "resource:///modules/UrlbarPrefs.jsm",
-  UrlbarProvidersManager: "resource:///modules/UrlbarProvidersManager.jsm",
 });
 
 var UrlbarUtils = {
@@ -571,37 +569,6 @@ var UrlbarUtils = {
     let str = value.trim();
     return this.REGEXP_SINGLE_WORD.test(str);
   },
-
-  /**
-   * Runs a search for the given string, and returns the heuristic result.
-   * @param {string} searchString The string to search for.
-   * @param {nsIDOMWindow} window The window requesting it.
-   * @returns {UrlbarResult} an heuristic result.
-   */
-  async getHeuristicResultFor(
-    searchString,
-    window = BrowserWindowTracker.getTopWindow()
-  ) {
-    if (!searchString) {
-      throw new Error("Must pass a non-null search string");
-    }
-    let context = new UrlbarQueryContext({
-      allowAutofill: false,
-      isPrivate: PrivateBrowsingUtils.isWindowPrivate(window),
-      maxResults: 1,
-      searchString,
-      userContextId: window.gBrowser.selectedBrowser.getAttribute(
-        "usercontextid"
-      ),
-      allowSearchSuggestions: false,
-      providers: ["UnifiedComplete"],
-    });
-    await UrlbarProvidersManager.startQuery(context);
-    if (!context.heuristicResult) {
-      throw new Error("There should always be an heuristic result");
-    }
-    return context.heuristicResult;
-  },
 };
 
 XPCOMUtils.defineLazyGetter(UrlbarUtils.ICON, "DEFAULT", () => {

browser/components/urlbar/UrlbarView.jsm

@@ -510,8 +510,9 @@ class UrlbarView {
             trimmedValue.length != 1)
       );
 
-      // Notify the input, so it can make adjustments based on the first result.
-      this.input.onFirstResult(firstResult);
+      // The input field applies autofill on input, without waiting for results.
+      // Once we get results, we can ask it to correct wrong predictions.
+      this.input.maybeClearAutofillPlaceholder(firstResult);
     }
 
     if (

browser/components/urlbar/tests/browser/browser.ini

@@ -62,7 +62,6 @@ skip-if = verify && os == 'linux' # Bug 1581635
 [browser_enter.js]
 [browser_enterAfterMouseOver.js]
 [browser_focusedCmdK.js]
-[browser_handleCommand_fallback.js]
 [browser_hashChangeProxyState.js]
 [browser_heuristicNotAddedFirst.js]
 [browser_ime_composition.js]

browser/components/urlbar/tests/browser/browser_handleCommand_fallback.js

@@ -1,88 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-/**
- * Tests that the fallback paths of handleCommand (no view and no previous
- * result) work consistently against the normal case of picking the heuristic
- * result.
- */
-
-const TEST_STRINGS = [
-  "test",
-  "test/",
-  "test.com",
-  "test.invalid",
-  "moz",
-  "moz test",
-  "@moz test",
-  "keyword",
-  "keyword test",
-  "test/test/",
-  "test /test/",
-];
-
-add_task(async function() {
-  sandbox = sinon.createSandbox();
-  let engine = await Services.search.addEngineWithDetails("MozSearch", {
-    alias: "moz",
-    method: "GET",
-    template: "http://example.com/?q={searchTerms}",
-  });
-  let engine2 = await Services.search.addEngineWithDetails("MozSearch2", {
-    alias: "@moz",
-    method: "GET",
-    template: "http://example.com/?q={searchTerms}",
-  });
-  let bm = await PlacesUtils.bookmarks.insert({
-    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
-    url: "http://example.com/?q=%s",
-    title: "test",
-  });
-  await PlacesUtils.keywords.insert({
-    keyword: "keyword",
-    url: "http://example.com/?q=%s",
-  });
-  registerCleanupFunction(async () => {
-    sandbox.restore();
-    await Services.search.removeEngine(engine);
-    await Services.search.removeEngine(engine2);
-    await PlacesUtils.bookmarks.remove(bm);
-  });
-
-  async function promiseLoadURL() {
-    return new Promise(resolve => {
-      sandbox.stub(gURLBar, "_loadURL").callsFake(function() {
-        sandbox.restore();
-        // The last arguments are optional and apply only to some cases, so we
-        // could not use deepEqual with them.
-        resolve(Array.from(arguments).slice(0, 3));
-      });
-    });
-  }
-
-  // Run the string through a normal search where the user types the string
-  // and confirms the heuristic result, store the arguments to _loadURL, then
-  // confirm the same string without a view and without an input event, and
-  // compare the arguments.
-  for (let value of TEST_STRINGS) {
-    let promise = promiseLoadURL();
-    await UrlbarTestUtils.promiseAutocompleteResultPopup({
-      window,
-      waitForFocus,
-      value,
-    });
-    EventUtils.synthesizeKey("KEY_Enter");
-    let args = await promise;
-    Assert.ok(args.length, "Sanity check");
-    // Close the panel and confirm again.
-    promise = promiseLoadURL();
-    await UrlbarTestUtils.promisePopupClose(window);
-    EventUtils.synthesizeKey("KEY_Enter");
-    Assert.deepEqual(await promise, args, "Check arguments are coherent");
-    // Set the value directly and Enter.
-    promise = promiseLoadURL();
-    gURLBar.value = value;
-    EventUtils.synthesizeKey("KEY_Enter");
-    Assert.deepEqual(await promise, args, "Check arguments are coherent");
-  }
-});

browser/components/urlbar/tests/browser/browser_loadRace.js

@@ -22,61 +22,50 @@ async function checkShortcutLoading(modifierKeys) {
     opening: "about:robots",
   });
 
-  // We stub getHeuristicResultFor to guarentee it doesn't resolve until after
-  // we've loaded a new page.
-  let original = UrlbarUtils.getHeuristicResultFor;
+  // We stub getShortcutOrURIAndPostData so that we can guarentee it doesn't resolve
+  // until after we've loaded a new page.
   sandbox
-    .stub(UrlbarUtils, "getHeuristicResultFor")
-    .callsFake(async searchString => {
-      await deferred.promise;
-      return original.call(this, searchString);
-    });
+    .stub(UrlbarUtils, "getShortcutOrURIAndPostData")
+    .callsFake(() => deferred.promise);
 
-  // This load will be blocked until the deferred is resolved.
-  // Use a string that will be interepreted as a local URL to avoid hitting the
-  // network.
   gURLBar.focus();
-  gURLBar.value = "example.com";
+  gURLBar.value = "search";
   gURLBar.userTypedValue = true;
   EventUtils.synthesizeKey("KEY_Enter", modifierKeys);
 
   Assert.ok(
-    UrlbarUtils.getHeuristicResultFor.calledOnce,
-    "should have called getHeuristicResultFor"
+    UrlbarUtils.getShortcutOrURIAndPostData.calledOnce,
+    "should have called getShortcutOrURIAndPostData"
   );
 
-  // Now load a different page.
   BrowserTestUtils.loadURI(tab.linkedBrowser, "about:license");
   await BrowserTestUtils.browserLoaded(tab.linkedBrowser);
-  Assert.equal(gBrowser.visibleTabs.length, 2, "Should have 2 tabs");
 
-  // Now that the new page has loaded, unblock the previous urlbar load.
-  deferred.resolve();
+  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) {
-    let openedTab = await new Promise(resolve => {
-      window.addEventListener(
-        "TabOpen",
-        event => {
-          resolve(event.target);
-        },
-        { once: true }
-      );
-    });
-    await BrowserTestUtils.browserLoaded(openedTab.linkedBrowser);
-    Assert.ok(
-      openedTab.linkedBrowser.currentURI.spec.includes("example.com"),
-      "Should have attempted to open the shortcut page"
-    );
+    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"
+    );
   }
 
-  Assert.equal(
-    tab.linkedBrowser.currentURI.spec,
-    "about:license",
-    "Tab url should not have changed"
-  );
-  Assert.equal(gBrowser.visibleTabs.length, 2, "Should still have 2 tabs");
-
+  window.removeEventListener("TabOpen", listener);
   BrowserTestUtils.removeTab(tab);
   sandbox.restore();
 }

docshell/base/URIFixup.jsm

@@ -14,7 +14,7 @@
 
 // getFixupURIInfo has a complex logic, that likely could be simplified, but
 // the risk of regressions is high, thus that should be done with care.
-/* eslint complexity: ["error", 40] */
+/* eslint complexity: ["error", 39] */
 
 var EXPORTED_SYMBOLS = ["URIFixup", "URIFixupInfo"];
 
@@ -389,11 +389,6 @@ URIFixup.prototype = {
         info.fixupChangedProtocol = true;
         maybeSetAlternateFixedURI(info, fixupFlags);
         info.preferredURI = info.fixedURI;
-        // Check if it's a forced visit. The user can enforce a visit by
-        // appending a slash, but the string must be in a valid uri format.
-        if (uriString.endsWith("/")) {
-          return info;
-        }
       }
     }
 

docshell/test/browser/browser_search_notification.js

@@ -2,13 +2,6 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 add_task(async function() {
-  // Our search would be handled by the urlbar normally and not by the docshell,
-  // thus we must force going through dns first, so that the urlbar thinks
-  // the value may be a url, and asks the docshell to visit it.
-  // On NS_ERROR_UNKNOWN_HOST the docshell will fix it up.
-  await SpecialPowers.pushPrefEnv({
-    set: [["browser.fixup.dns_first_for_single_words", true]],
-  });
   const kSearchEngineID = "test_urifixup_search_engine";
   const kSearchEngineURL = "http://localhost/?search={searchTerms}";
   await Services.search.addEngineWithDetails(kSearchEngineID, {
@@ -41,7 +34,7 @@ add_task(async function() {
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser);
   gBrowser.selectedTab = tab;
 
-  gURLBar.value = "firefox";
+  gURLBar.value = "firefox health report";
   gURLBar.handleCommand();
 
   let [subject, data] = await TestUtils.topicObserved("keyword-search");
@@ -49,7 +42,11 @@ add_task(async function() {
   let engine = Services.search.defaultEngine;
   Assert.ok(engine, "Have default search engine.");
   Assert.equal(engine, subject, "Notification subject is engine.");
-  Assert.equal(data, "firefox", "Notification data is search term.");
+  Assert.equal(
+    data,
+    "firefox health report",
+    "Notification data is search term."
+  );
 
   gBrowser.removeTab(tab);
 });

docshell/test/unit/test_URIFixup_info.js

@@ -411,7 +411,7 @@ var testcases = [
     protocolChange: true,
   },
   {
-    input: "mozilla.nonexistent",
+    input: "mozilla.nonexistent/",
     fixedURI: "http://mozilla.nonexistent/",
     alternateURI: "http://www.mozilla.nonexistent/",
     keywordLookup: true,
@@ -603,22 +603,6 @@ var testcases = [
     input: " moz\ti\tlla.org ",
     keywordLookup: true,
   },
-  {
-    input: "mozilla/",
-    fixedURI: "http://mozilla/",
-    alternateURI: "http://www.mozilla.com/",
-    protocolChange: true,
-  },
-  {
-    input: "mozilla/ test /",
-    fixedURI: "http://mozilla/%20test%20/",
-    alternateURI: "http://www.mozilla.com/%20test%20/",
-    protocolChange: true,
-  },
-  {
-    input: "mozilla /test/",
-    keywordLookup: true,
-  },
 ];
 
 if (AppConstants.platform == "win") {