Last Comment Bug 366324 - SVG site icons (favicons, shortcut icons) support
: SVG site icons (favicons, shortcut icons) support
Status: VERIFIED FIXED
[see comment 14, comment 24, comment ...
: dev-doc-complete, site-compat, testcase
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: Trunk
: All All
: P5 enhancement with 43 votes (vote)
: mozilla41
Assigned To: Justin Dolske [:Dolske]
:
Mentors:
: 1168893 1172342 (view as bug list)
Depends on: 650556 878922 276431 1174548 1174552 1174568 1181681 1218041
Blocks: 120352
  Show dependency treegraph
 
Reported: 2007-01-08 09:07 PST by [:Aleksej]
Modified: 2015-12-11 09:02 PST (History)
68 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
41+


Attachments
Testcase v1 Should be accessed from a web server (I guess). (9.90 KB, application/x-7z-compressed)
2007-01-08 09:11 PST, [:Aleksej]
no flags Details
ZIPped testcase v1. Should be put onto a web server to view. (10.30 KB, application/x-zip-compressed)
2007-02-16 05:38 PST, [:Aleksej]
no flags Details
testcase (not using viewBox) (7.52 KB, text/html)
2007-02-20 07:59 PST, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details
screenshot of testcase working, with tryserver build from bug 276431 (49.92 KB, image/png)
2010-05-18 13:15 PDT, Daniel Holbert [:dholbert]
no flags Details
testcase 2 (using viewBox) (7.07 KB, text/html)
2010-09-29 14:06 PDT, Daniel Holbert [:dholbert]
no flags Details
sniff-less workaround? (2.22 KB, patch)
2015-06-07 12:41 PDT, Justin Dolske [:Dolske]
mak77: feedback+
Details | Diff | Splinter Review
Patch v.2 (6.14 KB, patch)
2015-06-11 16:07 PDT, Justin Dolske [:Dolske]
mak77: review+
Details | Diff | Splinter Review
Patch v.3 (5.87 KB, patch)
2015-06-12 11:49 PDT, Justin Dolske [:Dolske]
no flags Details | Diff | Splinter Review

Description [:Aleksej] 2007-01-08 09:07:34 PST
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.1) Gecko/20061208 Firefox/2.0.0.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a2pre) Gecko/20070108 Minefield/3.0a2pre

Firefox should support SVG favicons. They could then be shown scaled up in Page Info, and be more useful at higher resolutions.

Reproducible: Always

Steps to Reproduce:
1. Load the testcase (see the URL or the attachment, if available).
2. Look at the tab and at the location bar, where a favicon should be.
Actual Results:  
The "no favicon" image is shown

Expected Results:  
The SVG favicon should be shown, as specified in the testcase.
Comment 1 [:Aleksej] 2007-01-08 09:11:12 PST Comment hidden (typo)
Comment 2 [:Aleksej] 2007-02-16 05:34:11 PST Comment hidden (typo)
Comment 3 [:Aleksej] 2007-02-16 05:38:07 PST
Created attachment 255332 [details]
ZIPped testcase v1.  Should be put onto a web server to view.
Comment 4 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-02-20 07:57:48 PST
This depends on bug 276431, since the favicon in the url bar is a <xul:image/>.
Comment 5 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-02-20 07:59:12 PST
Created attachment 255775 [details]
testcase (not using viewBox)

This testcase can be viewed online, because it uses the data uri to embed the svg.
Comment 6 Daniel Pope 2007-02-20 09:51:43 PST
Antialiasing results in blurriness at low resolutions. Current wisdom among icon designers is that icons below a certain size (certainly at 32x32 and below) need to be specially optimised for the size they are to be used at.

While I can't see that there would be any problems with using SVG for favicons under the same constraints as for img tags, it doesn't follow that it would be correct to scale these up for any other purpose. If such a use was established, designers/developers would want an alternative way of configuring the larger icon.

