Closed Bug 620975 Opened 9 years ago Closed 9 years ago

Add-on restart notification does not go away after install

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set

Tracking

(fennec2.0+)

VERIFIED FIXED
Tracking Status
fennec 2.0+ ---

People

(Reporter: ashah, Assigned: alexp)

Details

(Keywords: regression, Whiteboard: [4.0b3])

Attachments

(1 file, 1 obsolete file)

ID:
Mozilla/5.0 (Android; Linux armv7l; rv:2.0b8) Gecko/20101221 Firefox/4.0b8 Fennec/4.0b3

Device: Nexus One

STR:
1. Go to addons.mozilla.org
2. Install any add-on by clicking "Add to Mobile"
3. Click on "Install" on the context menu that shows up.
4. There will be a notification in the android system menu. Swipe down the system bar to see the notification. But make sure you dont click the notification.
5. Open the control panel and go to the addons manager.
6. Click the restart button on the blue notification bar.

Expected Result:
The system notification should not be present.

Actual result:
The system notification is still there. Clicking on that takes you to the addons manager.
OS: Mac OS X → Android
Hardware: x86 → ARM
Whiteboard: [4.0b3]
tracking-fennec: --- → ?
Keywords: regression
Assignee: nobody → alexp
tracking-fennec: ? → 2.0+
Should we clear certain types of notification from Java when we quit? Add-on and Download notifications maybe?
I don't think we need any new nsI interface for this. Just clear the notifications from Java impl.
(In reply to comment #1)
> Should we clear certain types of notification from Java when we quit? Add-on
> and Download notifications maybe?

Makes sense, but there are two issues with that:
1. In the current implementation Java layer doesn't differentiate these notifications from the other - they are all the same, with generic implementation.
2. As I mentioned in bug 604491, we cannot rely on the shut down callback to do anything. Most of the time Fennec gets killed without onDestroy() being called.

But we can and should clear the add-on notification on restart, as this is a standard workflow for the add-ons installation. And we don't need any new interface, as nsIAlertsProgressListener already has onCancel() method, so the following call will work:
Cc["@mozilla.org/alerts-service;1"].getService(Ci.nsIAlertsService).QueryInterface(Ci.nsIAlertsProgressListener).onCancel("addons");
We just need to add this in the "Restart" button handler.
Attached patch Fix (obsolete) — Splinter Review
Cancel the add-on notification on restart.
Attachment #501514 - Flags: review?(mark.finkle)
Comment on attachment 501514 [details] [diff] [review]
Fix

will this get the notification removed if we are killed or crash? If not, then maybe we should just close the add-on notification when we init ExtensionsView?
(In reply to comment #5)
> will this get the notification removed if we are killed or crash?

No, only when the Restart button is clicked.

> If not, then
> maybe we should just close the add-on notification when we init ExtensionsView?

Yes, we can do this.

Somewhere around here?
http://mxr.mozilla.org/mobile-browser/source/chrome/content/extensions.js#240
Let's do it here:
http://mxr.mozilla.org/mobile-browser/source/chrome/content/extensions.js#235

"delayInit" won't be called until the user displays the Add-on Manager. "init" will be called right after the initial startpage loads.
(In reply to comment #7)
> Let's do it here:
> http://mxr.mozilla.org/mobile-browser/source/chrome/content/extensions.js#235
> 
> "delayInit" won't be called until the user displays the Add-on Manager. "init"
> will be called right after the initial startpage loads.

Oh, you mean to hide it _after_ restart? Yes this probably is the best way.
(In reply to comment #8)
> (In reply to comment #7)
> > Let's do it here:
> > http://mxr.mozilla.org/mobile-browser/source/chrome/content/extensions.js#235
> > 
> > "delayInit" won't be called until the user displays the Add-on Manager. "init"
> > will be called right after the initial startpage loads.
> 
> Oh, you mean to hide it _after_ restart? Yes this probably is the best way.

Yes
Attached patch Fix v2Splinter Review
Hide the notification in the ExtensionsView.init().
Attachment #501514 - Attachment is obsolete: true
Attachment #502090 - Flags: review?(mark.finkle)
Attachment #501514 - Flags: review?(mark.finkle)
Attachment #502090 - Flags: review?(mark.finkle) → review+
pushed:
http://hg.mozilla.org/mobile-browser/rev/a9aba61eb3d9
Status: NEW → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
VERIFIED FIXED on Build ID: Mozilla /5.0 (Android;Linux armv7l;rv:2.0b9pre) Gecko/20110111 Firefox/4.0b9pre Fennec /4.0b4pre

Device: Motorola Droid 2 (Android 2.2)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.