[ICS] Home Screen icons are too small on ICS

VERIFIED FIXED in Firefox 11

Status

()

defect
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: madhava, Assigned: mfinkle)

Tracking

unspecified
Firefox 12
x86
macOS
Points:
---

Firefox Tracking Flags

(firefox11 verified, firefox12 verified)

Details

Attachments

(2 attachments)

When you use the "Add to Home Screen" context menu item, the resulting icon (which is otherwise beautiful) is too small on ICS. Looks fine on gingerbread.
Posted patch patchSplinter Review
The Galaxy Nexus is an extra high density device and Android wants the launcher icons to be 96px for that density. This patch tweaks the icon size and the overlay size to be inline with what Android wants for medium, high and extra high densities:

http://developer.android.com/guide/practices/ui_guidelines/icon_design_launcher.html#size
Assignee: nobody → mark.finkle
Attachment #585654 - Flags: review?(wjohnston)
Posted image screenshot
Shows a bugzilla home screen bookmark. Note the size of the icon is right and the size of the "favicon" is not too small.
Comment on attachment 585654 [details] [diff] [review]
patch

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

I'm a bit nervous about how this will handle high resolution icons (the original code wasn't very well designed for those either). The "star" icon we use for pages without a favicon will also travel through that path. We just basically draw it on top of the overlay (essentially assuming the icon is 72x72 so that the two will match). Can you try that out and maybe mbrubeck's homepage (http://limpet.net/mbrubeck/) or USA Today (which for me has a high res icon)?

::: mobile/android/base/GeckoAppShell.java
@@ +671,5 @@
> +                break;
> +            case DisplayMetrics.DENSITY_HIGH:
> +                kIconSize = 72;
> +                kOverlaySize = 32;
> +                break;

Not sure what coding style you want, but I'd probably collapse this with the default state below.
Comment on attachment 585654 [details] [diff] [review]
patch

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

Whoops. r+ assuming the high res icons (at least the star) work. We can file a follow up for pages with their own high res icons (where we shouldn't draw a background at all probably.
Attachment #585654 - Flags: review?(wjohnston) → review+
The USA Today page works pretty well. We can tweak this feature in new bugs as needed.
Comment on attachment 585654 [details] [diff] [review]
patch

Needed for making the home screen shortcuts not look silly on ICS
Attachment #585654 - Flags: approval-mozilla-aurora?
I'll leave this in the queue until we've had a day on m-c.
https://hg.mozilla.org/mozilla-central/rev/0cdaf0773073
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 12
Comment on attachment 585654 [details] [diff] [review]
patch

[Triage Comment]
Mobile only - approved for Aurora.
Attachment #585654 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified fixed on Firefox 11.0a2 (2012-01-10)
                          12.0a1 (2012-01-10)
Device: Samsung Nexus (ICS)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.