860578 - No favicons in Panorama view

Status: RESOLVED FIXED

(Firefox Graveyard :: Panorama, defect)

Description

[S Cohen]

Attachment: Screenshot of problem

In recent Nightly builds (I'm not sure when this first arose, I don't all too often have enough tabs open to warrant grouping), I have noticed that none of the thumbnails in Panorama view display favicons. The generic "dotted box" icon is shown instead for all normal and app tabs.

I have attached a screenshot to demonstrate.

Comment 1

[Loic]

Regression range:
m-c
good=2013-03-27
bad=2013-03-28
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=178a4a770bb1&tochange=962f5293f87f

Suspected bug:
Jonathan Kew — bug 828508 - use higher-res favicons for tab titles in hidpi mode if available. r=dao

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: fix missing favicons in Panorama view

Similar to bug 866444, we need to strip the -moz-resolution fragment from the favicon URI as stored in the tab's image attribute.

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 744665 [details] [diff] [review]
fix missing favicons in Panorama view

Please use gBrowser.getIcon(tab).

Comment 4

[Jonathan Kew [:jfkthame]]

That doesn't work as well - it fails to get icons for restored-session tabs that have not actually been loaded, whereas reading the tab's image attribute does get them.

Comment 5

[Jonathan Kew [:jfkthame]]

Comment on attachment 744665 [details] [diff] [review]
fix missing favicons in Panorama view

Re-requesting r? on this version of the patch, in view of comment #4.

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: fix missing favicons in Panorama view

Comment 7

[Jonathan Kew [:jfkthame]]

Pushed this to inbound; the behavior for "pending" restored tabs will still be broken until bug 866444 lands, but aside from that it should fix the issue.

https://hg.mozilla.org/integration/mozilla-inbound/rev/f6525b901c95

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f6525b901c95

Comment 9

[Jonathan Kew [:jfkthame]]

Comment on attachment 746416 [details] [diff] [review]
fix missing favicons in Panorama view

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 828508

User impact if declined: missing favicons in Panorama view

Testing completed (on m-c, etc.): in Nightly for a few days

Risk to taking this patch (and alternatives if risky): minimal - simple one-liner in tabview code, no effect elsewhere

String or IDL/UUID changes made by this patch: none

Comment 10

[Jonathan Kew [:jfkthame]]

Comment on attachment 746416 [details] [diff] [review]
fix missing favicons in Panorama view

Moving approval request from aurora to beta, as FF22 has now merged to beta.

Comment 11

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6e6d7e84431a

Comment 12

[Manuela Muntean [Away]]

Verified fixed with Firefox 22 beta 1 (build ID: 20130514181517) on Mac OSX 10.8.3 32bit mode.

Comment 13

[Manuela Muntean [Away]]

Verified fixed with Firefox 23 beta 7 (build ID: 20130718163513) on Mac OSX 10.8.3 32bit mode.