For a 16x16 icon, SVG is also likely to be a much larger file than a raster image.
Comment 7 Mike Connor [:mconnor] 2009-04-17 23:12:36 PDT
Not sure this should be wontfixed outright, but absolutely not a priority.
Comment 8 d 2009-06-21 06:26:53 PDT
My opinion is that SVG favicons should be supported like any other format. The "scale up" thingy was just a suggestion, but this is about implementing SVG support for favicons.
Comment 9 Tristan Miller 2010-02-21 15:01:00 PST
I'd like to see this feature in SeaMonkey as well.  Is it necessary to file a separate report?
Comment 10 Dão Gottwald [:dao] 2010-02-21 15:06:17 PST
Not sure if there's any Firefox specific change needed to make this work. This might essentially just be a duplicate of bug 276431, in which case a separate bug for SeaMonkey wouldn't make a lot of sense.
Comment 11 Tristan Miller 2010-03-04 05:11:29 PST
If there's nothing Firefox-specific about this, perhaps the product should be changed from Firefox to Core.
Comment 12 Dão Gottwald [:dao] 2010-03-04 05:15:45 PST
No, in that case this would actually be duplicate of bug 276431.
Comment 13 Daniel Holbert [:dholbert] 2010-05-18 13:15:55 PDT
Created attachment 446032 [details]
screenshot of testcase working, with tryserver build from bug 276431

