Last Comment Bug 553631 - Changing state of installed addon does not prompting a restart (enable, disable)
: Changing state of installed addon does not prompting a restart (enable, disable)
Status: VERIFIED FIXED
[rewrite][mozmill-test-needed]
:
Product: Toolkit
Classification: Components
Component: Add-ons Manager (show other bugs)
: Trunk
: All All
: P2 normal (vote)
: mozilla2.0b5
Assigned To: Dave Townsend [:mossop]
:
: Andy McKay [:andym]
Mentors:
Depends on: 585950
Blocks: 550048
  Show dependency treegraph
 
Reported: 2010-03-19 10:20 PDT by Tony Chung [:tchung]
Modified: 2011-04-01 07:53 PDT (History)
10 users (show)
dtownsend: in‑testsuite+
hskupin: in‑litmus+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta5+


Attachments
Diagram: Disabling an add-on with no restart, disabling an add-on with a restart, and re-enabling an add-on with a restart (720.78 KB, image/png)
2010-03-25 16:25 PDT, Jennifer Morrow [:Boriss] (UX)
no flags Details
Diagram: Multiple notifications in the add-ons manager replacing each other (669.49 KB, image/png)
2010-04-06 16:04 PDT, Jennifer Morrow [:Boriss] (UX)
no flags Details
Diagram: Multiple notifications in the add-ons manager replacing each other (corrected) (671.79 KB, image/png)
2010-04-06 16:08 PDT, Jennifer Morrow [:Boriss] (UX)
no flags Details

Description Tony Chung [:tchung] 2010-03-19 10:20:06 PDT
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 2010-03-22 08:18:33 PDT
I thought part of the point of the new backend was so that restart was not needed.
Comment 2 Dave Townsend [:mossop] 2010-03-22 08:38:31 PDT
(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) 2010-03-25 16:25:03 PDT
Created attachment 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.
Comment 4 Dave Townsend [:mossop] 2010-03-25 16:51:51 PDT
(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] 2010-03-25 16:54:50 PDT
(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) 2010-04-06 16:04:58 PDT
Created attachment 437435 [details]
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) 2010-04-06 16:08:03 PDT
Created attachment 437437 [details]
Diagram: Multiple notifications in the add-ons manager replacing each other (corrected)
Comment 8 Henrik Skupin (:whimboo) 2010-04-07 01:16:18 PDT
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) 2010-04-28 01:04:01 PDT
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] 2010-04-28 13:58:15 PDT
What we have is good enough for the trunk landing I think, thanks Blair.
Comment 11 Jennifer Morrow [:Boriss] (UX) 2010-04-29 14:25:58 PDT
(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] 2010-08-05 09:36:56 PDT
Needed by string freeze
Comment 13 Blair McBride [:Unfocused] (UNAVAILABLE) 2010-08-20 16:00:52 PDT
The rest of this will fixed by the work done in bug 585950.
Comment 14 Dave Townsend [:mossop] 2010-08-23 14:37:43 PDT
Fixed by bug 585950
Comment 15 Henrik Skupin (:whimboo) 2010-08-25 06:00:19 PDT
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] 2010-08-25 07:43:18 PDT
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) 2011-04-01 07:53:25 PDT
This is covered by:
https://litmus.mozilla.org/show_test.cgi?id=10150

Note You need to log in before you can comment on or make changes to this bug.