1265497 - Not-so-great favicon handling

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Favicon Handling, defect)

Description

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

In https://bugzilla.mozilla.org/show_bug.cgi?id=1228680#c11, we identified there's a case where we have an icon, so we'll use it, but it's not IDEAL. 

So here's how we'll handle that situation.

Comment 1

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: Screen Shot 2016-04-18 at 12.20.11 PM.png

1) Use the not-so-great favicon, centered.
2) Detect dominant color
3) Fill remaining space with color from (2).

In most cases, it'll look something like this. 

Home panels list (left), possible top sites (right).

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: not-so-great-icons.png

This is a WIP screenshot. I kept the 27.5% color (dominant color & 0x46FFFFFF). It looks much better but I'm not super happy with it yet. What do you think. Your mock seems to use a darker version of the dominant color (or: less transparent) - what values did you use?

Comment 3

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Comment on attachment 8742940 [details]
not-so-great-icons.png

This is looking great!

I'm not sure why the 27.5% is there, since I don't think we use that value anywhere else in our product. But I do like the transparency in that it separates the favicon visually.

I suggest we try a cleaner % value (not 100% since it looks broken).

Can we try just making it 50%?

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: 50percent.png

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8743274 [details]
50percent.png

> Can we try just making it 50%?

Yep. :)

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: MozReview Request: Bug 1265497 - FaviconView: New favicon background with rounded corners. r?ahunt

Review commit: https://reviewboard.mozilla.org/r/47711/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/47711/

Comment 7

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Comment on attachment 8743274 [details]
50percent.png

Awesome!

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8743318 [details]
MozReview Request: Bug 1265497 - FaviconView: New favicon background with rounded corners. r?ahunt

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47711/diff/1-2/

Comment 9

[Andrzej Hunt :ahunt]

Comment on attachment 8743318 [details]
MozReview Request: Bug 1265497 - FaviconView: New favicon background with rounded corners. r?ahunt

https://reviewboard.mozilla.org/r/47711/#review44561

Nice!

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

https://hg.mozilla.org/integration/fx-team/rev/1a4b7ff56f657c95591a22286f56478cefdba64b
Bug 1265497 - FaviconView: New favicon background with rounded corners. r=ahunt

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1a4b7ff56f65

Comment 12

[Alexandru Simonca [:asimonca], Release Desktop QA]

Verified as fixed in Firefox Nightly 49.0a1 (2016-05-18) on Motorola Nexus 6 (Android 6.0)

Comment 13

[Barbara Bermes [:barbara] - NI please!]

Release Note Request (optional, but appreciated)
[Why is this notable]:
[Suggested wording]: Polish of displaying favicons
[Links (documentation, blog post, etc)]:

Comment 14

[Marcia Knous [:marcia]]

Relnote 49+ added "Visual improvements in favicon display"