This "just works" with my current patch for bug 276431. (using testing build linked in bug 276431 comment 77, which is built from m-c + patches from http://hg.mozilla.org/users/dholbert_mozilla.com/svg-patches/file/20d4eae2b924 )

Here's a screenshot of this bug's testcase, working, in that testing build.

I think this should just be a duplicate of bug 276431, as hinted in comment 10 & comment 12.
Comment 14 Daniel Holbert [:dholbert] 2010-09-08 15:08:29 PDT
Comment 13 isn't entirely correct. bug 276431 (which just landed - yay!) does the bulk of the work required for this, but favicons still don't quite work reliably.

The current behavior (with bug 276431) is that the favicon displays correctly the first time, but then the next time (when it's retrieved from a cache), it's not displayed.

I think all we need here is to get imgLoader::GetMimeTypeFromContent* working for SVG content. Basically, given a big character-buffer of our SVG content, we need to infer that it's an SVG file.

I used to have a hack in my bug 276431 patch-queue that simply checked whether the first four characters of our buffer matches "<svg", but that's clearly not sensitive enough to catch all (or even most) SVG files.

I don't know whether it'd be best to just search through our content for a <svg> root node, or instead to rework the imgLoader code so it caches the mimetype together with the data. (so GetMimeTypeFromContent wouldn't even be required anymore)  Need to look into this more.

* http://mxr.mozilla.org/mozilla-central/source/modules/libpr0n/src/imgLoader.cpp#1872
Comment 15 Rimas Kudelis 2010-09-29 13:23:52 PDT
Actually, I only saw a black square in the favicon area instead of the cat...

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100926 Firefox/4.0b7pre
Comment 16 Daniel Holbert [:dholbert] 2010-09-29 14:06:41 PDT
Created attachment 479549 [details]
testcase 2 (using viewBox)

Ah yes.  You actually are seeing the image -- you're just seeing the upper-left 16x16 chunk of it. :) That's because the first testcase didn't use a viewBox on its SVG content, which means it's not requesting to be scaled up/down to fit its viewport.  (You'll find that you get the same results in Firefox, Opera, & Webkit if you view that SVG image in a 16px-by-16px <object>, <embed> tag, or browser-window -- they'll all just crop out a 16-by-16 black rectangle from the upper-left corner).

FWIW, it worked in the screenshot that I'd posted because an earlier of my patch handled heights/widths a bit differently.

I'm attaching an updated version of the testcase, including a viewBox on the SVG content.  That fixes the problem.

Side note: It may be that, for SVG content that has pixel-valued width & height attributes, we want to effectively synthesize a viewBox using that width/height.  No spec currently calls for that, but it's what Opera does for <img> and for favicons, and Erik filed an issue on it in the SVG WG tracker:
  http://www.w3.org/Graphics/SVG/WG/track/issues/2258

At the moment, though, SVG content needs to have a viewBox attribute, to tell us what region should be scaled up/down, in order for Firefox to auto-size it to fit a favicon-box or an <img> (or any other container).
Comment 17 Daniel Holbert [:dholbert] 2010-10-19 14:30:28 PDT
Requesting blocking status for this.  Right now SVG-as-a-favicon is kind of broken -- it only loads the first time, but then doesn't show up when we load it from cache, as noted in comment 14.
Comment 18 Johnathan Nightingale [:johnath] 2010-11-02 07:36:06 PDT
Broken is bad, but how often is this actually used? And does this represent a regression? No debate from me on fixing it, but holding the release is a higher bar.
Comment 19 Johnathan Nightingale [:johnath] 2010-11-02 07:36:39 PDT
Blocking- for now, but feel free to re-nom with answers to comment 18 if you think I'm underestimating the bustage.
Comment 20 Daniel Holbert [:dholbert] 2010-11-02 07:59:17 PDT
Not a regression, & not used frequently -- both of which are because SVG-as-an-image is a new feature in Gecko.

I'd rather not ship with this un-fixed, but I can understand the blocking- reasoning.  Hoping fix this & land with approval+, I guess.
Comment 21 Daniel Holbert [:dholbert] 2010-11-02 08:02:43 PDT
gah, sorry, didn't meant to renom. bugzilla preserving-old-flag-state-across-page-reload fail.
Comment 22 Daniel Holbert [:dholbert] 2011-04-16 17:04:40 PDT
(In reply to comment #15)
> Actually, I only saw a black square in the favicon area instead of the cat...
(In reply to comment #16)
> Ah yes.  You actually are seeing the image -- you're just seeing the upper-left
> 16x16 chunk of it. :) That's because the first testcase didn't use a viewBox

BTW, this behavior changed in Bug 614649, so now both testcases (with- and without-viewbox) look right on the first load.

(favicon still doesn't show up on subsequent loads, though, per comment 17)
Comment 23 Daniel Holbert [:dholbert] 2011-08-08 13:34:44 PDT
Moving to Core|ImageLib, since (per comment 14) that's where the code is that needs fixing.
Comment 24 Daniel Holbert [:dholbert] 2011-08-08 22:28:20 PDT
Given bug 111373 comment 49 (basically: shifting away from allowing/supporting animated favicons), it might be simpler to fix this bug here by rasterizing the SVG's first rendered frame, storing that in a PNG (or whatever's convenient), and sticking *that* in the favicon cache.

That would work around the issue brought up in Comment 14. (trying to detect "this is definitely SVG" given a buffer of data from the favicon cache)
Comment 25 c1602037 2013-02-22 04:38:22 PST Comment hidden (advocacy)
Comment 26 Andre Klapper 2013-02-22 11:48:53 PST Comment hidden (obsolete)
Comment 27 John Drinkwater (:beta) 2013-03-14 07:00:36 PDT
What’s left for this, is it just a mime check required in GetMimeTypeFromContent to fix the cache issue?
Comment 28 John Drinkwater (:beta) 2013-03-14 08:35:39 PDT
Putting some rudimentary checks into that function does solve it, though I wonder how best to test for ‘<?xml…<svg’ without wasting users or firefox’s time.
Comment 29 Daniel Holbert [:dholbert] 2013-03-14 09:10:15 PDT
Not to mention if there's a huge <!-- ... --> comment before the <svg> that you have to sniff through.

Ideally we should just redesign the cache to store the mimetype as well as the image data.  Sniffing through possible-XML to try to guess whether it's SVG is kind of an ugly hack.
Comment 30 e281857 2013-03-28 20:40:22 PDT Comment hidden (advocacy)
Comment 31 Andre Klapper 2013-03-29 02:41:22 PDT Comment hidden (obsolete)
Comment 32 tu 2013-03-30 03:59:47 PDT
If all browsers don't support SVG favicons sometime soon we're going to have some damn horrible favicons on high PPI screens.
Comment 33 wind 2013-04-10 04:37:09 PDT Comment hidden (off-topic)
Comment 34 STUNGMEBAS 2013-05-24 21:13:47 PDT
Oh the horrors of bitmap icons on retina displays.
Comment 35 Anne (:annevk) 2014-03-15 13:44:27 PDT
We should have the same pipeline as for <img>. Meaning we should check if the content's MIME type is image/svg+xml and use an SVG codepath in that case and just hand the content (ignoring its MIME type) to the image decoder otherwise.
Comment 36 Daniel Holbert [:dholbert] 2014-03-15 16:15:18 PDT
(In reply to Anne (:annevk) from comment #35)
> We should have the same pipeline as for <img>.

We do, in general.

> Meaning we should check if
> the content's MIME type is image/svg+xml and use an SVG codepath in that
> case and just hand the content (ignoring its MIME type) to the image decoder
> otherwise.

That's what we do the first time.

The problem is, we don't have the mimetype on *subsequent* loads of the favicon (because we pull the data from the favicon cache on subsequent loads, which only stores the raw data and assumes that's enough).

See comment 14 / comment 29.
Comment 37 Daniel Holbert [:dholbert] 2014-03-15 16:19:14 PDT
(and comment 24 has an alternate solution I'd forgotten about: rasterize once and stick that in the favicon cache.)

[I forgot this was assigned to me -- unassigning, since I'm not actively working on this at the moment, so that someone else can take it if they like. I want to see this fixed and will hopefully have time to circle back to it eventually, so I'm tagging myself with "needinfo" so that I don't forget about it entirely.]
Comment 38 Simen Mangseth 2014-09-23 09:05:59 PDT
Any news on this?
Comment 39 Seth Fowler [:seth] [:s2h] 2014-11-19 17:55:36 PST
It's possible (though the matter needs greater study) that the best solution here is to get rid of the favicon cache entirely, and just artificially boost the expiration time of favicons in our normal caches. We could really benefit from a simpler model here; if this problem existed in our normal (necko/imagelib) caches, we'd have fixed it long ago, but since it's in obscure code that nobody ever touches, this bug is still open after 7 years.
Comment 40 Daniel Holbert [:dholbert] 2014-11-19 18:20:10 PST
Sorry for introducing imprecise terminology up above -- for the record, I believe the mentions of "favicon cache" on this bug are really talking about the "Favicon Service" aka "nsFaviconService", which is accessed (in toolkit/places code) via nsFaviconService::SetAndFetchFaviconForPage():
https://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/nsFaviconService.cpp?rev=96a566fa1599#211

I believe the relevant troublesome mimetype sniffing is happening via a call to NS_SniffContent(), inside of AsyncFetchAndSetIconFromNetwork::OnStopRequest():
https://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/AsyncFaviconHelpers.cpp?rev=8340783e3145#638
Comment 41 Johannes Pfrang [:johnp] 2015-05-27 09:20:47 PDT
*** Bug 1168893 has been marked as a duplicate of this bug. ***
Comment 42 Tom Schuster [:evilpie] 2015-06-07 11:19:18 PDT
*** Bug 1172342 has been marked as a duplicate of this bug. ***
Comment 43 Justin Dolske [:Dolske] 2015-06-07 11:33:51 PDT
From 1172342 -- I see both Twitter and Pinterest replying with an explicit "Content-Type: image/svg+xml" -- so it seems like at least for this case there shouldn't need to be any content type sniffing going on?
Comment 44 Justin Dolske [:Dolske] 2015-06-07 11:39:47 PDT
(In reply to Seth Fowler [:seth] - PTO until 6/12 from comment #39)
> It's possible (though the matter needs greater study) that the best solution
> here is to get rid of the favicon cache entirely

I had similar thoughts while looking at 1172342. Maybe not _entirely_ (since a cache is useful for, say, the history menu or the awesomebar). But it does seem a little unusual that we do this extra caching/checking for just the favicon specified by a page we're currently loading.
Comment 45 Daniel Holbert [:dholbert] 2015-06-07 11:45:30 PDT
(In reply to Justin Dolske [:Dolske] from comment #43)
> From 1172342 -- I see both Twitter and Pinterest replying with an explicit
> "Content-Type: image/svg+xml" -- so it seems like at least for this case
> there shouldn't need to be any content type sniffing going on?

IIRC, we handle that correctly the first time the favicon is loaded. But then, we stick it in the favicon cache (which *only* stores the data, and assumes it can sniff the content-type from that later on as-needed). So on subsequent loads, we pull the favicon out of the cache & fail to sniff the content-type.
Comment 46 Justin Dolske [:Dolske] 2015-06-07 12:04:06 PDT
(In reply to Daniel Holbert [:dholbert] from comment #45)
> (In reply to Justin Dolske [:Dolske] from comment #43)
> > From 1172342 -- I see both Twitter and Pinterest replying with an explicit
> > "Content-Type: image/svg+xml" -- so it seems like at least for this case
> > there shouldn't need to be any content type sniffing going on?
> 
> IIRC, we handle that correctly the first time the favicon is loaded. But
> then, we stick it in the favicon cache (which *only* stores the data, and
> assumes it can sniff the content-type from that later on as-needed). So on
> subsequent loads, we pull the favicon out of the cache & fail to sniff the
> content-type.

Well, hmm, there is a mimetype column in the favicon cache:

CREATE TABLE moz_favicons (  id INTEGER PRIMARY KEY, url LONGVARCHAR UNIQUE, data BLOB, mime_type VARCHAR(32), expiration LONG, guid TEXT);

Except it ends up blank for SVG. :| I wonder if something just isn't hooked up properly, or it's just harder than one would think to get it from point A to point B... *looks*
Comment 47 Justin Dolske [:Dolske] 2015-06-07 12:41:29 PDT
Created attachment 8616488 [details] [diff] [review]
sniff-less workaround?

This isn't a complete fix, but seems like an incremental improvement? Sites explicitly setting the content-type for SVG would work (as Twitter/Pinterest seem to do at the moment, intentional or not).

I suppose one downside is that native menus on OS X don't seem to like SVG images, and so the History/Bookmarks menu will be missing a favicon when SVG is used.
Comment 48 Daniel Holbert [:dholbert] 2015-06-07 15:33:47 PDT
Interesting! I wasn't aware that the favicon cache stores the mimetype. (or intends to store it, at least)

Anyway, as I recall, comment 45 describes basically what happened for all favicons, when I was poking at this back in 2010; favicon image-data is treated as a local blob whose content-type needs to be sniffed on subsequent loads.  It sounds like we can do better by pulling out the mimetype, though (as long as it's been set) perhaps. It's also possible I misunderstood what was going on.
Comment 49 Daniel Holbert [:dholbert] 2015-06-07 15:39:44 PDT
(In reply to Justin Dolske [:Dolske] from comment #47)
> This isn't a complete fix, but seems like an incremental improvement? Sites
> explicitly setting the content-type for SVG would work (as Twitter/Pinterest
> seem to do at the moment, intentional or not).

I think this is a reasonable thing to depend on (sites manually sending the correct mimetype), FWIW.  I'm pretty sure we depend on that for SVG-as-an-image in other contexts (e.g. <img>, backgrounds), too.
Comment 50 Anne (:annevk) 2015-06-08 22:04:27 PDT
Yes, sites MUST set the MIME type correctly for SVG content. We don't want to ever sniff for SVG.
Comment 51 Marco Bonardo [::mak] 2015-06-10 09:37:28 PDT
Comment on attachment 8616488 [details] [diff] [review]
sniff-less workaround?

Review of attachment 8616488 [details] [diff] [review]:
-----------------------------------------------------------------

FWIW, the reason the favicon service exists, even if we already have a cache, is because we have views showing thousands of favicons, but we clearly don't want to open thousands of file handles. It's very useful for perf reasons to have all blobs stored in a single file. It also enables other interesting facts like allowing to set custom icons for bookmarks, but we don't expose those in Firefox UI (some add-ons do) so this is a minor fact. The important thing is that we have views showing multiple favicons at once.

The approach looks sane to me, if you tested it working, it only needs a small xpcshell-test (in toolkit/components/places/tests/unit/) that tries to setAndFetch an SVG icon and checks it with getFaviconData. (I shortened API names here, just look at mozIAsyncFavicons.idl)
Comment 52 Justin Dolske [:Dolske] 2015-06-11 16:07:01 PDT
Created attachment 8621304 [details] [diff] [review]
Patch v.2

Added the xpcshell test.
Comment 53 Marco Bonardo [::mak] 2015-06-12 03:05:31 PDT
Comment on attachment 8621304 [details] [diff] [review]
Patch v.2

Review of attachment 8621304 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/places/tests/unit/test_svg_favicon.js
@@ +1,2 @@
> +/* Any copyright is dedicated to the Public Domain.
> +   http://creativecommons.org/publicdomain/zero/1.0/ */

no more needed (the default is PD if no header exists)

@@ +2,5 @@
> +   http://creativecommons.org/publicdomain/zero/1.0/ */
> +
> +function run_test() {
> +  run_next_test();
> +}

no more needed as well, if you use add_task

@@ +12,5 @@
> +    uri: pageuri,
> +    transition: TRANSITION_LINK,
> +    visitDate: Date.now() * 1000
> +  }).then(run_next_test());
> +});

please use add_task and yield PlacesTestUtils.addVisits

I'd like to see the whole test in a single add_task call

@@ +16,5 @@
> +});
> +
> +// Test adding the favicon
> +add_test(function test_setFavicon() {
> +  function onSetComplete(aURI, aDataLen, aData, aMimeType) {

yield new Promise(resolve => {
  ...whatever...
  resolve();
});

@@ +30,5 @@
> +});
> +
> +// Test fetching the favicon
> +add_test(function test_getFavicon() {
> +  function onGetComplete(aURI, aDataLen, aData, aMimeType) {

let data = yield PlacesUtils.promiseFaviconData(pageuri);

data is { uri, dataLen, data, mimeType }
Comment 54 Justin Dolske [:Dolske] 2015-06-12 11:49:19 PDT
Created attachment 8621738 [details] [diff] [review]
Patch v.3

Fixed nits.
Comment 56 Justin Dolske [:Dolske] 2015-06-12 12:09:19 PDT
Just to summarize what got fixed here:

SVG favicons already kinda-sorta worked, but only for the first time a page was loaded in a session. Prior to this patch, Places wasn't able to properly cache the favicon so it got treated as a bad/corrupt icon, and wouldn't show up on future page loads (and always failed for places that only use the favicon cache, such as menus). See bug 1172342 comment 0 (which got duped here) for exactly what was happening.
Comment 57 Justin Dolske [:Dolske] 2015-06-12 12:45:51 PDT
(In reply to Justin Dolske [:Dolske] from comment #47)

> I suppose one downside is that native menus on OS X don't seem to like SVG
> images, and so the History/Bookmarks menu will be missing a favicon when SVG
> is used.

Filed bug 1174284 to get SVG images working in native OS X menus.
Comment 58 Simen Mangseth 2015-06-12 15:25:30 PDT
It's great that this bug seems to finally get a patch after all these years. So when, or in which release, do you think this will arrive?
Comment 59 :Gijs Kruitbosch (away 26-29 incl.) 2015-06-13 02:47:26 PDT
(In reply to Simen Mangseth from comment #58)
> It's great that this bug seems to finally get a patch after all these years.
> So when, or in which release, do you think this will arrive?

This is landing in Firefox 41, and that's what it will ship with unless it gets uplifted to an earlier release. See https://wiki.mozilla.org/RapidRelease/Calendar for more details.
Comment 60 Simen Mangseth 2015-06-13 02:55:34 PDT
(In reply to :Gijs Kruitbosch from comment #59)
> (In reply to Simen Mangseth from comment #58)
> > It's great that this bug seems to finally get a patch after all these years.
> > So when, or in which release, do you think this will arrive?
> 
> This is landing in Firefox 41, and that's what it will ship with unless it
> gets uplifted to an earlier release. See
> https://wiki.mozilla.org/RapidRelease/Calendar for more details.

Thanks for the information. However, I just tried Firefox 41a1 from the Nightly channel, and in both sites like Twitter and in my own testcases, the SVG favicon still seems to appear only on the first page load - am I doing something wrong or is it just not fixed in 41 yet?
Comment 61 :Gijs Kruitbosch (away 26-29 incl.) 2015-06-13 03:18:00 PDT
(In reply to Simen Mangseth from comment #60)
> (In reply to :Gijs Kruitbosch from comment #59)
> > (In reply to Simen Mangseth from comment #58)
> > > It's great that this bug seems to finally get a patch after all these years.
> > > So when, or in which release, do you think this will arrive?
> > 
> > This is landing in Firefox 41, and that's what it will ship with unless it
> > gets uplifted to an earlier release. See
> > https://wiki.mozilla.org/RapidRelease/Calendar for more details.
> 
> Thanks for the information. However, I just tried Firefox 41a1 from the
> Nightly channel, and in both sites like Twitter and in my own testcases, the
> SVG favicon still seems to appear only on the first page load - am I doing
> something wrong or is it just not fixed in 41 yet?

It's not fixed until this bug is marked fixed and the change lands on mozilla-central instead of just fx-team. And then of course, you'd still need to wait for the first nightly build that includes the change. It's the weekend now, and it's not merged to m-c yet, so you'll have to wait for a later build - maybe tomorrow's, maybe Monday, maybe even Tuesday, depending on circumstances. All assuming the change sticks and doesn't cause problems that require it to be backed out.

If you have more questions about the lifecycle of patches/bugs, please just email me - this bug is not really the best place to have this discussion.
Comment 62 Phil Ringnalda (:philor) 2015-06-13 18:34:57 PDT
https://hg.mozilla.org/mozilla-central/rev/ed791601d8e7
Comment 63 Guilherme Lima 2015-06-14 12:56:58 PDT
On today's Nightly, Twitter favicon is always black!
Comment 64 Anthony Ricaud (:rik) 2015-06-14 13:10:06 PDT
Twitter is providing a black icon [1]. I think this is related to Safari 9 pinned tabs implementation [2]. They provide a <meta name="theme-color" content="#55acee"> for Safari. I don't know if websites can provide SVG icons with anything else than black to get the same result in Safari but a normal icon elsewhere. We should reach out to Twitter and Apple here, before Safari 9 ships.

[1] https://abs.twimg.com/a/1434065683/img/t1/favicon.svg
[2] https://developer.apple.com/library/safari/releasenotes/General/WhatsNewInSafari/Articles/Safari_9.html#//apple_ref/doc/uid/TP40014305-CH9-SW20
Comment 65 Daniel Holbert [:dholbert] 2015-06-14 13:43:10 PDT
Yelp has the same issue (providing a black SVG favicon, with <meta name="theme-color"> to get it colorized in Safari I guess). Filed bug 1174548.
Comment 66 Daniel Holbert [:dholbert] 2015-06-14 13:54:40 PDT
I spun off bug 1174552 to cover Tech Evang for Twitter (comment 63 / comment 64).
Comment 67 Daniel Holbert [:dholbert] 2015-06-24 22:13:38 PDT
This bugfix probably merits mentioning in the release notes for Firefox 41 --> adding relnote keyword. (I think that's the right way to remind ourselves to do that?)
Comment 68 Daniel Holbert [:dholbert] 2015-06-24 23:40:10 PDT
Release Note Request (optional, but appreciated)
[Why is this notable]: New feature (or rather, fixing an existing feature to reliably work in a case where it previously was mostly-broken; see comment 56.)

[Suggested wording]: SVG images can be used as favicons. (Fixed a bug that made them disappear after first pageload.)

[Links (documentation, blog post, etc)]: none yet, AFAIK
Comment 69 Ritu Kothari (:ritu) 2015-07-01 17:34:39 PDT
Release note added to Firefox 41.0a2
Comment 71 Jean-Yves Perrier [:teoli] 2015-09-26 01:53:21 PDT
There is also a note there: https://developer.mozilla.org/en-US/Firefox/Releases/41#SVG
Comment 72 Fahmida Noor 2015-12-11 08:59:24 PST
I have reproduced this bug with Firefox 3.0a2pre on Windows 7(64-bit) with the instructions from comment 0.

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9a2pre) Gecko/20070108 Minefield/3.0a2pre

Verified as fixed with Latest Firefox Aurora(Build ID : 20151211004013)

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0

[bugday-20151211]
Comment 73 Daniel Holbert [:dholbert] 2015-12-11 09:02:47 PST
Thanks! Marking as "Verified".

Note You need to log in before you can comment on or make changes to this bug.