408116 - Move restart button to notification bar

Status: VERIFIED FIXED

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

Description

[Dave Townsend [:mossop]]

From UX the idea is to only show the restart button when necessary as an item in the notification bar.

Comment 1

[Dave Townsend [:mossop]]

The concept here will be fairly simple. Every time an operation is performed we will have to spin through to see whether the restart button should be visible or not. Implementation time is a little long though since testing as many of the various interactions that can go on will take time.

Main thing we need to take care of is how to deal with the restart button showing and hiding repeatedly in a single session. Do we pop the entire notification bar up and down over and over or leave the bar up once it is there and disable the button or something?

Comment 2

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

iirc there is code so that only one notification is shown per notification type so there won't be redundant notifications below the same notification in the notification stack. I'd display it whenever an action that requires a restart is performed. If it is displayed then there will no ui change and if there is one below the current notification it will be displayed.

Comment 3

[Dave Townsend [:mossop]]

Sure, but what if I click to disable an add-on (notification bar appears). Go to get add-ons, click to install (notification bar goes away), install completes (notification bar appears).

In particular if the install is fast it will look quite ugly to hide the bar and re-show it again. The alternative of say disabling the restart button in the bar is not currently possible without extending the notification widget I think.

Comment 4

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

Thanks for clearing up what the phases are you are concerned with.

I'd suggest never dismissing the bar unless the user dismisses it with the current exception of auto-dismissing it on click when no updates have been found. When an action that disallows restarting is being performed I'd prefer it if the button were disabled.

Comment 5

[Reed Loden [:reed]]

I use the restart button all the time in order to restart Firefox while keeping my session history. Does this mean you're taking that functionality away from me? :(

Comment 6

[Dave Townsend [:mossop]]

(In reply to comment #5)
> I use the restart button all the time in order to restart Firefox while keeping
> my session history. Does this mean you're taking that functionality away from
> me? :(

Yes.

Comment 7

[Dão Gottwald [:dao]]

The permanent Restart button is actually a Fx3 product requirement (ADD-003b, bug 369075).

Comment 8

[Dave Townsend [:mossop]]

I believe that the actual PRD item was somewhat poorly worded. The spirit of it was to allow you to restart the browser whenever necessary due to add-on changes. This is what will be implemented here.

Comment 9

[Dão Gottwald [:dao]]

That sounds like what was tried for Firefox 2, which was a failure (e.g. bug 351819).
There are also changes to extensions' preferences that could require a restart and are currently not tracked.

Comment 10

[Dave Townsend [:mossop]]

I don't recall that we ever tried this for Firefox 2, there we only ever showed the restart button on the installs pane, and regardless whether we tried it and failed before isn't a good reason for not doing it. bug 396129 should allow us to take care of the problem case in bug 351819.

Comment 11

[Dão Gottwald [:dao]]

What was tried for Firefox 2 was to "show the restart button when necessary", which is what this bug proposes too, even though it surely doesn't aim for the same buggy implementation that led to bug 369075. But even if you get this bug-free, there are some uses cases that would become unsupported. Can't we just leave the button where it is and make it flash when there's an action that we know needs restarting?

Comment 12

[Dave Townsend [:mossop]]

(In reply to comment #11)
> What was tried for Firefox 2 was to "show the restart button when necessary",
> which is what this bug proposes too

I can't find where this was tried, can you point me to bugs please?

> even though it surely doesn't aim for the
> same buggy implementation that led to bug 369075

The implementation we had previous to bug 396075 was just to only show the restart buttons in certain situations and yes it had problems and we don't want to go back to that.

> there are some uses cases that would become unsupported.

Beyond the case of an extension having options that require a restart to take effect are there any others?

Comment 13

[Reed Loden [:reed]]

(In reply to comment #12)
> Beyond the case of an extension having options that require a restart to take
> effect are there any others?

Having the restart button permanently there gives users an easy place to use for restarting their browser while saving session history?

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

1. If you have multiple windows open and a high memory fragmentation forces you to restart, this is the only possibility to do this.
2. The user had to perform an action like copying a file to the profile directory or installing something which can only be detected/implemented on startup.

Comment 15

[Dão Gottwald [:dao]]

(In reply to comment #12)
> (In reply to comment #11)
> > What was tried for Firefox 2 was to "show the restart button when necessary",
> > which is what this bug proposes too
> 
> I can't find where this was tried, can you point me to bugs please?
> 
> > even though it surely doesn't aim for the
> > same buggy implementation that led to bug 369075
> 
> The implementation we had previous to bug 396075 was just to only show the
> restart buttons in certain situations and yes it had problems and we don't want
> to go back to that.

"show the restart buttons in certain situations" is what I meant -- assuming that there was some sort of reasoning behind it, as in "when necessary".

> Beyond the case of an extension having options that require a restart to take
> effect are there any others?

All kinds of customizations, depending on how deep you dig into Firefox. The most common use case would probably be hidden prefs. So this isn't critical for inexperienced users, but if there's a way to support both groups, then we should consider that. Most inexperienced users don't even manage Add-ons. ;)

Comment 16

[Dave Townsend [:mossop]]

(In reply to comment #13)
> Having the restart button permanently there gives users an easy place to use
> for restarting their browser while saving session history?

(In reply to comment #14)
> 1. If you have multiple windows open and a high memory fragmentation forces you
> to restart, this is the only possibility to do this.
> 2. The user had to perform an action like copying a file to the profile
> directory or installing something which can only be detected/implemented on
> startup.

(In reply to comment #15)
> All kinds of customizations, depending on how deep you dig into Firefox. The
> most common use case would probably be hidden prefs. So this isn't critical for
> inexperienced users, but if there's a way to support both groups, then we
> should consider that. Most inexperienced users don't even manage Add-ons. ;)

None of these cases are related to add-ons so why should the restart option be
in the add-ons manager for them?

Comment 17

[Dão Gottwald [:dao]]

Because we don't have a better place for it? The uses cases that I had in mind are customizations at least, and thereby remotely related to Add-ons.

Comment 18

[Alvaro Muñoz]

(In reply to comment #17)
> Because we don't have a better place for it?
> 

maybe an item in the file menu above exit?

Comment 19

[Tony Mechelynck [:tonymec]]

(In reply to comment #16)
[...]
> None of these cases are related to add-ons so why should the restart option be
> in the add-ons manager for them?
> 

OK, here's an addons-related use case, two of them even, which ought to be obvious enough to many Firefox users to make the feature "discoverable":

- After installing a new extension (or upgrading an existing one), a restart is required before the extension (or the upgrade) starts to "work".
- After selecting a different theme, a restart is required in order to use the newly selected theme.

This said, IMHO a "Restart" button in the add-ons manager is not incompatible with having, even at the same time, a "Restart" button on a toolbar and/or a "File -> Restart Firefox" menu item -- as provided, e.g., by the MR Tech Local Install extension.

Comment 20

[Dave Townsend [:mossop]]

(In reply to comment #19)
> OK, here's an addons-related use case, two of them even, which ought to be
> obvious enough to many Firefox users to make the feature "discoverable":
> 
> - After installing a new extension (or upgrading an existing one), a restart is
> required before the extension (or the upgrade) starts to "work".
> - After selecting a different theme, a restart is required in order to use the
> newly selected theme.

And both of these cases will cause a restart button to appear, something far more discoverable than a button that was already there in the first place.

Comment 21

[Tony Mechelynck [:tonymec]]

P.S. In case it wasn't evident, I'm in favour of a permanent "Restart" button in the add-ons manager, possibly together with other "Restart" buttons and/or menuitems elsewhere, because restarting the browser has uses related to add-ons (which makes the add-ons manager one possible place to search for such a button) and other uses not related to them (which makes it useful for that button to be parmanent).

If that means I'm in favour of WONTFIXing the bug, so be it.

Comment 22

[Dão Gottwald [:dao]]

(In reply to comment #20)
> And both of these cases will cause a restart button to appear, something far
> more discoverable than a button that was already there in the first place.

See my proposal from comment 11: Can't we just leave the button where it is and make it flash when there's an action that we know needs restarting?

Comment 23

[Dave Townsend [:mossop]]

Attachment: patch rev 1

Note that this really works best with the patch from bug 408118 fixing some of the theme switch code. Also there will be a minor addition to this patch once that lands to observe the theme switch pref and update the restart button accordingly but that s a 2 line change and this applies and works as it is now so might as well get on.

Basically this removes the restart button. Makes sure that for every path that cna change the state of an add-on we call updateOptionalViews followed by updateGlobalCommands.

updateOptionalViews already scans over the add-ons so just add a new check to spot any with pending operations.

Then update the state of the restart button. If there are any installs in progress then restarting is not available, otherwise restarting is available if there are pending operations or a theme change has been requested.

I left it has just showing and hiding the notification bar, in practice this seems to work better than any alternative that I came across.

The exact wording for the notification bar and button are still awaiting confirmation from Madhava.

Comment 24

[Dão Gottwald [:dao]]

Thanks for taking all comments into account.

Comment 25

[Dave Townsend [:mossop]]

Comment on attachment 297145 [details] [diff] [review]
patch rev 1

Madhava, can you just confirm that this is the behaviour we want to go for here, the patch just implements the notifcation bar as I showed you a week or two ago

Comment 26

[Dave Townsend [:mossop]]

Attachment: patch rev 2 [landed strings]

Neglected to localise the message. This is the updated version and the string is agreed now.

Comment 27

[Dave Townsend [:mossop]]

Attachment: Screenshot for ui review

Screenshot of the behaviour

Comment 28

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

Comment on attachment 297173 [details] [diff] [review]
patch rev 2 [landed strings]

Looks good. Is there any reason not to put the link back over on the right now that there are no buttons on the right so apps that don't use the get add-ons pane will have a nicer layout?

Comment 29

[Dave Townsend [:mossop]]

(In reply to comment #28)
> (From update of attachment 297173 [details] [diff] [review])
> Looks good. Is there any reason not to put the link back over on the right now
> that there are no buttons on the right so apps that don't use the get add-ons
> pane will have a nicer layout?

Don't think so, will double check with madhava all the same, thanks for the review!

Comment 30

[Dave Townsend [:mossop]]

Comment on attachment 297173 [details] [diff] [review]
patch rev 2 [landed strings]

Landed just the strings for now.

Checking in toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties,v  <--  extensions.properties
new revision: 1.48; previous revision: 1.47
done

Comment 31

[Stefan Breunig]

What happens when the user accidentally misses the restart button and instead dismisses the notification bar?
If there was File > Restart Firefox it would neither be discoverable for starts nor convenient for experienced users. Should I open a new bug on this?

Comment 32

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

That seems rather like a solution looking for a problem in that it would be a very rare occurrence.

Comment 33

[AndrewM]

(In reply to comment #31)
> What happens when the user accidentally misses the restart button and instead
> dismisses the notification bar?
> If there was File > Restart Firefox it would neither be discoverable for starts
> nor convenient for experienced users. Should I open a new bug on this?

I'm guessing that only a small number of users use the restart button regularly to restart in situations other than after installing an add-on. After all, you can just kill Firefox and then restore the session to achieve the same goal.

But given that some users do find it useful, someone could create an extension that adds a 'Restart Firefox' option to the File menu and/or re-adds a permanent restart button to the Add-ons Manager.

Comment 34

[Tony Mechelynck [:tonymec]]

(In reply to comment #33)
[...]
> But given that some users do find it useful, someone could create an extension
> that adds a 'Restart Firefox' option to the File menu and/or re-adds a
> permanent restart button to the Add-ons Manager.
> 

I don't know if the "Restart Firefox" extension is still maintained, but "MR Tech Local Install" includes the functionality (both a menuitem and a toolbar button for Fx/Tb, the menuitem only for Sm which hasn't yet got customizable toolbars).

Comment 35

[Ed Hume]

This is one of those times that I wish Bugzilla had a Vote Against capacity.

Comment 36

[rdz_bug]

What about about:config changes that need restart?

Comment 37

[AndrewM]

(In reply to comment #36)
> What about about:config changes that need restart?

See comment 34, and there are at least 4 other extensions on AMO that add restart buttons or menuitems (although none of them seem at the moment to be updated for nightlies or Fx 3 betas): https://addons.mozilla.org/en-US/firefox/search?q=restart

Or, you could just kill Firefox and then restore the session :)

Comment 38

[Ed Hume]

I have not seen any justification for this change. "From UX the idea is . . . " hardly states any necessity for this change. I think most users, if briefed on the issue, would prefer to have a permanent restart button rather than one that shows up on an ephemeral part of the UI.

Better to just leave it alone and use precious programmer time on bugs that represent actual broken things that need fixing.

Comment 39

[Alex Vincent [:WeirdAl]]

Ed:  What's your justification for slamming people who are volunteering their time on this?

* This bug is marked blocking-firefox3+, which means it must be fixed for Firefox 3.
* As the bug has not been marked FIXED yet, the implication is there's more work to be done.  So it's not a waste of time.
* We are not supposed to "brief" users on anything.  The user-interface should be intuitive by design.  That's one of the big lessons of Firefox over ye olde Mozilla Application Suite.

I will fully admit I have not followed this issue in great detail, nor have I participated in this bug to date.  But when a bug blocks the release, it's not fair to say further work on it does not "represent actual broken things that need fixing".

</soapbox>

Comment 41

[Dave Townsend [:mossop]]

Checking in toolkit/mozapps/extensions/content/extensions.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/content/extensions.js,v  <--  extensions.js
new revision: 1.155; previous revision: 1.154
done
Checking in toolkit/mozapps/extensions/content/extensions.xul;
/cvsroot/mozilla/toolkit/mozapps/extensions/content/extensions.xul,v  <--  extensions.xul
new revision: 1.68; previous revision: 1.67
done

Comment 42

[Tony Chung [:tchung]]

Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008012904 Minefield/3.0b3pre

Comment 43

[Alfred Kayser]

About the discussion of 'show the button when necessary'.
It is also needed when one 'uninstalls' an extension or theme.

Comment 44

[Dave Townsend [:mossop]]

(In reply to comment #43)
> About the discussion of 'show the button when necessary'.
> It is also needed when one 'uninstalls' an extension or theme.

When uninstalling an unused theme it is not needed, since there is no need to restart. Otherwise yes it is needed and it appears in this case for me. If you can provide steps to show it doesn't them please open a new bug.

Comment 45

[Alfred Kayser]

I was expecting (and was needing the restart button to test something else) that the restart button would also appear when uninstalling an unused theme.

Comment 46

[cris]

Restart button is what I use when I try to restart the browser, NOT ALways related to install/uninstall themes/extensions.

This move is unwise, IMHO.

Comment 47

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

See bug 414961 for adding general restart capabilities beyond the need to restart due to add-on operations.

Comment 48

[Alfred Kayser]

A way to make the 'restart' button disappear is as follows:
1. Install a new theme or extension.
2. Before pressing 'Restart', switch back to the theme (or extension) page.
3. The notification disappears.
4. Switch back to 'Installation' pane: now still there is no restart button.

So, at least the button and notification should remain visible as long as it is needed (i.e. till FF is restarted).

Comment 49

[Dave Townsend [:mossop]]

This bug is fixed. Please do not reopen it. Further issues should be filed as separate bugs.

Comment 50

[Alfred Kayser]

Created bug 415542 for the disappearing restart button (and notification bar).
Note this is not the same as bug 414961.

Comment 51

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

Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008020104 Minefield/3.0b3pre ID:2008020104

Comment 53

[Onno Ekker [:nONoNonO UTC+1]]

Restart isn't only wanted for extension updating, but also for locale switching.
It would be nice to have locale/language switching integrated in the add-ons manager, much the same way like theme switcher. This would make it possible.

At the moment the only way to change locales is to go to about:config, find general.useragent.locale preference and edit it. Or installing an extension for it, but the one I found, only adds a menuitem. So you need a second extension to restart Firefox. It could be so much more user-friendly...

Comment 54

[Tony Chung [:tchung]]

Added litmus test case: 5292

Comment 55

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

(In reply to comment #53)
> Restart isn't only wanted for extension updating, but also for locale
> switching.
> It would be nice to have locale/language switching integrated in the add-ons
> manager, much the same way like theme switcher. This would make it possible.

See bug 377881 for that.

Comment 56

[timeless]

this bug is incorrectly referenced by changeset:
http://hg.mozilla.org/mozilla-central/rev/47e4c4d472e3
it is actually for bug 519886