973014 - "Firefox must restart to enable this feature" should not use "OK" as a button label

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect)

Description

[Jesse Ruderman]

Attachment: screenshot

I clicked "OK" thinking I was just acknowledging that I'd have to restart Firefox for the change to take effect. Instead, clicking "OK" immediately restarted my browser!

Using "OK" as a button label violates Mac OS X user interface guidelines and we usually avoid it.  The button should be labeled "Restart" or "Restart Firefox".

http://hg.mozilla.org/mozilla-central/annotate/eac89fb04bb9/browser/components/preferences/privacy.js#l271

Comment 1

[Dão Gottwald [:dao]]

Also, what does "Cancel" mean? "I'll restart later" or "Don't disable this feature"?

Comment 2

[Jesse Ruderman]

Firefox takes it to mean "Don't disable this feature", which is also not what I expected as a user.

Comment 3

[(no longer active)]

What would a good label for "Cancel" be?  Also, do we want to change this on all platforms?

Comment 4

[Jesse Ruderman]

"Restart later"? ;)

All platforms, please.

Comment 5

[(no longer active)]

"Restart later" is not a good choice, because pressing that button will revert the setting in the UI, and cancel the restart.  In other words, it's "Don't disable this feature", but we obviously need a better title for it.

Comment 6

[(no longer active)]

Here is some information on the code changes required for this bug (as I plan to mentor it.)

The code for showing this prompt lives here: <http://mxr.mozilla.org/mozilla-central/source/browser/components/preferences/privacy.js#279>  We need to switch to the confirmEx function which lets us customize the default buttons' titles.  This method is documented here: <http://mxr.mozilla.org/mozilla-central/source/embedding/components/windowwatcher/public/nsIPromptService.idl#173>.  The string resources live here: <http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/preferences/preferences.properties#134>.

Once we pick the strings we want, we need to add those additional two strings to the properties file, and then use confirmEx to set those strings.  The code changes must be copied to browser/components/preferences/in-content/privacy.js as well.

Comment 7

[(no longer active)]

Needinfo-ing Jesse because he's a native speaker.  :-)

Comment 8

[Jesse Ruderman]

Can you ask a UX expert? Assuming you won't change the behavior of the "Cancel" button in this bug, I can't think of a label I like better than "Cancel".

Comment 9

[(no longer active)]

