Closed Bug 760218 Opened 12 years ago Closed 12 years ago

Missing favicons for pages that don't specify favicon size

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(firefox14 unaffected, firefox15+ fixed)

RESOLVED FIXED
Firefox 16
Tracking Status
firefox14 --- unaffected
firefox15 + fixed

People

(Reporter: aaronmt, Assigned: bnicholson)

References

Details

(Keywords: regression)

Attachments

(1 file)

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?
Assignee: nobody → margaret.leibovic
(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.
blocking1.9.2: --- → ?
blocking1.9.2: ? → ---
blocking-fennec1.0: --- → ?
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.
blocking-fennec1.0: ? → ---
Attached patch patchSplinter Review
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).
Assignee: margaret.leibovic → bnicholson
Attachment #631206 - Flags: review?(margaret.leibovic)
Summary: Branding favicon on about: pages missing → Missing favicons for pages that don't specify favicon size
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!
Attachment #631206 - Flags: review?(margaret.leibovic) → review+
https://hg.mozilla.org/mozilla-central/rev/e64eb13b8b5b

(Merged by Ed Morley)
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
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
Attachment #631206 - Flags: approval-mozilla-aurora?
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
Attachment #631206 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: