Bug 1502039 - Move addToUrlbarHistory, getShortcutOrURIAndPostData and getPostDataStream to UrlbarUtils. r=mak
authorMark Banner <standard8@mozilla.com>
Thu, 25 Oct 2018 14:35:32 +0000
changeset 491346 7ba779ebed434ca3c839e152c2c2b6efb7a45b6a
parent 491345 976ebd2f5e68831956c355309ed8eec52a0e7fa1
child 491347 bd3f7151c697162af1a815a6b8cea81a94e45279
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersmak
bugs1502039
milestone65.0a1
Bug 1502039 - Move addToUrlbarHistory, getShortcutOrURIAndPostData and getPostDataStream to UrlbarUtils. r=mak Differential Revision: https://phabricator.services.mozilla.com/D9783
browser/base/content/browser.js
browser/base/content/test/general/browser.ini
browser/base/content/test/general/browser_contentAreaClick.js
browser/base/content/test/general/browser_getshortcutoruri.js
browser/base/content/urlbarBindings.xml
browser/components/places/tests/browser/browser_bookmarkProperties_addKeywordForThisSearch.js
browser/components/urlbar/UrlbarController.jsm
browser/components/urlbar/UrlbarUtils.jsm
browser/components/urlbar/tests/unit/test_UrlbarUtils_addToUrlbarHistory.js
browser/components/urlbar/tests/unit/test_UrlbarUtils_getShortcutOrURIAndPostData.js
browser/components/urlbar/tests/unit/xpcshell.ini
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -65,16 +65,17 @@ XPCOMUtils.defineLazyModuleGetters(this,
   TabCrashHandler: "resource:///modules/ContentCrashHandlers.jsm",
   TelemetryEnvironment: "resource://gre/modules/TelemetryEnvironment.jsm",
   TelemetryStopwatch: "resource://gre/modules/TelemetryStopwatch.jsm",
   Translation: "resource:///modules/translation/Translation.jsm",
   UITour: "resource:///modules/UITour.jsm",
   UpdateUtils: "resource://gre/modules/UpdateUtils.jsm",
   UrlbarInput: "resource:///modules/UrlbarInput.jsm",
   UrlbarPrefs: "resource:///modules/UrlbarPrefs.jsm",
+  UrlbarUtils: "resource:///modules/UrlbarUtils.jsm",
   UrlbarValueFormatter: "resource:///modules/UrlbarValueFormatter.jsm",
   Utils: "resource://gre/modules/sessionstore/Utils.jsm",
   Weave: "resource://services-sync/main.js",
   WebNavigationFrames: "resource://gre/modules/WebNavigationFrames.jsm",
   fxAccounts: "resource://gre/modules/FxAccounts.jsm",
   webrtcUI: "resource:///modules/webrtcUI.jsm",
   ZoomUI: "resource:///modules/ZoomUI.jsm",
 });
@@ -2460,90 +2461,16 @@ function loadURI(uri, referrer, postData
                  forceAboutBlankViewerInCurrent,
                  allowInheritPrincipal,
                });
   } catch (e) {
     Cu.reportError(e);
   }
 }
 
