Bug 1449523 - ContentLinkHandler only picks perfectly sized icons. r=Mardak draft
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 28 Mar 2018 12:40:02 +0200
changeset 774643 125201afee6771d22ed5079e70b966bb0d193f07
parent 773730 5bf126434fac78a31256c994b9dbf4b1031b0350
push id104463
push usermak77@bonardo.net
push dateThu, 29 Mar 2018 09:14:55 +0000
reviewersMardak
bugs1449523
milestone61.0a1
Bug 1449523 - ContentLinkHandler only picks perfectly sized icons. r=Mardak MozReview-Commit-ID: IY5vOIliWWC
browser/base/content/test/favicons/browser_preferred_icons.js
browser/modules/ContentLinkHandler.jsm
--- a/browser/base/content/test/favicons/browser_preferred_icons.js
+++ b/browser/base/content/test/favicons/browser_preferred_icons.js
@@ -159,8 +159,33 @@ add_task(async function guess_invalid() 
     { href: "data:text/plain,icon" },
     { href: "file:///icon" },
     { href: "about:icon" },
   ]);
   await promise;
   // Must have at least one test.
   Assert.ok(true, "The expected icon has been set");
 });
+
+add_task(async function guess_bestSized() {
+  let preferredWidth = 16 * Math.ceil(window.devicePixelRatio);
+  let promise = waitIcon(ROOT + "icon3.png");
+  await createLinks([
+    { href: ROOT + "icon.png",
+      type: "image/png",
+      size: preferredWidth - 1
+    },
+    { href: ROOT + "icon2.png",
+      type: "image/png",
+    },
+    { href: ROOT + "icon3.png",
+      type: "image/png",
+      size: preferredWidth + 1
+    },
+    { href: ROOT + "icon4.png",
+      type: "image/png",
+      size: preferredWidth + 2
+    },
+  ]);
+  await promise;
+  // Must have at least one test.
+  Assert.ok(true, "The expected icon has been set");
+});
--- a/browser/modules/ContentLinkHandler.jsm
+++ b/browser/modules/ContentLinkHandler.jsm
@@ -155,16 +155,17 @@ function guessType(icon) {
  */
 function faviconTimeoutCallback(aFaviconLoads, aPageUrl, aChromeGlobal) {
   let load = aFaviconLoads.get(aPageUrl);
   if (!load)
     return;
 
   let preferredIcon;
   let preferredWidth = 16 * Math.ceil(aChromeGlobal.content.devicePixelRatio);
+  let bestSizedIcon;
   // Other links with the "icon" tag are the default icons
   let defaultIcon;
   // Rich icons are either apple-touch or fluid icons, or the ones of the
   // dimension 96x96 or greater
   let largestRichIcon;
 
   for (let icon of load.iconInfos) {
     if (!icon.isRichIcon) {
@@ -173,39 +174,49 @@ function faviconTimeoutCallback(aFavicon
       // ico files. When multiple icons are in the same set, the latest wins.
       if (guessType(icon) == TYPE_SVG) {
         preferredIcon = icon;
       } else if (icon.width == preferredWidth && guessType(preferredIcon) != TYPE_SVG) {
         preferredIcon = icon;
       } else if (guessType(icon) == TYPE_ICO && (!preferredIcon || guessType(preferredIcon) == TYPE_ICO)) {
         preferredIcon = icon;
       }
+
+      // Check for an icon larger yet closest to preferredWidth, that can be
+      // downscaled efficiently.
+      if (icon.width >= preferredWidth &&
+          (!bestSizedIcon || bestSizedIcon.width >= icon.width)) {
+        bestSizedIcon = icon;
+      }
     }
 
     // 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 {
       defaultIcon = icon;
     }
   }
 
   // Now set the favicons for the page in the following order:
   // 1. Set the best rich icon if any.
-  // 2. Set the preferred one if any, otherwise use the default one.
+  // 2. Set the preferred one if any, otherwise check if there's a better
+  //    sized fit.
   // This order allows smaller icon frames to eventually override rich icon
   // frames.
   if (largestRichIcon) {
     setIconForLink(largestRichIcon, aChromeGlobal);
   }
   if (preferredIcon) {
     setIconForLink(preferredIcon, aChromeGlobal);
+  } else if (bestSizedIcon) {
+    setIconForLink(bestSizedIcon, aChromeGlobal);
   } else if (defaultIcon) {
     setIconForLink(defaultIcon, aChromeGlobal);
   }
 
   load.timer = null;
   aFaviconLoads.delete(aPageUrl);
 }