Bug 1067899 - Add autocomplete result type for arbitrary URLs. r=mak
authorBlair McBride <bmcbride@mozilla.com>
Sat, 27 Sep 2014 01:17:25 +1200
changeset 207461 8e4b92b32b1556b673211302f56288d7ab0c8a9e
parent 207460 6351ff630f1a81e5d7d1c584b126ac192fb69111
child 207462 e16c4c4eb07a99753935057ff91d9a8966d9d613
push id27555
push userryanvm@gmail.com
push dateFri, 26 Sep 2014 20:30:28 +0000
treeherderautoland@4ff52be673f6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1067899
milestone35.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 1067899 - Add autocomplete result type for arbitrary URLs. r=mak
browser/base/content/urlbarBindings.xml
toolkit/components/places/PlacesUtils.jsm
toolkit/components/places/UnifiedComplete.js
toolkit/components/places/tests/unifiedcomplete/test_tabmatches.js
toolkit/components/places/tests/unifiedcomplete/test_visiturl.js
toolkit/components/places/tests/unifiedcomplete/xpcshell.ini
toolkit/content/widgets/autocomplete.xml
toolkit/themes/linux/global/autocomplete.css
toolkit/themes/osx/global/autocomplete.css
toolkit/themes/windows/global/autocomplete.css
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -146,17 +146,18 @@
         <parameter name="aValue"/>
         <body><![CDATA[
           this._value = aValue;
           var returnValue = aValue;
           var action = this._parseActionUrl(aValue);
 
           if (action) {
             switch (action.type) {
-              case "switchtab": {
+              case "switchtab": // Fall through.
+              case "visiturl": {
                 returnValue = action.params.url;
                 break;
               }
               case "keyword": // Fall through.
               case "searchengine": {
                 returnValue = action.params.input;
                 break;
               }
@@ -322,16 +323,18 @@
               } else if (action.type == "keyword") {
                 url = action.params.url;
               } else if (action.type == "searchengine") {
                 let engine = Services.search.getEngineByName(action.params.engineName);
                 let submission = engine.getSubmission(action.params.searchQuery);
 
                 url = submission.uri.spec;
                 postData = submission.postData;
+              } else if (action.type == "visiturl") {
+                url = action.params.url;
               }
             }
             continueOperation.call(this);
           }
           else {
             this._canonizeURL(aTriggeringEvent, response => {
               [url, postData, mayInheritPrincipal] = response;
               if (url) {
@@ -1017,18 +1020,19 @@
             controller.handleEscape();
 
             // Check if this is meant to be an action
             let action = this.mInput._parseActionUrl(url);
             if (action) {
               // TODO (bug 1054816): Centralise the implementation of actions
               // into a JS module.
               switch (action.type) {
-                case "switchtab": //Fall through.
-                case "keyword": {
+                case "switchtab": // Fall through.
+                case "keyword": // Fall through.
+                case "visiturl": {
                   url = action.params.url;
                   break;
                 }
                 case "searchengine": {
                   let engine = Services.search.getEngineByName(action.params.engineName);
                   let submission = engine.getSubmission(action.params.searchQuery);
                   url = submission.uri.spec;
                   options.postData = submission.postData;
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -1509,16 +1509,39 @@ this.PlacesUtils = {
         } else {
           deferred.reject();
         }
       });
     return deferred.promise;
   },
 
   /**
+   * Gets the favicon link url (moz-anno:) for a given page url.
+   *
+   * @param aPageURL url of the page to lookup the favicon for.
+   * @resolves to the nsIURL of the favicon link
+   * @rejects if the given url has no associated favicon.
+   */
+  promiseFaviconLinkUrl: function (aPageUrl) {
+    let deferred = Promise.defer();
+    if (!(aPageUrl instanceof Ci.nsIURI))
+      aPageUrl = NetUtil.newURI(aPageUrl);
+
+    PlacesUtils.favicons.getFaviconURLForPage(aPageUrl, uri => {
+      if (uri) {
+        uri = PlacesUtils.favicons.getFaviconLinkForIcon(uri);
+        deferred.resolve(uri);
+      } else {
+        deferred.reject();
+      }
+    });
+    return deferred.promise;
+  },
+
+  /**
    * Returns the passed URL with a #-moz-resolution fragment
    * for the specified dimensions and devicePixelRatio.
    *
    * @param aWindow
    *        A window from where we want to get the device
    *        pixel Ratio
    *
    * @param aURL
--- a/toolkit/components/places/UnifiedComplete.js
+++ b/toolkit/components/places/UnifiedComplete.js
@@ -706,39 +706,53 @@ Search.prototype = {
       hasFirstResult = true;
     }
 
     if (this._enableActions && !hasFirstResult) {
       // If it's not a bookmarked keyword, then it may be a search engine
       // with an alias - which works like a keyword.
       hasFirstResult = yield this._matchSearchEngineAlias();
     }
-
     let shouldAutofill = this._shouldAutofill;
     if (this.pending && !hasFirstResult && shouldAutofill) {
       // Or it may look like a URL we know about from search engines.
       hasFirstResult = yield this._matchSearchEngineUrl();
     }
 
     if (this.pending && !hasFirstResult && shouldAutofill) {
       // It may also look like a URL we know from the database.
+      // Here we can only try to predict whether the URL autofill query is
+      // likely to return a result.  If the prediction ends up being wrong,
+      // later we will need to make up for the lack of a special first result.
       hasFirstResult = yield this._matchKnownUrl(conn, queries);
     }
 
     if (this.pending && this._enableActions && !hasFirstResult) {
-      // When all else fails, we search using the current search engine.
-      yield this._matchCurrentSearchEngine();
+      // If we don't have a result that matches what we know about, then
+      // we use a fallback for things we don't know about.
+      yield this._matchHeuristicFallback();
     }
 
+    // IMPORTANT: No other first result heuristics should run after
+    // _matchHeuristicFallback().
+
     yield this._sleep(Prefs.delay);
     if (!this.pending)
       return;
 
     for (let [query, params] of queries) {
-      yield conn.executeCached(query, params, this._onResultRow.bind(this));
+      let hasResult = yield conn.executeCached(query, params, this._onResultRow.bind(this));
+
+      if (this.pending && params.query_type == QUERYTYPE_AUTOFILL_URL &&
+          !hasResult) {
+        // If we predicted that our URL autofill query might have gotten a
+        // result, but it didn't, then we need to recover.
+        yield this._matchHeuristicFallback();
+      }
+
       if (!this.pending)
         return;
     }
 
     // If we do not have enough results, and our match type is
     // MATCH_BOUNDARY_ANYWHERE, search again with MATCH_ANYWHERE to get more
     // results.
     if (this._matchBehavior == MATCH_BOUNDARY_ANYWHERE &&
@@ -876,16 +890,80 @@ Search.prototype = {
       comment: match.engineName,
       icon: match.iconUrl,
       style: "action searchengine",
       finalCompleteValue: this._trimmedOriginalSearchString,
       frecency: FRECENCY_SEARCHENGINES_DEFAULT,
     });
   },
 
+  // These are separated out so we can run them in two distinct cases:
+  // (1) We didn't match on anything that we know about
+  // (2) Our predictive query for URL autofill thought we may get a result,
+  //     but we didn't.
+  _matchHeuristicFallback: function* () {
+    // We may not have auto-filled, but this may still look like a URL.
+    let hasFirstResult = yield this._matchUnknownUrl();
+    // However, even if the input is a valid URL, we may not want to use
+    // it as such. This can happen if the host would require whitelisting,
+    // but isn't in the whitelist.
+
+    if (this.pending && !hasFirstResult) {
+      // When all else fails, we search using the current search engine.
+      yield this._matchCurrentSearchEngine();
+    }
+  },
+
+  // TODO (bug 1054814): Use visited URLs to inform which scheme to use, if the
+  // scheme isn't specificed.
+  _matchUnknownUrl: function* () {
+    let flags = Ci.nsIURIFixup.FIXUP_FLAG_FIX_SCHEME_TYPOS |
+                Ci.nsIURIFixup.FIXUP_FLAG_REQUIRE_WHITELISTED_HOST;
+    let fixupInfo = null;
+    try {
+      fixupInfo = Services.uriFixup.getFixupURIInfo(this._originalSearchString,
+                                                    flags);
+    } catch (e) {
+      return false;
+    }
+
+    let uri = fixupInfo.preferredURI;
+    // 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 (!uri || (hostExpected.has(uri.scheme) && !uri.host))
+      return false;
+
+    let value = makeActionURL("visiturl", {
+      url: uri.spec,
+      input: this._originalSearchString,
+    });
+
+    let match = {
+      value: value,
+      comment: uri.spec,
+      style: "action visiturl",
+      finalCompleteValue: this._originalSearchString,
+      frecency: 0,
+    };
+
+    try {
+      let favicon = yield PlacesUtils.promiseFaviconLinkUrl(uri);
+      if (favicon)
+        match.icon = favicon.spec;
+    } catch (e) {
+      // It's possible we don't have a favicon for this - and that's ok.
+    };
+
+    this._addMatch(match);
+    return true;
+  },
+
   _onResultRow: function (row) {
     TelemetryStopwatch.finish(TELEMETRY_1ST_RESULT);
     let queryType = row.getResultByIndex(QUERYINDEX_QUERYTYPE);
     let match;
     switch (queryType) {
       case QUERYTYPE_AUTOFILL_HOST:
         this._result.setDefaultIndex(0);
         // Fall through.
--- a/toolkit/components/places/tests/unifiedcomplete/test_tabmatches.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_tabmatches.js
@@ -22,17 +22,17 @@ add_task(function* test_tab_matches() {
   // Pages that cannot be registered in history.
   addOpenPages(uri3, 1);
   addOpenPages(uri4, 1);
 
   do_log_info("two results, normal result is a tab match");
   yield check_autocomplete({
     search: "abc.com",
     searchParam: "enable-actions",
-    matches: [ { uri: makeActionURI("searchengine", {engineName: "MozSearch", input: "abc.com", searchQuery: "abc.com"}), title: "MozSearch" },
+    matches: [ { uri: makeActionURI("visiturl", {url: "http://abc.com/", input: "abc.com"}), title: "http://abc.com/" },
                { uri: makeActionURI("switchtab", {url: "http://abc.com/"}), title: "ABC rocks" } ]
   });
 
   do_log_info("three results, one tab match");
   yield check_autocomplete({
     search: "abc",
     searchParam: "enable-actions",
     matches: [ { uri: makeActionURI("searchengine", {engineName: "MozSearch", input: "abc", searchQuery: "abc"}), title: "MozSearch" },
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/unifiedcomplete/test_visiturl.js
@@ -0,0 +1,53 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+
+add_task(function*() {
+  do_log_info("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/" }, ]
+  });
+
+  do_log_info("visit url, with protocol");
+  yield check_autocomplete({
+    search: "https://mozilla.org",
+    searchParam: "enable-actions",
+    matches: [ { uri: makeActionURI("visiturl", {url: "https://mozilla.org/", input: "https://mozilla.org"}), title: "https://mozilla.org/" }, ]
+  });
+
+  do_log_info("visit url, about: protocol (no host)");
+  yield check_autocomplete({
+    search: "about:config",
+    searchParam: "enable-actions",
+    matches: [ { uri: makeActionURI("visiturl", {url: "about:config", input: "about:config"}), title: "about:config" }, ]
+  });
+
+  // This is distinct because of how we predict being able to url autofill via
+  // host lookups.
+  do_log_info("visit url, host matching visited host but not visited url");
+  yield promiseAddVisits([
+    { uri: NetUtil.newURI("http://mozilla.org/wine/"), title: "Mozilla Wine", transition: TRANSITION_TYPED },
+  ]);
+  yield check_autocomplete({
+    search: "mozilla.org/rum",
+    searchParam: "enable-actions",
+    matches: [ { uri: makeActionURI("visiturl", {url: "http://mozilla.org/rum", input: "mozilla.org/rum"}), title: "http://mozilla.org/rum" }, ]
+  });
+
+  // And hosts with no dot in them are special, due to requiring whitelisting.
+  do_log_info("visit url, host matching visited host but not visited url, non-whitelisted host");
+  Services.search.addEngineWithDetails("MozSearch", "", "", "", "GET",
+                                       "http://s.example.com/search");
+  let engine = Services.search.getEngineByName("MozSearch");
+  Services.search.currentEngine = engine;
+  yield promiseAddVisits([
+    { uri: NetUtil.newURI("http://mozilla/bourbon/"), title: "Mozilla Bourbon", transition: TRANSITION_TYPED },
+  ]);
+  yield check_autocomplete({
+    search: "mozilla/rum",
+    searchParam: "enable-actions",
+    matches: [ { uri: makeActionURI("searchengine", {engineName: "MozSearch", input: "mozilla/rum", searchQuery: "mozilla/rum"}), title: "MozSearch" }, ]
+  });
+});
--- a/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini
+++ b/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini
@@ -30,10 +30,11 @@ tail =
 [test_searchEngine_current.js]
 [test_searchEngine_host.js]
 [test_searchEngine_restyle.js]
 [test_special_search.js]
 [test_swap_protocol.js]
 [test_tabmatches.js]
 [test_trimming.js]
 [test_typed.js]
+[test_visiturl.js]
 [test_word_boundary_search.js]
 [test_zero_frecency.js]
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -1583,16 +1583,24 @@ extends="chrome://global/content/binding
               let sourceStr = this._stringBundle.GetStringFromName("searchWithEngineForQuery");
               title = this._generateEmphasisPairs(sourceStr, [
                                                     [action.params.engineName, false],
                                                     [action.params.searchQuery, true]
                                                   ]);
               // Remove the image, so we can style it ourselves with a generic
               // search icon.
               this.removeAttribute("image");
+            } else if (action.type == "visiturl") {
+              emphasiseUrl = false;
+              url = action.params.url;
+
+              let sourceStr = this._stringBundle.GetStringFromName("visitURL");
+              title = this._generateEmphasisPairs(sourceStr, [
+                                                    [trimURL(url), true],
+                                                  ]);
             }
 
             // Remove the "action" substring so that the correct style, if any,
             // is applied below.
             types.delete("action");
           }
 
           // Check if we have a search engine name
--- a/toolkit/themes/linux/global/autocomplete.css
+++ b/toolkit/themes/linux/global/autocomplete.css
@@ -114,22 +114,24 @@ treechildren.autocomplete-treebody::-moz
 
 .autocomplete-richlistitem {
   padding: 6px 2px;
   color: MenuText;
 }
 
 .autocomplete-richlistitem[actiontype="keyword"] .ac-url-box,
 .autocomplete-richlistitem[actiontype="searchengine"] .ac-url-box,
+.autocomplete-richlistitem[actiontype="visiturl"] .ac-url-box,
 .autocomplete-richlistitem[type~="autofill"] .ac-url-box {
   display: none;
 }
 
 .autocomplete-richlistitem[actiontype="keyword"] .ac-title-box,
 .autocomplete-richlistitem[actiontype="searchengine"] .ac-title-box,
+.autocomplete-richlistitem[actiontype="visiturl"] .ac-title-box,
 .autocomplete-richlistitem[type~="autofill"] .ac-title-box {
   margin-top: 12px;
   margin-bottom: 12px;
 }
 
 .ac-url-box {
   margin-top: 1px;
 }
--- a/toolkit/themes/osx/global/autocomplete.css
+++ b/toolkit/themes/osx/global/autocomplete.css
@@ -99,22 +99,24 @@ treechildren.autocomplete-treebody::-moz
 }
 
 .autocomplete-richlistitem {
   padding: 5px 2px;
 }
 
 .autocomplete-richlistitem[actiontype="keyword"] .ac-url-box,
 .autocomplete-richlistitem[actiontype="searchengine"] .ac-url-box,
+.autocomplete-richlistitem[actiontype="visiturl"] .ac-url-box,
 .autocomplete-richlistitem[type~="autofill"] .ac-url-box {
   display: none;
 }
 
 .autocomplete-richlistitem[actiontype="keyword"] .ac-title-box,
 .autocomplete-richlistitem[actiontype="searchengine"] .ac-title-box,
+.autocomplete-richlistitem[actiontype="visiturl"] .ac-title-box,
 .autocomplete-richlistitem[type~="autofill"] .ac-title-box {
   margin-top: 12px;
   margin-bottom: 12px;
 }
 
 .ac-url-box {
   margin-top: 1px;
 }
--- a/toolkit/themes/windows/global/autocomplete.css
+++ b/toolkit/themes/windows/global/autocomplete.css
@@ -127,22 +127,24 @@ treechildren.autocomplete-treebody::-moz
     -moz-outline-radius: 3px;
     outline-offset: -2px;
   }
 }
 %endif
 
 .autocomplete-richlistitem[actiontype="keyword"] .ac-url-box,
 .autocomplete-richlistitem[actiontype="searchengine"] .ac-url-box,
+.autocomplete-richlistitem[actiontype="visiturl"] .ac-url-box,
 .autocomplete-richlistitem[type~="autofill"] .ac-url-box {
   display: none;
 }
 
 .autocomplete-richlistitem[actiontype="keyword"] .ac-title-box,
 .autocomplete-richlistitem[actiontype="searchengine"] .ac-title-box,
+.autocomplete-richlistitem[actiontype="visiturl"] .ac-title-box,
 .autocomplete-richlistitem[type~="autofill"] .ac-title-box {
   margin-top: 12px;
   margin-bottom: 12px;
 }
 
 .ac-title-box {
   margin-top: 4px;
 }