Closed Bug 608428 Opened 11 years ago Closed 10 years ago

Replace default icon in desktop notifications

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(fennec2.0+)

VERIFIED FIXED
Tracking Status
fennec 2.0+ ---

People

(Reporter: dougt, Assigned: alexp)

Details

Attachments

(2 files, 3 obsolete files)

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.
tracking-fennec: --- → ?
madhava, could you help here?

AlexP, could you provide the exact specifications (size, type) for this icon?
Assignee: nobody → madhava
tracking-fennec: ? → 2.0+
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
Attached image notification alert icon
Assignee: madhava → alexp
Attachment #490167 - Flags: ui-review?(madhava)
Should this new icon be used for the update notifications as well?
(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).
That's right - this icon is specifically intended to mean "the web page wants to tell you something."
Attachment #490167 - Flags: ui-review?(madhava) → ui-review+
Attached patch Fix (obsolete) — Splinter Review
Use R.drawable.alert in GeckoAppShell.showAlertNotification() by default.
Attachment #491984 - Flags: review?(blassey.bugs)
Attached patch Fix (mobile) (obsolete) — Splinter Review
- Added the new icon
- Made UpdatePrompt to continue using the main app icon for the update notifications
Attachment #491986 - Flags: review?(mark.finkle)
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
(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 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.).
Attachment #491984 - Flags: review?(blassey.bugs) → review-
what brad said.
Attached patch Fix v2 (obsolete) — Splinter Review
Specify a new icon in desktop notifications.
Attachment #491984 - Attachment is obsolete: true
Attachment #491986 - Attachment is obsolete: true
Attachment #492472 - Flags: review?(blassey.bugs)
Attachment #491986 - Flags: review?(mark.finkle)
Attachment #492472 - Flags: review?(blassey.bugs) → review+
The patch contains the icon itself as well.
r=blassey a=blocking-fennec
Attachment #492472 - Attachment is obsolete: true
Attachment #492541 - Flags: review+
Keywords: checkin-needed
pushed http://hg.mozilla.org/mozilla-central/rev/fe7649100f19
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
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)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.