Last Comment Bug 732303 - Redesign safe mode dialog with the profile reset option
: Redesign safe mode dialog with the profile reset option
Status: VERIFIED FIXED
: user-doc-needed
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: -- enhancement with 2 votes (vote)
: Firefox 15
Assigned To: Matthew N. [:MattN]
:
Mentors:
http://people.mozilla.org/~mverdi/scr...
Depends on: 294260 717070 881199
Blocks: safe-mode reset-firefox
  Show dependency treegraph
 
Reported: 2012-03-01 18:32 PST by Matthew N. [:MattN]
Modified: 2013-12-27 14:35 PST (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v.1 Use dropdown to choose actions including reset (23.11 KB, patch)
2012-03-09 04:06 PST, Matthew N. [:MattN]
dolske: review+
Details | Diff | Splinter Review
Screenshot of three actions for safe mode (122.15 KB, image/png)
2012-03-09 04:14 PST, Matthew N. [:MattN]
no flags Details
Screenshot for forced safe showing dropdown (54.27 KB, image/png)
2012-03-09 04:19 PST, Matthew N. [:MattN]
no flags Details
Dialog for implicit safe mode (by crashing) (56.14 KB, image/png)
2012-03-12 14:53 PDT, Alex Limi (:limi) — Firefox UX Team
no flags Details
Explicitly triggered Safe Mode (shift/option, menu, command line) (61.67 KB, image/png)
2012-03-12 14:54 PDT, Alex Limi (:limi) — Firefox UX Team
no flags Details
v.2 Based on Limi's design (24.65 KB, patch)
2012-03-27 17:20 PDT, Matthew N. [:MattN]
dolske: review+
limi: ui‑review+
Details | Diff | Splinter Review
Screenshot for ui-r. Stand-alone reset-profile dialog is same as on about:support (36.34 KB, image/png)
2012-03-27 17:28 PDT, Matthew N. [:MattN]
no flags Details
v.1 Changes for comment 19 (4.39 KB, patch)
2012-05-18 14:14 PDT, Matthew N. [:MattN]
dolske: review+
Details | Diff | Splinter Review

Description Matthew N. [:MattN] 2012-03-01 18:32:04 PST
When we detect too many startup crashes (bug 294260), offer the user the option to reset their profile.

The idea in the mockup (in the URL field) was to show a new dialog before the safe mode dialog.  I'd rather avoid adding this additional step that leads to more choices. I'd prefer adding this option on the safe mode dialog.  

Possible UI:
Add a checkbox as a parent of the existing ones labelled "Reset Firefox to defaults" or something similar in wording to bug 717070. When checked, the options below it will become disabled and/or checked.  This is similar to the custom history preferences.
Comment 1 Alex Limi (:limi) — Firefox UX Team 2012-03-05 14:01:15 PST
I think this is pretty close, some quick comments:

* If we think that they should Reset Firefox as the default action, it should also be the first (or last, in the case of OS X) in the list of buttons. For the record, I think starting in safe mode to do your own troubleshooting is an advanced use case, so I agree that Reset Firefox is the main option, and should be the default one.

* There's a spelling error in the initial dialog ("it's", should be "its").

* The current "make some of these changes permanent" in the Safe Mode is incredibly bad UI. By looking at the dialog, you think that you may have to check one or more of these to make it happen for the safe mode, whereas it's actually only if you want to make them permanent. I don't even see what the rationale for doing this is once we have a Reset option. We should either hide those options under an advanced control, or preferably remove them altogether.

___


Since this is also an exceptional case, it's more important to be clear than concise, and to make sure people feel comfortable, so I'm actually not opposed to using a wizard-like structure here (which I normally dislike).
Comment 2 Matthew N. [:MattN] 2012-03-09 04:06:17 PST
Created attachment 604360 [details] [diff] [review]
v.1 Use dropdown to choose actions including reset

Taking some of the feedback from Limi about hiding the checkboxes, I switched to using a dropdown to choose the action and removed the third button.  This should be much less confusing than the old dialog.  Screenshots and try builds coming up.  Aiming to get this landed for 13 if possible.
Comment 3 Matthew N. [:MattN] 2012-03-09 04:14:29 PST
Created attachment 604364 [details]
Screenshot of three actions for safe mode

Try builds:
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mozilla@noorenberghe.ca-a3213912ea2c
Comment 4 Matthew N. [:MattN] 2012-03-09 04:19:01 PST
Created attachment 604367 [details]
Screenshot for forced safe showing dropdown
Comment 5 Justin Dolske [:Dolske] 2012-03-10 21:23:03 PST
Comment on attachment 604360 [details] [diff] [review]
v.1 Use dropdown to choose actions including reset

Review of attachment 604360 [details] [diff] [review]:
-----------------------------------------------------------------

Didn't look at the XUL/strings/CSS in detail since that depends on ui-r.

::: browser/base/content/safeMode.js
@@ +37,5 @@
>  #
>  # ***** END LICENSE BLOCK *****
>  
> +const Cu = Components.utils;
> +Cu.import("resource://gre/modules/AddonManager.jsm");

I would be so happy if you also (in a separate bug/patch) added Cc/Ci and cleaned up such usage... :)

