369075 - Add permanent button for restarting Firefox to Add-ons Mgr

Status: VERIFIED FIXED

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

Description

[Dão Gottwald [:dao]]

Attachment: patch

From the wiki:

> Should always be a "Restart Firefox" button available.
> Design TBD. Leaving as a P1.

This has annoyed me for some time, so here's my proposal:

The button is always enabled and always visible, except for the "update-only" case (i.e. when starting Firefox). The "Get Extensions" resp. "Get Themes" link is moved to the center, since it looked unbalanced and the button was moving when switching panels. (Alternatively, the link could stay right-aligned, but on the button's left.)

Comment 1

[Ria Klaassen (not reading all bugmail)]

Would be nice if it didn't login everywhere automatically after restarting. I have currently a button that does this. That's the reason why I don't use it very often.

Comment 2

[Ria Klaassen (not reading all bugmail)]

(In reply to comment #1)
> 
I guess that's Bug 345345.

Comment 4

[Robert Strong (they/them - no direct email)]

The button should not be available when an installation is being performed as well.

Comment 5

[Robert Strong (they/them - no direct email)]

(In reply to comment #4)
> The button should not be available when an installation is being performed as
> well.
or warn the user and provide the option to not restart.

Comment 6

[Dão Gottwald [:dao]]

yes, I went over the top. I'll revert the change in updateGlobalCommands().

Comment 7

[Dão Gottwald [:dao]]

Attachment: patch, hopefully unraveled

looks like gExtensionManager doesn't notify gDownloadManager when the initial state for a new download is set. Hence, an extra updateGlobalCommands() for each gExtensionManager.addDownloads(...), except for the case where showView("installs") gets already called (it includes updateGlobalCommands()).

besides, the logics of updateGlobalCommands() seemed weird to me, so I changed that.

Comment 8

[Robert Strong (they/them - no direct email)]

Comment on attachment 253818 [details] [diff] [review]
patch, hopefully unraveled

Could you attach a screenshot of the add-ons mgr. with the patch applied? Thanks

Comment 9

[Dão Gottwald [:dao]]

Attachment: screenshots

Comment 10

[Robert Strong (they/them - no direct email)]

Comment on attachment 254398 [details]
screenshots

beltzner: can you think of a better way to lay this out or are you fine with it?

Comment 11

[Robert Strong (they/them - no direct email)]

Attachment: search engine mgr screenshot

beltzner: how about putting the link in the same place as the search engine mgr? We should probably show a link all of the time if this is done since the list height would change when changing views to locales, plugins, etc. which don't have an url but I think that would be cleaner.

Comment 12

[Robert Strong (they/them - no direct email)]

Comment on attachment 253818 [details] [diff] [review]
patch, hopefully unraveled

clearing review until ui review has been performed

Comment 14

[Ray Kiddy]

Is 300414 actually a duplicate of this bug? If this bug is about adding the button to the Add-on window, can it be renamed to "Add permanent button for restarting Firefox to Add-on window"? Otherwise, it is unclear. If there are other windows that the "Restart Firefox" button should appear in, can they be listed out in this bug?

What is the window shown in the screen shot? Is it an old window? Is it Windows only? If that window still exists in Firefox, how does one get to it?

Comment 15

[Robert Strong (they/them - no direct email)]

(In reply to comment #14)
> Is 300414 actually a duplicate of this bug?
It is
> If this bug is about adding the
> button to the Add-on window, can it be renamed to "Add permanent button for
> restarting Firefox to Add-on window"?
Done

> Otherwise, it is unclear. If there are
> other windows that the "Restart Firefox" button should appear in, can they be
> listed out in this bug?
Then it would be filed under a different component but I agree with changing the summary so the point is moot.
 
> What is the window shown in the screen shot? Is it an old window? Is it Windows
> only? If that window still exists in Firefox, how does one get to it?
The theming is just different between Mac OS X and Windows / Linux

Comment 16

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 254398 [details]
screenshots

To be honest, I'd rather we pull the "Get Extensions" text out entirely rather than center it. Let's deal with that in a separate bug, though.

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #11)
> beltzner: how about putting the link in the same place as the search engine
> mgr? We should probably show a link all of the time if this is done since the
> list height would change when changing views to locales, plugins, etc. which
> don't have an url but I think that would be cleaner.

To be honest, I think I'd rather have a better way of promoting getting extensions. Do we know how many people click on this link? I can't remember if we added something to the URI target or not, but if we can pull that info out of Urchin, it'd be great.

Comment 18

[Dave Townsend [:mossop] (PTO until Aug 21st)]

The url is http://%LOCALE%.add-ons.mozilla.com/%LOCALE%/%APP%/%VERSION%/extensions/ so I'm guessing not.

Comment 19

[Mike Beltzner [:beltzner, not reading bugmail]]

Well, I think pulling stats for how many people link directly there (ie: no referrer URL) instead of to the AMO front page will give us some idea.

Comment 21

[Robert Strong (they/them - no direct email)]

Comment on attachment 253818 [details] [diff] [review]
patch, hopefully unraveled

Thanks!

Comment 22

[Robert Strong (they/them - no direct email)]

Filed bug 385394 to cover the replacement of the links.

Comment 23

[Dave Townsend [:mossop] (PTO until Aug 21st)]

Checking in extensions.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/content/extensions.js,v  <--  extensions.js
new revision: 1.120; previous revision: 1.119
done
Checking in extensions.xul;
/cvsroot/mozilla/toolkit/mozapps/extensions/content/extensions.xul,v  <--  extensions.xul
new revision: 1.56; previous revision: 1.55
done

Comment 24

[Jesse Ruderman]

I don't like this.  I'm worried that users will click the button accidentally, or worse, think that the only way to leave the Add-ons Manager window is to restart Firefox.

Comment 25

[Ray Kiddy]

On the Mac this is not a problem because every window has a close button and everyone knows what it is. Is this more of a problem on other platforms?

Comment 26

[Tony Chung [:tchung]]

regarding comment 24, can we add a pop up confirmation dialog asking users if they really want to restart firefox?   It'd be even nice to use the quit confirmation dialog from Bug 383760

Comment 27

[Tony Chung [:tchung]]

nevermind, looks like that quit confirmation dialog is there.  i was testing on a funky profile where the setting was getting ignored.  

Comment 28

[Simon Bünzli]

(In reply to comment #27)
This might change with bug 385425 though (see also bug 385582 comment #0).

(In reply to comment #24)
> I'm worried that users will click the button accidentally,

What about disabling the button until the user makes a change which would require a restart (enabling/disabling/uninstalling an extension or changing/uninstalling a theme)? Or is that restart button the official way to restart an application even for non-extension related issues?

Comment 29

[Dave Townsend [:mossop] (PTO until Aug 21st)]

(In reply to comment #28)
> What about disabling the button until the user makes a change which would
> require a restart (enabling/disabling/uninstalling an extension or
> changing/uninstalling a theme)? Or is that restart button the official way to
> restart an application even for non-extension related issues?

To be honest that is what I thought this PRD was meant to do. I had assumed the patch did this given the disabled button in the screenshot but it looks like that's only during installs.

Comment 30

[Robert Strong (they/them - no direct email)]

(In reply to comment #29)
> (In reply to comment #28)
> > What about disabling the button until the user makes a change which would
> > require a restart (enabling/disabling/uninstalling an extension or
> > changing/uninstalling a theme)? Or is that restart button the official way to
> > restart an application even for non-extension related issues?
> 
> To be honest that is what I thought this PRD was meant to do. I had assumed the
> patch did this given the disabled button in the screenshot but it looks like
> that's only during installs.
> 

(In reply to comment #0)
> Created an attachment (id=253743) [details]
> patch
> 
> From the wiki:
> 
> > Should always be a "Restart Firefox" button available.
> > Design TBD. Leaving as a P1.
> 
> This has annoyed me for some time, so here's my proposal:
> 
> The button is always enabled and always visible, except for the "update-only"
> case (i.e. when starting Firefox). The "Get Extensions" resp. "Get Themes" link
> is moved to the center, since it looked unbalanced and the button was moving
> when switching panels. (Alternatively, the link could stay right-aligned, but
> on the button's left.)

I prefer the only available when applicable but both bug summary and comment #0 are pretty clear about it being permanently available. Bueller??? I mean Beltzner???

Comment 31

[Dão Gottwald [:dao]]

(In reply to comment #28)
> (In reply to comment #27)
> This might change with bug 385425 though (see also bug 385582 comment #0).
> 
> (In reply to comment #24)
> > I'm worried that users will click the button accidentally,
> 
> What about disabling the button until the user makes a change which would
> require a restart (enabling/disabling/uninstalling an extension or
> changing/uninstalling a theme)?

We don't know all possible actions, like changing extension prefs that need an restart.

I'd rather swap "Find Updates" and "Restart Minfield". Since close/cancel buttons are generally on the bottom right, this would reduce the risk for accidental clicks.

> Or is that restart button the official way to
> restart an application even for non-extension related issues?

Restarting makes sense for all kinds of customizations like changing hidden prefs or modifying userChrome.css, which isn't alien to the Add-ons metaphor. Also, it seems unlikely that there will be another way by default (e.g. File -> Restart). So yes, I'd say this is the official way.

Comment 32

[Adam Kowalczyk]

(In reply to comment #31)
> I'd rather swap "Find Updates" and "Restart Minfield". Since close/cancel
> buttons are generally on the bottom right, this would reduce the risk for
> accidental clicks.

However, the Restart button _is_ kind of a confirmation button, as in "restart to apply the changes". Placing it on the right side seems more intuitive to me.

Comment 33

[Tony Chung [:tchung]]

Verified the fix on trunk. 

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a6pre) Gecko/20070625 Minefield/3.0a6pre

Comment 34

[Reed Loden [:reed]]

FYI, bug 408116 is going to change the way this button gets shown and where. I like it how it currently is. :(

Comment 35

[Onno Ekker [:nONoNonO UTC+1]]

Bug 408116 is going to block this one. Moving it to the notification bar makes it non-permanent.

Comment 36

[Kim Sullivan]

This issue has been reopened as bug 414961 (due to #408116 being fixed).