Closed Bug 408116 Opened 12 years ago Closed 12 years ago

Move restart button to notification bar

Categories

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

defect

Tracking

()

VERIFIED FIXED
mozilla1.9beta3

People

(Reporter: mossop, Assigned: mossop)

References

(Depends on 1 open bug, )

Details

Attachments

(2 files, 1 obsolete file)

From UX the idea is to only show the restart button when necessary as an item in the notification bar.
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?
Whiteboard: [swag 2]
Blocks: 404024
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.
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.
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.
Flags: blocking-firefox3?
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P3
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? :(
(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.
The permanent Restart button is actually a Fx3 product requirement (ADD-003b, bug 369075).
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.
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.
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.
Depends on: 396129
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?
(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?
(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?
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 #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. ;)
(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?
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.
(In reply to comment #17)
> Because we don't have a better place for it?
> 

maybe an item in the file menu above exit?
(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.
(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.
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.
(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?
Attached patch patch rev 1 (obsolete) — Splinter Review
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.
Assignee: nobody → dtownsend
Status: NEW → ASSIGNED
Attachment #297145 - Flags: review?(robert.bugzilla)
Whiteboard: [swag 2] → [has patch]
No longer depends on: 396129
Thanks for taking all comments into account.
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
Attachment #297145 - Flags: ui-review?(madhava)
Neglected to localise the message. This is the updated version and the string is agreed now.
Attachment #297145 - Attachment is obsolete: true
Attachment #297173 - Flags: ui-review?
Attachment #297173 - Flags: review?(robert.bugzilla)
Attachment #297145 - Flags: ui-review?(madhava)
Attachment #297145 - Flags: review?(robert.bugzilla)
Attachment #297173 - Flags: ui-review?
Screenshot of the behaviour
Attachment #297180 - Flags: ui-review?(madhava)
Attachment #297180 - Flags: ui-review?(madhava) → ui-review+
Keywords: late-l10n
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?
Attachment #297173 - Flags: review?(robert.bugzilla) → review+
(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!
Whiteboard: [has patch]
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
Attachment #297173 - Attachment description: patch rev 2 → patch rev 2 [landed strings]
Keywords: late-l10n
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?
That seems rather like a solution looking for a problem in that it would be a very rare occurrence.
(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.
(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).
This is one of those times that I wish Bugzilla had a Vote Against capacity.
What about about:config changes that need restart?
(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 :)
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.
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>
Duplicate of this bug: 414149
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
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M11
Depends on: 414403
Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008012904 Minefield/3.0b3pre
Status: RESOLVED → VERIFIED
About the discussion of 'show the button when necessary'.
It is also needed when one 'uninstalls' an extension or theme.
(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.
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.
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.
See bug 414961 for adding general restart capabilities beyond the need to restart due to add-on operations.
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).
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
This bug is fixed. Please do not reopen it. Further issues should be filed as separate bugs.
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Created bug 415542 for the disappearing restart button (and notification bar).
Note this is not the same as bug 414961.
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
Status: RESOLVED → VERIFIED
Flags: in-litmus?
Duplicate of this bug: 425492
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...
Added litmus test case: 5292
Flags: in-litmus? → in-litmus+
Product: Firefox → Toolkit
(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.
Depends on: 506084
No longer depends on: 506084
this bug is incorrectly referenced by changeset:
http://hg.mozilla.org/mozilla-central/rev/47e4c4d472e3
it is actually for bug 519886
You need to log in before you can comment on or make changes to this bug.