Bug 1503338 - Move url loading handling functions from controller to input. r=dao
authorMark Banner <standard8@mozilla.com>
Wed, 14 Nov 2018 11:17:23 +0000
changeset 502987 ba1aae6c2949a2c1b9cbb0e4aad403c5abe2bb69
parent 502986 d76f007bee63b3730068f1a5549bd4d6513593c7
child 502988 72b025a3da8164ed67ff92258a945acbd73bc4d4
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdao
bugs1503338
milestone65.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 1503338 - Move url loading handling functions from controller to input. r=dao Differential Revision: https://phabricator.services.mozilla.com/D11479
browser/components/urlbar/UrlbarController.jsm
browser/components/urlbar/UrlbarInput.jsm
browser/components/urlbar/tests/browser/browser.ini
browser/components/urlbar/tests/browser/browser_UrlbarController_resultOpening.js
--- a/browser/components/urlbar/UrlbarController.jsm
+++ b/browser/components/urlbar/UrlbarController.jsm
@@ -7,17 +7,16 @@
 var EXPORTED_SYMBOLS = ["QueryContext", "UrlbarController"];
 
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 XPCOMUtils.defineLazyModuleGetters(this, {
   AppConstants: "resource://gre/modules/AppConstants.jsm",
   // BrowserUsageTelemetry: "resource:///modules/BrowserUsageTelemetry.jsm",
   UrlbarPrefs: "resource:///modules/UrlbarPrefs.jsm",
   UrlbarProvidersManager: "resource:///modules/UrlbarProvidersManager.jsm",
-  UrlbarUtils: "resource:///modules/UrlbarUtils.jsm",
 });
 
 /**
  * QueryContext defines a user's autocomplete input from within the Address Bar.
  * It supplements it with details of how the search results should be obtained
  * and what they consist of.
  */
 class QueryContext {
@@ -140,110 +139,16 @@ class UrlbarController {
    *
    * @param {QueryContext} queryContext The query details.
    */
   receiveResults(queryContext) {
     this._notify("onQueryResults", queryContext);
   }
 
   /**
-   * Handles the case where a url or other text has been entered into the
-   * urlbar. This will either load the URL, or some text that could be a keyword
-   * or a simple value to load via the default search engine.
-   *
-   * @param {Event} event The event that triggered this.
-   * @param {string} text The text that was entered into the urlbar.
-   * @param {string} [openWhere] Where we expect the result to be opened.
-   * @param {object} [openParams]
-   *   The parameters related to how and where the result will be opened.
-   *   For possible properties @see {_loadURL}
-   */
-  handleEnteredText(event, text, openWhere, openParams = {}) {
-    let where = openWhere || this._whereToOpen(event);
-
-    openParams.postData = null;
-    openParams.allowInheritPrincipal = false;
-
-    // TODO: Work out how we get the user selection behavior, probably via passing
-    // it in, since we don't have the old autocomplete controller to work with.
-    // BrowserUsageTelemetry.recordUrlbarSelectedResultMethod(
-    //   event, this.userSelectionBehavior);
-
-    text = text.trim();
-
-    try {
-      new URL(text);
-    } 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);
-      //   }
-      // });
-      return;
-    }
-
-    this._loadURL(text, where, openParams);
-  }
-
-  /**
-   * Opens a specific result that has been selected.
-   *
-   * @param {Event} event The event that triggered this.
-   * @param {UrlbarMatch} result The result that was selected.
-   * @param {string} [openWhere] Where we expect the result to be opened.
-   * @param {object} [openParams]
-   *   The parameters related to how and where the result will be opened.
-   *   For possible properties @see {_loadURL}
-   */
-  resultSelected(event, result, openWhere, openParams = {}) {
-    // TODO: Work out how we get the user selection behavior, probably via passing
-    // it in, since we don't have the old autocomplete controller to work with.
-    // BrowserUsageTelemetry.recordUrlbarSelectedResultMethod(
-    //   event, this.userSelectionBehavior);
-
-    let where = openWhere || this._whereToOpen(event);
-    openParams.postData = null;
-    openParams.allowInheritPrincipal = false;
-    let url = result.url;
-
-    switch (result.type) {
-      case UrlbarUtils.MATCH_TYPE.TAB_SWITCH: {
-        // TODO: Implement handleRevert or equivalent on the input.
-        // this.input.handleRevert();
-        let prevTab = this.browserWindow.gBrowser.selectedTab;
-        let loadOpts = {
-          adoptIntoActiveWindow: UrlbarPrefs.get("switchTabs.adoptIntoActiveWindow"),
-        };
-
-        if (this.browserWindow.switchToTabHavingURI(url, false, loadOpts) &&
-            prevTab.isEmpty) {
-          this.browserWindow.gBrowser.removeTab(prevTab);
-        }
-        return;
-
-        // TODO: How to handle meta chars?
-        // Once we get here, we got a TAB_SWITCH match but the user
-        // bypassed it by pressing shift/meta/ctrl. Those modifiers
-        // might otherwise affect where we open - we always want to
-        // open in the current tab.
-        // where = "current";
-
-      }
-    }
-
-    this._loadURL(url, where, openParams);
-  }
-
-  /**
    * Adds a listener for query actions and results.
    *
    * @param {object} listener The listener to add.
    * @throws {TypeError} Throws if the listener is not an object.
    */
   addQueryListener(listener) {
     if (!listener || typeof listener != "object") {
       throw new TypeError("Expected listener to be an object");
@@ -278,117 +183,9 @@ class UrlbarController {
     for (let listener of this._listeners) {
       try {
         listener[name](...params);
       } catch (ex) {
         Cu.reportError(ex);
       }
     }
   }
-
-  /**
-   * Loads the url in the appropriate place.
-   *
-   * @param {string} url
-   *   The URL to open.
-   * @param {string} openUILinkWhere
-   *   Where we expect the result to be opened.
-   * @param {object} params
-   *   The parameters related to how and where the result will be opened.
-   *   Further supported paramters are listed in utilityOverlay.js#openUILinkIn.
-   * @param {object} params.triggeringPrincipal
-   *   The principal that the action was triggered from.
-   * @param {nsIInputStream} [params.postData]
-   *   The POST data associated with a search submission.
-   * @param {boolean} [params.allowInheritPrincipal]
-   *   If the principal may be inherited
-   */
-  _loadURL(url, openUILinkWhere, params) {
-    let browser = this.browserWindow.gBrowser.selectedBrowser;
-
-    // TODO: These should probably be set by the input field.
-    // this.value = url;
-    // browser.userTypedValue = url;
-    if (this.browserWindow.gInitialPages.includes(url)) {
-      browser.initialPageLoadedFromURLBar = url;
-    }
-    try {
-      UrlbarUtils.addToUrlbarHistory(url);
-    } 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;
-
-    if (openUILinkWhere == "current") {
-      params.targetBrowser = browser;
-      params.indicateErrorPageLoad = true;
-      params.allowPinnedTabHostChange = true;
-      params.allowPopups = url.startsWith("javascript:");
-    } else {
-      params.initiatingDoc = this.browserWindow.document;
-    }
-
-    // Focus the content area before triggering loads, since if the load
-    // occurs in a new tab, we want focus to be restored to the content
-    // area when the current tab is re-selected.
-    browser.focus();
-
-    if (openUILinkWhere != "current") {
-      // TODO: Implement handleRevert or equivalent on the input.
-      // this.input.handleRevert();
-    }
-
-    try {
-      this.browserWindow.openTrustedLinkIn(url, openUILinkWhere, params);
-    } catch (ex) {
-      // This load can throw an exception in certain cases, which means
-      // we'll want to replace the URL with the loaded URL:
-      if (ex.result != Cr.NS_ERROR_LOAD_SHOWED_ERRORPAGE) {
-        // 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;
-  }
-
-  /**
-   * Determines where a URL/page should be opened.
-   *
-   * @param {Event} event the event triggering the opening.
-   * @returns {"current" | "tabshifted" | "tab" | "save" | "window"}
-   */
-  _whereToOpen(event) {
-    let isMouseEvent = event instanceof MouseEvent;
-    let reuseEmpty = !isMouseEvent;
-    let where = undefined;
-    if (!isMouseEvent && event && event.altKey) {
-      // We support using 'alt' to open in a tab, because ctrl/shift
-      // might be used for canonizing URLs:
-      where = event.shiftKey ? "tabshifted" : "tab";
-    } else if (!isMouseEvent && this._ctrlCanonizesURLs && event && event.ctrlKey) {
-      // If we're allowing canonization, and this is a key event with ctrl
-      // pressed, open in current tab to allow ctrl-enter to canonize URL.
-      where = "current";
-    } else {
-      where = this.browserWindow.whereToOpenLink(event, false, false);
-    }
-    if (this.openInTab) {
-      if (where == "current") {
-        where = "tab";
-      } else if (where == "tab") {
-        where = "current";
-      }
-      reuseEmpty = true;
-    }
-    if (where == "tab" &&
-        reuseEmpty &&
-        this.browserWindow.gBrowser.selectedTab.isEmpty) {
-      where = "current";
-    }
-    return where;
-  }
 }
--- a/browser/components/urlbar/UrlbarInput.jsm
+++ b/browser/components/urlbar/UrlbarInput.jsm
@@ -9,16 +9,17 @@ var EXPORTED_SYMBOLS = ["UrlbarInput"];
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetters(this, {
   PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.jsm",
   QueryContext: "resource:///modules/UrlbarController.jsm",
   Services: "resource://gre/modules/Services.jsm",
   UrlbarController: "resource:///modules/UrlbarController.jsm",
   UrlbarPrefs: "resource:///modules/UrlbarPrefs.jsm",
+  UrlbarUtils: "resource:///modules/UrlbarUtils.jsm",
   UrlbarValueFormatter: "resource:///modules/UrlbarValueFormatter.jsm",
   UrlbarView: "resource:///modules/UrlbarView.jsm",
 });
 
 XPCOMUtils.defineLazyServiceGetter(this, "ClipboardHelper",
                                    "@mozilla.org/widget/clipboardhelper;1",
                                    "nsIClipboardHelper");
 
@@ -213,30 +214,97 @@ class UrlbarInput {
     //   return;
     // }
 
     let url = this.value;
     if (!url) {
       return;
     }
 
-    this.controller.handleEnteredText(event, url);
+    let where = openWhere || this._whereToOpen(event);
+
+    openParams.postData = null;
+    openParams.allowInheritPrincipal = false;
+
+    // TODO: Work out how we get the user selection behavior, probably via passing
+    // it in, since we don't have the old autocomplete controller to work with.
+    // 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);
+      //   }
+      // });
+      return;
+    }
+
+    this._loadURL(url, where, openParams);
 
     this.view.close();
   }
 
   /**
    * Called by the view when a result is selected.
    *
    * @param {Event} event The event that selected the result.
    * @param {UrlbarMatch} result The result that was selected.
    */
   resultSelected(event, result) {
     this.setValueFromResult(result);
-    this.controller.resultSelected(event, result);
+
+    // TODO: Work out how we get the user selection behavior, probably via passing
+    // it in, since we don't have the old autocomplete controller to work with.
+    // BrowserUsageTelemetry.recordUrlbarSelectedResultMethod(
+    //   event, this.userSelectionBehavior);
+
+    let where = this._whereToOpen(event);
+    let openParams = {
+      postData: null,
+      allowInheritPrincipal: false,
+    };
+    let url = result.url;
+
+    switch (result.type) {
+      case UrlbarUtils.MATCH_TYPE.TAB_SWITCH: {
+        // TODO: Implement handleRevert or equivalent on the input.
+        // this.input.handleRevert();
+        let prevTab = this.browserWindow.gBrowser.selectedTab;
+        let loadOpts = {
+          adoptIntoActiveWindow: UrlbarPrefs.get("switchTabs.adoptIntoActiveWindow"),
+        };
+
+        if (this.browserWindow.switchToTabHavingURI(url, false, loadOpts) &&
+            prevTab.isEmpty) {
+          this.browserWindow.gBrowser.removeTab(prevTab);
+        }
+        return;
+
+        // TODO: How to handle meta chars?
+        // Once we get here, we got a TAB_SWITCH match but the user
+        // bypassed it by pressing shift/meta/ctrl. Those modifiers
+        // might otherwise affect where we open - we always want to
+        // open in the current tab.
+        // where = "current";
+
+      }
+    }
+
+    this._loadURL(url, where, openParams);
   }
 
   /**
    * Called by the view when moving through results with the keyboard.
    *
    * @param {UrlbarMatch} result The result that was selected.
    */
   setValueFromResult(result) {
@@ -420,16 +488,124 @@ class UrlbarInput {
       } catch (e) {
         action.params.displayUrl = action.params.url;
       }
     }
 
     return action;
   }
 
+  /**
+   * Loads the url in the appropriate place.
+   *
+   * @param {string} url
+   *   The URL to open.
+   * @param {string} openUILinkWhere
+   *   Where we expect the result to be opened.
+   * @param {object} params
+   *   The parameters related to how and where the result will be opened.
+   *   Further supported paramters are listed in utilityOverlay.js#openUILinkIn.
+   * @param {object} params.triggeringPrincipal
+   *   The principal that the action was triggered from.
+   * @param {nsIInputStream} [params.postData]
+   *   The POST data associated with a search submission.
+   * @param {boolean} [params.allowInheritPrincipal]
+   *   If the principal may be inherited
+   */
+  _loadURL(url, openUILinkWhere, params) {
+    let browser = this.browserWindow.gBrowser.selectedBrowser;
+
+    // TODO: These should probably be set by the input field.
+    // this.value = url;
+    // browser.userTypedValue = url;
+    if (this.browserWindow.gInitialPages.includes(url)) {
+      browser.initialPageLoadedFromURLBar = url;
+    }
+    try {
+      UrlbarUtils.addToUrlbarHistory(url);
+    } 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;
+
+    if (openUILinkWhere == "current") {
+      params.targetBrowser = browser;
+      params.indicateErrorPageLoad = true;
+      params.allowPinnedTabHostChange = true;
+      params.allowPopups = url.startsWith("javascript:");
+    } else {
+      params.initiatingDoc = this.browserWindow.document;
+    }
+
+    // Focus the content area before triggering loads, since if the load
+    // occurs in a new tab, we want focus to be restored to the content
+    // area when the current tab is re-selected.
+    browser.focus();
+
+    if (openUILinkWhere != "current") {
+      // TODO: Implement handleRevert or equivalent on the input.
+      // this.input.handleRevert();
+    }
+
+    try {
+      this.browserWindow.openTrustedLinkIn(url, openUILinkWhere, params);
+    } catch (ex) {
+      // This load can throw an exception in certain cases, which means
+      // we'll want to replace the URL with the loaded URL:
+      if (ex.result != Cr.NS_ERROR_LOAD_SHOWED_ERRORPAGE) {
+        // 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;
+  }
+
+  /**
+   * Determines where a URL/page should be opened.
+   *
+   * @param {Event} event the event triggering the opening.
+   * @returns {"current" | "tabshifted" | "tab" | "save" | "window"}
+   */
+  _whereToOpen(event) {
+    let isMouseEvent = event instanceof MouseEvent;
+    let reuseEmpty = !isMouseEvent;
+    let where = undefined;
+    if (!isMouseEvent && event && event.altKey) {
+      // We support using 'alt' to open in a tab, because ctrl/shift
+      // might be used for canonizing URLs:
+      where = event.shiftKey ? "tabshifted" : "tab";
+    } else if (!isMouseEvent && this._ctrlCanonizesURLs && event && event.ctrlKey) {
+      // If we're allowing canonization, and this is a key event with ctrl
+      // pressed, open in current tab to allow ctrl-enter to canonize URL.
+      where = "current";
+    } else {
+      where = this.browserWindow.whereToOpenLink(event, false, false);
+    }
+    if (this.openInTab) {
+      if (where == "current") {
+        where = "tab";
+      } else if (where == "tab") {
+        where = "current";
+      }
+      reuseEmpty = true;
+    }
+    if (where == "tab" &&
+        reuseEmpty &&
+        this.browserWindow.gBrowser.selectedTab.isEmpty) {
+      where = "current";
+    }
+    return where;
+  }
+
   // Event handlers below.
 
   _on_blur(event) {
     this.formatValue();
   }
 
   _on_focus(event) {
     this._updateUrlTooltip();
--- a/browser/components/urlbar/tests/browser/browser.ini
+++ b/browser/components/urlbar/tests/browser/browser.ini
@@ -1,16 +1,15 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 [DEFAULT]
 support-files =
   head.js
 
-[browser_UrlbarController_resultOpening.js]
 [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
deleted file mode 100644
--- a/browser/components/urlbar/tests/browser/browser_UrlbarController_resultOpening.js
+++ /dev/null
@@ -1,84 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-/**
- * These tests unit test the result/url loading functionality of UrlbarController.
- */
-
-"use strict";
-
-let controller;
-
-add_task(async function setup() {
-  sandbox = sinon.sandbox.create();
-
-  controller = new UrlbarController({
-    browserWindow: window,
-  });
-
-  registerCleanupFunction(async () => {
-    sandbox.restore();
-  });
-});
-
-add_task(function test_handleEnteredText_url() {
-  sandbox.stub(window, "openTrustedLinkIn");
-
-  const event = new KeyboardEvent("keyup", {key: "Enter"});
-  // Additional spaces in the url to check that we trim correctly.
-  controller.handleEnteredText(event, " https://example.com ");
-
-  Assert.ok(window.openTrustedLinkIn.calledOnce,
-    "Should have triggered opening the url.");
-
-  let args = window.openTrustedLinkIn.args[0];
-
-  Assert.equal(args[0], "https://example.com",
-    "Should have triggered opening with the correct url");
-  Assert.equal(args[1], "current",
-    "Should be opening in the current browser");
-  Assert.deepEqual(args[2], {
-    allowInheritPrincipal: false,
-    allowPinnedTabHostChange: true,
-    allowPopups: false,
-    allowThirdPartyFixup: true,
-    indicateErrorPageLoad: true,
-    postData: null,
-    targetBrowser: gBrowser.selectedBrowser,
-  }, "Should have the correct additional parameters for opening");
-
-  sandbox.restore();
-});
-
-add_task(function test_resultSelected_switchtab() {
-  sandbox.stub(window, "switchToTabHavingURI").returns(true);
-  sandbox.stub(window.gBrowser.selectedTab, "isEmpty").returns(false);
-  sandbox.stub(window.gBrowser, "removeTab");
-
-  const event = new MouseEvent("click", {button: 0});
-  const url = "https://example.com/1";
-  const result = new UrlbarMatch(UrlbarUtils.MATCH_TYPE.TAB_SWITCH,
-                                 UrlbarUtils.MATCH_SOURCE.TABS,
-                                 { url });
-
-  Assert.equal(gURLBar.value, "", "urlbar input is empty before selecting a result");
-  if (Services.prefs.getBoolPref("browser.urlbar.quantumbar", true)) {
-    gURLBar.resultSelected(event, result);
-    Assert.equal(gURLBar.value, url, "urlbar value updated for selected result");
-  } else {
-    controller.resultSelected(event, result);
-  }
-
-  Assert.ok(window.switchToTabHavingURI.calledOnce,
-    "Should have triggered switching to the tab");
-
-  let args = window.switchToTabHavingURI.args[0];
-
-  Assert.equal(args[0], url, "Should have passed the expected url");
-  Assert.ok(!args[1], "Should not attempt to open a new tab");
-  Assert.deepEqual(args[2], {
-    adoptIntoActiveWindow: UrlbarPrefs.get("switchTabs.adoptIntoActiveWindow"),
-  }, "Should have the correct additional parameters for opening");
-
-  sandbox.restore();
-});