(In reply to Jesse Ruderman from comment #8)
> Can you ask a UX expert? Assuming you won't change the behavior of the
> "Cancel" button in this bug, I can't think of a label I like better than
> "Cancel".

Needinfo-ing Madhava, and CCing mconley to see if there is a quicker way for us to get a response here.

Comment 10

[Askar Safin]

I absolutely agree with the bug reporter. You absolutely definitely should change somehow this "Firefox must restart to enable this feature" or button label.

I had one tab opened in my Iceweasel 24.6.0 on Debian GNU/Linux (of course, Iceweasel is nearly identical to Firefox) and this tab contented some important string entered to input field (string was long and didn't fit to screen). I cleared my browser history. Then I opened "Preferences -> Privacy -> Iceweasel will" and changed "Remember history" to "Never remember history". I wanted to change this setting, then to save somehow that important string and then to close the browser (and so end with fresh browser which will not remember history anymore). Iceweasel said me: "Iceweasel must restart to enable this feature". I thought this means "Iceweasel must restart to enable this feature, so you should manually restart the browser, do you understand me?" I thought that "OK" button means "Yes, I understand you, I will restart the browser manually later" and "Cancel" means "No, I am stupid, I don't understand whatever you said me, my lovely browser". In fact, I didn't really realize at that moment what "Cancel" means. Yes, now I understand that this is very unlikely that computer may ask me "Do you understand?" and give "OK" and "Cancel" buttons. I pressed "OK" and then the browser suddenly restarted. And I got fresh "New tab" and no way to restore that data (because history was already cleared and the browser now doesn't remember it).

So, please search through all software by Mozilla Foundation including Firefox, Thunderburd etc, search all dialogs like "XXX must restart to enable this feature" and make sure that they all are unambiguous in all platforms and all human languages.

Of course, this bug should be tagged as "dataloss", because it really cases data loss.

Comment 11

[Askar Safin]

Also, the browser (and all programs from Mozilla Foundation) should support triggering/deferring settings change to the next start. It is possible that someone want to change some setting (which requires restart) but don't want to restart the browser now (for example, he may have some Flash game opened in the browser which doesn't preserve its state on restart). So, the interface should be one of the following:

First variant. Firefox shows "Please, restart you browser manually to enable this feature" and one "OK" button. User presses this button, the feature enables, but will activate on the next run. This is my preferred variant, because, of course, I can manually restart browser in any case. So, any other buttons are unneeded, in my opinion. Moreover, you can disable all popup windows and just to write "This feature requires Firefox restart" near that feature. If I remember correctly, this is behavior of Internet Explorer (version nearly 7). IE shows text like "This feature will be activated after browser restart" near options to enable tab view.

Second variant. Firefox shows "Firefox must restart to enable this feature. Restart now?" and two buttons "Yes" and "No" (or "Restart now" and "Restart later"). "Yes" restarts immediately, "No" enables the feature, but it will be activated on the next start. This variant is not my preferred one, because it present to user some unneeded choice problem. But this variant is traditional and present in a lot of programs.

Comment 12

[Kim Ludvigsen]

Someone please take this and correct it. I just lost a lot of work translating a SUMO article and of course all my open tabs. I needed to check the strings for the translation and pushed the OK button, believing I could change it back after working on the article, and poof! everything was gone. Very bad.

Comment 13

[Jesper Kristensen]

Is this bug stalled because we are waiting for a response from one single person, and no one is allowed to touch it until that one person answers? If I made a patch, would someone be able to accept it, or would it also have to wait for that one person, who has not answered in six months?

Comment 14

[Josh Matthews [:jdm]]

Gavin, can we get some help prodding UX here?

Comment 15

[:Gavin Sharp [email: [e-mail]]]

"Undo" and "Restart" seem like better button labels to me.

Comment 16

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Let's do
»Revert« and »Restart $product now«

Also, I vaguely recall a different bug about that same thing, but I can't find it right now.

Comment 17

[Askar Safin]

Let me repeat: please don't just rename buttons. Add support for deferring changes. https://bugzilla.mozilla.org/show_bug.cgi?id=973014#c11

Comment 18

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Found the other bug!
This comment (https://bugzilla.mozilla.org/show_bug.cgi?id=1055284#c6) in bug 1055284 explains why it might be hard to support deferring this specific restart.

Comment 19

[Askar Safin]

(In reply to Philipp Sackl [:phlsa] from comment #18)
> This comment (https://bugzilla.mozilla.org/show_bug.cgi?id=1055284#c6) in
> bug 1055284 explains why it might be hard to support deferring this specific
> restart.

That comment just shows that this is hard to change privacy mode immediately. But it still seems for me that deferring change is somewhat easy

Comment 21

[:Gijs (he/him)]

(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #10)
> > (In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from
> > comment #7)
> > > The issue is that on OSX, the OS convention is that the setting changes are
> > > made immediately as you interact with the controls, so it's not clear to me
> > > how to build a UX around that which both makes the setting take effect
> > > immediately and makes it not do anything until a restart.  Maybe there are
> > > tricks like that, but that seems self-contradictory.
> > 
> > OS X Human Interface Guidelines (
> > https://developer.apple.com/librarY/mac/documentation/UserExperience/
> > Conceptual/AppleHIGuidelines ) says in "UI Element Guidelines:
> > Windows/Dialogs/Accepting and Applying User Input in a Dialog":
> > > _For the most part_, changes that a user makes in a dialog should appear to take effect immediately.
> > 
> > Note "For the most post". Of course, not all changes should take effect
> > immediately.
> 
> That will be different than all of Firefox's other preferences, so I don't
> think that is a good idea.  It will also not be obvious to users because
> they have no way of knowing that the change only has an effect after
> restarting.

It's no different from what many non-restartless add-on actions do in the add-on manager, and various options in the devtools have used similar language in the past. As for it not being obvious - we should insert a message in the prefs saying "this change will take (full) effect once you restart Firefox", which is exactly what the add-on manager does.

It is harder than just updating the buttons, and if we can do that quickly we certainly should do that first, but I would tend to agree that not forcing the user to choose between "restart now" and "don't change the pref" would be a better solution.

Comment 22

[obrufau]

(In reply to Philipp Sackl [:phlsa] from comment #18)
> Found the other bug!
> This comment (https://bugzilla.mozilla.org/show_bug.cgi?id=1055284#c6) in
> bug 1055284 explains why it might be hard to support deferring this specific
> restart.

That comment didn't explain anything. It was just a question:

> [...] is this going to be some kind of strange inbetween state where you're kind of in
> private browsing mode and kind of not, until you restart? How harmful is that going to be?

If I go to about:config and change browser.privatebrowsing.autostart, I don't have to restart immediately and doesn't seem to be any problem nor inbetween state.

Comment 23

[obrufau]

Since it may be hard to fix this properly, maybe it could be solved in three phases:

 1. Improve the buttons in the dialog
    - Maintain current message "Firefox must restart to enable this feature"
    - Change "OK" to "Restart now"
    - Change "Cancel" to "Undo"

 2. Don't require immediate restart
    - Change dialog message to "This feature will be enabled after you restart Firefox"
    - Change "OK" to "Restart now"
    - Change "Cancel" to "I will restart later"

 3. Don't use any dialog (like non-restartless add-ons)
    - Insert a message in the prefs saying "This feature will be enabled after you restart Firefox"
    - Insert a "Restart now" button
    - Insert an "Undo" button

The texts above are only illustrative.

Comment 24

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Let's use this bug to fix the messaging. I'll file a follow-up for enabling a deferred restart.
The interim solution would be to change the dialog copy as follows:

Firefox must restart to enable this feature.
[Restart Firefox now] [Revert]

Comment 25

[obrufau]

The same problem applies to "Enable E10S (multi-process)"

Comment 26

[arni2033]

(In reply to obrufau from comment #25)
> The same problem applies to "Enable E10S (multi-process)"
I confirm this one. See also bug 1169306 comment 3 - my very sad story about lost work.
See also what GoogleChrome browser did to their "Use HWA" option: that's a masterpiece IMO.

Comment 27

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: MozReview Request: Bug 973014: Restart due to privacy setting change now has better button labels than OK and Cancel. r?past

Review commit: https://reviewboard.mozilla.org/r/49745/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/49745/

Comment 28

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Milan Sreckovic [:milan] from comment #27)
> Created attachment 8747143 [details]
> MozReview Request: Bug 973014: Restart due to privacy setting change now has
> better button labels than OK and Cancel. r?past
> 
> Review commit: https://reviewboard.mozilla.org/r/49745/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/49745/

While we're discussing the rest, here is what was "approved" by UX in comment 24.  It does not contain the E10S issue mentioned in comment 25 and comment 26, but it would be trivial to add.  Given that we've been talking about it for two years without any movement, here's at least something that we've agreed on so far.

Comment 29

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8747143 [details]
MozReview Request: Bug 973014: Restart due to privacy setting change now has better button labels than OK and Cancel. r?past

https://reviewboard.mozilla.org/r/49745/#review46671

Comment 30

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a8340df297f1

Comment 31

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a8340df297f1

Comment 32

[Milan Sreckovic [:milan] (needinfo for best results)]

Some conversation on this continuing in bug 1285328.

Comment 33

[Azmina]

I have reproduced this bug with Firefox Nightly 30.0a1 (2014-02-14) on windows 8.1 , 64 bit

The bug's fix is verified on Aurora 49.0a2

Build ID : 20160709004037

User Agent : Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:49.0) Gecko/20100101 Firefox/49.0

[bugday-20160713]

Comment 34

[Rezaul Huque Nayeem]

Reproduced the bug in firefox nightly 31.0a1 (2014-04-14) with ubuntu 16.04 (64 bit)

Verified as this bugs fixed with latest firefox aurora 49.0a2 (Build ID: 20160716004012)
Mozilla/5.0 (X11; Linux x86_64; rv:49.0) Gecko/20100101 Firefox/49.0

As it is also verified on windows (Comment 33), Marking it as verified!

Comment 35

[Bogdan Maris, Desktop QA]

(In reply to Milan Sreckovic [:milan] from comment #28)
> (In reply to Milan Sreckovic [:milan] from comment #27)
> > Created attachment 8747143 [details]
> > MozReview Request: Bug 973014: Restart due to privacy setting change now has
> > better button labels than OK and Cancel. r?past
> > 
> > Review commit: https://reviewboard.mozilla.org/r/49745/diff/#index_header
> > See other reviews: https://reviewboard.mozilla.org/r/49745/
> 
> While we're discussing the rest, here is what was "approved" by UX in
> comment 24.  It does not contain the E10S issue mentioned in comment 25 and
> comment 26, but it would be trivial to add.  Given that we've been talking
> about it for two years without any movement, here's at least something that
> we've agreed on so far.

Is there any plan to also change the message from disabling/enabling e10s for consistency?

Comment 36

[Mike Conley (:mconley) (:⚙️)]

(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #35)
> (In reply to Milan Sreckovic [:milan] from comment #28)
> > (In reply to Milan Sreckovic [:milan] from comment #27)
> > > Created attachment 8747143 [details]
> > > MozReview Request: Bug 973014: Restart due to privacy setting change now has
> > > better button labels than OK and Cancel. r?past
> > > 
> > > Review commit: https://reviewboard.mozilla.org/r/49745/diff/#index_header
> > > See other reviews: https://reviewboard.mozilla.org/r/49745/
> > 
> > While we're discussing the rest, here is what was "approved" by UX in
> > comment 24.  It does not contain the E10S issue mentioned in comment 25 and
> > comment 26, but it would be trivial to add.  Given that we've been talking
> > about it for two years without any movement, here's at least something that
> > we've agreed on so far.
> 
> Is there any plan to also change the message from disabling/enabling e10s
> for consistency?

If it makes any difference, the toggle for enabling / disabling e10s will not be shipping on the beta or release channel. Only pre-beta users will see this toggle.

Comment 37

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #35)
> ...
> 
> Is there any plan to also change the message from disabling/enabling e10s
> for consistency?

I think it's being treated as low priority because it will go away within a release or two - there won't be an option as such.

What kind of message did you have in mind for consistency?

Comment 38

[Bogdan Maris, Desktop QA]

(In reply to Milan Sreckovic [:milan] (PTO July 28-29) from comment #37)
> (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #35)
> > ...
> > 
> > Is there any plan to also change the message from disabling/enabling e10s
> > for consistency?
> 
> I think it's being treated as low priority because it will go away within a
> release or two - there won't be an option as such.
> 
> What kind of message did you have in mind for consistency?

By consistency I meant that the e10s prompt should have the same message as other prompts for restart like Never remember history and so on.

Comment 39

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #38)
> ...
> 
> By consistency I meant that the e10s prompt should have the same message as
> other prompts for restart like Never remember history and so on.

I believe they do now - the only difference is the E10S default is "restart", while the "never remember history" default is "revert".