741435 - Web App shortcuts should use the App Icon 'as is' and not add a background

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Web Apps (PWAs), defect)

Description

[Jason Smith [:jsmith]]

Steps:

	1. Install the latest fennec native nightly
	2. Go to apps.mozillalabs.com/appdir
	3. Select an app to install and hit ok
	4. Go to about:apps
	5. Long press the app you installed
	6. Select "Add to Home Screen"
	7. Go to Home Screen

Expected:

The app shortcut should be there at the same size of an Android App Icon. The background of the icon should be transparent.

Actual:

The app shortcut is far smaller than an Android app icon, making the icon a bit hard to read. There is also a colored background around the icon that is unnecessary and does not follow what native Android app icons look like.

Device: Samsung Galaxy Nexus
OS: Android 4.02
Build: Fennec Native Nightly

Comment 1

[Jason Smith [:jsmith]]

Attachment: Web App Android Shortcut

Comment 2

[Jason Smith [:jsmith]]

Moving to Fennec Native General - This applies to more than just web apps, as the bug also occurs in this scenario:

Steps:

1. Start Fennec Native
2. Click the URL bar
3. Go to bookmarks
4. Hard press a bookmark
5. Select add to home screen

Expected/Actual behavior same as what happened with web apps.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

No. Bookmarks typically have a 16px favicon. Stretching 16px image to >64px (depends on the screen res) creates an ugly icon. We intentionally keep the bookmark image small and add the background.

Moving back to webapps

Comment 4

[Aaron Train [:aaronmt]]

Sounds like this is WONT-FIX.

Comment 5

[Wesley Johnston (:wesj)]

There's two potential bugs here.

One is to use a high res favicon if we can. What is high res "enough" will probably depend on the dpi of the device to some extent.

Two, the screenshot posted looks different than shortcuts typically do on my phone. I wonder if we need to make some adjustments on xhdpi screens? I have seen things not work correctly on my tablet (which i assume is mdpi) as well.

Comment 6

[Wesley Johnston (:wesj)]

I think we have a bug somewhere for the high res favicons. We also already use them if the page has a sizes attribute on its icons. What's missing (I think) is support for Apple's proprietary extensions.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Wesley Johnston (:wesj) from comment #5)

> One is to use a high res favicon if we can. What is high res "enough" will
> probably depend on the dpi of the device to some extent.

For webapps, the icons provided _should_ be high res enough. At least one of them should be anyway.

> Two, the screenshot posted looks different than shortcuts typically do on my
> phone. I wonder if we need to make some adjustments on xhdpi screens? I have
> seen things not work correctly on my tablet (which i assume is mdpi) as well.

I think we need to tell the shortcut code when to use the "fancy background" and when to just use the image "as is". For webapps, we'd use the image "as is".

Comment 8

[Jason Smith [:jsmith]]

