1307621 - To optimize web icon after adding this page to Home Screen

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect)

Description

[sherryyang12321]

Attachment: Screenshot_2016-10-05-10-15-47.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2486.0 Safari/537.36 Edge/13.10586

Steps to reproduce:

1.Type Firefox 
2.Input www.google.co.nz
3.Tab setting at right button->Page->Add to Home Screen.
4.Go back to home screen and check the created icon.


Actual results:

This page icon was created with poor quality at home screen.


Expected results:

This web icon was created withy high quality at home screen.

Comment 1

[Kevin Brosnan [Ex-Mozilla]]

Not a security bug.

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

There's an unintentional behavior change introduced by bug 1290014:

The previous code bypassed the memory cache and always loaded the icon from disk or performed a download of the icon before adding it to the home screen:
https://dxr.mozilla.org/mozilla-beta/source/mobile/android/base/java/org/mozilla/gecko/favicons/Favicons.java#628

The new code does not perform a download and only loads an icon from disk:
https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java#1922

Comment 4

[Nevin Chen(Not active on Bugzilla)]

Attachment: device-2016-11-02-102606.png

I've tried several website and it seems okay. I'll try to reproduce it

Comment 5

[Nevin Chen(Not active on Bugzilla)]

I found the issue. The downloaded favicon size for Google is 64*64. while the launcher icon size on Android is 144*144. That's why it is still blur even we've added some inset and background to it. Suggest to use the original size if the favicon is not big enough. PS. since the favicon has the same width/height I'll assume it has only two type of ratio: one is as large as launcher icon, the other is it's original size.

Comment 6

[Nevin Chen(Not active on Bugzilla)]

Attachment: Bug 1307621 - Use the original size if the downloaded favicon is not big enough.

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

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

Thanks for the patch. I'll review it later.

Did you look at comment 3 too? I think we want to skip memory here (because it may contain a small version), but not disk or network (to load the icon in the original size):
https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java#1874

Comment 8

[Nevin Chen(Not active on Bugzilla)]

Yes, I've read comment 3. Sorry I think I missed the "skip memory" part. I'll update later

Comment 9

[Nevin Chen(Not active on Bugzilla)]

Yes, I've read comment 3. Sorry I think I missed the "skip memory" part. I'll update later.

Comment 10

[Nevin Chen(Not active on Bugzilla)]

Attachment: Bug 1307621 - Skip memory because it may contain a small version.

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

Comment 11

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8807041 [details]
Bug 1307621 - Skip memory because it may contain a small version.

https://reviewboard.mozilla.org/r/90322/#review90334

Comment 12

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8806627 [details]
Bug 1307621 - Use the original size if the downloaded favicon is not big enough.

https://reviewboard.mozilla.org/r/90018/#review90466

Looking with Web IDE at the HTML code our app gets, the actual problem seems to be that google.co.nz only serves a single icon to us:
http://www.google.co.nz/images/branding/product/ico/googleg_lodp.ico

And that's a 32x32 icon. No matter what we do this will always have low resolution.

Comment 13

[Nevin Chen(Not active on Bugzilla)]

Yes. But should we keep the low resolution image as it and put it in the center of the launch image(this is my current approach)? Or we just stretch it (The original approach)? 
I think the first approach is better because it's the website's owner's choice to provide what resolution of their favicon.

Comment 14

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: fallback-launcher-icons.png

That's a good question for UX. Antlam what do you think is the best to do here?

See screenshot:

A) Keep our current fallback. Stretch the icon and draw the colored background.

B) New patch: Draw the colored background and draw the icon in original size

C) (I think that's what Chrome does): Ignore too small icons and just generate a default icon (colored background + letter, like we do in the UI)

Everyone of them is not perfect. Maybe we want to create better icon templates/stencils for fallback icons or somehow generate 'material' launcher icons as fallback.

Comment 15

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8809361 [details]
fallback-launcher-icons.png

See comment above.

Comment 16

[Sebastian Kaspari (:sebastian; :pocmo)]

Additionally, but that's something for a different bug, Chrome seems to take this high resolution image (if available) and then adds it to a a round launcher icon template:

<meta content="/images/branding/googleg/1x/googleg_standard_color_128dp.png" itemprop="image">

Comment 17

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

Ahunt was working on something similar in bug 1254663 and it seems similar enough that I think we should be following the same logic in both places.

Ahunt, where did we land on that bug? We can even maybe even use the tippy top pack for ideal situations as well.

Comment 18

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

Comment on attachment 8809361 [details]
fallback-launcher-icons.png

Spoke in person, we should be consistent here with the logic we establish in bug 1254663. 

So I don't want to make a call here yet. Waiting for what our logic is there.

Comment 19

[Nevin Chen(Not active on Bugzilla)]

Hi Joe
Can you please help decide what icon should be shown here?  Cause I think bug 1254663 is not confirmed the solution either. I prefer approach C in Attachment #8809361 [details] cause it looks better. And we should use the same logic for every site. (e.g. google.co.nz for example it'll still use approach C since their favico is 16*16)

Comment 20

[Joe Cheng [:jcheng] (please needinfo)]

agree with :antlam on comment #18 that we should be consistent in our approach
base on the discussions in bug 1254663, approach A is out of the question, so that leaves us with B and/or C depending on the size of the favicon. The background for approach B seems a bit off to me but it's really not my expertise so let's have :pekochen to take a look as well

Comment 21

[Nevin Chen(Not active on Bugzilla)]

Since there's already PWA work on this, I think we should close this bug.
Please feel free to open it again if you think it's still valid.