Bug 1422289 - ContentLinkHandler should guess icon type from the extension when type is not defined. r=mak
MozReview-Commit-ID: HfuqudW77jM
--- a/browser/base/content/test/favicons/browser_preferred_icons.js
+++ b/browser/base/content/test/favicons/browser_preferred_icons.js
@@ -1,14 +1,17 @@
/* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/publicdomain/zero/1.0/ */
const ROOT = "http://mochi.test:8888/browser/browser/base/content/test/favicons/";
function waitIcon(url) {
+ // Make sure we don't miss out on an icon if it was previously used in a test
+ PlacesUtils.favicons.removeFailedFavicon(makeURI(url));
+
// 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.
return BrowserTestUtils.waitForCondition(
() => {
let tabIcon = gBrowser.getIcon();
info("Found icon " + tabIcon);
return tabIcon == url;
@@ -19,17 +22,18 @@ function waitIcon(url) {
function createLinks(linkInfos) {
return ContentTask.spawn(gBrowser.selectedBrowser, linkInfos, links => {
let doc = content.document;
let head = doc.getElementById("linkparent");
for (let l of links) {
let link = doc.createElement("link");
link.rel = "icon";
link.href = l.href;
- link.type = l.type;
+ if (l.type)
+ link.type = l.type;
if (l.size)
link.setAttribute("sizes", `${l.size}x${l.size}`);
head.appendChild(link);
}
});
}
add_task(async function setup() {
@@ -73,25 +77,90 @@ add_task(async function prefer_sized() {
type: "image/x-icon"
},
]);
await promise;
// Must have at least one test.
Assert.ok(true, "The expected icon has been set");
});
-add_task(async function prefer_ico() {
+add_task(async function prefer_last_ico() {
let promise = waitIcon(ROOT + "icon2.ico");
await createLinks([
{ href: ROOT + "icon.ico",
type: "image/x-icon"
},
{ href: ROOT + "icon.png",
type: "image/png",
},
{ href: ROOT + "icon2.ico",
- type: "image/x-icon"
- },
+ type: "image/x-icon"
+ },
+ ]);
+ await promise;
+ // Must have at least one test.
+ Assert.ok(true, "The expected icon has been set");
+});
+
+add_task(async function fuzzy_ico() {
+ let promise = waitIcon(ROOT + "microsoft.ico");
+ await createLinks([
+ { href: ROOT + "icon.ico",
+ type: "image/x-icon"
+ },
+ { href: ROOT + "icon.png",
+ type: "image/png",
+ },
+ { href: ROOT + "microsoft.ico",
+ type: "image/vnd.microsoft.icon"
+ },
]);
await promise;
// Must have at least one test.
Assert.ok(true, "The expected icon has been set");
});
+
+add_task(async function guess_svg() {
+ let promise = waitIcon(ROOT + "icon.svg");
+ await createLinks([
+ { href: ROOT + "icon.svg" },
+ { href: ROOT + "icon.png",
+ type: "image/png",
+ size: 16 * Math.ceil(window.devicePixelRatio)
+ },
+ { href: ROOT + "icon.ico",
+ type: "image/x-icon"
+ },
+ ]);
+ await promise;
+ // Must have at least one test.
+ Assert.ok(true, "The expected icon has been set");
+});
+
+add_task(async function guess_ico() {
+ let promise = waitIcon(ROOT + "icon.ico");
+ await createLinks([
+ { href: ROOT + "icon.ico" },
+ { href: ROOT + "icon.png",
+ type: "image/png",
+ },
+ ]);
+ await promise;
+ // Must have at least one test.
+ Assert.ok(true, "The expected icon has been set");
+});
+
+add_task(async function guess_invalid() {
+ let promise = waitIcon(ROOT + "icon.svg");
+ // Create strange links to make sure they don't break us
+ await createLinks([
+ { href: ROOT + "icon.svg" },
+ { href: ROOT + "icon" },
+ { href: ROOT + "icon?.svg" },
+ { href: ROOT + "icon#.svg" },
+ { 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");
+});
--- a/browser/modules/ContentLinkHandler.jsm
+++ b/browser/modules/ContentLinkHandler.jsm
@@ -23,16 +23,19 @@ const SIZES_TELEMETRY_ENUM = {
ANY: 1,
DIMENSION: 2,
INVALID: 3,
};
const FAVICON_PARSING_TIMEOUT = 100;
const FAVICON_RICH_ICON_MIN_WIDTH = 96;
+const TYPE_ICO = "image/x-icon";
+const TYPE_SVG = "image/svg+xml";
+
/*
* Create a nsITimer.
*
* @param {function} aCallback A timeout callback function.
* @param {Number} aDelay A timeout interval in millisecond.
* @return {nsITimer} A nsITimer object.
*/
function setTimeout(aCallback, aDelay) {
@@ -115,20 +118,37 @@ function setIconForLink(aIconInfo, aChro
{ url: aIconInfo.iconUri.spec,
loadingPrincipal: aIconInfo.loadingPrincipal,
requestContextID: aIconInfo.requestContextID,
canUseForTab: !aIconInfo.isRichIcon,
});
}
/**
- * Checks whether the icon info represents an ICO image.
+ * Guess a type for an icon based on its declared type or file extension.
*/
-function isICO(icon) {
- return icon.type == "image/x-icon" || icon.type == "image/vnd.microsoft.icon";
+function guessType(icon) {
+ // No type with no icon
+ if (!icon) {
+ return "";
+ }
+
+ // Use the file extension to guess at a type we're interested in
+ if (!icon.type) {
+ let extension = icon.iconUri.filePath.split(".").pop();
+ switch (extension) {
+ case "ico":
+ return TYPE_ICO;
+ case "svg":
+ return TYPE_SVG;
+ }
+ }
+
+ // Fuzzily prefer the type or fall back to the declared type
+ return icon.type == "image/vnd.microsoft.icon" ? TYPE_ICO : icon.type || "";
}
/*
* 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,
@@ -137,36 +157,34 @@ function isICO(icon) {
* @param {String} aPageUrl A page URL string for this callback.
* @param {Object} aChromeGlobal A global chrome object.
*/
function faviconTimeoutCallback(aFaviconLoads, aPageUrl, aChromeGlobal) {
let load = aFaviconLoads.get(aPageUrl);
if (!load)
return;
- let preferredIcon = {
- type: null
- };
+ let preferredIcon;
let preferredWidth = 16 * Math.ceil(aChromeGlobal.content.devicePixelRatio);
// 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) {
// First check for svg. If it's not available check for an icon with a
// size adapt to the current resolution. If both are not available, prefer
// ico files. When multiple icons are in the same set, the latest wins.
- if (icon.type == "image/svg+xml") {
+ if (guessType(icon) == TYPE_SVG) {
preferredIcon = icon;
- } else if (icon.width == preferredWidth && preferredIcon.type != "image/svg+xml") {
+ } else if (icon.width == preferredWidth && guessType(preferredIcon) != TYPE_SVG) {
preferredIcon = icon;
- } else if (isICO(icon) && (preferredIcon.type == null || isICO(preferredIcon))) {
+ } else if (guessType(icon) == TYPE_ICO && (!preferredIcon || guessType(preferredIcon) == TYPE_ICO)) {
preferredIcon = 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) {
@@ -180,17 +198,17 @@ function faviconTimeoutCallback(aFavicon
// 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.
// This order allows smaller icon frames to eventually override rich icon
// frames.
if (largestRichIcon) {
setIconForLink(largestRichIcon, aChromeGlobal);
}
- if (preferredIcon.type) {
+ if (preferredIcon) {
setIconForLink(preferredIcon, aChromeGlobal);
} else if (defaultIcon) {
setIconForLink(defaultIcon, aChromeGlobal);
}
load.timer = null;
aFaviconLoads.delete(aPageUrl);
}