(In reply to Mark Finkle (:mfinkle) from comment #3)
> No. Bookmarks typically have a 16px favicon. Stretching 16px image to >64px
> (depends on the screen res) creates an ugly icon. We intentionally keep the
> bookmark image small and add the background.

Looking here - http://en.wikipedia.org/wiki/Favicon and other sources states that favicons can have other sizes other than 16x16. This similar to how web apps icons work, although for that case, the typical size used I believe is 64x64, but other sizes are possible.

I should note though that the small icon size with the background basically makes the icon very hard to read. I suggest rethinking the approach here.

> 
> Moving back to webapps

I disagree that this is a webapps-specific issue, given that this has to do with how you handle both favicons and web app icons. Or more generally icon management of different sizes other than 16x16. This sounds like an issue that applies fennec native generally, especially if there are other features I'm not thinking of that utilize home screen shortcut features. I think you need to be able to handle cases where icons are not 16x16.

I agree that stretching an icon from 16x16 to 64x64 probably isn't a good idea. The concern I think here is having to do with handling icons of sizes other than 16x16. It's less typical for a website to do it, but for a web app icon, 64x64 is more likely. I still think this applies to general, not web apps (the issue is more likely in web apps, but not a web apps specific issue).

(In reply to Aaron Train [:aaronmt] from comment #4)
> Sounds like this is WONT-FIX.

I don't think so. With web apps, icons are typically 64x64. We need to handle that case.

(In reply to Mark Finkle (:mfinkle) from comment #7)
> (In reply to Wesley Johnston (:wesj) from comment #5)
> 
> > One is to use a high res favicon if we can. What is high res "enough" will
> > probably depend on the dpi of the device to some extent.
> 
> For webapps, the icons provided _should_ be high res enough. At least one of
> them should be anyway.
> 
> > Two, the screenshot posted looks different than shortcuts typically do on my
> > phone. I wonder if we need to make some adjustments on xhdpi screens? I have
> > seen things not work correctly on my tablet (which i assume is mdpi) as well.
> 
> I think we need to tell the shortcut code when to use the "fancy background"
> and when to just use the image "as is". For webapps, we'd use the image "as
> is".

As a note - Icons can be various sizes for webapps, so be sure to take that into account in designing this (even the example here https://developer.mozilla.org/en/Apps/Manifest shows 16, 48, 128 being used).

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Jason Smith from comment #8)
> (In reply to Mark Finkle (:mfinkle) from comment #3)
> > No. Bookmarks typically have a 16px favicon. Stretching 16px image to >64px
> > (depends on the screen res) creates an ugly icon. We intentionally keep the
> > bookmark image small and add the background.
> 
> Looking here - http://en.wikipedia.org/wiki/Favicon and other sources states
> that favicons can have other sizes other than 16x16. This similar to how web
> apps icons work, although for that case, the typical size used I believe is
> 64x64, but other sizes are possible.

Those are so freaking rare on the web, we could ignore them. However, we do have a separate bug for using bigger favicons in the Fennec UI when they are available.

> > Moving back to webapps
> 
> I disagree that this is a webapps-specific issue, given that this has to do
> with how you handle both favicons and web app icons. Or more generally icon
> management of different sizes other than 16x16. This sounds like an issue
> that applies fennec native generally, especially if there are other features
> I'm not thinking of that utilize home screen shortcut features. I think you
> need to be able to handle cases where icons are not 16x16.

We purposefully show icons to bookmarks with a background so users are not confused into thinking the icons are apps. I want to treat these issues separately.

> As a note - Icons can be various sizes for webapps, so be sure to take that
> into account in designing this (even the example here
> https://developer.mozilla.org/en/Apps/Manifest shows 16, 48, 128 being used).

Then I hope the 128 version is used when we ask for a 64 or 72 px image. Downsizing a big image looks better than trying to upscale a small image.

Comment 10

[:Margaret Leibovic]

(In reply to Mark Finkle (:mfinkle) from comment #9)
> (In reply to Jason Smith from comment #8)
> > (In reply to Mark Finkle (:mfinkle) from comment #3)
> > > No. Bookmarks typically have a 16px favicon. Stretching 16px image to >64px
> > > (depends on the screen res) creates an ugly icon. We intentionally keep the
> > > bookmark image small and add the background.
> > 
> > Looking here - http://en.wikipedia.org/wiki/Favicon and other sources states
> > that favicons can have other sizes other than 16x16. This similar to how web
> > apps icons work, although for that case, the typical size used I believe is
> > 64x64, but other sizes are possible.
> 
> Those are so freaking rare on the web, we could ignore them. However, we do
> have a separate bug for using bigger favicons in the Fennec UI when they are
> available.

For reference, this is bug 715263.

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

This patch does two things:
* Makes a full-size 64px 'favicon' in about:apps for use as the homescreen shortcut. No padding is needed. We want the full image.
* Adds a simple code path to getLauncherIcon that just uses the image as-is

Comment 12

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: screenshot

Shows the app (Bar Fight) and a bookmark (Firefox: Support)

Comment 13

[Wesley Johnston (:wesj)]

Comment on attachment 616571 [details] [diff] [review]
patch

Review of attachment 616571 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/GeckoAppShell.java
@@ +781,5 @@
>  
>          Bitmap bitmap = Bitmap.createBitmap(kIconSize, kIconSize, Bitmap.Config.ARGB_8888);
>          Canvas canvas = new Canvas(bitmap);
>  
> +        if (aType.equalsIgnoreCase("webapp")) {

We use this string twice in this file. I'd move it to a constant.

Comment 14

[Mark Finkle (:mfinkle) (use needinfo?)]

Soft nom. Basic webapp support has been in Fennec for a while. This just cleans up the creation of the homescreen icons.

Comment 15

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1bb268b42bbe

Comment 16

[(no longer active)]

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

Comment 17

[Aaron Train [:aaronmt]]

Verified Fixed

Nightly (04/29)
Samsung Galaxy Note (Android 2.3), Samsung Galaxy Nexus (Android 4.0.4)

Comment 18

[Aaron Train [:aaronmt]]

MozTrap: https://moztrap.allizom.org/manage/cases/_detail/569/