873146 - about: page favicons are not displayed in the "Your tabs from last time" section of about:home screen

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Nick Alexander :nalexander [he/him]]

Attachment: Fennec Feedback with favicon

Screenshots will say it best.

Comment 1

[Nick Alexander :nalexander [he/him]]

Attachment: about:home with blank about:feedback favicon

Comment 2

[Nick Alexander :nalexander [he/him]]

Attachment: Fennec Health Report with favicon

This is not specific to about:feedback.

Comment 3

[Nick Alexander :nalexander [he/him]]

Attachment: about:home with blank about:healthreport favicon

Comment 4

[Nick Alexander :nalexander [he/him]]

margaret, wesj: I think y'all were working in this area recently.

Comment 5

[:Margaret Leibovic]

I can look into this.

Comment 6

[:Margaret Leibovic]

So, there are two issues here.

The main issue with about: pages is that we don't store about: URLs in history, so we're not finding a favicon for them when we call getFaviconForUrl in readLastTabs. The favicon *does* show up for about:firefox because it's a bookmark, so if you bookmark about:feedback, you will see the favicon show up ;) I'm not sure if it's worth fixing this issue, since it's rooted in how we handle history entries for about: pages.

The second issue here is that when we don't find a favicon, we never end up calling updateImage, so we get that awkward white square. This is something that should definitely be fixed.

Comment 7

[:Margaret Leibovic]

Attachment: Use default favicon in "tabs from last time" if there's no stored favicon

Here's a patch to fix the second issue I mentioned in my last comment.

Comment 8

[:Margaret Leibovic]

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

Comment 9

[:Margaret Leibovic]

This was a regression from bug 839855 (I tested in beta, and we show the default favicon for about:feedback in tabs from last time).

Comment 10

[:Margaret Leibovic]

Comment on attachment 750682 [details] [diff] [review]
Use default favicon in "tabs from last time" if there's no stored favicon

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 839855
User impact if declined: about: pages have an ugly favicon in tabs from last time
Testing completed (on m-c, etc.): tested locally
Risk to taking this patch (and alternatives if risky): low-risk background change
String or IDL/UUID changes made by this patch: n/a

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bf0fe0f70138

Comment 13

[Teodora Vermesan (:TeoVermesan)]

Verified fixed on:
Build: Firefox for Android 24.0a1(2013-06-20) and Firefox for Android 23.0a2(2013-06-20)
Device: Samsung Galaxy Nexus
OS: Android 4.1.1