@@ +120,2 @@
>  function onOK() {
> +  switch (document.getElementById("action").value) {

Add a "default" case; should probably just Cu.reportError("wtf") and fall though to the "safeMode" case.

@@ +180,5 @@
> +  try {
> +    let dataTypes = getMigratedData();
> +    for (let dataType of dataTypes) {
> +      let checkbox = document.createElement("label");
> +      checkbox.setAttribute("value", dataType);

Probably should connect the label and checkbox. Or just do <checkbox label="woo"/>

::: browser/locales/en-US/chrome/browser/safeMode.dtd
@@ +41,3 @@
>  
>  <!ENTITY autoSafeModeIntro.label      "&brandShortName; Closed Unexpectedly While Starting">
> +<!ENTITY autoSafeModeDescription2.label "We sincerely apologize for the inconvenience. Below are some options to repair &brandShortName;.">

Remove the "sincerely". Because... tee-hee...

http://en.wikiquote.org/wiki/The_Hitchhiker%27s_Guide_to_the_Galaxy#Chapter_40

Share and enjoy.

::: toolkit/content/aboutSupport.js
@@ +41,1 @@
>  const Cu = Components.utils;

let, let, const? O_o

::: toolkit/content/resetProfile.js
@@ +56,4 @@
>    // getMigrateData now won't give the correct result.
> +  let dataTypes = [];
> +  for (let i = 1; i < MAX_MIGRATED_TYPES; ++i) {
> +    let itemID = Math.pow(2, i);

Using a left-shift (<<) would be a bit clearer, meh..
Comment 6 Dão Gottwald [:dao] 2012-03-11 04:33:52 PDT
Comment on attachment 604360 [details] [diff] [review]
v.1 Use dropdown to choose actions including reset

>--- /dev/null
>+++ b/browser/base/content/safeMode.css

>+description, label {
>+  font-size: 1.1em;
>+}

Why should this window use a larger font size than others?

>+#resetProfileItems {
>+  -moz-margin-start: 1.5em;
>+}

Use the 'indent' class.
Comment 7 Verdi [:verdi] 2012-03-11 12:51:43 PDT
(In reply to Alex Limi (:limi) — Firefox UX Team from comment #1)
> I think this is pretty close, some quick comments:
> 
> * If we think that they should Reset Firefox as the default action, it
> should also be the first (or last, in the case of OS X) in the list of
> buttons. For the record, I think starting in safe mode to do your own
> troubleshooting is an advanced use case, so I agree that Reset Firefox is
> the main option, and should be the default one.
> 

So in this case, if we go with the dropdown menu, Reset Firefox should be the top and default (checked) option?
Comment 8 Alex Limi (:limi) — Firefox UX Team 2012-03-12 14:53:47 PDT
Created attachment 605142 [details]
Dialog for implicit safe mode (by crashing)
Comment 9 Alex Limi (:limi) — Firefox UX Team 2012-03-12 14:54:58 PDT
Created attachment 605144 [details]
Explicitly triggered Safe Mode (shift/option, menu, command line)
Comment 10 Alex Limi (:limi) — Firefox UX Team 2012-03-12 14:55:53 PDT
The previous two mock-ups illustrate the conclusions reached in the meeting with MattN and dolske earlier today.
Comment 11 Verdi [:verdi] 2012-03-12 17:25:17 PDT
This looks great to me. Before seeing these I was talking to Asa in irc and we agreed that if you invoke safe mode it should be the default but that it was reasonable to have reset be the default if you were crashing.
Comment 12 Alex Limi (:limi) — Firefox UX Team 2012-03-14 13:24:05 PDT
(In reply to Verdi from comment #11)
> This looks great to me. Before seeing these I was talking to Asa in irc and
> we agreed that if you invoke safe mode it should be the default but that it
> was reasonable to have reset be the default if you were crashing.

Excellent. That was exactly our thinking too.
Comment 13 Madhava Enros [:madhava] 2012-03-15 08:43:20 PDT
Excellent - thanks guys.
Comment 14 Matthew N. [:MattN] 2012-03-27 17:20:44 PDT
Created attachment 609958 [details] [diff] [review]
v.2 Based on Limi's design

This is mostly the same as Limi's mockups but with some wording changes.

The separate reset dialog uses a 'Cancel' button like on about:support because they are sharing code and I wasn't sure of the proper way to hide the first dialog while showing the second without having nsAppRunner do this.

(In reply to Dão Gottwald [:dao] from comment #6)
> Use the 'indent' class.

Thanks, I was looking for a class to do this.

(In reply to Justin Dolske [:Dolske] from comment #5)
> Review of attachment 604360 [details] [diff] [review]:
> -----------------------------------------------------------------
> ::: browser/base/content/safeMode.js
> @@ +37,5 @@
> >  #
> >  # ***** END LICENSE BLOCK *****
> >  
> > +const Cu = Components.utils;
> > +Cu.import("resource://gre/modules/AddonManager.jsm");
> 
> I would be so happy if you also (in a separate bug/patch) added Cc/Ci and
> cleaned up such usage... :)

I did it here now since I was touching the uses of CC/Ci now.

> @@ +180,5 @@
> > +  try {
> > +    let dataTypes = getMigratedData();
> > +    for (let dataType of dataTypes) {
> > +      let checkbox = document.createElement("label");
> > +      checkbox.setAttribute("value", dataType);
> 
> Probably should connect the label and checkbox. Or just do <checkbox
> label="woo"/>

There isn't actually a checkbox, it was a bad variable name.


> ::: toolkit/content/aboutSupport.js
> @@ +41,1 @@
> >  const Cu = Components.utils;
> 
> let, let, const? O_o

I was avoiding redefinition of the const from the other script file which is a fatal error.

I can file a follow-up to remove the browser.bookmarks.restore_default_bookmarks pref since I don't find any remaining consumers.
Comment 15 Matthew N. [:MattN] 2012-03-27 17:28:20 PDT
Created attachment 609959 [details]
Screenshot for ui-r. Stand-alone reset-profile dialog is same as on about:support
Comment 16 Dão Gottwald [:dao] 2012-03-28 00:06:34 PDT
Comment on attachment 609958 [details] [diff] [review]
v.2 Based on Limi's design

>+<?xml-stylesheet href="chrome://browser/content/safeMode.css"?>
> <?xml-stylesheet href="chrome://global/skin/"?>

chrome://global/skin/ should come first
Comment 17 Alex Limi (:limi) — Firefox UX Team 2012-04-03 10:42:25 PDT
Comment on attachment 609958 [details] [diff] [review]
v.2 Based on Limi's design

Looks great! Nice job. :)
Comment 18 Justin Dolske [:Dolske] 2012-05-15 19:13:41 PDT
Comment on attachment 609958 [details] [diff] [review]
v.2 Based on Limi's design

Review of attachment 609958 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/safeMode.js
@@ +41,5 @@
> +let Ci = Components.interfaces;
> +let Cu = Components.utils;
> +
> +const appStartup = Cc["@mozilla.org/toolkit/app-startup;1"]
> +                     .getService(Ci.nsIAppStartup);

Services.startup?

@@ +70,5 @@
> +  restartApp();
> +}
> +
> +function onOK() {
> +  if (defaultToReset) {

I think part of why this code is a bit confusing to me is that <dialog>'s API is very much built around ok-or-cancel, but the actual UI doesn't have those labels.

Not sure how to best clarify that. A comment here (and in onExtra1) would help. Maybe calling "onOK" something different would also help.

@@ +74,5 @@
> +  if (defaultToReset) {
> +    // Restart to reset the profile.
> +    resetProfile();
> +    restartApp();
> +    return false;

Add a comment here; wrt to we stay in the dialog because restartApp() starts the shutdown, and we don't want to exit the dialog because that would resume normal startup at the same time (which would be craaaaazy!).

@@ +91,5 @@
> +    // Continue in safe mode
> +    window.close();
> +    return true;
> +  } else {
> +    showResetDialog();

You could just inline this; I was confused about why there was no resetProfile+restartApp call right here. Maybe it's just me. Meh?

::: browser/locales/en-US/chrome/browser/safeMode.dtd
@@ +41,2 @@
>  
> +<!ENTITY autoSafeModeDescription2.label "&brandShortName; Closed Unexpectedly While Starting. This might be caused by add-ons or other problems. You can try to resolve the problem by resetting &brandShortName; to its default state.">

"Closed Unexpectedly While Starting" should be lowercase.

I'd also recommend using a different entity names (or localization note?) so that localizers don't get confused by the similarity or implied-adjacentness of autoSafeModeDescription2 / safeModeDescription3 / safeModeDescription4.

@@ +43,3 @@
>  
> +<!ENTITY startSafeMode.label          "Start in Safe Mode">
> +<!ENTITY resetProfile.label           "Reset &brandShortName;">

Should be "Reset Nightly" sometimes, "Reset Nightly..." others.

::: toolkit/content/resetProfile.js
@@ +63,4 @@
>    // getMigrateData now won't give the correct result.
> +  let dataTypes = [];
> +  for (let i = 1; i < MAX_MIGRATED_TYPES; ++i) {
> +    let itemID = Math.pow(2, i);

Normally one would write |1 << i| for 0..n. Free pass since it's existing code. :)

::: toolkit/locales/en-US/chrome/global/resetProfile.dtd
@@ +3,5 @@
>     - You can obtain one at http://mozilla.org/MPL/2.0/.  -->
>  
>  <!ENTITY resetProfile.dialog.title        "Reset &brandShortName;">
>  <!ENTITY resetProfile.dialog.description  "Are you sure you want to reset &brandShortName; to its initial state?">
> +<!ENTITY resetProfile.dialog.items.label2 "&brandShortName; will try to preserve your:">

items2.label?
Comment 19 Justin Dolske [:Dolske] 2012-05-15 19:31:02 PDT
I am still somewhat concerned that this is too quickly leading users down the path of profile-reset, and that we'd benefit from a more conservative change until we have more experience with how this goes in the real world.

Consider:

1) I'm browsing along, and load browsercrash.com. Oops, browser crashes. :)
...crash reporter, restart firefox...
2) Automatic session restore, start loading last tab
...crash reporter, restart firefox...
3) Session restore's "Well, this is embarrassing". User restores session.
...crash reporter, restart firefox...
4) Two startup crashes, so this bug's prompt is shown.

It seems rather easy to get to point 4, but a profile reset isn't really called for.

So, I'd suggest 3 changes.

1) Tweak the text (leftmost dialog, attachment 609959 [details]) to say a bit more about safemode (it doesn't explain it at all), and possibly tone down the urging for resetting a profile.

2) For 1 or 2 release cycles, change the default button in that dialog to be "Start In Safe Mode" instead of profile-reset. (And then make profile-reset the default if we're seeing good results from the users who are clicking it.)

3) Increase toolkit.startup.max_resumed_crashes from 2 to 3 (or even 4?), so that the user in the example above would need another crash to get the dialog. Again, we can dial this back down as we get more experience with how things fare.
Comment 20 Matthew N. [:MattN] 2012-05-18 14:14:45 PDT
Created attachment 625263 [details] [diff] [review]
v.1 Changes for comment 19
Comment 21 Asa Dotzler [:asa] 2012-05-18 15:31:52 PDT
(In reply to Justin Dolske [:Dolske] from comment #19)
> I am still somewhat concerned that this is too quickly leading users down
> the path of profile-reset, and that we'd benefit from a more conservative
> change until we have more experience with how this goes in the real world.

I'm not that worried about the accidental or premature reset. For some users, they'll wonder where their add-ons went, and that's the only downside I can think of. I'd prefer that we be aggressive rather than timid about suggesting a profile reset. If you've ever helped someone reset their profile, you'll get what an amazing experience it is for them to have a fresh Firefox. It's so good so much of the time that I think we should risk the occasional premature reset for all of the positive resets we can push.
Comment 22 Justin Dolske [:Dolske] 2012-05-26 15:57:36 PDT
I agree it's a useful feature, but from the engineering side of things there are enough unknowns that I believe we need to start off carefully.
Comment 23 Matthew N. [:MattN] 2012-05-26 15:59:12 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/6261d5c12c7c

(In reply to Asa Dotzler [:asa] from comment #21)
> (In reply to Justin Dolske [:Dolske] from comment #19)
> > I am still somewhat concerned that this is too quickly leading users down
> > the path of profile-reset, and that we'd benefit from a more conservative
> > change until we have more experience with how this goes in the real world.
> 
> I'm not that worried about the accidental or premature reset. For some
> users, they'll wonder where their add-ons went, and that's the only downside
> I can think of.

Tabs are also not migrated
Comment 24 Phil Ringnalda (:philor, back in August) 2012-05-26 21:36:46 PDT
https://hg.mozilla.org/mozilla-central/rev/6261d5c12c7c
Comment 25 Willy_ Foo_Foo 2012-05-29 00:46:34 PDT
Since this bug has landed
i can see "Start in safe mode" "Reset profile"
but what happened to the other window & options like
"Disable all addons(& start normally)"
"delete all bookmarks?"
"Reset bookmarks & toolbar"
"reset all user preference"
"restore search engine"
Like the second window in this screenshot
http://people.mozilla.org/~mverdi/screenshots/reset-after-crash-20110629-233522.jpg

As a developer i find it very useful & also so do my helpers
please don't remove it add it with "Start in safe mode" "Reset profile"


win8/Xp
Comment 26 Matthew N. [:MattN] 2012-05-29 01:08:04 PDT
(In reply to magnumarchonbasileus from comment #25)
> As a developer i find it very useful & also so do my helpers
> please don't remove it add it with "Start in safe mode" "Reset profile"

Could you give more details on which of the options you found useful and what problems you were trying to solve with them.  I was told by people on the SUMO (support.mozilla.com) team that they don't direct users to use the checkboxes.  They were removed in this bug because the dialog was confusing with the two different continue buttons and most of the items could be solved just as easily by deleting files from the profile (we now have a button to open the profile folder from about:support):

* Disable all add-ons
** Start in safe mode (disables extensions) and then disable the plugins from the add-ons manager
* Reset toolbars and controls
** Delete localstore.rdf from the profile
* Delete all bookmarks except for backups
** set browser.bookmarks.restore_default_bookmarks to true (before startup or from about:config then restart)
* Reset all user preferences
** Delete prefs.js (and the "preferences" directory if it exists) from the profile
* Restore default search engines
** Delete search* from the profile

If there was an option which commonly solved users' problems and is better than resetting Firefox then we can consider a way to address that.  Could you file bugs (blocking this one) which you have strong arguments for? Otherwise we can discuss this more on the newsgroup at https://www.mozilla.org/about/forums/#dev-apps-firefox
Comment 27 Willy_ Foo_Foo 2012-05-29 02:36:56 PDT
(In reply to Matthew N. [:MattN] from comment #26)

> Could you give more details on which of the options you found useful and
> what problems you were trying to solve with them.

Firstly sorry for the late reply(was discussing this option with other users)

Yes

> I was told by people on
> the SUMO (support.mozilla.com) team that they don't direct users to use the
> checkboxes.  They were removed in this bug because the dialog was confusing
> with the two different continue buttons and most of the items could be
> solved just as easily by deleting files from the profile (we now have a
> button to open the profile folder from about:support):

Agree to some extent!
But when helping people (newbies) Thatch a very tough task !! (to find a profile or delete a particular file when they don't know what a browser is!)
Resetting does not help sometimes (say just want to reset tool-bars or bookmarks & nothing else with minimal fuss & surety that the user will not mess up!)


> * Disable all add-ons
> ** Start in safe mode (disables extensions) and then disable the plugins
> from the add-ons manager

We here use approx 40+ extensions & plugins, when we find some problem we just disable all addons in a go(try disabling even 10 extension one by one you will see)
& then one by one check each addon to find the problem(leaks or bugs/website problem)


> * Reset toolbars and controls
> ** Delete localstore.rdf from the profile

As a Power  user ok but What about the general people/non-power users?(adds unnecessary extra steps plus a risk)
plus while helping them creates extra steps(say when simply restarting & resetting search-box does the trick without any sort of risk)


> * Delete all bookmarks except for backups
> ** set browser.bookmarks.restore_default_bookmarks to true (before start-up
> or from about:config then restart)
> * Reset all user preferences
> ** Delete prefs.js (and the "preferences" directory if it exists) from the
> profile
> * Restore default search engines
> ** Delete search* from the profile
> 

Same As ABOVE!

> If there was an option which commonly solved users' problems and is better
> than resetting Firefox then we can consider a way to address that.  Could
> you file bugs (blocking this one) which you have strong arguments for?


These are the options which commonly solved users' problems and is better than resetting Firefox , just add a button to keep the old way too by keeping the old code( & add a 3rd button) to start the old box(many people find it easy)
Comment 28 Willy_ Foo_Foo 2012-05-29 21:52:22 PDT
(In reply to Matthew N. [:MattN] from comment #26)

> * Disable all add-ons
> ** Start in safe mode (disables extensions) and then disable the plugins
> from the add-ons manager
> * Reset toolbars and controls
> ** Delete localstore.rdf from the profile
> * Delete all bookmarks except for backups
> ** set browser.bookmarks.restore_default_bookmarks to true (before startup
> or from about:config then restart)
> * Reset all user preferences
> ** Delete prefs.js (and the "preferences" directory if it exists) from the
> profile
> * Restore default search engines
> ** Delete search* from the profile
> 
> If there was an option which commonly solved users' problems and is better
> than resetting Firefox then we can consider a way to address that.  Could
> you file bugs (blocking this one) which you have strong arguments for?


Plus users or tech support has to remember settings whereas that menu provides a easy GUI
Comment 29 Lozzy 2012-06-01 07:43:14 PDT
As it stands, a backup of the reset profile folder is put on the desktop, but I read in another bug that future iterations of this feature would remove that safety net. If the reset button is ever a default I feel it would be best to keep the backup around so that users at least have some form of recourse available.
Comment 30 Ioana (away) 2012-06-11 08:53:51 PDT
Verified as fixed on Fx 15.0 Aurora(20120609042006) and Fx16.0 Nightly(20120611030526).

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