Closed Bug 732303 Opened 12 years ago Closed 12 years ago

Redesign safe mode dialog with the profile reset option

Categories

(Firefox :: General, enhancement)

enhancement
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 15

People

(Reporter: MattN, Assigned: MattN)

References

(Blocks 2 open bugs, )

Details

(Keywords: user-doc-needed)

Attachments

(5 files, 3 obsolete files)

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.
Assignee: nobody → mnoorenberghe+bmo
Status: NEW → ASSIGNED
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).
Keywords: uiwanted
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.
Attachment #604360 - Flags: review?(dolske)
Attachment #604367 - Flags: ui-review?(ux-review)
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..
Attachment #604360 - Flags: review?(dolske) → review+
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.
(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?
The previous two mock-ups illustrate the conclusions reached in the meeting with MattN and dolske earlier today.
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.
(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.
Excellent - thanks guys.
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.
Attachment #604360 - Attachment is obsolete: true
Attachment #604364 - Attachment is obsolete: true
Attachment #604367 - Attachment is obsolete: true
Attachment #609958 - Flags: ui-review?(limi)
Attachment #609958 - Flags: review?(dolske)
Attachment #604364 - Flags: ui-review?(ux-review)
Attachment #604367 - Flags: ui-review?(ux-review)
Summary: Add profile reset option in forced safe mode → Redesign safe mode dialog with the profile reset option
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 on attachment 609958 [details] [diff] [review]
v.2 Based on Limi's design

Looks great! Nice job. :)
Attachment #609958 - Flags: ui-review?(limi) → ui-review+
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?
Attachment #609958 - Flags: review?(dolske) → review+
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.
Attachment #625263 - Flags: review?(dolske)
(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.
Attachment #625263 - Flags: review?(dolske) → review+
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.
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
https://hg.mozilla.org/mozilla-central/rev/6261d5c12c7c
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 15
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
(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
(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)
(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
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.
Verified as fixed on Fx 15.0 Aurora(20120609042006) and Fx16.0 Nightly(20120611030526).
Status: RESOLVED → VERIFIED
Keywords: user-doc-needed
Depends on: 765609
No longer depends on: 765609
Depends on: 881199
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: