615897 - Use notification text rather than title for android tickerText

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Wesley Johnston (:wesj)]

Attachment: 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.

Comment 1

[Wesley Johnston (:wesj)]

Attachment: Patch

Marked as a patch

Comment 2

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

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.

Comment 3

[Tony Chung [:tchung]]

flagging for blocking

Comment 4

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

We'd probably take a patch, but it doesn't block

Comment 5

[Wesley Johnston (:wesj)]

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?

Comment 6

[Wesley Johnston (:wesj)]

Attachment: Patch v2

Now with more stuff. Defaults to using the text if we have something. Otherwise uses the title.

Comment 7

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

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?

Comment 8

[Wesley Johnston (:wesj)]

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?

Comment 9

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

(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 10

[Doug Turner (:dougt)]

Comment on attachment 498339 [details] [diff] [review]
Patch v2

android only.  make sure you test locally alot before pushing. zero regressions.

Comment 11

[Wesley Johnston (:wesj)]

Pushed:

http://hg.mozilla.org/mozilla-central/rev/2e64d11f6885

Comment 12

[Anna (Waverley)]

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)