Bug 1306639 - Searching in locationbar by typing something and pressing enter is not accounted in telemetry. r=adw draft
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 06 Oct 2016 17:40:13 +0200
changeset 423276 039983cca262164bfc004e05bf77dc8ced0b1f29
parent 421960 4b9944879c9a60a9aba4a744a7401bc38e0f39c4
child 423646 e39c62b06355a9f60a632e1bca31d1339adbb5c7
child 423777 c77f8cf5005c081b309dca909e433efcfdd79ede
push id31860
push usermak77@bonardo.net
push dateMon, 10 Oct 2016 19:37:40 +0000
reviewersadw
bugs1306639
milestone52.0a1
Bug 1306639 - Searching in locationbar by typing something and pressing enter is not accounted in telemetry. r=adw MozReview-Commit-ID: 9r8IyyyxruC
browser/base/content/test/urlbar/browser_canonizeURL.js
browser/base/content/test/urlbar/browser_urlbarSearchTelemetry.js
browser/base/content/urlbarBindings.xml
toolkit/components/autocomplete/nsAutoCompleteController.cpp
toolkit/components/autocomplete/nsIAutoCompleteController.idl
toolkit/components/autocomplete/tests/unit/test_finalCompleteValueSelectedIndex.js
toolkit/components/places/UnifiedComplete.js
toolkit/components/places/tests/unifiedcomplete/test_searchSuggestions.js
toolkit/components/places/tests/unifiedcomplete/test_visiturl.js
--- a/browser/base/content/test/urlbar/browser_canonizeURL.js
+++ b/browser/base/content/test/urlbar/browser_canonizeURL.js
@@ -19,59 +19,16 @@ add_task(function*() {
   // Disable autoFill for this test, since it could mess up the results.
   let autoFill = Preferences.get("browser.urlbar.autoFill");
   Preferences.set("browser.urlbar.autoFill", false);
   registerCleanupFunction(() => {
     Preferences.set("browser.urlbar.autoFill", autoFill);
   });
 
   for (let [inputValue, expectedURL] of pairs) {
-    let focusEventPromise = BrowserTestUtils.waitForEvent(gURLBar, "focus");
-    let messagePromise = BrowserTestUtils.waitForMessage(gBrowser.selectedBrowser.messageManager,
-                                                         "browser_canonizeURL:start");
-
-    let stoppedLoadPromise = ContentTask.spawn(gBrowser.selectedBrowser, [inputValue, expectedURL],
-      function([inputValue, expectedURL]) {
-        return new Promise(resolve => {
-          let wpl = {
-            onStateChange(aWebProgress, aRequest, aStateFlags, aStatus) {
-              if (aStateFlags & Ci.nsIWebProgressListener.STATE_START &&
-                  aStateFlags & Ci.nsIWebProgressListener.STATE_IS_NETWORK) {
-                if (!aRequest || !(aRequest instanceof Ci.nsIChannel)) {
-                  return;
-                }
-                aRequest.QueryInterface(Ci.nsIChannel);
-                is(aRequest.originalURI.spec, expectedURL,
-                   "entering '" + inputValue + "' loads expected URL");
-
-                webProgress.removeProgressListener(filter);
-                filter.removeProgressListener(wpl);
-                docShell.QueryInterface(Ci.nsIWebNavigation);
-                docShell.stop(docShell.STOP_ALL);
-                resolve();
-              }
-            },
-          };
-          let filter = Cc["@mozilla.org/appshell/component/browser-status-filter;1"]
-                           .createInstance(Ci.nsIWebProgress);
-          filter.addProgressListener(wpl, Ci.nsIWebProgress.NOTIFY_ALL);
-
-          let webProgress = docShell.QueryInterface(Ci.nsIInterfaceRequestor)
-                                    .getInterface(Ci.nsIWebProgress);
-          webProgress.addProgressListener(filter, Ci.nsIWebProgress.NOTIFY_ALL);
-          // We're sending this off to trigger the start of the this test, when all the
-          // listeners are in place:
-          sendAsyncMessage("browser_canonizeURL:start");
-        });
-      }
-    );
-
-    gBrowser.selectedBrowser.focus();
+    let promiseLoad = waitForDocLoadAndStopIt(expectedURL);
     gURLBar.focus();
-
-    yield Promise.all([focusEventPromise, messagePromise]);
-
     gURLBar.inputField.value = inputValue.slice(0, -1);
     EventUtils.synthesizeKey(inputValue.slice(-1), {});
     EventUtils.synthesizeKey("VK_RETURN", { shiftKey: true });
-    yield stoppedLoadPromise;
+    yield promiseLoad;
   }
 });
--- a/browser/base/content/test/urlbar/browser_urlbarSearchTelemetry.js
+++ b/browser/base/content/test/urlbar/browser_urlbarSearchTelemetry.js
@@ -6,62 +6,96 @@ const SUGGEST_URLBAR_PREF = "browser.url
 const TEST_ENGINE_BASENAME = "searchSuggestionEngine.xml";
 
 // Must run first.
 add_task(function* prepare() {
   Services.prefs.setBoolPref(SUGGEST_URLBAR_PREF, true);
   let engine = yield promiseNewSearchEngine(TEST_ENGINE_BASENAME);
   let oldCurrentEngine = Services.search.currentEngine;
   Services.search.currentEngine = engine;
+
   registerCleanupFunction(function* () {
     Services.prefs.clearUserPref(SUGGEST_URLBAR_PREF);
     Services.search.currentEngine = oldCurrentEngine;
 
     // Clicking urlbar results causes visits to their associated pages, so clear
     // that history now.
     yield PlacesTestUtils.clearHistory();
 
     // Make sure the popup is closed for the next test.
     gURLBar.blur();
     Assert.ok(!gURLBar.popup.popupOpen, "popup should be closed");
   });
+
   // Move the mouse away from the urlbar one-offs so that a one-off engine is
   // not inadvertently selected.
   yield new Promise(resolve => {
     EventUtils.synthesizeNativeMouseMove(window.document.documentElement, 0, 0,
                                          resolve);
   });
 });
 
-add_task(function* heuristicResult() {
+add_task(function* heuristicResultMouse() {
   yield compareCounts(function* () {
-    gBrowser.selectedTab = gBrowser.addTab();
+    let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser);
+    gURLBar.focus();
+    yield promiseAutocompleteResultPopup("heuristicResult");
+    let action = getActionAtIndex(0);
+    Assert.ok(!!action, "there should be an action at index 0");
+    Assert.equal(action.type, "searchengine", "type should be searchengine");
+    let loadPromise = BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+    gURLBar.popup.richlistbox.getItemAtIndex(0).click();
+    yield loadPromise;
+    yield BrowserTestUtils.removeTab(tab);
+  });
+});
+
+add_task(function* heuristicResultKeyboard() {
+  yield compareCounts(function* () {
+    let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser);
+    gURLBar.focus();
     yield promiseAutocompleteResultPopup("heuristicResult");
     let action = getActionAtIndex(0);
     Assert.ok(!!action, "there should be an action at index 0");
     Assert.equal(action.type, "searchengine", "type should be searchengine");
-    let item = gURLBar.popup.richlistbox.getItemAtIndex(0);
-    let loadPromise = promiseTabLoaded(gBrowser.selectedTab);
-    item.click();
+    let loadPromise = BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+    EventUtils.sendKey("return");
     yield loadPromise;
-    gBrowser.removeTab(gBrowser.selectedTab);
+    yield BrowserTestUtils.removeTab(tab);
   });
 });
 
