The default bug view has changed. See this FAQ.

Missing favicons for pages that don't specify favicon size

RESOLVED FIXED in Firefox 15

Status

()

Firefox for Android
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: aaronmt, Assigned: bnicholson)

Tracking

({regression})

Trunk
Firefox 16
ARM
Android
regression
Points:
---

Firefox Tracking Flags

(firefox14 unaffected, firefox15+ fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
Regression from bug 715263?
(Reporter)

Comment 1

5 years ago
Yep.

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=65fa5cb6f79c&tochange=3aa566994890
Blocks: 715263
Keywords: regressionwindow-wanted

Comment 2

5 years ago
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
tracking-firefox15: ? → +
(Assignee)

Comment 3

5 years ago
(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.
(Assignee)

Updated

5 years ago
blocking1.9.2: --- → ?
(Assignee)

Updated

5 years ago
blocking1.9.2: ? → ---
blocking-fennec1.0: --- → ?

Comment 4

5 years ago
This isn't in Firefox 14 (introduced by bug 715263), so it shouldn't need to be a blocker.
(Assignee)

Comment 5

5 years ago
Oh, I'll take that away then.
blocking-fennec1.0: ? → ---
status-firefox14: --- → unaffected
(Assignee)

Comment 6

5 years ago
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).
Assignee: margaret.leibovic → bnicholson
Attachment #631206 - Flags: review?(margaret.leibovic)
(Assignee)

Updated

5 years ago
Summary: Branding favicon on about: pages missing → Missing favicons for pages that don't specify favicon size

Comment 7

5 years ago
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+
(Assignee)

Comment 8

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/e64eb13b8b5b
https://hg.mozilla.org/mozilla-central/rev/e64eb13b8b5b

(Merged by Ed Morley)
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
(Assignee)

Comment 10

5 years ago
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?
(Assignee)

Comment 11

5 years ago
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+
(Assignee)

Comment 13

5 years ago
http://hg.mozilla.org/releases/mozilla-aurora/rev/c7a66cfaa20b
status-firefox15: affected → fixed
You need to log in before you can comment on or make changes to this bug.