Bug 1401777 - don't use rich icons for tabs. r=Mardak
☠☠ backed out by 076a7c57b868 ☠ ☠
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 21 Sep 2017 10:21:50 +0200
changeset 382670 698252497ed8387d10de995262938316faaf41de
parent 382650 8676ddcded7b5e2079f5d399fb462bf2f9a3d3b8
child 382671 f44a80769c80ef7aec9d175f88dc68897928275d
push id95394
push userarchaeopteryx@coole-files.de
push dateMon, 25 Sep 2017 10:03:57 +0000
treeherdermozilla-inbound@ff48fab67d3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMardak
bugs1401777
milestone58.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 1401777 - don't use rich icons for tabs. r=Mardak MozReview-Commit-ID: DcqxEme7hfx
browser/base/content/browser.js
browser/base/content/tabbrowser.xml
browser/base/content/test/general/browser_discovery.js
browser/base/content/test/general/browser_favicon_change.js
browser/components/uitour/test/browser_UITour_availableTargets.js
browser/modules/ContentLinkHandler.jsm
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -3696,17 +3696,18 @@ const DOMEventHandler = {
   receiveMessage(aMsg) {
     switch (aMsg.name) {
       case "Link:AddFeed":
         let link = {type: aMsg.data.type, href: aMsg.data.href, title: aMsg.data.title};
         FeedHandler.addFeed(link, aMsg.target);
         break;
 
       case "Link:SetIcon":
-        this.setIcon(aMsg.target, aMsg.data.url, aMsg.data.loadingPrincipal, aMsg.data.requestContextID);
+        this.setIcon(aMsg.target, aMsg.data.url, aMsg.data.loadingPrincipal,
+                     aMsg.data.requestContextID, aMsg.data.canUseForTab);
         break;
 
       case "Link:AddSearch":
         this.addSearch(aMsg.target, aMsg.data.engine, aMsg.data.url);
         break;
 
       case "Meta:SetPageInfo":
         this.setPageInfo(aMsg.data);
@@ -3715,25 +3716,40 @@ const DOMEventHandler = {
   },
 
   setPageInfo(aData) {
     const {url, description, previewImageURL} = aData;
     gBrowser.setPageInfo(url, description, previewImageURL);
     return true;
   },
 
-  setIcon(aBrowser, aURL, aLoadingPrincipal, aRequestContextID) {
+  setIcon(aBrowser, aURL, aLoadingPrincipal, aRequestContextID = 0, aCanUseForTab = true) {
     if (gBrowser.isFailedIcon(aURL))
       return false;
 
     let tab = gBrowser.getTabForBrowser(aBrowser);
     if (!tab)
       return false;
 
-    gBrowser.setIcon(tab, aURL, aLoadingPrincipal, aRequestContextID);
+    let loadingPrincipal = aLoadingPrincipal ||
+                           Services.scriptSecurityManager.getSystemPrincipal();
+    if (aURL) {
+      try {
+        if (!(aURL instanceof Ci.nsIURI)) {
+          aURL = makeURI(aURL);
+        }
+        PlacesUIUtils.loadFavicon(aBrowser, loadingPrincipal, aURL, aRequestContextID);
+      } catch (ex) {
+        Components.utils.reportError(ex);
+      }
+    }
+
+    if (aCanUseForTab) {
+      gBrowser.setIcon(tab, aURL, loadingPrincipal, aRequestContextID);
+    }
     return true;
   },
 
   addSearch(aBrowser, aEngine, aURL) {
     let tab = gBrowser.getTabForBrowser(aBrowser);
     if (!tab)
       return;
 
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -975,28 +975,19 @@
         <parameter name="aTab"/>
         <parameter name="aURI"/>
         <parameter name="aLoadingPrincipal"/>
         <parameter name="aRequestContextID"/>
         <body>
           <![CDATA[
             let browser = this.getBrowserForTab(aTab);
             browser.mIconURL = aURI instanceof Ci.nsIURI ? aURI.spec : aURI;
-            let loadingPrincipal = aLoadingPrincipal
-              ? aLoadingPrincipal
-              : Services.scriptSecurityManager.getSystemPrincipal();
+            let loadingPrincipal = aLoadingPrincipal ||
+                                   Services.scriptSecurityManager.getSystemPrincipal();
             let requestContextID = aRequestContextID || 0;
-
-            if (aURI) {
-              if (!(aURI instanceof Ci.nsIURI)) {
-                aURI = makeURI(aURI);
-              }
-              PlacesUIUtils.loadFavicon(browser, loadingPrincipal, aURI, requestContextID);
-            }
-
             let sizedIconUrl = browser.mIconURL || "";
             if (sizedIconUrl != aTab.getAttribute("image")) {
               if (sizedIconUrl) {
                 if (!browser.mIconLoadingPrincipal ||
                     !browser.mIconLoadingPrincipal.equals(loadingPrincipal)) {
                   aTab.setAttribute("iconLoadingPrincipal",
                     this.serializationHelper.serializeToString(loadingPrincipal));
                   aTab.setAttribute("requestcontextid", requestContextID);
--- a/browser/base/content/test/general/browser_discovery.js
+++ b/browser/base/content/test/general/browser_discovery.js
@@ -1,225 +1,171 @@
-var browser;
-
-function doc() {
-  return browser.contentDocument;
-}
-
-function setHandlerFunc(aResultFunc) {
-  gBrowser.addEventListener("DOMLinkAdded", function(event) {
-    executeSoon(aResultFunc);
-  }, {once: true});
-}
+/* eslint-disable mozilla/no-arbitrary-setTimeout */
 
-function test() {
-  waitForExplicitFinish();
+add_task(async function() {
+  let rootDir = getRootDirectory(gTestPath);
+  let url = rootDir + "discovery.html";
+  info("Test icons discovery");
+  await BrowserTestUtils.withNewTab(url, iconDiscovery);
+  info("Test search discovery");
+  await BrowserTestUtils.withNewTab(url, searchDiscovery);
+});
 
-  gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser);
-  browser = gBrowser.selectedBrowser;
-  browser.addEventListener("load", function(event) {
-    iconDiscovery();
-  }, {capture: true, once: true});
-  var rootDir = getRootDirectory(gTestPath);
-  content.location = rootDir + "discovery.html";
-}
-
-var iconDiscoveryTests = [
+let iconDiscoveryTests = [
   { text: "rel icon discovered" },
   { rel: "abcdefg icon qwerty", text: "rel may contain additional rels separated by spaces" },
   { rel: "ICON", text: "rel is case insensitive" },
   { rel: "shortcut-icon", pass: false, text: "rel shortcut-icon not discovered" },
   { href: "moz.png", text: "relative href works" },
   { href: "notthere.png", text: "404'd icon is removed properly" },
   { href: "data:image/x-icon,%00", type: "image/x-icon", text: "data: URIs work" },
-  { type: "image/png; charset=utf-8", text: "type may have optional parameters (RFC2046)" }
+  { type: "image/png; charset=utf-8", text: "type may have optional parameters (RFC2046)" },
+  { richIcon: true, rel: "apple-touch-icon", text: "apple-touch-icon discovered" },
+  { richIcon: true, rel: "apple-touch-icon-precomposed", text: "apple-touch-icon-precomposed discovered" },
+  { richIcon: true, rel: "fluid-icon", text: "fluid-icon discovered" },
+  { richIcon: true, rel: "unknown-icon", pass: false, text: "unknown icon not discovered" }
 ];
 
-function runIconDiscoveryTest() {
-  let testCase = iconDiscoveryTests[0];
-  let head = doc().getElementById("linkparent");
-
-  // Because there is debounce logic in ContentLinkHandler.jsm to reduce the
-  // favicon loads, we have to wait some time before checking that icon was
-  // stored properly.
-  BrowserTestUtils.waitForCondition(() => {
-    return gBrowser.getIcon() != null;
-  }, "wait for icon load to finish", 100, 5)
-  .then(() => {
-    ok(testCase.pass, testCase.text);
-  })
-  .catch(() => {
-    ok(!testCase.pass, testCase.text);
-  })
-  .then(() => {
-    head.removeChild(head.getElementsByTagName("link")[0]);
-    iconDiscoveryTests.shift();
-    iconDiscovery(); // Run the next test.
-  });
-}
-
-function iconDiscovery() {
-  if (iconDiscoveryTests.length) {
-    setHandlerFunc(runIconDiscoveryTest);
-    gBrowser.setIcon(gBrowser.selectedTab, null,
-                     Services.scriptSecurityManager.getSystemPrincipal());
-
-    var testCase = iconDiscoveryTests[0];
-    var head = doc().getElementById("linkparent");
-    var link = doc().createElement("link");
-
-    var rootDir = getRootDirectory(gTestPath);
-    var rel = testCase.rel || "icon";
-    var href = testCase.href || rootDir + "moz.png";
-    var type = testCase.type || "image/png";
+async function iconDiscovery() {
+  let mm = window.messageManager;
+  for (let testCase of iconDiscoveryTests) {
     if (testCase.pass == undefined)
       testCase.pass = true;
+    testCase.rootDir = getRootDirectory(gTestPath);
 
-    link.rel = rel;
-    link.href = href;
-    link.type = type;
-    head.appendChild(link);
-  } else {
-    richIconDiscovery();
+    // Clear the current icon.
+    gBrowser.setIcon(gBrowser.selectedTab, null);
+
+    let promiseLinkAdded =
+      BrowserTestUtils.waitForEvent(gBrowser.selectedBrowser, "DOMLinkAdded",
+                                    false, null, true);
+    let promiseMessage = new Promise(resolve => {
+      testCase.listener = resolve;
+      mm.addMessageListener("Link:SetIcon", testCase.listener);
+    });
+
+    await ContentTask.spawn(gBrowser.selectedBrowser, testCase, test => {
+      let doc = content.document;
+      let head = doc.getElementById("linkparent");
+      let link = doc.createElement("link");
+      link.rel = test.rel || "icon";
+      link.href = test.href || test.rootDir + "moz.png";
+      link.type = test.type || "image/png";
+      head.appendChild(link);
+    });
+
+    await promiseLinkAdded;
+
+    if (!testCase.richIcon) {
+      // Because there is debounce logic in ContentLinkHandler.jsm to reduce the
+      // favicon loads, we have to wait some time before checking that icon was
+      // stored properly.
+      try {
+        await BrowserTestUtils.waitForCondition(() => {
+          return gBrowser.getIcon() != null;
+        }, "wait for icon load to finish", 100, 5);
+        ok(testCase.pass, testCase.text);
+      } catch (ex) {
+        ok(!testCase.pass, testCase.text);
+      }
+    } else {
+      // Rich icons are not set as tab icons, so just check for the SetIcon message.
+      try {
+        let msg = await Promise.race([promiseMessage,
+                                      new Promise((resolve, reject) => setTimeout(reject(), 2000))]);
+        is(msg.data.canUseForTab, false, "Rich icons cannot be used for tabs");
+        ok(testCase.pass, testCase.text);
+      } catch (ex) {
+        ok(!testCase.pass, testCase.text);
+      } finally {
+        mm.removeMessageListener("Link:SetIcon", testCase.listener);
+      }
+    }
+
+    await ContentTask.spawn(gBrowser.selectedBrowser, null, () => {
+      let head = content.document.getElementById("linkparent");
+      head.removeChild(head.getElementsByTagName("link")[0]);
+    });
   }
 }
 
-let richIconDiscoveryTests = [
-  { rel: "apple-touch-icon", text: "apple-touch-icon discovered" },
-  { rel: "apple-touch-icon-precomposed", text: "apple-touch-icon-precomposed discovered" },
-  { rel: "fluid-icon", text: "fluid-icon discovered" },
-  { rel: "unknown-icon", pass: false, text: "unknown icon not discovered" }
-];
-
-function runRichIconDiscoveryTest() {
-  let testCase = richIconDiscoveryTests[0];
-  let head = doc().getElementById("linkparent");
-
-  // Because there is debounce logic in ContentLinkHandler.jsm to reduce the
-  // favicon loads, we have to wait some time before checking that icon was
-  // stored properly.
-  BrowserTestUtils.waitForCondition(() => {
-    return gBrowser.getIcon() != null;
-  }, "wait for icon load to finish", 100, 5)
-  .then(() => {
-    ok(testCase.pass, testCase.text);
-  })
-  .catch(() => {
-    ok(!testCase.pass, testCase.text);
-  })
-  .then(() => {
-    head.removeChild(head.getElementsByTagName("link")[0]);
-    richIconDiscoveryTests.shift();
-    richIconDiscovery(); // Run the next test.
-  });
-}
-
-function richIconDiscovery() {
-  if (richIconDiscoveryTests.length) {
-    setHandlerFunc(runRichIconDiscoveryTest);
-    gBrowser.setIcon(gBrowser.selectedTab, null,
-                     Services.scriptSecurityManager.getSystemPrincipal()
-    );
-
-    let testCase = richIconDiscoveryTests[0];
-    let head = doc().getElementById("linkparent");
-    let link = doc().createElement("link");
-
-    let rel = testCase.rel;
-    let rootDir = getRootDirectory(gTestPath);
-    let href = testCase.href || rootDir + "moz.png";
-    let type = testCase.type || "image/png";
-    if (testCase.pass === undefined)
-      testCase.pass = true;
-
-    link.rel = rel;
-    link.href = href;
-    link.type = type;
-    head.appendChild(link);
-  } else {
-    searchDiscovery();
-  }
-}
-
-var searchDiscoveryTests = [
+let searchDiscoveryTests = [
   { text: "rel search discovered" },
   { rel: "SEARCH", text: "rel is case insensitive" },
   { rel: "-search-", pass: false, text: "rel -search- not discovered" },
   { rel: "foo bar baz search quux", text: "rel may contain additional rels separated by spaces" },
   { href: "https://not.mozilla.com", text: "HTTPS ok" },
   { href: "ftp://not.mozilla.com", text: "FTP ok" },
   { href: "data:text/foo,foo", pass: false, text: "data URI not permitted" },
   { href: "javascript:alert(0)", pass: false, text: "JS URI not permitted" },
   { type: "APPLICATION/OPENSEARCHDESCRIPTION+XML", text: "type is case insensitve" },
   { type: " application/opensearchdescription+xml ", text: "type may contain extra whitespace" },
   { type: "application/opensearchdescription+xml; charset=utf-8", text: "type may have optional parameters (RFC2046)" },
   { type: "aapplication/opensearchdescription+xml", pass: false, text: "type should not be loosely matched" },
   { rel: "search search search", count: 1, text: "only one engine should be added" }
 ];
 
-function runSearchDiscoveryTest() {
-  var testCase = searchDiscoveryTests[0];
-  var title = testCase.title || searchDiscoveryTests.length;
-  if (browser.engines) {
-    var hasEngine = (testCase.count) ? (browser.engines[0].title == title &&
-                                        browser.engines.length == testCase.count) :
-                                       (browser.engines[0].title == title);
-    ok(hasEngine, testCase.text);
-    browser.engines = null;
-  } else
-    ok(!testCase.pass, testCase.text);
-
-  searchDiscoveryTests.shift();
-  searchDiscovery(); // Run the next test.
-}
+async function searchDiscovery() {
+  let browser = gBrowser.selectedBrowser;
 
-// This handler is called twice, once for each added link element.
-// Only want to check once the second link element has been added.
-var ranOnce = false;
-function runMultipleEnginesTestAndFinalize() {
-  if (!ranOnce) {
-    ranOnce = true;
-    return;
-  }
-  ok(browser.engines, "has engines");
-  is(browser.engines.length, 1, "only one engine");
-  is(browser.engines[0].uri, "http://first.mozilla.com/search.xml", "first engine wins");
-
-  gBrowser.removeCurrentTab();
-  finish();
-}
-
-function searchDiscovery() {
-  let head = doc().getElementById("linkparent");
-
-  if (searchDiscoveryTests.length) {
-    setHandlerFunc(runSearchDiscoveryTest);
-    let testCase = searchDiscoveryTests[0];
-    let link = doc().createElement("link");
-
-    let rel = testCase.rel || "search";
-    let href = testCase.href || "http://so.not.here.mozilla.com/search.xml";
-    let type = testCase.type || "application/opensearchdescription+xml";
-    let title = testCase.title || searchDiscoveryTests.length;
+  for (let testCase of searchDiscoveryTests) {
     if (testCase.pass == undefined)
       testCase.pass = true;
+    testCase.title = testCase.title || searchDiscoveryTests.indexOf(testCase);
 
-    link.rel = rel;
-    link.href = href;
-    link.type = type;
-    link.title = title;
-    head.appendChild(link);
-  } else {
-    setHandlerFunc(runMultipleEnginesTestAndFinalize);
-    setHandlerFunc(runMultipleEnginesTestAndFinalize);
-    // Test multiple engines with the same title
-    let link = doc().createElement("link");
+    let promiseLinkAdded =
+      BrowserTestUtils.waitForEvent(gBrowser.selectedBrowser, "DOMLinkAdded",
+                                    false, null, true);
+
+    await ContentTask.spawn(gBrowser.selectedBrowser, testCase, test => {
+      let doc = content.document;
+      let head = doc.getElementById("linkparent");
+      let link = doc.createElement("link");
+      link.rel = test.rel || "search";
+      link.href = test.href || "http://so.not.here.mozilla.com/search.xml";
+      link.type = test.type || "application/opensearchdescription+xml";
+      link.title = test.title;
+      head.appendChild(link);
+    });
+
+    await promiseLinkAdded;
+    await new Promise(resolve => executeSoon(resolve));
+
+    if (browser.engines) {
+      info(`Found ${browser.engines.length} engines`);
+      info(`First engine title: ${browser.engines[0].title}`);
+      let hasEngine = testCase.count ?
+        (browser.engines[0].title == testCase.title && browser.engines.length == testCase.count) :
+        (browser.engines[0].title == testCase.title);
+      ok(hasEngine, testCase.text);
+      browser.engines = null;
+    } else {
+      ok(!testCase.pass, testCase.text);
+    }
+  }
+
+  info("Test multiple engines with the same title");
+  let count = 0;
+  let promiseLinkAdded =
+    BrowserTestUtils.waitForEvent(gBrowser.selectedBrowser, "DOMLinkAdded",
+                                  false, () => ++count == 2, true);
+  await ContentTask.spawn(gBrowser.selectedBrowser, null, () => {
+    let doc = content.document;
+    let head = doc.getElementById("linkparent");
+    let link = doc.createElement("link");
     link.rel = "search";
     link.href = "http://first.mozilla.com/search.xml";
     link.type = "application/opensearchdescription+xml";
     link.title = "Test Engine";
     let link2 = link.cloneNode(false);
     link2.href = "http://second.mozilla.com/search.xml";
-
     head.appendChild(link);
     head.appendChild(link2);
-  }
+  });
+
+  await promiseLinkAdded;
+  await new Promise(resolve => executeSoon(resolve));
+
+  ok(browser.engines, "has engines");
+  is(browser.engines.length, 1, "only one engine");
+  is(browser.engines[0].uri, "http://first.mozilla.com/search.xml", "first engine wins");
+  browser.engines = null;
 }
--- a/browser/base/content/test/general/browser_favicon_change.js
+++ b/browser/base/content/test/general/browser_favicon_change.js
@@ -12,17 +12,18 @@ add_task(async function() {
   let expectedFavicon = "http://example.org/one-icon";
   let haveChanged = PromiseUtils.defer();
   let observer = new MutationObserver(function(mutations) {
     for (let mut of mutations) {
       if (mut.attributeName != "image") {
         continue;
       }
       let imageVal = extraTab.getAttribute("image").replace(/#.*$/, "");
-      if (!imageVal) {
+      // Ignore chrome favicons set on the tab before the actual page load.
+      if (!imageVal || !imageVal.startsWith("http://example.org/")) {
         // The value gets removed because it doesn't load.
         continue;
       }
       is(imageVal, expectedFavicon, "Favicon image should correspond to expected image.");
       haveChanged.resolve();
     }
   });
   observer.observe(extraTab, {attributes: true});
--- a/browser/components/uitour/test/browser_UITour_availableTargets.js
+++ b/browser/components/uitour/test/browser_UITour_availableTargets.js
@@ -111,24 +111,21 @@ add_UITour_task(async function test_avai
   for (let [ targetName, expectedNodeId ] of expectedActions) {
     await assertTargetNode(targetName, expectedNodeId);
   }
   pageActionsHelper.restoreActionsUrlbarState();
 });
 
 function ok_targets(actualData, expectedTargets) {
   // Depending on how soon after page load this is called, the selected tab icon
-  // may or may not be showing the loading throbber.  Check for its presence and
-  // insert it into expectedTargets if it's visible.
-  let selectedTabIcon =
-    document.getAnonymousElementByAttribute(gBrowser.selectedTab,
-                                            "anonid",
-                                            "tab-icon-image");
-  if (selectedTabIcon && UITour.isElementVisible(selectedTabIcon))
-    expectedTargets.push("selectedTabIcon");
+  // may or may not be showing the loading throbber.  We can't be sure whether
+  // it appears in the list of targets, so remove it.
+  let index = actualData.targets.indexOf("selectedTabIcon");
+  if (index != -1)
+    actualData.targets.splice(index, 1);
 
   ok(Array.isArray(actualData.targets), "data.targets should be an array");
   is(actualData.targets.sort().toString(), expectedTargets.sort().toString(),
      "Targets should be as expected");
 }
 
 async function assertTargetNode(targetName, expectedNodeId) {
   let target = await UITour.getTarget(window, targetName);
--- a/browser/modules/ContentLinkHandler.jsm
+++ b/browser/modules/ContentLinkHandler.jsm
@@ -109,17 +109,19 @@ function getLinkIconURI(aLink) {
  * }.
  * @param {Object} aChromeGlobal A global chrome object.
  */
 function setIconForLink(aIconInfo, aChromeGlobal) {
   aChromeGlobal.sendAsyncMessage(
     "Link:SetIcon",
     { url: aIconInfo.iconUri.spec,
       loadingPrincipal: aIconInfo.loadingPrincipal,
-      requestContextID: aIconInfo.requestContextID });
+      requestContextID: aIconInfo.requestContextID,
+      canUseForTab: !aIconInfo.isRichIcon,
+    });
 }
 
 /*
  * Timeout callback function for loading favicon.
  *
  * @param {Map} aFaviconLoads A map of page URL and FaviconLoad object pairs,
  *   where the FaviconLoad object looks like {
  *     timer: a nsITimer object,
@@ -144,37 +146,41 @@ function faviconTimeoutCallback(aFavicon
   for (let icon of load.iconInfos) {
     if (icon.type === "image/svg+xml" ||
       icon.type === "image/x-icon" ||
       icon.type === "image/vnd.microsoft.icon") {
       preferredIcon = icon;
       continue;
     }
 
-    if (icon.isRichIcon) {
+    // Note that some sites use hi-res icons without specifying them as
+    // apple-touch or fluid icons.
+    if (icon.isRichIcon || icon.width >= FAVICON_RICH_ICON_MIN_WIDTH) {
       if (!largestRichIcon || largestRichIcon.width < icon.width) {
         largestRichIcon = icon;
       }
     } else if (!defaultIcon) {
       defaultIcon = icon;
     }
   }
 
   // Now set the favicons for the page in the following order:
-  // 1. Set the preferred one if any, otherwise use the default one.
-  // 2. Set the best rich icon if any.
+  // 1. Set the best rich icon if any.
+  // 2. Set the preferred one if any, otherwise use the default one.
+  // This order allows smaller icon frames to eventually override rich icon
+  // frames.
+  if (largestRichIcon) {
+    setIconForLink(largestRichIcon, aChromeGlobal);
+  }
   if (preferredIcon) {
     setIconForLink(preferredIcon, aChromeGlobal);
   } else if (defaultIcon) {
     setIconForLink(defaultIcon, aChromeGlobal);
   }
 
-  if (largestRichIcon) {
-    setIconForLink(largestRichIcon, aChromeGlobal);
-  }
   load.timer = null;
   aFaviconLoads.delete(aPageUrl);
 }
 
 /*
  * Get request context ID of the link dom node's document.
  *
  * @param {DOMNode} aLink A link dom node.
@@ -199,22 +205,18 @@ function getLinkRequestContextID(aLink) 
  * @return {bool} Returns true if the link is successfully handled.
  */
 function handleFaviconLink(aLink, aIsRichIcon, aChromeGlobal, aFaviconLoads) {
   let pageUrl = aLink.ownerDocument.documentURI;
   let iconUri = getLinkIconURI(aLink);
   if (!iconUri)
     return false;
 
-  // Extract the size type and width. Note that some sites use hi-res icons
-  // without specifying them as apple-touch or fluid icons.
+  // Extract the size type and width.
   let width = extractIconSize(aLink.sizes);
-  if (width >= FAVICON_RICH_ICON_MIN_WIDTH)
-    aIsRichIcon = true;
-
   let iconInfo = {
     iconUri,
     width,
     isRichIcon: aIsRichIcon,
     type: aLink.type,
     loadingPrincipal: aLink.ownerDocument.nodePrincipal,
     requestContextID: getLinkRequestContextID(aLink)
   };