608428 - Replace default icon in desktop notifications

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Doug Turner (:dougt)]

On android, because we can not specify the icon of our notifications, we are using the default application icon for all desktop notifications:

http://dougt.org/wordpress/wp-content/uploads/n2.png

We discussed this a bit and would like to use a different icon on Android.  The icon should convey the idea of a notification from fennec, but not fennec itself.

Comment 1

[Doug Turner (:dougt)]

madhava, could you help here?

AlexP, could you provide the exact specifications (size, type) for this icon?

Comment 2

[Alex Pakhotin (:alexp)]

We need hi-DPI icons, which are 38x38 PNGs.

The guidelines are here: http://developer.android.com/guide/practices/ui_guidelines/icon_design.html#statusbarstructure

But the actual icon sizes used in real Android system and apps are different than mentioned in that document (which says 48x48).

Here you can find the actual image files for reference (stat_*.png are the ones used for the status bar):
http://android.git.kernel.org/?p=platform/frameworks/base.git;a=tree;f=core/res/res/drawable-hdpi;hb=HEAD

Comment 3

[Sean Martell]

Attachment: notification alert icon

Comment 4

[Alex Pakhotin (:alexp)]

Should this new icon be used for the update notifications as well?

Comment 5

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

(In reply to comment #4)
> Should this new icon be used for the update notifications as well?

No. This new icon should only be used for web-based notifications (notifications created by the web page).

Comment 6

[Madhava Enros [:madhava]]

That's right - this icon is specifically intended to mean "the web page wants to tell you something."

Comment 7

[Alex Pakhotin (:alexp)]

Attachment: Fix

Use R.drawable.alert in GeckoAppShell.showAlertNotification() by default.

Comment 8

[Alex Pakhotin (:alexp)]

Attachment: Fix (mobile)

- Added the new icon
- Made UpdatePrompt to continue using the main app icon for the update notifications

Comment 9

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 491984 [details] [diff] [review]
Fix

why do we want this change? I think this bug is just about using the desktop notification icon for desktop notifications

Comment 10

[Alex Pakhotin (:alexp)]

(In reply to comment #9)
> why do we want this change? I think this bug is just about using the desktop
> notification icon for desktop notifications

As far as I understand this is about default icon - if no icon is provided in the call to showAlertNotification(), or the one is not found in the existing resources, use the new notification icon instead of the app icon.

Comment 11

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 491984 [details] [diff] [review]
Fix

(In reply to comment #10)
> (In reply to comment #9)
> > why do we want this change? I think this bug is just about using the desktop
> > notification icon for desktop notifications
> 
> As far as I understand this is about default icon - if no icon is provided in
> the call to showAlertNotification(), or the one is not found in the existing
> resources, use the new notification icon instead of the app icon.

No, this bug is about notifications form web pages having the same icon as notifications from the application (with all the implied trust level etc. etc.).

Comment 12

[Doug Turner (:dougt)]

what brad said.

Comment 13

[Alex Pakhotin (:alexp)]

Attachment: Fix v2

Specify a new icon in desktop notifications.

Comment 14

[Alex Pakhotin (:alexp)]

Attachment: Fix - ready for submission

The patch contains the icon itself as well.
r=blassey a=blocking-fennec

Comment 15

[Brad Lassey [:blassey] (use needinfo?)]

pushed http://hg.mozilla.org/mozilla-central/rev/fe7649100f19

Comment 16

[Anna (Waverley)]

VERIFIED FIXED:

Mozilla /5.0 (Android;Linux armv7l;rv:7.0a1) Gecko/20110608 Firefox/7.0a1 Fennec/7.0a1 

Mozilla /5.0 (Android;Linux armv7l;rv:6.0a2) Gecko/20110607 Firefox/6.0a2 Fennec/6.0a2 

Device: HTC Desire Z (Android 2.2)