1172342 - Sometimes get black favicons on Twitter and Pinterest

Status: RESOLVED DUPLICATE of bug 366324

(Toolkit :: Places, defect)

Description

[Justin Dolske [:Dolske]]

Started in this Twitter thread, but had been previously noted by others:

https://twitter.com/dria/status/607516335778762752

Both sites are basically doing:

  <link href="favicon.png" rel="icon">
  <link href="favicon.svg" rel="icon">

Since the SVG icon is specified last, it _should_ be shown all the time. And the SVG is indeed a black image in both Chrome and Firefox, so that's expected. (Not sure why both sites are doing this -- seems like a mistake. Also not sure why Chrome doesn't seem to show the SVG version at all.)

What's unexpected here is that sometimes the (colored) PNG favicon is shown, and sometimes the (black) SVG. It should be the same thing all the time. I added some logging to ContentLinkHandler.jsm to see if the sites were updating the icon, but it's only being called when the page loads.

I was able to get the favicon to change on Pinterest (from SVG to PNG) by reloading the page. Some extra logging shows that after the 1st load, Places AsyncFetchAndSetIconFromNetwork::OnStopRequest is calling the FaviconService's AddFailedFavicon() here:

642   // If the icon does not have a valid MIME type, add it to the failed cache.
643   if (mIcon.mimeType.IsEmpty()) {
644     nsCOMPtr<nsIURI> iconURI;
645     rv = NS_NewURI(getter_AddRefs(iconURI), mIcon.spec);
646     NS_ENSURE_SUCCESS(rv, rv);
647     rv = favicons->AddFailedFavicon(iconURI);
648     NS_ENSURE_SUCCESS(rv, rv);
649     return NS_OK;
650   }

When the page loads the 2nd time, we attempt to use the SVG as the favicon but it failed in browser.js's DOMLinkHandler check to see if it's a failed icon.

3354   setIcon: function(aBrowser, aURL) {
3355     if (gBrowser.isFailedIcon(aURL))
3356       return false;
3357 
3358     let tab = gBrowser.getTabForBrowser(aBrowser);
3359     if (!tab)
3360       return false;
3361 
3362     gBrowser.setIcon(tab, aURL);
3363     return true;
3364   },

Since the PNG is specified first during page load, that's what ends up being used when the following SVG load is canceled here.

The question is if the first "no mime type" error is correct or not.