-/**
- * Given a string, will generate a more appropriate urlbar value if a Places
- * keyword or a search alias is found at the beginning of it.
- *
- * @param url
- *        A string that may begin with a keyword or an alias.
- *
- * @return {Promise}
- * @resolves { url, postData, mayInheritPrincipal }. If it's not possible
- *           to discern a keyword or an alias, url will be the input string.
- */
-async function getShortcutOrURIAndPostData(url) {
-  let mayInheritPrincipal = false;
-  let postData = null;
-  // Split on the first whitespace.
-  let [keyword, param = ""] = url.trim().split(/\s(.+)/, 2);
-
-  if (!keyword) {
-    return { url, postData, mayInheritPrincipal };
-  }
-
-  let engine = Services.search.getEngineByAlias(keyword);
-  if (engine) {
-    let submission = engine.getSubmission(param, null, "keyword");
-    return { url: submission.uri.spec,
-             postData: submission.postData,
-             mayInheritPrincipal };
-  }
-
-  // A corrupt Places database could make this throw, breaking navigation
-  // from the location bar.
-  let entry = null;
-  try {
-    entry = await PlacesUtils.keywords.fetch(keyword);
-  } catch (ex) {
-    Cu.reportError(`Unable to fetch Places keyword "${keyword}": ${ex}`);
-  }
-  if (!entry || !entry.url) {
-    // This is not a Places keyword.
-    return { url, postData, mayInheritPrincipal };
-  }
-
-  try {
-    [url, postData] =
-      await BrowserUtils.parseUrlAndPostData(entry.url.href,
-                                             entry.postData,
-                                             param);
-    if (postData) {
-      postData = getPostDataStream(postData);
-    }
-
-    // Since this URL came from a bookmark, it's safe to let it inherit the
-    // current document's principal.
-    mayInheritPrincipal = true;
-  } catch (ex) {
-    // It was not possible to bind the param, just use the original url value.
-  }
-
-  return { url, postData, mayInheritPrincipal };
-}
-
-function getPostDataStream(aPostDataString,
-                           aType = "application/x-www-form-urlencoded") {
-  let dataStream = Cc["@mozilla.org/io/string-input-stream;1"]
-                     .createInstance(Ci.nsIStringInputStream);
-  dataStream.data = aPostDataString;
-
-  let mimeStream = Cc["@mozilla.org/network/mime-input-stream;1"]
-                     .createInstance(Ci.nsIMIMEInputStream);
-  mimeStream.addHeader("Content-Type", aType);
-  mimeStream.setData(dataStream);
-  return mimeStream.QueryInterface(Ci.nsIInputStream);
-}
-
 function getLoadContext() {
   return window.docShell.QueryInterface(Ci.nsILoadContext);
 }
 
 function readFromClipboard() {
   var url;
 
   try {
@@ -3698,17 +3625,17 @@ var newTabButtonObserver = {
                                                                   window);
       if (!answer) {
         return;
       }
     }
 
     for (let link of links) {
       if (link.url) {
-        let data = await getShortcutOrURIAndPostData(link.url);
+        let data = await UrlbarUtils.getShortcutOrURIAndPostData(link.url);
         // Allow third-party services to fixup this URL.
         openNewTabWith(data.url, shiftKey, {
           // TODO fix allowInheritPrincipal
           // (this is required by javascript: drop to the new window) Bug 1475201
           allowInheritPrincipal: true,
           postData: data.postData,
           allowThirdPartyFixup: true,
           triggeringPrincipal,
@@ -3733,17 +3660,17 @@ var newWindowButtonObserver = {
                                                                   window);
       if (!answer) {
         return;
       }
     }
 
     for (let link of links) {
       if (link.url) {
-        let data = await getShortcutOrURIAndPostData(link.url);
+        let data = await UrlbarUtils.getShortcutOrURIAndPostData(link.url);
         // Allow third-party services to fixup this URL.
         openNewWindowWith(data.url, {
           // TODO fix allowInheritPrincipal
           // (this is required by javascript: drop to the new window) Bug 1475201
           allowInheritPrincipal: true,
           postData: data.postData,
           allowThirdPartyFixup: true,
           triggeringPrincipal,
@@ -4380,24 +4307,16 @@ function FillHistoryMenu(aParent) {
   // don't display the popup for a single item
   if (sessionHistory.entries.length <= 1)
     return false;
 
   updateSessionHistory(sessionHistory, true);
   return true;
 }
 
-function addToUrlbarHistory(aUrlToAdd) {
-  if (!PrivateBrowsingUtils.isWindowPrivate(window) &&
-      aUrlToAdd &&
-      !aUrlToAdd.includes(" ") &&
-      !/[\x00-\x1F]/.test(aUrlToAdd)) // eslint-disable-line no-control-regex
-    PlacesUIUtils.markPageAsTyped(aUrlToAdd);
-}
-
 function BrowserDownloadsUI() {
   if (PrivateBrowsingUtils.isWindowPrivate(window)) {
     openTrustedLinkIn("about:downloads", "tab");
   } else {
     PlacesCommandHook.showPlacesOrganizer("Downloads");
   }
 }
 
@@ -6235,26 +6154,26 @@ function middleMousePaste(event) {
   // if it's not the current tab, we don't need to do anything because the
   // browser doesn't exist.
   let where = whereToOpenLink(event, true, false);
   let lastLocationChange;
   if (where == "current") {
     lastLocationChange = gBrowser.selectedBrowser.lastLocationChange;
   }
 
-  getShortcutOrURIAndPostData(clipboard).then(data => {
+  UrlbarUtils.getShortcutOrURIAndPostData(clipboard).then(data => {
     try {
       makeURI(data.url);
     } catch (ex) {
       // Not a valid URI.
       return;
     }
 
     try {
-      addToUrlbarHistory(data.url);
+      UrlbarUtils.addToUrlbarHistory(data.url, 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);
     }
 
     if (where != "current" ||
         lastLocationChange == gBrowser.selectedBrowser.lastLocationChange) {
@@ -6321,17 +6240,17 @@ function handleDroppedLink(event, urlOrL
       if (!answer) {
         return;
       }
     }
 
     let urls = [];
     let postDatas = [];
     for (let link of links) {
-      let data = await getShortcutOrURIAndPostData(link.url);
+      let data = await UrlbarUtils.getShortcutOrURIAndPostData(link.url);
       urls.push(data.url);
       postDatas.push(data.postData);
     }
     if (lastLocationChange == gBrowser.selectedBrowser.lastLocationChange) {
       gBrowser.loadTabs(urls, {
         inBackground,
         replace: true,
         allowThirdPartyFixup: false,
--- a/browser/base/content/test/general/browser.ini
+++ b/browser/base/content/test/general/browser.ini
@@ -296,18 +296,16 @@ skip-if = true # browser_drag.js is disa
 # DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
 [browser_fullscreen-window-open.js]
 tags = fullscreen
 skip-if = os == "linux" # Linux: Intermittent failures - bug 941575.
 # DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
 [browser_gestureSupport.js]
 skip-if = e10s # Bug 863514 - no gesture support.
 # DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
-[browser_getshortcutoruri.js]
-# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
 [browser_hide_removing.js]
 # DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
 [browser_homeDrop.js]
 # DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
 [browser_invalid_uri_back_forward_manipulation.js]
 # DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
 [browser_keywordBookmarklets.js]
 # DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
--- a/browser/base/content/test/general/browser_contentAreaClick.js
+++ b/browser/base/content/test/general/browser_contentAreaClick.js
@@ -164,28 +164,42 @@ var gReplacedMethods = [
   "urlSecurityCheck",
   "loadURI",
   "gatherTextUnder",
   "saveURL",
   "openLinkIn",
   "getShortcutOrURIAndPostData",
 ];
 
+// Returns the target object for the replaced method.
+function getStub(replacedMethod) {
+  let targetObj = replacedMethod == "getShortcutOrURIAndPostData" ? UrlbarUtils : gTestWin;
+  return targetObj[replacedMethod];
+}
+
 // Reference to the new window.
 var gTestWin = null;
 
-// List of methods invoked by a specific call to contentAreaClick.
-var gInvokedMethods = [];
-
 // The test currently running.
 var gCurrentTest = null;
 
+var sandbox;
+
 function test() {
   waitForExplicitFinish();
 
+  /* global sinon */
+  Services.scriptloader.loadSubScript("resource://testing-common/sinon-2.3.2.js");
+  sandbox = sinon.sandbox.create();
+
+  registerCleanupFunction(function() {
+    sandbox.restore();
+    delete window.sinon;
+  });
+
   gTestWin = openDialog(location, "", "chrome,all,dialog=no", "about:blank");
   whenDelayedStartupFinished(gTestWin, function() {
     info("Browser window opened");
     waitForFocus(function() {
       info("Browser window focused");
       waitForFocus(function() {
         info("Setting up browser...");
         setupTestBrowserWindow();
@@ -205,50 +219,43 @@ var gClickHandler = {
 
     let isPanelClick = linkId == "panellink";
     gTestWin.contentAreaClick(event, isPanelClick);
     let prevent = event.defaultPrevented;
     is(prevent, gCurrentTest.preventDefault,
        gCurrentTest.desc + ": event.defaultPrevented is correct (" + prevent + ")");
 
     // Check that all required methods have been called.
-    gCurrentTest.expectedInvokedMethods.forEach(function(aExpectedMethodName) {
-      isnot(gInvokedMethods.indexOf(aExpectedMethodName), -1,
-            gCurrentTest.desc + ":" + aExpectedMethodName + " was invoked");
-    });
+    for (let expectedMethod of gCurrentTest.expectedInvokedMethods) {
+      ok(getStub(expectedMethod).called,
+        `${gCurrentTest.desc}:${expectedMethod} should have been invoked`);
+    }
 
-    if (gInvokedMethods.length != gCurrentTest.expectedInvokedMethods.length) {
-      ok(false, "Wrong number of invoked methods");
-      gInvokedMethods.forEach(method => info(method + " was invoked"));
+    for (let method of gReplacedMethods) {
+      if (getStub(method).called &&
+          !gCurrentTest.expectedInvokedMethods.includes(method)) {
+        ok(false, `Should have not called ${method}`);
+      }
     }
 
     event.preventDefault();
     event.stopPropagation();
 
     executeSoon(runNextTest);
   },
 };
 
-// Wraps around the methods' replacement mock function.
-function wrapperMethod(aInvokedMethods, aMethodName) {
-  return function() {
-    aInvokedMethods.push(aMethodName);
-    // At least getShortcutOrURIAndPostData requires to return url
-    return (aMethodName == "getShortcutOrURIAndPostData") ? arguments.url : arguments[0];
-  };
-}
-
 function setupTestBrowserWindow() {
   // Steal click events and don't propagate them.
   gTestWin.addEventListener("click", gClickHandler, true);
 
   // Replace methods.
-  gReplacedMethods.forEach(function(aMethodName) {
-    gTestWin["old_" + aMethodName] = gTestWin[aMethodName];
-    gTestWin[aMethodName] = wrapperMethod(gInvokedMethods, aMethodName);
+  gReplacedMethods.forEach(function(methodName) {
+    let targetObj = methodName == "getShortcutOrURIAndPostData" ? UrlbarUtils : gTestWin;
+    sandbox.stub(targetObj, methodName).returnsArg(0);
   });
 
   // Inject links in content.
   let doc = gTestWin.content.document;
   let mainDiv = doc.createElement("div");
   mainDiv.innerHTML =
     '<p><a id="commonlink" href="http://mochi.test/moz/">Common link</a></p>' +
     '<p><a id="panellink" href="http://mochi.test/moz/">Panel link</a></p>' +
@@ -274,32 +281,25 @@ function runNextTest() {
       gCurrentTest.setup();
     } else {
       finishTest();
       return;
     }
   }
 
   // Move to next target.
-  gInvokedMethods.length = 0;
+  sandbox.resetHistory();
   let target = gCurrentTest.targets.shift();
 
   info(gCurrentTest.desc + ": testing " + target);
 
   // Fire click event.
   let targetElt = gTestWin.content.document.getElementById(target);
   ok(targetElt, gCurrentTest.desc + ": target is valid (" + targetElt.id + ")");
   EventUtils.synthesizeMouseAtCenter(targetElt, gCurrentTest.event, gTestWin.content);
 }
 
 function finishTest() {
   info("Restoring browser...");
   gTestWin.removeEventListener("click", gClickHandler, true);
-
-  // Restore original methods.
-  gReplacedMethods.forEach(function(aMethodName) {
-    gTestWin[aMethodName] = gTestWin["old_" + aMethodName];
-    delete gTestWin["old_" + aMethodName];
-  });
-
   gTestWin.close();
   finish();
 }
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -716,17 +716,17 @@ file, You can obtain one at http://mozil
                 }
                 url = action.params.url;
                 break;
               case "remotetab":
                 url = action.params.url;
                 break;
               case "keyword":
                 if (action.params.postData) {
-                  postData = getPostDataStream(action.params.postData);
+                  postData = UrlbarUtils.getPostDataStream(action.params.postData);
                 }
                 mayInheritPrincipal = true;
                 url = action.params.url;
                 break;
               case "switchtab":
                 url = action.params.url;
                 if (this.hasAttribute("actiontype")) {
                   this.handleRevert();
@@ -778,17 +778,17 @@ file, You can obtain one at http://mozil
             // This is a fallback for add-ons and old testing code that directly
             // set value and try to confirm it. UnifiedComplete should always
             // resolve to a valid url.
             try {
               url = url.trim();
               new URL(url);
             } catch (ex) {
               let lastLocationChange = browser.lastLocationChange;
-              getShortcutOrURIAndPostData(url).then(data => {
+              UrlbarUtils.getShortcutOrURIAndPostData(url).then(data => {
                 if (where != "current" ||
                     browser.lastLocationChange == lastLocationChange) {
                   this._loadURL(data.url, browser, data.postData, where,
                                 openUILinkParams, data.mayInheritPrincipal,
                                 triggeringPrincipal);
                 }
               });
               return;
@@ -828,17 +828,17 @@ file, You can obtain one at http://mozil
         <parameter name="triggeringPrincipal"/>
         <body><![CDATA[
           this.value = url;
           browser.userTypedValue = url;
           if (gInitialPages.includes(url)) {
             browser.initialPageLoadedFromURLBar = url;
           }
           try {
-            addToUrlbarHistory(url);
+            UrlbarUtils.addToUrlbarHistory(url, 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);
           }
 
           let params = {
             postData,
--- a/browser/components/places/tests/browser/browser_bookmarkProperties_addKeywordForThisSearch.js
+++ b/browser/components/places/tests/browser/browser_bookmarkProperties_addKeywordForThisSearch.js
@@ -41,17 +41,17 @@ add_task(async function() {
       Assert.equal(entry.url.href, TEST_URL, "URL is correct");
       Assert.equal(entry.postData, "accenti%3D%E0%E8%EC%F2%F9&search%3D%25s", "POST data is correct");
 
       info("Check the charset has been saved");
       let pageInfo = await PlacesUtils.history.fetch(TEST_URL, {includeAnnotations: true});
       Assert.equal(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), "windows-1252", "charset is correct");
 
       // Now check getShortcutOrURI.
-      let data = await getShortcutOrURIAndPostData("kw test");
+      let data = await UrlbarUtils.getShortcutOrURIAndPostData("kw test");
       Assert.equal(getPostDataString(data.postData), "accenti=\u00E0\u00E8\u00EC\u00F2\u00F9&search=test", "getShortcutOrURI POST data is correct");
       Assert.equal(data.url, TEST_URL, "getShortcutOrURI URL is correct");
     }, closeHandler);
   });
 });
 
 add_task(async function reopen_same_field() {
   await PlacesUtils.keywords.insert({
--- a/browser/components/urlbar/UrlbarController.jsm
+++ b/browser/components/urlbar/UrlbarController.jsm
@@ -162,20 +162,18 @@ class UrlbarController {
     //   event, this.userSelectionBehavior);
 
     text = text.trim();
 
     try {
       new URL(text);
     } catch (ex) {
       // TODO: Figure out why we need lastLocationChange here.
-      // TODO: Possibly move getShortcutOrURIAndPostData into a utility function
-      // in a jsm (there's nothing window specific about it).
       // let lastLocationChange = browser.lastLocationChange;
-      // getShortcutOrURIAndPostData(text).then(data => {
+      // 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);
       //   }
       // });
@@ -303,18 +301,17 @@ class UrlbarController {
   _loadURL(url, browser, openUILinkWhere, params) {
     // 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 {
-      // TODO: Move function to PlacesUIUtils.
-      this.window.addToUrlbarHistory(url);
+      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;
 
--- a/browser/components/urlbar/UrlbarUtils.jsm
+++ b/browser/components/urlbar/UrlbarUtils.jsm
@@ -6,16 +6,26 @@
 
 /**
  * This module exports the UrlbarUtils singleton, which contains constants and
  * helper functions that are useful to all components of the urlbar.
  */
 
 var EXPORTED_SYMBOLS = ["UrlbarUtils"];
 
+ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
+
+XPCOMUtils.defineLazyModuleGetters(this, {
+  BrowserUtils: "resource://gre/modules/BrowserUtils.jsm",
+  PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.jsm",
+  PlacesUIUtils: "resource:///modules/PlacesUIUtils.jsm",
+  PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",
+  Services: "resource://gre/modules/Services.jsm",
+});
+
 var UrlbarUtils = {
   // Values for browser.urlbar.insertMethod
   INSERTMETHOD: {
     // Just append new results.
     APPEND: 0,
     // Merge previous and current results if search strings are related.
     MERGE_RELATED: 1,
     // Always merge previous and current results.
@@ -51,9 +61,105 @@ var UrlbarUtils = {
   },
 
   // Defines UrlbarMatch types.
   MATCH_TYPE: {
     // Indicates an open tab.
     // The payload is: { url, userContextId }
     TAB_SWITCH: 1,
   },
+
+  /**
+   * Adds a url to history as long as it isn't in a private browsing window,
+   * and it is valid.
+   *
+   * @param {string} url The url to add to history.
+   * @param {nsIDomWindow} window The window from where the url is being added.
+   */
+  addToUrlbarHistory(url, window) {
+    if (!PrivateBrowsingUtils.isWindowPrivate(window) &&
+        url &&
+        !url.includes(" ") &&
+        !/[\x00-\x1F]/.test(url)) // eslint-disable-line no-control-regex
+      PlacesUIUtils.markPageAsTyped(url);
+  },
+
+  /**
+   * Given a string, will generate a more appropriate urlbar value if a Places
+   * keyword or a search alias is found at the beginning of it.
+   *
+   * @param {string} url
+   *        A string that may begin with a keyword or an alias.
+   *
+   * @returns {Promise}
+   * @resolves { url, postData, mayInheritPrincipal }. If it's not possible
+   *           to discern a keyword or an alias, url will be the input string.
+   */
+  async getShortcutOrURIAndPostData(url) {
+    let mayInheritPrincipal = false;
+    let postData = null;
+    // Split on the first whitespace.
+    let [keyword, param = ""] = url.trim().split(/\s(.+)/, 2);
+
+    if (!keyword) {
+      return { url, postData, mayInheritPrincipal };
+    }
+
+    let engine = Services.search.getEngineByAlias(keyword);
+    if (engine) {
+      let submission = engine.getSubmission(param, null, "keyword");
+      return { url: submission.uri.spec,
+               postData: submission.postData,
+               mayInheritPrincipal };
+    }
+
+    // A corrupt Places database could make this throw, breaking navigation
+    // from the location bar.
+    let entry = null;
+    try {
+      entry = await PlacesUtils.keywords.fetch(keyword);
+    } catch (ex) {
+      Cu.reportError(`Unable to fetch Places keyword "${keyword}": ${ex}`);
+    }
+    if (!entry || !entry.url) {
+      // This is not a Places keyword.
+      return { url, postData, mayInheritPrincipal };
+    }
+
+    try {
+      [url, postData] =
+        await BrowserUtils.parseUrlAndPostData(entry.url.href,
+                                               entry.postData,
+                                               param);
+      if (postData) {
+        postData = this.getPostDataStream(postData);
+      }
+
+      // Since this URL came from a bookmark, it's safe to let it inherit the
+      // current document's principal.
+      mayInheritPrincipal = true;
+    } catch (ex) {
+      // It was not possible to bind the param, just use the original url value.
+    }
+
+    return { url, postData, mayInheritPrincipal };
+  },
+
+  /**
+   * Returns an input stream wrapper for the given post data.
+   *
+   * @param {string} postDataString The string to wrap.
+   * @param {string} [type] The encoding type.
+   * @returns {nsIInputStream} An input stream of the wrapped post data.
+   */
+  getPostDataStream(postDataString,
+                    type = "application/x-www-form-urlencoded") {
+    let dataStream = Cc["@mozilla.org/io/string-input-stream;1"]
+                       .createInstance(Ci.nsIStringInputStream);
+    dataStream.data = postDataString;
+
+    let mimeStream = Cc["@mozilla.org/network/mime-input-stream;1"]
+                       .createInstance(Ci.nsIMIMEInputStream);
+    mimeStream.addHeader("Content-Type", type);
+    mimeStream.setData(dataStream);
+    return mimeStream.QueryInterface(Ci.nsIInputStream);
+  },
 };
new file mode 100644
--- /dev/null
+++ b/browser/components/urlbar/tests/unit/test_UrlbarUtils_addToUrlbarHistory.js
@@ -0,0 +1,49 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * These tests unit test the functionality of the functions in UrlbarUtils.
+ * Some functions are bigger, and split out into sepearate test_UrlbarUtils_* files.
+ */
+
+"use strict";
+
+ChromeUtils.import("resource://gre/modules/PrivateBrowsingUtils.jsm");
+ChromeUtils.import("resource:///modules/PlacesUIUtils.jsm");
+
+let sandbox;
+
+add_task(function setup() {
+  sandbox = sinon.sandbox.create();
+});
+
+add_task(function test_addToUrlbarHistory() {
+  sandbox.stub(PlacesUIUtils, "markPageAsTyped");
+  sandbox.stub(PrivateBrowsingUtils, "isWindowPrivate").returns(false);
+
+  UrlbarUtils.addToUrlbarHistory("http://example.com");
+  Assert.ok(PlacesUIUtils.markPageAsTyped.calledOnce,
+    "Should have marked a simple URL as typed.");
+  PlacesUIUtils.markPageAsTyped.resetHistory();
+
+  UrlbarUtils.addToUrlbarHistory();
+  Assert.ok(PlacesUIUtils.markPageAsTyped.notCalled,
+    "Should not have attempted to mark a null URL as typed.");
+  PlacesUIUtils.markPageAsTyped.resetHistory();
+
+  UrlbarUtils.addToUrlbarHistory("http://exam ple.com");
+  Assert.ok(PlacesUIUtils.markPageAsTyped.notCalled,
+    "Should not have marked a URL containing a space as typed.");
+  PlacesUIUtils.markPageAsTyped.resetHistory();
+
+  UrlbarUtils.addToUrlbarHistory("http://exam\x01ple.com");
+  Assert.ok(PlacesUIUtils.markPageAsTyped.notCalled,
+    "Should not have marked a URL containing a control character as typed.");
+  PlacesUIUtils.markPageAsTyped.resetHistory();
+
+  PrivateBrowsingUtils.isWindowPrivate.returns(true);
+  UrlbarUtils.addToUrlbarHistory("http://example.com");
+  Assert.ok(PlacesUIUtils.markPageAsTyped.notCalled,
+    "Should not have marked a URL provided by a private browsing page as typed.");
+  PlacesUIUtils.markPageAsTyped.resetHistory();
+});
rename from browser/base/content/test/general/browser_getshortcutoruri.js
rename to browser/components/urlbar/tests/unit/test_UrlbarUtils_getShortcutOrURIAndPostData.js
--- a/browser/base/content/test/general/browser_getshortcutoruri.js
+++ b/browser/components/urlbar/tests/unit/test_UrlbarUtils_getShortcutOrURIAndPostData.js
@@ -1,32 +1,42 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * These tests unit test the functionality of UrlbarController by stubbing out the
+ * model and providing stubs to be called.
+ */
+
+"use strict";
+
 function getPostDataString(aIS) {
   if (!aIS)
     return null;
 
-  var sis = Cc["@mozilla.org/scriptableinputstream;1"].
+  let sis = Cc["@mozilla.org/scriptableinputstream;1"].
             createInstance(Ci.nsIScriptableInputStream);
   sis.init(aIS);
-  var dataLines = sis.read(aIS.available()).split("\n");
+  let dataLines = sis.read(aIS.available()).split("\n");
 
   // only want the last line
   return dataLines[dataLines.length - 1];
 }
 
 function keywordResult(aURL, aPostData, aIsUnsafe) {
   this.url = aURL;
   this.postData = aPostData;
   this.isUnsafe = aIsUnsafe;
 }
 
 function keyWordData() {}
 keyWordData.prototype = {
   init(aKeyWord, aURL, aPostData, aSearchWord) {
     this.keyword = aKeyWord;
-    this.uri = makeURI(aURL);
+    this.uri = Services.io.newURI(aURL);
     this.postData = aPostData;
     this.searchWord = aSearchWord;
 
     this.method = (this.postData ? "POST" : "GET");
   },
 };
 
 function bmKeywordData(aKeyWord, aURL, aPostData, aSearchWord) {
@@ -95,22 +105,24 @@ add_task(async function test_getshortcut
   await setupKeywords();
 
   for (let item of testData) {
     let [data, result] = item;
 
     let query = data.keyword;
     if (data.searchWord)
       query += " " + data.searchWord;
-    let returnedData = await getShortcutOrURIAndPostData(query);
+    let returnedData = await UrlbarUtils.getShortcutOrURIAndPostData(query);
     // null result.url means we should expect the same query we sent in
     let expected = result.url || query;
-    is(returnedData.url, expected, "got correct URL for " + data.keyword);
-    is(getPostDataString(returnedData.postData), result.postData, "got correct postData for " + data.keyword);
-    is(returnedData.mayInheritPrincipal, !result.isUnsafe, "got correct mayInheritPrincipal for " + data.keyword);
+    Assert.equal(returnedData.url, expected, "got correct URL for " + data.keyword);
+    Assert.equal(getPostDataString(returnedData.postData), result.postData,
+      "got correct postData for " + data.keyword);
+    Assert.equal(returnedData.mayInheritPrincipal, !result.isUnsafe,
+      "got correct mayInheritPrincipal for " + data.keyword);
   }
 
   await cleanupKeywords();
 });
 
 var folder = null;
 var gAddedEngines = [];
 
--- a/browser/components/urlbar/tests/unit/xpcshell.ini
+++ b/browser/components/urlbar/tests/unit/xpcshell.ini
@@ -3,8 +3,10 @@ head = head.js
 firefox-appdir = browser
 
 [test_providerOpenTabs.js]
 [test_providersManager.js]
 [test_QueryContext.js]
 [test_tokenizer.js]
 [test_UrlbarController_unit.js]
 [test_UrlbarController_integration.js]
+[test_UrlbarUtils_addToUrlbarHistory.js]
+[test_UrlbarUtils_getShortcutOrURIAndPostData.js]