814460 - Dark favicon is barely visible on homescreen

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Homescreen, defect, P1)

Description

[Ian Barlow (:ibarlow)]

Steps to reproduce
1. Go to a website. 
2. Tap the bookmark star
3. Select "Add to Home Screen"

Expected results
Page is added to homescreen, with some kind of placeholder icon or favicon and title (similar to other apps)

Actual results
Page is added to homescreen, but no icon is shown -- only a title is shown, so it looks broken.

Comment 1

[Ian Barlow (:ibarlow)]

Attachment: screenshot of issue

Scratch that. We do show favicons, the one I tried just happened to be completely black, and thus invisible on this background. 

Given this, it might make sense to place favicons on some kind of default background, similar to the generic "apps" circle with the rocket

Comment 2

[Ben Francis [:benfrancis]]

(In reply to Ian Barlow (:ibarlow) from comment #1)
> Given this, it might make sense to place favicons on some kind of default
> background, similar to the generic "apps" circle with the rocket

This was https://github.com/mozilla-b2g/gaia/issues/5453 but didn't get implemented.

A stop gap solution was implemented in bug 797377 which just scales up small favicons.

Comment 3

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Josh, should we revisit https://github.com/mozilla-b2g/gaia/issues/5453 based on this use case?

Comment 4

[Patryk Adamczyk [:patryk] UX]

We had visual designs for this. It should be loaded onto a gray plate.

Comment 5

[Patryk Adamczyk [:patryk] UX]

Visual design reference
https://www.dropbox.com/sh/c97cd46hu8e5q84/3AQBuFWt_Y

Comment 6

[Josh Carpenter [:jcarpenter]]

Renoming. This is UX-P1 ("must fix"). The current state creates unacceptable usability problems. The user literally cannot see their icon. Triagers please see the screenshot attached.

Comment 7

[Ben Francis [:benfrancis]]

Does this not apply to app icons as well?

Generating bookmark icons as per the original proposal is really new feature work and not feasible for v1 IMO (it called for implementing upscaling algorithms in JavaScript and other complexities).

I can imagine a simpler solution in which we just display the favicon as provided, but with the disc as a background image. That should be relatively easy but requires developer time, it probably isn't just a CSS/visual design change.

We'd need to differentiate between e.me bookmarks and bookmarks added from elsewhere :(

Comment 8

[Patryk Adamczyk [:patryk] UX]

Yes the background image solution would be fine for v.1. And yes this could happen to app icons, but app icons are generally designed with that in mind, we also give our 3rd party developers tools to design app icons. Favicons however are designed to for bright backgrounds, which is why this is an issue.

Comment 9

[Josh Carpenter [:jcarpenter]]

Yep, what Patryk said.

Comment 10

[Sam Joch [:samjoch]]

Attachment: My github branch

Comment 11

[Patryk Adamczyk [:patryk] UX]

Overall looks good.

1. Can you scale the favicons without any resampling… basically make them look pixelated / nearest neighbour?
2. Living with them on device they look a bit small, can you upscale them 3x (48px) instead of 2x (32px)
3. The fall back image currently looks like its the rocketship plate, Peter will send you a different fall back (48x48px), so it looks better.

Comment 12

[Jinghua zhang]

Sam, are you fixing it in the UX branch?

Comment 13

[Sam Joch [:samjoch]]

I am working on my branch. So Patryk can test it easyly. Then i'll push all the fixing code, as a PR, to the UX branch.

Comment 14

[Peter La]

Attachment: Graphical Assets for Fix

Attached are the graphical assets needed to fix this issue.

We've opted for a light grey circular underlay (vs. dark grey) as most favicons are designed to look best using this background colour.  It's the same colour used in the firefox tabs (and very close to other browser like Chrome).

Favicons from websites are to be scaled up 2x to 32x32 pixels with 'nearest neighbour' resampling, ie. no softening at all.

If no favicon is available, use the default 32x32 grey firefox logo provided, overlayed on top of the circular plate (as shown in FaviconReference.jpg).

If you need further clarification, don't hesitate to ask.

Thanks!

Comment 15

[Peter La]

I should clarify Patryk's comment above about sizing.  At the time, we thought what we were seeing on device was 32x32 but it was actually smaller.  We tried 48x48 but it's too large, so 32x32 is actually a pretty optimal size (as seen in FaviconReference.jpg in the attachment above).

Comment 16

[Peter La]

Also, note that the favicon for TheVerge.com in FaviconReference.jpg contains no tranparency, but the actual favicon from the website actually has transparency.  We will want to render ours with transparency if specified, and if it's at all possible for the best look.  Example: www.theverge.com.

Comment 17

[Sam Joch [:samjoch]]

Comment on attachment 691509 [details]
My github branch

I update my branch. 
I add the default icon for bookmark with no icon.
Also i use canvas, just like icon app are rendered.

Comment 18

[Patryk Adamczyk [:patryk] UX]

Attachment: What I see...

Everything look great now! EXCEPT! The no favicon instance (see google) is the gray rocket ship not the gray firefox icon peter provide. Can you please make the change.

Comment 19

[Sam Joch [:samjoch]]

have you got this image apps/homescreen/style/images/default_favicon.png in your test branch ?

It's odd cause, it is include in the commit https://github.com/guiora/gaia/commit/257f120ddb26ff0dddafa08f41b24896573a910b

Comment 20

[Patryk Adamczyk [:patryk] UX]

(In reply to Sam Joch:samjoch from comment #19)
> have you got this image apps/homescreen/style/images/default_favicon.png in
> your test branch ?
> 
> It's odd cause, it is include in the commit
> https://github.com/guiora/gaia/commit/
> 257f120ddb26ff0dddafa08f41b24896573a910b

Seems to work for me now. Not sure what happened before.

Comment 21

[Sam Joch [:samjoch]]

Attachment: Github PR

Comment 22

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 696284 [details]
Github PR

https://github.com/mozilla-b2g/gaia/commit/a3ce4c048053883becd953d93359bca83b77c225

Comment 23

[Darren]

Verified as fixed in build 20130102070202 on Unagi.