303279 - Safe Mode: Use "safe" localstore

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, enhancement)

Description

[Jaime Mitchell (use [e-mail] for email)]

When Firefox is opened in safe mode (either invoked by the user or
automatically) a series of pages with questions should be offered with
troubshooting steps. This is similar in concept to the help wizard in windows help.

___
&brandName is running in safe mode, this is used to troubleshoot problems with
&brandName.

What is the problem:
o &brandName wont start
o menus are messed up
etc...
____

Clicking each option provides the user with more choices as well as offering
suggestions as to a fix. Such as instructions how to use the default theme and
then to restart in normal mode to see if the problem is fixed.

If safe mode is invoked automatically (not yet possible) the wizard should offer
instructions relevant to the reason the browser was started in safe mode. Such
as if Firefox hangs when trying to load the files for an extension and on the
next restart safemode is activated (see bug 294260) then the instructions should
tell you which extension to try disabling. 

If it is possible to give these pages/wizard chrome privileges then buttons
should be provided to carryout operations rather than the user being given
complex instructions as to how to do the necessary steps.

The wizard should try certain actions such as to open the mozilla.org homepage
and if that fails then offer relevant information about configuring
firewalls/proxy settings.

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

Jaime clarified in IRC that he was picturing that these wizard pages would be
rendered in the browser en lieu of the start page, and should have a "Take me to
my start page" link on them to allow users to skip past them.

Comment 2

[Benjamin Smedberg]

Attachment: Use "special" localstore in safe mode, rev. 1

Comment 3

[Darin Fisher]

Comment on attachment 192407 [details] [diff] [review]
Use "special" localstore in safe mode, rev. 1

Shouldn't there be some sort of NS_ prefix on LOCALSTORE_UNSAFE_FILE?

Comment 4

[Benjamin Smedberg]

I'm happy to prefix with NS_ if that seems useful.

Comment 5

[Darin Fisher]

Comment on attachment 192407 [details] [diff] [review]
Use "special" localstore in safe mode, rev. 1

>Index: toolkit/xre/nsAppRunner.h

>+#define LOCALSTORE_UNSAFE_FILE "LStoreS"

Yeah, I think it's a good idea to namespace this identifier.  Afterall,
this header file may be included by several modules, and it does define
other things with proper namespaces, so why is this any different?


>Index: toolkit/xre/nsXREDirProvider.cpp

>+      else if (!strcmp(aProperty, NS_APP_LOCALSTORE_50_FILE)) {
>+        rv = mProfileDir->Clone(getter_AddRefs(file));
>+        if (gSafeMode) {
>+          rv |= file->AppendNative(NS_LITERAL_CSTRING("localstore-safe.rdf"));
>+          PRBool exists;
>+          file->Remove(PR_FALSE);
>+        }

I'm not understanding something... what is the |exists| variable for?
Why are you removing the localstore file?

Comment 6

[Mike Connor [:mconnor]]

Comment on attachment 192407 [details] [diff] [review]
Use "special" localstore in safe mode, rev. 1

>+      else if (!strcmp(aProperty, NS_APP_LOCALSTORE_50_FILE)) {
>+        rv = mProfileDir->Clone(getter_AddRefs(file));
>+        if (gSafeMode) {
>+          rv |= file->AppendNative(NS_LITERAL_CSTRING("localstore-safe.rdf"));
>+          PRBool exists;
>+          file->Remove(PR_FALSE);

missing a couple lines here?  I'm assuming you were going to check
file->Exists() before calling remove, especially since iirc that crashes hard
if !exists?

Comment 7

[Mike Connor [:mconnor]]

Comment on attachment 192407 [details] [diff] [review]
Use "special" localstore in safe mode, rev. 1

Probably need a non-darin SR here, if we need a second review here.  I'd like
to get this done ASAP so I can finish up 304403

Comment 8

[Benjamin Smedberg]

Attachment: Use "special" localstore in safe mode, rev. 1.1 [checked in on trunk]

Comment 9

[Darin Fisher]

Comment on attachment 193816 [details] [diff] [review]
Use "special" localstore in safe mode, rev. 1.1 [checked in on trunk]

By the way, it's not clear to me why NS_LOCALSTORE_UNSAFE_FILE exists.	Who is
the consumer?

sr=darin FWIW

Comment 10

[Benjamin Smedberg]

The consumer is to-be-written UI code which lets the user remove a (potentially
corrupted) localstore.rdf file in safe mode.

Comment 11

[Benjamin Smedberg]

Comment on attachment 193816 [details] [diff] [review]
Use "special" localstore in safe mode, rev. 1.1 [checked in on trunk]

This has been landed on trunk. Maybe we should just morph this bug to cover
this patch, and file followups as necessary?

Comment 12

[Benjamin Smedberg]

I'm going to morph this bug into the localstore specficially, and let bug 304403
be about the UI.

Comment 13

[Benjamin Smedberg]

*** Bug 311158 has been marked as a duplicate of this bug. ***

Comment 14

[[e-mail]]

Bah, this was ugly, why didn't anyone think to use an in-memory data source in safe mode? (LocalStoreImpl already did this after profile-before-change.)