Replace default icon in desktop notifications

VERIFIED FIXED

Status

Firefox for Android Graveyard
General
VERIFIED FIXED
8 years ago
7 years ago

People

(Reporter: dougt, Assigned: alexp)

Tracking

Trunk
All
Android

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

8 years ago
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.
(Reporter)

Updated

8 years ago
tracking-fennec: --- → ?
(Reporter)

Comment 1

8 years ago
madhava, could you help here?

AlexP, could you provide the exact specifications (size, type) for this icon?
Assignee: nobody → madhava
tracking-fennec: ? → 2.0+
(Assignee)

Comment 2

8 years ago
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

8 years ago
Created attachment 490167 [details]
notification alert icon
Assignee: madhava → alexp
(Assignee)

Updated

8 years ago
Attachment #490167 - Flags: ui-review?(madhava)
(Assignee)

Comment 4

8 years ago
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+
(Assignee)

Comment 7

8 years ago
Created attachment 491984 [details] [diff] [review]
Fix

Use R.drawable.alert in GeckoAppShell.showAlertNotification() by default.
Attachment #491984 - Flags: review?(blassey.bugs)
(Assignee)

Comment 8

8 years ago
Created attachment 491986 [details] [diff] [review]
Fix (mobile)

- 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
(Assignee)

Comment 10

8 years ago
(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-
(Reporter)

Comment 12

8 years ago
what brad said.
(Assignee)

Comment 13

8 years ago
Created attachment 492472 [details] [diff] [review]
Fix v2

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+
(Assignee)

Comment 14

8 years ago
Created attachment 492541 [details] [diff] [review]
Fix - ready for submission

The patch contains the icon itself as well.
r=blassey a=blocking-fennec
Attachment #492472 - Attachment is obsolete: true
Attachment #492541 - Flags: review+
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
pushed http://hg.mozilla.org/mozilla-central/rev/fe7649100f19
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Keywords: checkin-needed

Comment 16

7 years ago
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.