-add_task(function* searchSuggestion() {
+add_task(function* searchSuggestionMouse() {
   yield compareCounts(function* () {
-    gBrowser.selectedTab = gBrowser.addTab();
+    let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser);
+    gURLBar.focus();
     yield promiseAutocompleteResultPopup("searchSuggestion");
     let idx = getFirstSuggestionIndex();
     Assert.ok(idx >= 0, "there should be a first suggestion");
-    let item = gURLBar.popup.richlistbox.getItemAtIndex(idx);
-    let loadPromise = promiseTabLoaded(gBrowser.selectedTab);
-    item.click();
+    let loadPromise = BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+    gURLBar.popup.richlistbox.getItemAtIndex(idx).click();
     yield loadPromise;
-    gBrowser.removeTab(gBrowser.selectedTab);
+    yield BrowserTestUtils.removeTab(tab);
+  });
+});
+
+add_task(function* searchSuggestionKeyboard() {
+  yield compareCounts(function* () {
+    let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser);
+    gURLBar.focus();
+    yield promiseAutocompleteResultPopup("searchSuggestion");
+    let idx = getFirstSuggestionIndex();
+    Assert.ok(idx >= 0, "there should be a first suggestion");
+    let loadPromise = BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+    while (idx--) {
+      EventUtils.sendKey("down");
+    }
+    EventUtils.sendKey("return");
+    yield loadPromise;
+    yield BrowserTestUtils.removeTab(tab);
   });
 });
 
 /**
  * This does three things: gets current telemetry/FHR counts, calls
  * clickCallback, gets telemetry/FHR counts again to compare them to the old
  * counts.
  *
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -402,16 +402,31 @@ file, You can obtain one at http://mozil
           let browser = gBrowser.selectedBrowser;
           let lastLocationChange = gBrowser.selectedBrowser.lastLocationChange;
           let matchLastLocationChange = true;
 
           let action = this._parseActionUrl(url);
           if (action) {
             switch (action.type) {
               case "visiturl":
+                // Unifiedcomplete uses fixupURI to tell if something is a visit
+                // or a search, and passes out the fixedURI as the url param.
+                // By using that uri we would end up passing a different string
+                // to the docshell that may run a different not-found heuristic.
+                // For example, "mozilla/run" would be fixed by unifiedcomplete
+                // to "http://mozilla/run". The docshell, once it can't resolve
+                // mozilla, would note the string has a scheme, and try to load
+                // http://mozilla.com/run instead of searching "mozilla/run".
+                // So, if we have the original input at hand, we pass it through
+                // and let the docshell handle it.
+                if (action.params.input) {
+                  url = action.params.input;
+                  break;
+                }
+                // Fall-through.
               case "keyword":
               case "remotetab":
                 url = action.params.url;
                 break;
               case "switchtab":
                 url = action.params.url;
                 if (this.hasAttribute("actiontype")) {
                   this.handleRevert();
@@ -458,17 +473,16 @@ file, You can obtain one at http://mozil
                 this._recordSearchEngineLoad(selectedOneOff.engine, value,
                                              event, where, openUILinkParams);
               this._loadURL(url, browser, postData, where, openUILinkParams,
                             matchLastLocationChange, mayInheritPrincipal);
               return;
             }
           }
 
-          url = this._canonizeURL(event, url);
           getShortcutOrURIAndPostData(url).then(({url, postData, mayInheritPrincipal}) => {
             if (url) {
               matchLastLocationChange =
                 lastLocationChange == browser.lastLocationChange;
               this._loadURL(url, browser, postData, where, openUILinkParams,
                             matchLastLocationChange, mayInheritPrincipal);
             }
           });
@@ -572,72 +586,69 @@ file, You can obtain one at http://mozil
           BrowserSearch.recordSearchInTelemetry(engine, "urlbar");
           this.popup.oneOffSearchButtons
               .maybeRecordTelemetry(event, openUILinkWhere, openUILinkParams);
           let submission = engine.getSubmission(query, null, "keyword");
           return [submission.uri.spec, submission.postData];
         ]]></body>
       </method>
 
-      <method name="_canonizeURL">
+      <method name="maybeCanonizeURL">
         <parameter name="aTriggeringEvent"/>
         <parameter name="aUrl"/>
         <body><![CDATA[
-          let url = aUrl;
-
           // Only add the suffix when the URL bar value isn't already "URL-like",
           // and only if we get a keyboard event, to match user expectations.
-          if (/^\s*[^.:\/\s]+(?:\/.*|\s*)$/i.test(url) &&
-              (aTriggeringEvent instanceof KeyEvent)) {
-            let accel = this.AppConstants.platform == "macosx" ?
-                        aTriggeringEvent.metaKey :
-                        aTriggeringEvent.ctrlKey;
-            let shift = aTriggeringEvent.shiftKey;
-
-            let suffix = "";
-
-            switch (true) {
-              case (accel && shift):
-                suffix = ".org/";
-                break;
-              case (shift):
-                suffix = ".net/";
-                break;
-              case (accel):
-                try {
-                  suffix = gPrefService.getCharPref("browser.fixup.alternate.suffix");
-                  if (suffix.charAt(suffix.length - 1) != "/")
-                    suffix += "/";
-                } catch (e) {
-                  suffix = ".com/";
-                }
-                break;
-            }
-
-            if (suffix) {
-              // trim leading/trailing spaces (bug 233205)
-              url = url.trim();
-
-              // Tack www. and suffix on.  If user has appended directories, insert
-              // suffix before them (bug 279035).  Be careful not to get two slashes.
-
-              let firstSlash = url.indexOf("/");
-
-              if (firstSlash >= 0) {
-                url = url.substring(0, firstSlash) + suffix +
-                      url.substring(firstSlash + 1);
-              } else {
-                url = url + suffix;
-              }
-
-              url = "http://www." + url;
-            }
+          if (!/^\s*[^.:\/\s]+(?:\/.*|\s*)$/i.test(aUrl) ||
+              !(aTriggeringEvent instanceof KeyEvent)) {
+            return;
           }
 
-          return url;
+          let url = aUrl;
+          let accel = this.AppConstants.platform == "macosx" ?
+                      aTriggeringEvent.metaKey :
+                      aTriggeringEvent.ctrlKey;
+          let shift = aTriggeringEvent.shiftKey;
+          let suffix = "";
+
+          switch (true) {
+            case (accel && shift):
+              suffix = ".org/";
+              break;
+            case (shift):
+              suffix = ".net/";
+              break;
+            case (accel):
+              try {
+                suffix = gPrefService.getCharPref("browser.fixup.alternate.suffix");
+                if (suffix.charAt(suffix.length - 1) != "/")
+                  suffix += "/";
+              } catch (e) {
+                suffix = ".com/";
+              }
+              break;
+          }
+
+          if (!suffix)
+            return;
+
+          // trim leading/trailing spaces (bug 233205)
+          url = url.trim();
+
+          // Tack www. and suffix on.  If user has appended directories, insert
+          // suffix before them (bug 279035).  Be careful not to get two slashes.
+          let firstSlash = url.indexOf("/");
+          if (firstSlash >= 0) {
+            url = url.substring(0, firstSlash) + suffix +
+                  url.substring(firstSlash + 1);
+          } else {
+            url = url + suffix;
+          }
+
+          this.popup.overrideValue = "http://www." + url;
         ]]></body>
       </method>
 
       <field name="_contentIsCropped">false</field>
 
       <method name="_initURLTooltip">
         <body><![CDATA[
           if (this.focused || !this._contentIsCropped)
@@ -1046,16 +1057,17 @@ file, You can obtain one at http://mozil
           // Note this is also used to detect if we should perform a delayed
           // handleEnter, in such a case it won't have been cleared.
           this.handleEnterInstance = {
             searchString: this.mController.searchString,
             event: event
           };
 
           if (this.popup.selectedIndex != 0 || this.gotResultForCurrentQuery) {
+            this.maybeCanonizeURL(event, this.value);
             let rv = this.mController.handleEnter(false, event);
             this.handleEnterInstance = null;
             return rv;
           }
 
           return true;
         ]]></body>
       </method>
@@ -1339,16 +1351,21 @@ file, You can obtain one at http://mozil
 
       <field name="oneOffSearchButtons" readonly="true">
         document.getAnonymousElementByAttribute(this, "anonid",
                                                 "one-off-search-buttons");
       </field>
 
       <field name="_oneOffSearchesEnabled">false</field>
 
+      <field name="_overrideValue">null</field>
+      <property name="overrideValue"
+                onget="return this._overrideValue;"
+                onset="this._overrideValue = val; return val;"/>
+
       <method name="enableOneOffSearches">
         <parameter name="enable"/>
         <body><![CDATA[
           this._oneOffSearchesEnabled = enable;
           if (enable) {
             this.oneOffSearchButtons.style.display = "-moz-box";
             this.oneOffSearchButtons.popup = this;
             this.oneOffSearchButtons.textbox = this.input;
@@ -1501,17 +1518,17 @@ file, You can obtain one at http://mozil
         <parameter name="aInput"/>
         <parameter name="aElement"/>
         <body><![CDATA[
           if (this.mPopupOpen) {
             return;
           }
 
           this.mInput = aInput;
-          this.selectedIndex = this._isFirstResultHeuristic ? 0 : -1;
+          aInput.controller.setInitiallySelectedIndex(this._isFirstResultHeuristic ? 0 : -1);
           this.view = aInput.controller.QueryInterface(Components.interfaces.nsITreeView);
           this._invalidate();
 
           var rect = window.document.documentElement.getBoundingClientRect();
           var width = rect.right - rect.left;
           this.setAttribute("width", width);
 
           // Adjust the direction of the autocomplete popup list based on the textbox direction, bug 649840
@@ -1765,32 +1782,34 @@ file, You can obtain one at http://mozil
         <body>
           <![CDATA[
             // If nothing is selected yet, select the first result if it is a
             // pre-selected "heuristic" result.  (See UnifiedComplete.js.)
             if (this.selectedIndex == -1 && this._isFirstResultHeuristic) {
               // Don't fire DOMMenuItemActive so that screen readers still see
               // the input as being focused.
               this.richlistbox.suppressMenuItemEvent = true;
-              this.selectedIndex = 0;
+              this.input.controller.setInitiallySelectedIndex(0);
               this.richlistbox.suppressMenuItemEvent = false;
             }
 
             this.input.gotResultForCurrentQuery = true;
 
             // Check if we should perform a delayed handleEnter.
             if (this.input.handleEnterInstance) {
               let instance = this.input.handleEnterInstance;
               this.input.handleEnterInstance = null;
               // Don't handle this immediately or we could cause a recursive
               // loop where the controller sets popupOpen and re-enters here.
               setTimeout(() => {
                 // Safety check: handle only if the search string didn't change.
-                if (this.input.mController.searchString == instance.searchString) {
-                  this.input.mController.handleEnter(false, instance.event);
+                let { event, searchString } = instance;
+                if (this.input.mController.searchString == searchString) {
+                  this.input.maybeCanonizeURL(event, searchString);
+                  this.input.mController.handleEnter(false, event);
                 }
               }, 0);
             }
           ]]>
         </body>
       </method>
 
       <method name="_onSearchBegin">
@@ -1801,17 +1820,18 @@ file, You can obtain one at http://mozil
           // This is required to properly manage delayed handleEnter:
           // 1. if a search starts we set selectedIndex to 0 here, and it will
           //    be updated by onResultsAdded. Since selectedIndex is 0,
           //    handleEnter will delay the action if a result didn't arrive yet.
           // 2. if a search doesn't start (for example if autocomplete is
           //    disabled), this won't be called, and the selectedIndex will be
           //    the default -1 value. Then handleEnter will know it should not
           //    delay the action, cause a result wont't ever arrive.
-          this.selectedIndex = 0;
+          this.input.controller.setInitiallySelectedIndex(0);
+          this.overrideValue = null;
         ]]></body>
       </method>
 
     </implementation>
     <handlers>
 
       <handler event="SelectedOneOffButtonChanged"><![CDATA[
         this._selectedOneOffChanged();
--- a/toolkit/components/autocomplete/nsAutoCompleteController.cpp
+++ b/toolkit/components/autocomplete/nsAutoCompleteController.cpp
@@ -85,16 +85,36 @@ NS_IMETHODIMP
 nsAutoCompleteController::GetInput(nsIAutoCompleteInput **aInput)
 {
   *aInput = mInput;
   NS_IF_ADDREF(*aInput);
   return NS_OK;
 }
 
 NS_IMETHODIMP
+nsAutoCompleteController::SetInitiallySelectedIndex(int32_t aSelectedIndex)
+{
+  // First forward to the popup.
+  nsCOMPtr<nsIAutoCompleteInput> input(mInput);
+  NS_ENSURE_STATE(input);
+  nsCOMPtr<nsIAutoCompletePopup> popup;
+  input->GetPopup(getter_AddRefs(popup));
+  NS_ENSURE_STATE(popup);
+  popup->SetSelectedIndex(aSelectedIndex);
+
+  // Now take care of internal stuff.
+  bool completeSelection;
+  if (NS_SUCCEEDED(input->GetCompleteSelectedIndex(&completeSelection)) &&
+      completeSelection) {
+    mCompletedSelectionIndex = aSelectedIndex;
+  }
+  return NS_OK;
+}
+
+NS_IMETHODIMP
 nsAutoCompleteController::SetInput(nsIAutoCompleteInput *aInput)
 {
   // Don't do anything if the input isn't changing.
   if (mInput == aInput)
     return NS_OK;
 
   // Clear out the current search context
   if (mInput) {
@@ -1420,19 +1440,17 @@ nsAutoCompleteController::EnterMatch(boo
       } else if (mCompletedSelectionIndex != -1) {
         // If completeselectedindex is true, and EnterMatch was not invoked by
         // mouse-clicking a match (for example the user pressed Enter),
         // don't fill in the value as it will have already been filled in as
         // needed, unless the selected match has a final complete value that
         // differs from the user-facing value.
         nsAutoString finalValue;
         GetResultValueAt(mCompletedSelectionIndex, true, finalValue);
-        nsAutoString completedValue;
-        GetResultValueAt(mCompletedSelectionIndex, false, completedValue);
-        if (completedValue.Equals(inputValue) && !finalValue.Equals(inputValue)) {
+        if (!inputValue.Equals(finalValue)) {
           value = finalValue;
         }
         // Note that if the user opens the popup, mouses over entries without
         // ever selecting one with the keyboard, and then hits enter, none of
         // the above cases will be hit, since mouseover doesn't activate
         // completeselectedindex and thus mCompletedSelectionIndex would be
         // -1.
       }
--- a/toolkit/components/autocomplete/nsIAutoCompleteController.idl
+++ b/toolkit/components/autocomplete/nsIAutoCompleteController.idl
@@ -154,9 +154,20 @@ interface nsIAutoCompleteController : ns
    * when the user confirms the match at the given index
    */
   AString getFinalCompleteValueAt(in long index);
 
   /*
    * Get / set the current search string.  Note, setting will not start searching
    */
   attribute AString searchString;
