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.