620975 - Add-on restart notification does not go away after install

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Ayan Shah[:ashah]]

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.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Should we clear certain types of notification from Java when we quit? Add-on and Download notifications maybe?

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

I don't think we need any new nsI interface for this. Just clear the notifications from Java impl.

Comment 3

[Alex Pakhotin (:alexp)]

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

Comment 4

[Alex Pakhotin (:alexp)]

Attachment: Fix

Cancel the add-on notification on restart.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

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?

Comment 6

[Alex Pakhotin (:alexp)]

(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

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

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.

Comment 8

[Alex Pakhotin (:alexp)]

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

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

(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

Comment 10

[Alex Pakhotin (:alexp)]

Attachment: Fix v2

Hide the notification in the ExtensionsView.init().

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
http://hg.mozilla.org/mobile-browser/rev/a9aba61eb3d9

Comment 12

[Anna (Waverley)]

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)