792373 - Request for an XHDPI branded product icon

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Description

[Henri Sivonen (:hsivonen)]

Steps to reproduce:
1) Place the icons of Nightly, Chrome, and Google Earth next to each other in the launcher on Galaxy Nexus.

Actual results:
The round edge of the Nightly icon looks more pixelated than the round shapes of the Chrome and Google Earth icons.

Expected results:
Expected not to be able to see pixelation.

Additional info:
Maybe Nightly lacks the right icon size and the icon gets scaled by Android?

Comment 1

[Ian Barlow (:ibarlow)]

Thanks for filing this Henri. Can someone please verify the app icon sizes we are shipping right now, and what exact pixel size is needed for XHDPI, if we don't have one yet?

Comment 2

[Aaron Train [:aaronmt]]

I think it's 96x96 (http://developer.android.com/guide/practices/ui_guidelines/icon_design_launcher.html)

http://mxr.mozilla.org/mozilla-central/source/mobile/android/branding/nightly/content/
http://mxr.mozilla.org/mozilla-central/source/mobile/android/branding/official/content/
http://mxr.mozilla.org/mozilla-central/source/mobile/android/branding/beta/content/
http://mxr.mozilla.org/mozilla-central/source/mobile/android/branding/aurora/content/

Fennec
http://mxr.mozilla.org/mozilla-central/source/mobile/android/branding/unofficial/content/

Comment 4

[Eric Pang [:epang] UX]

Attachment: Official Logos sized at XHDPI (96x96px)

Comment 5

[Henri Sivonen (:hsivonen)]

This bug is now highlighted in the Ars Technica review of Nexus 10.

http://arstechnica.com/gadgets/2012/11/nexus-10-tablet-is-a-solid-house-built-on-shifting-sands/

Comment 6

[Aaron Train [:aaronmt]]

This just needs to be converted to a patch and land.

Comment 7

[Lucas Rocha (:lucasr)]

Actually, app icons in the Nexus 10 need to be xxhdpi, this is probably why our icon is blurry there. See: 

https://plus.google.com/u/0/118292708268361843293/posts/ePQya3KsTjW

Ian, we need a 144*144px app icon.

Comment 8

[Sriram Ramasubramanian [:sriram]]

When did they ever come up with an xxhdpi? That's shocking me! Are we having one whole big list of 3x icons and textures?

Comment 9

[Sriram Ramasubramanian [:sriram]]

http://stackoverflow.com/questions/13215587/android-xxhdpi-resources
Thank goodness. It's only for the launcher icon.

Comment 10

[Aaron Train [:aaronmt]]

Wow!

Comment 11

[Eric Pang [:epang] UX]

Attachment: Official Logos sized at XXHDPI (144x144px)

Comment 12

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

Icons in all sharpness. (We should probably uplift all the way to release. This is a trivial bug).

Comment 13

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

Comment on attachment 683318 [details] [diff] [review]
Patch

>diff --git a/mobile/android/branding/aurora/content/Makefile.in b/mobile/android/branding/aurora/content/Makefile.in

> LINUX_BRANDING_FILES = \
> 	fennec_48x48.png \
> 	fennec_72x72.png \
>+	fennec_96x96.png \
>+	fennec_144x144.png \

I don't think you need these changes. Your Makefile changes grab the files from their SRC folder location. We should only need the LINUX_BRANDING_FILES for making .deb packages. Which means we might be able to remove that code.

r+, but test without the LINUX_BRANDING_FILES changes

Comment 14

[Sriram Ramasubramanian [:sriram]]

Yup. It does build fine without those changes. I'll push it with removing those.

Comment 15

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch: Cleanup

Cleaned up icons that aren't used.

Comment 16

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

Comment on attachment 683347 [details] [diff] [review]
Patch: Cleanup

>diff --git a/mobile/android/base/Makefile.in b/mobile/android/base/Makefile.in

> # Bug 567884 - Need a way to find appropriate icons during packaging

Remove the comment too

>-  res/drawable/bookmarkdefaults_favicon_support.png \
>-  res/drawable/bookmarkdefaults_favicon_addons.png \

Do not remove. We use these here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/db/BrowserProvider.java.in#963

>diff --git a/mobile/android/base/resources/drawable/bookmarkdefaults_favicon_addons.png b/mobile/android/base/resources/drawable/bookmarkdefaults_favicon_addons.png
>deleted file mode 100644
>Binary file mobile/android/base/resources/drawable/bookmarkdefaults_favicon_addons.png has changed
>diff --git a/mobile/android/base/resources/drawable/bookmarkdefaults_favicon_support.png b/mobile/android/base/resources/drawable/bookmarkdefaults_favicon_support.png
>deleted file mode 100644
>Binary file mobile/android/base/resources/drawable/bookmarkdefaults_favicon_support.png has changed

Keep these

>diff --git a/mobile/android/branding/aurora/android-resources.mn b/mobile/android/branding/aurora/android-resources.mn

>-mobile/android/branding/aurora/content/bookmarkdefaults_favicon_aboutfirefox.png

Keep

