Bug 1363621 - Favicon in the first location bar suggestion flickers when typing url. r=florian
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 25 May 2017 17:12:36 +0200
changeset 409701 5df9b21d7a40944c9a01f75abfa63c7dba183ed4
parent 409700 244e7cfea7317c67c322a2b4fba6c2b2e64feb34
child 409702 a7fdbcd53500160faee6ffa43a88ab90ed5e1829
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersflorian
bugs1363621
milestone55.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 1363621 - Favicon in the first location bar suggestion flickers when typing url. r=florian MozReview-Commit-ID: JtoBMl0BQFC
toolkit/components/places/UnifiedComplete.js
--- a/toolkit/components/places/UnifiedComplete.js
+++ b/toolkit/components/places/UnifiedComplete.js
@@ -1594,18 +1594,18 @@ Search.prototype = {
     if (!fixupInfo.fixedURI || fixupInfo.keywordAsSent)
       return false;
 
     let uri = fixupInfo.fixedURI;
     // 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"]);
-    if (hostExpected.has(uri.scheme) && !uri.host)
+    let hostExpected = ["http", "https", "ftp", "chrome"].includes(uri.scheme);
+    if (hostExpected && !uri.host)
       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;
@@ -1616,19 +1616,29 @@ Search.prototype = {
       input: this._originalSearchString,
     });
 
     let match = {
       value,
       comment: displayURL,
       style: "action visiturl",
       frecency: 0,
-      icon: "page-icon:" + escapedURL
     };
 
+    // We don't know if this url is in Places or not, and checking that would
+    // be expensive. Thus we also don't know if we may have an icon.
+    // If we'd just try to fetch the icon for the typed string, we'd cause icon
+    // flicker, since the url keeps changing while the user types.
+    // By default we won't provide an icon, but for the subset of urls with a
+    // host we'll check for a typed slash and set favicon for the host part.
+    if (hostExpected &&
+        (this._trimmedOriginalSearchString.endsWith("/") || uri.path.length > 1)) {
+      match.icon = `page-icon:${uri.prePath}/`;
+    }
+
     this._addMatch(match);
     return true;
   },
 
   _onResultRow(row) {
     if (this._localMatchesCount == 0) {
       TelemetryStopwatch.finish(TELEMETRY_1ST_RESULT, this);
     }