553631 - Changing state of installed addon does not prompting a restart (enable, disable)

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P2)

Description

[Tony Chung [:tchung]]

Changing state of installed addon does not prompting a restart (enable, disable). 

Repro:
1) install addons branch build: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a4pre) Gecko/20100319 Minefield/3.7a4pre
2) open addons manager
3) install 2 or more addons to the pane
4) Verify if you change the state of the addon (toggle enable, disable), there is not notification or text requiring the user to restart to accept changes.

Expected:
- a note saying restart required

Actual:
- no restart message.

Comment 1

[alanjstr]

I thought part of the point of the new backend was so that restart was not needed.

Comment 2

[Dave Townsend [:mossop]]

(In reply to comment #1)
> I thought part of the point of the new backend was so that restart was not
> needed.

Making traditional extensions work without restarts was never a goal of this rewrite.

Comment 3

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Diagram: Disabling an add-on with no restart, disabling an add-on with a restart, and re-enabling an add-on with a restart

Disabling and re-enabling will indeed require restart for traditional add-ons, and they should prompt the user to restart similarly to an add-on installation or removal.  The attached image shows the case of disabling an add-on without a restart (jetpack case), and disabling and re-enabling with a restart (non-jetpack).  Please note that the "success" message shown after restart is a placeholder for a newer notification system; the current method is the yellow drop-down notification bar.

Comment 4

[Dave Townsend [:mossop]]

(In reply to comment #3)
> Created an attachment (id=435031) [details]
> Diagram: Disabling an add-on with no restart, disabling an add-on with a
> restart, and re-enabling an add-on with a restart
> 
> Disabling and re-enabling will indeed require restart for traditional add-ons,
> and they should prompt the user to restart similarly to an add-on installation
> or removal.  The attached image shows the case of disabling an add-on without a
> restart (jetpack case), and disabling and re-enabling with a restart
> (non-jetpack).  Please note that the "success" message shown after restart is a
> placeholder for a newer notification system; the current method is the yellow
> drop-down notification bar.

How does this cope with the case where you have multiple extensions waiting for a restart or you are looking at the appearance pane but only have extensions requiring a restart?

Comment 5

[Tony Chung [:tchung]]

(In reply to comment #4)
> (In reply to comment #3)
> > Created an attachment (id=435031) [details] [details]
> > Diagram: Disabling an add-on with no restart, disabling an add-on with a
> > restart, and re-enabling an add-on with a restart
> > 
> > Disabling and re-enabling will indeed require restart for traditional add-ons,
> > and they should prompt the user to restart similarly to an add-on installation
> > or removal.  The attached image shows the case of disabling an add-on without a
> > restart (jetpack case), and disabling and re-enabling with a restart
> > (non-jetpack).  Please note that the "success" message shown after restart is a
> > placeholder for a newer notification system; the current method is the yellow
> > drop-down notification bar.
> 
> How does this cope with the case where you have multiple extensions waiting for
> a restart or you are looking at the appearance pane but only have extensions
> requiring a restart?

Kinda like bug 553460 that i describe.

Comment 6

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Diagram: Multiple notifications in the add-ons manager replacing each other

(In reply to comment #4)
>How does this cope with the case where you have multiple extensions waiting for a restart or you are looking at the appearance pane but only have extensions requiring a restart?

The yellow restart notifications are transient - if another action is taken that requires a restart, the first notification is merely replaced by the second.

Comment 7

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Diagram: Multiple notifications in the add-ons manager replacing each other (corrected)

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks Jennifer for those mockups. It looks good. But I wonder if we could use a better color as the yellow one you have chosen now. The current one is really to lighten. What about the color and shading of the old notification bar?

Comment 9

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Got a temporary solution that's different from the mockups:
http://hg.mozilla.org/projects/addonsmgr/rev/991b7daba3cc

I'm still not sure about the notification mockups. It feels wrong having it both transient and specific to the latest action, while being sorted in place with the addons. At the very least, I don't think its going to be possible to (sanely) remember what was the last action performed (ie, which addon was last enabled/disabled/uninstalled) - for when the addon manager is reloaded, opened again, or the pane changed.

Comment 10

[Dave Townsend [:mossop]]

What we have is good enough for the trunk landing I think, thanks Blair.

Comment 11

[Jennifer Morrow [:Boriss] (UX)]

(In reply to comment #10)
> What we have is good enough for the trunk landing I think, thanks Blair.

wfm

Comment 12

[Dave Townsend [:mossop]]

Needed by string freeze

Comment 13

[Blair McBride [:Unfocused] (UNAVAILABLE)]

The rest of this will fixed by the work done in bug 585950.

Comment 14

[Dave Townsend [:mossop]]

Fixed by bug 585950

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b5pre) Gecko/20100824 Minefield/4.0b5pre

What's the coverage of the automated test? Do we need a manual test?

Comment 16

[Dave Townsend [:mossop]]

This is pretty well covered by the automated tests, the only bit that isn't covered is the actual restart itself. I'm going to guess we'd hear pretty quickly if that was a problem but it might be worth a litmus test nonetheless.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This is covered by:
https://litmus.mozilla.org/show_test.cgi?id=10150