>diff --git a/mobile/android/branding/aurora/content/bookmarkdefaults_favicon_aboutfirefox.png b/mobile/android/branding/aurora/content/bookmarkdefaults_favicon_aboutfirefox.png

Keep

>diff --git a/mobile/android/branding/aurora/content/jar.mn b/mobile/android/branding/aurora/content/jar.mn

>-  content/branding/favicon32.png                 (favicon32.png)

favicon32.png is used in our about pages. Do not remove


>diff --git a/mobile/android/branding/beta/android-resources.mn b/mobile/android/branding/beta/android-resources.mn

>-mobile/android/branding/beta/content/bookmarkdefaults_favicon_aboutfirefox.png

Keep. See above.

>diff --git a/mobile/android/branding/beta/content/bookmarkdefaults_favicon_aboutfirefox.png b/mobile/android/branding/beta/content/bookmarkdefaults_favicon_aboutfirefox.png

Keep

>diff --git a/mobile/android/branding/beta/content/jar.mn b/mobile/android/branding/beta/content/jar.mn

>-  content/branding/favicon32.png                 (favicon32.png)

Keep

>diff --git a/mobile/android/branding/nightly/android-resources.mn b/mobile/android/branding/nightly/android-resources.mn

>-mobile/android/branding/nightly/content/bookmarkdefaults_favicon_aboutfirefox.png

Keep. See above.

>diff --git a/mobile/android/branding/nightly/content/bookmarkdefaults_favicon_aboutfirefox.png b/mobile/android/branding/nightly/content/bookmarkdefaults_favicon_aboutfirefox.png

Keep

>diff --git a/mobile/android/branding/nightly/content/jar.mn b/mobile/android/branding/nightly/content/jar.mn

>-  content/branding/favicon32.png                 (favicon32.png)

Still used in about pages

>diff --git a/mobile/android/branding/official/android-resources.mn b/mobile/android/branding/official/android-resources.mn

>-mobile/android/branding/official/content/bookmarkdefaults_favicon_aboutfirefox.png

Still needed. See above

>diff --git a/mobile/android/branding/official/content/bookmarkdefaults_favicon_aboutfirefox.png b/mobile/android/branding/official/content/bookmarkdefaults_favicon_aboutfirefox.png

Keep

>diff --git a/mobile/android/branding/official/content/jar.mn b/mobile/android/branding/official/content/jar.mn

>-  content/branding/favicon32.png                 (favicon32.png)

Still needed

>diff --git a/mobile/android/branding/unofficial/android-resources.mn b/mobile/android/branding/unofficial/android-resources.mn

>-mobile/android/branding/unofficial/content/bookmarkdefaults_favicon_aboutfirefox.png

Still needed

>diff --git a/mobile/android/branding/unofficial/content/bookmarkdefaults_favicon_aboutfirefox.png b/mobile/android/branding/unofficial/content/bookmarkdefaults_favicon_aboutfirefox.png

Still needed

>diff --git a/mobile/android/branding/unofficial/content/jar.mn b/mobile/android/branding/unofficial/content/jar.mn

>-  content/branding/favicon32.png                 (favicon32.png)

Still needed

Comment 17

[Sriram Ramasubramanian [:sriram]]

Part (1/2): 
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a18061fec2e

Comment 18

[Ryan VanderMeulen [:RyanVM]]

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

Comment 19

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 683318 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): -
User impact if declined: The launcher showing the icon will be fuzzy and of low quality as mentioned in http://arstechnica.com/gadgets/2012/11/nexus-10-tablet-is-a-solid-house-built-on-shifting-sands/ (almost bottom of the article). This is a problem for our brand.
Testing completed (on m-c, etc.): Landed yesterday.
Risk to taking this patch (and alternatives if risky): Low to None.
String or UUID changes made by this patch: None.

Comment 20

[Alex Keybl [:akeybl]]

Comment on attachment 683318 [details] [diff] [review]
Patch

Asset swap - low/no risk.

Comment 21

[Alex Keybl [:akeybl]]

s/swap/addition/

Comment 22

[Sriram Ramasubramanian [:sriram]]

Aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/73f7d2c5fe4f
Beta: https://hg.mozilla.org/releases/mozilla-beta/rev/36cbd3e6a3ac

Comment 23

[Aaron Train [:aaronmt]]

Is this resolved now?

Comment 24

[Sriram Ramasubramanian [:sriram]]

(In reply to Mark Finkle (:mfinkle) from comment #16)
> Comment on attachment 683347 [details] [diff] [review]
> Patch: Cleanup
> 
> >diff --git a/mobile/android/base/Makefile.in b/mobile/android/base/Makefile.in
> 
> > # Bug 567884 - Need a way to find appropriate icons during packaging
> 
> Remove the comment too
> 
> >-  res/drawable/bookmarkdefaults_favicon_support.png \
> >-  res/drawable/bookmarkdefaults_favicon_addons.png \
> 
> Do not remove. We use these here:
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/db/
> BrowserProvider.java.in#963

http://mxr.mozilla.org/mozilla-central/search?string=bookmarkdefaults_favicon_addons

This doesn't show up.