Use notification text rather than title for android tickerText

VERIFIED FIXED

Status

Fennec Graveyard
General
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: wesj, Assigned: wesj)

Tracking

({polish})

Trunk
All
Android
polish

Details

Attachments

(1 attachment, 2 obsolete attachments)

3.19 KB, patch
blassey
: review+
Details | Diff | Splinter Review
(Assignee)

Description

7 years ago
Created attachment 494408 [details]
Fix

Currently we use the Android notification title for ticker text (text shown in the notification bar when the notification first appears). For consistency this is always "Downloads" or "Addons" or something pertaining to the component that's notifying.

It would (usually?) be more useful to show the text which tells the user what is happening. i.e. "Downloading: logo.png", "logo.png has finished downloading" "Downloading addon" "Installation complete. Restart Required". Rather than requiring them to open the notification bar to see what has happened.
(Assignee)

Comment 1

7 years ago
Created attachment 494411 [details] [diff] [review]
Patch

Marked as a patch
Attachment #494408 - Attachment is obsolete: true
Attachment #494411 - Flags: ui-review?(madhava)
Attachment #494411 - Flags: review?(blassey.bugs)
Comment on attachment 494411 [details] [diff] [review]
Patch

The AlertNotification class still names this parameter and the associated member aTitle and mTitle. Please make it all consistent.
Attachment #494411 - Flags: review?(blassey.bugs) → review-

Comment 3

7 years ago
flagging for blocking
tracking-fennec: --- → ?
tracking-fennec: ? → 2.0-
Keywords: polish
We'd probably take a patch, but it doesn't block
(Assignee)

Comment 5

7 years ago
I had this in for a bit and liked it. The text actually IS used as a title in AlertNotification when a progress bar is shown. The more detailed text is helpful as a title during progress, as it has the filename.

Do we want AlertNotification to take in two parameters, one to be used for the titles of progress notifications, and one for the ticker text? And if so, can we still leave the one which will be used as the Title with the names aTitle/mTitle?
(Assignee)

Comment 6

7 years ago
Created attachment 498339 [details] [diff] [review]
Patch v2

Now with more stuff. Defaults to using the text if we have something. Otherwise uses the title.
Attachment #494411 - Attachment is obsolete: true
Attachment #498339 - Flags: review?(blassey.bugs)
Attachment #494411 - Flags: ui-review?(madhava)
Comment on attachment 498339 [details] [diff] [review]
Patch v2


>+        super(aIcon, (aText.length() > 0) ? aText : aTitle, aWhen);

>+            view.setTextViewText(R.id.notificationTitle, (mText.length() > 0) ? mText : mTitle);
can mText ever be null?
Attachment #498339 - Flags: review?(blassey.bugs) → review+
(Assignee)

Comment 8

7 years ago
This is across the bridge, and as far as I know we force these strings to not be null. Do you want an extra check, just to be safe?
(In reply to comment #8)
> This is across the bridge, and as far as I know we force these strings to not
> be null. Do you want an extra check, just to be safe?

nope, that's fine
Comment on attachment 498339 [details] [diff] [review]
Patch v2

android only.  make sure you test locally alot before pushing. zero regressions.
Attachment #498339 - Flags: approval2.0+
(Assignee)

Comment 11

7 years ago
Pushed:

http://hg.mozilla.org/mozilla-central/rev/2e64d11f6885
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Updated

7 years ago
Assignee: nobody → wjohnston

Comment 12

7 years ago
VERIFIED FIXED on:

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

Build Id: Mozilla /5.0 (Android;Linux armv7l;rv:6.0a2) Gecko/20110529 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.