+
+  /*
+   * Set the index of the result item that should be initially selected.
+   * This should be used when a search wants to pre-select an element before
+   * the user starts using results.
+   *
+   * @note Setting this is not the same as just setting selectedIndex in
+   * nsIAutocompletePopup, since this will take care of updating any internal
+   * tracking variables of features like completeSelectedIndex.
+   */
+  void setInitiallySelectedIndex(in long index);
 };
--- a/toolkit/components/autocomplete/tests/unit/test_finalCompleteValueSelectedIndex.js
+++ b/toolkit/components/autocomplete/tests/unit/test_finalCompleteValueSelectedIndex.js
@@ -14,21 +14,17 @@ function AutoCompleteInput(aSearches) {
     Assert.equal(reverse, false);
     Assert.equal(page, false);
     this.selectedIndex += (reverse ? -1 : 1) * (page ? 100 : 1);
   };
   this.completeSelectedIndex = true;
 }
 AutoCompleteInput.prototype = Object.create(AutoCompleteInputBase.prototype);
 
-function run_test() {
-  run_next_test();
-}
-
-add_test(function test_handleEnter() {
+add_test(function test_handleEnter_key() {
   let results = [
     ["mozilla.com", "http://www.mozilla.com"],
     ["mozilla.org", "http://www.mozilla.org"],
   ];
   // First check the case where we do select a value with the keyboard:
   doSearch("moz", results, function(aController) {
     Assert.equal(aController.input.textValue, "moz");
     Assert.equal(aController.getFinalCompleteValueAt(0), "http://www.mozilla.com");
@@ -41,17 +37,23 @@ add_test(function test_handleEnter() {
     // ie NOT keyboard interaction:
     aController.input.popup.selectedIndex = 0;
 
     aController.handleEnter(false);
     // Verify that the keyboard-selected thing got inserted,
     // and not the mouse selection:
     Assert.equal(aController.input.textValue, "http://www.mozilla.org");
   });
+});
 
+add_test(function test_handleEnter_mouse() {
+  let results = [
+    ["mozilla.com", "http://www.mozilla.com"],
+    ["mozilla.org", "http://www.mozilla.org"],
+  ];
   // Then the case where we do not:
   doSearch("moz", results, function(aController) {
     Assert.equal(aController.input.textValue, "moz");
     Assert.equal(aController.getFinalCompleteValueAt(0), "http://www.mozilla.com");
     Assert.equal(aController.getFinalCompleteValueAt(1), "http://www.mozilla.org");
 
     Assert.equal(aController.input.popup.selectedIndex, 0);
     aController.input.popupOpen = true;
@@ -63,16 +65,36 @@ add_test(function test_handleEnter() {
 
     aController.handleEnter(false);
     // Verify that the input stayed the same, because no selection was made
     // with the keyboard:
     Assert.equal(aController.input.textValue, "moz");
   });
 });
 
+add_test(function test_handleEnter_preselected() {
+  let results = [
+    ["mozilla.com", "http://www.mozilla.com"],
+    ["mozilla.org", "http://www.mozilla.org"],
+  ];
+  // Then test a preselection.
+  doSearch("moz", results, function(aController) {
+    Assert.equal(aController.input.textValue, "moz");
+    Assert.equal(aController.getFinalCompleteValueAt(0), "http://www.mozilla.com");
+    Assert.equal(aController.getFinalCompleteValueAt(1), "http://www.mozilla.org");
+
+    aController.setInitiallySelectedIndex(0);
+
+    aController.handleEnter(false);
+    // Verify that the input stayed the same, because no selection was made
+    // with the keyboard:
+    Assert.equal(aController.input.textValue, "http://www.mozilla.com");
+  });
+});
+
 function doSearch(aSearchString, aResults, aOnCompleteCallback) {
   selectByWasCalled = false;
   let search = new AutoCompleteSearchBase(
     "search",
     new AutoCompleteResult(aResults)
   );
   registerAutoCompleteSearch(search);
 
--- a/toolkit/components/places/UnifiedComplete.js
+++ b/toolkit/components/places/UnifiedComplete.js
@@ -1302,26 +1302,16 @@ Search.prototype = {
     // Check the host, as "http:///" is a valid nsIURI, but not useful to us.
     // But, some schemes are expected to have no host. So we check just against
     // schemes we know should have a host. This allows new schemes to be
     // implemented without us accidentally blocking access to them.
     let hostExpected = new Set(["http", "https", "ftp", "chrome", "resource"]);
     if (hostExpected.has(uri.scheme) && !uri.host)
       return false;
 
-    // If the result is something that looks like a single-worded hostname
-    // we need to check the domain whitelist to treat it as such.
-    // We also want to return a "visit" if keyword.enabled is false.
-    if (uri.asciiHost &&
-        Prefs.keywordEnabled &&
-        REGEXP_SINGLEWORD_HOST.test(uri.asciiHost) &&
-        !Services.uriFixup.isDomainWhitelisted(uri.asciiHost, -1)) {
-      return false;
-    }
-
     // getFixupURIInfo() escaped the URI, so it may not be pretty.  Embed the
     // escaped URL in the action URI since that URL should be "canonical".  But
     // pass the pretty, unescaped URL as the match comment, since it's likely
     // to be displayed to the user, and in any case the front-end should not
     // rely on it being canonical.
     let escapedURL = uri.spec;
     let displayURL = textURIService.unEscapeURIForUI("UTF-8", uri.spec);
 
--- a/toolkit/components/places/tests/unifiedcomplete/test_searchSuggestions.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_searchSuggestions.js
@@ -583,24 +583,24 @@ add_task(function* prohibit_suggestions(
     matches: [
       makeVisitMatch("[2001::1]:30", "http://[2001::1]:30/", { heuristic: true }),
     ],
   });
   yield check_autocomplete({
     search: "user:pass@test",
     searchParam: "enable-actions",
     matches: [
-      makeSearchMatch("user:pass@test", { engineName: ENGINE_NAME, heuristic: true }),
+      makeVisitMatch("user:pass@test", "http://user:pass@test/", { heuristic: true }),
     ],
   });
   yield check_autocomplete({
     search: "test/test",
     searchParam: "enable-actions",
     matches: [
-      makeSearchMatch("test/test", { engineName: ENGINE_NAME, heuristic: true }),
+      makeVisitMatch("test/test", "http://test/test", { heuristic: true }),
     ],
   });
   yield check_autocomplete({
     search: "data:text/plain,Content",
     searchParam: "enable-actions",
     matches: [
       makeVisitMatch("data:text/plain,Content", "data:text/plain,Content", { heuristic: true }),
     ],
--- a/toolkit/components/places/tests/unifiedcomplete/test_visiturl.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_visiturl.js
@@ -1,12 +1,8 @@
-/* Any copyright is dedicated to the Public Domain.
- * http://creativecommons.org/publicdomain/zero/1.0/ */
-
-
 add_task(function*() {
   do_print("visit url, no protocol");
   yield check_autocomplete({
     search: "mozilla.org",
     searchParam: "enable-actions",
     matches: [ { uri: makeActionURI("visiturl", {url: "http://mozilla.org/", input: "mozilla.org"}), title: "http://mozilla.org/", style: [ "action", "visiturl", "heuristic" ] } ]
   });
 
@@ -32,24 +28,58 @@ add_task(function*() {
   ]);
   yield check_autocomplete({
     search: "mozilla.org/rum",
     searchParam: "enable-actions",
     matches: [ makeVisitMatch("mozilla.org/rum", "http://mozilla.org/rum", { heuristic: true }) ]
   });
 
   // And hosts with no dot in them are special, due to requiring whitelisting.
-  do_print("visit url, host matching visited host but not visited url, non-whitelisted host");
-  yield PlacesTestUtils.addVisits([
-    { uri: NetUtil.newURI("http://mozilla/bourbon/"), title: "Mozilla Bourbon", transition: TRANSITION_TYPED },
-  ]);
+  do_print("non-whitelisted host");
+  yield check_autocomplete({
+    search: "firefox",
+    searchParam: "enable-actions",
+    matches: [ makeSearchMatch("firefox", { heuristic: true }) ]
+  });
+
+  do_print("url with non-whitelisted host");
+  yield check_autocomplete({
+    search: "firefox/get",
+    searchParam: "enable-actions",
+    matches: [ makeVisitMatch("firefox/get", "http://firefox/get", { heuristic: true }) ]
+  });
+
+  Services.prefs.setBoolPref("browser.fixup.domainwhitelist.firefox", true);
+  do_register_cleanup(() => {
+    Services.prefs.clearUserPref("browser.fixup.domainwhitelist.firefox");
+  });
+
+  do_print("whitelisted host");
+  yield check_autocomplete({
+    search: "firefox",
+    searchParam: "enable-actions",
+    matches: [ makeVisitMatch("firefox", "http://firefox/", { heuristic: true }) ]
+  });
+
+  do_print("url with whitelisted host");
+  yield check_autocomplete({
+    search: "firefox/get",
+    searchParam: "enable-actions",
+    matches: [ makeVisitMatch("firefox/get", "http://firefox/get", { heuristic: true }) ]
+  });
+
+  do_print("visit url, host matching visited host but not visited url, whitelisted host");
+  Services.prefs.setBoolPref("browser.fixup.domainwhitelist.mozilla", true);
+  do_register_cleanup(() => {
+    Services.prefs.clearUserPref("browser.fixup.domainwhitelist.mozilla");
+  });
   yield check_autocomplete({
     search: "mozilla/rum",
     searchParam: "enable-actions",
-    matches: [ makeSearchMatch("mozilla/rum", { heuristic: true }) ]
+    matches: [ makeVisitMatch("mozilla/rum", "http://mozilla/rum", { heuristic: true }) ]
   });
 
   // ipv4 and ipv6 literal addresses should offer to visit.
   do_print("visit url, ipv4 literal");
   yield check_autocomplete({
     search: "127.0.0.1",
     searchParam: "enable-actions",
     matches: [ makeVisitMatch("127.0.0.1", "http://127.0.0.1/", { heuristic: true }) ]
@@ -58,19 +88,56 @@ add_task(function*() {
   do_print("visit url, ipv6 literal");
   yield check_autocomplete({
     search: "[2001:db8::1]",
     searchParam: "enable-actions",
     matches: [ makeVisitMatch("[2001:db8::1]", "http://[2001:db8::1]/", { heuristic: true }) ]
   });
 
   // Setting keyword.enabled to false should always try to visit.
+  let keywordEnabled = Services.prefs.getBoolPref("keyword.enabled");
   Services.prefs.setBoolPref("keyword.enabled", false);
   do_register_cleanup(() => {
     Services.prefs.clearUserPref("keyword.enabled");
   });
   do_print("visit url, keyword.enabled = false");
   yield check_autocomplete({
     search: "bacon",
     searchParam: "enable-actions",
     matches: [ makeVisitMatch("bacon", "http://bacon/", { heuristic: true }) ]
   });
+  Services.prefs.setBoolPref("keyword.enabled", keywordEnabled);
+
+  do_print("visit url, scheme+host");
+  yield check_autocomplete({
+    search: "http://example",
+    searchParam: "enable-actions",
+    matches: [ makeVisitMatch("http://example", "http://example/", { heuristic: true }) ]
+  });
+
+  do_print("visit url, scheme+host");
+  yield check_autocomplete({
+    search: "ftp://example",
+    searchParam: "enable-actions",
+    matches: [ makeVisitMatch("ftp://example", "ftp://example/", { heuristic: true }) ]
+  });
+
+  do_print("visit url, host+port");
+  yield check_autocomplete({
+    search: "example:8080",
+    searchParam: "enable-actions",
+    matches: [ makeVisitMatch("example:8080", "http://example:8080/", { heuristic: true }) ]
+  });
+
+  do_print("numerical operations that look like urls should search");
+  yield check_autocomplete({
+    search: "123/12",
+    searchParam: "enable-actions",
+    matches: [ makeSearchMatch("123/12", { heuristic: true }) ]
+  });
+
+  do_print("numerical operations that look like urls should search");
+  yield check_autocomplete({
+    search: "123.12/12.1",
+    searchParam: "enable-actions",
+    matches: [ makeSearchMatch("123.12/12.1", { heuristic: true }) ]
+  });
 });