Closed Bug 619432 Opened 9 years ago Closed 9 years ago

AUS notification fails and is not offered again if Fennec closes during update

Categories

(Firefox for Android Graveyard :: General, defect, major)

defect
Not set
major

Tracking

(fennec2.0+)

VERIFIED FIXED
Tracking Status
fennec 2.0+ ---

People

(Reporter: dre, Assigned: alexp)

Details

Attachments

(1 file, 1 obsolete file)

Mozilla/5.0 (Android; Linux armv7l; rv:2.0b8pre) Gecko/20101214 Firefox/4.0b8pre Fennec/4.0b3pre

STR:

Terminate any running instance of Fennec
Start Fennec
Go to "About Fennec" page
Click Check for Updates
See notification of new update available
Click notification
Begin downloading update
Terminate Fennec
Start Fennec
Go to "About Fennec" page
Click Check for Updates

Expected results:
Notification of new update is displayed again

Actual results:
The button says an update is available, but the notification icon does not appear.
At some point in the future it fixes itself and the update will be offered, but I don't know what the criteria is for fixing it.
Assignee: nobody → alexp
i've seen this myself.  Triggering the check for updates should bring the notification back.
tracking-fennec: --- → ?
tracking-fennec: ? → 2.0+
Attached patch Fix (obsolete) — Splinter Review
Check for update in progress on browser startup.
Attachment #500153 - Flags: review?(mark.finkle)
Comment on attachment 500153 [details] [diff] [review]
Fix

The only part of this patch I don't like is the need for gIsNotificationDisplayed. Do we really need it?

Why are we using the raw alerts service in checkForUpdates instead of using _showNotification?
(In reply to comment #3)

> The only part of this patch I don't like is the need for
> gIsNotificationDisplayed. Do we really need it?

Well, it's not really required, nothing should break without it, that's just an attempt to avoid extra calls to UpdateManager and AlertService in case if the notification was already displayed. But currently we'll probably have the only call to this checkForUpdates, so not having that global flag should not hurt.

> Why are we using the raw alerts service in checkForUpdates instead of using
> _showNotification?

The update being downloaded is not provided to this function, it is obtained from nsIUpdateManager.activeUpdate, which theoretically could be null, so passing it as-is to _showNotification and from there to _handleUpdate would be dangerous.
But I guess I could check aUpdate for null in _handleUpdate and just return from there if it's not valid.
(In reply to comment #4)
> But I guess I could check aUpdate for null in _handleUpdate and just return
> from there if it's not valid.

Looks like even this is not required, as _handleUpdate uses the aUpdate argument only for other modes. OK, I'll change to _showNotification (as a matter of fact, this was my original intention).
Attached patch Fix v2Splinter Review
Fixed review comments.
Attachment #500153 - Attachment is obsolete: true
Attachment #502188 - Flags: review?(mark.finkle)
Attachment #500153 - Flags: review?(mark.finkle)
Comment on attachment 502188 [details] [diff] [review]
Fix v2

Thanks for the extra tweaks. Looks good.
Attachment #502188 - Flags: review?(mark.finkle) → review+
pushed:
http://hg.mozilla.org/mobile-browser/rev/670e6b6c2140
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
verified FIXED on build:

Mozilla/5.0 (Android; Linux armv71; rv:2.0b9pre) Gecko/20100109 Namoroka/4.0b9pre Fennec/4.0b4pre
Status: RESOLVED → VERIFIED
Flags: in-litmus?
https://litmus.mozilla.org/show_test.cgi?id=15211
Flags: in-litmus? → in-litmus+
You need to log in before you can comment on or make changes to this bug.