Regression from bug 715263?
I guess that means I should take this... I'll do some debugging today. Does this affect all about: pages? Has anyone been seeing missing favicons on normal webpages?
(In reply to Margaret Leibovic [:margaret] from comment #2) > I guess that means I should take this... > > I'll do some debugging today. Does this affect all about: pages? Has anyone > been seeing missing favicons on normal webpages? Yes - http://nightly.mozilla.org.
This isn't in Firefox 14 (introduced by bug 715263), so it shouldn't need to be a blocker.
Oh, I'll take that away then.
Created attachment 631206 [details] [diff] [review] patch I noticed this while working on a number of other favicon bugs today. mFaviconSize is initialized to 0, so any favicons of undefined (0) size won't be used. This means that any page that that doesn't have a root favicon.ico won't have its favicon shown (unless it has a size attribute on its link item).
Comment on attachment 631206 [details] [diff] [review] patch That makes sense. Also if an additional favicon is added with the same size, I think the last one added is expected to be the one used, and this patch would make that true. Thank you, Brian!
https://hg.mozilla.org/mozilla-central/rev/e64eb13b8b5b (Merged by Ed Morley)
Comment on attachment 631206 [details] [diff] [review] patch I'm confused about what happened here...bug 715263 is apparently in Aurora, but it's not marked or mentioned in the bug. There was a rollup patch uploaded that included this one, but it looks like that wasn't the one that got landed. Anyway, we should probably uplift this too. [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 715263 User impact if declined: Missing icon on about:home, nightly.mozilla.org, and other sites Testing completed (on m-c, etc.): m-c Risk to taking this patch (and alternatives if risky): very low risk String or UUID changes made by this patch: none
Oh, I guess bug 715263 just made the 15 cutoff, and this one didn't?
Comment on attachment 631206 [details] [diff] [review] patch Safe, low risk fix. Mobile-only