Last Comment Bug 303279 - Safe Mode: Use "safe" localstore
: Safe Mode: Use "safe" localstore
Status: RESOLVED FIXED
: fixed1.8
Product: Toolkit
Classification: Components
Component: Startup and Profile System (show other bugs)
: Trunk
: x86 Windows XP
: -- enhancement (vote)
: ---
Assigned To: Benjamin Smedberg [:bsmedberg]
:
: Benjamin Smedberg [:bsmedberg]
Mentors:
: 311158 (view as bug list)
Depends on: 304403
Blocks: 1058436
  Show dependency treegraph
 
Reported: 2005-08-03 11:37 PDT by Jaime Mitchell (use bugmail@jaimem.org.uk for email)
Modified: 2016-03-06 00:38 PST (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Use "special" localstore in safe mode, rev. 1 (3.02 KB, patch)
2005-08-11 12:39 PDT, Benjamin Smedberg [:bsmedberg]
mconnor: review+
Details | Diff | Splinter Review
Use "special" localstore in safe mode, rev. 1.1 [checked in on trunk] (2.96 KB, patch)
2005-08-25 07:51 PDT, Benjamin Smedberg [:bsmedberg]
benjamin: review+
darin.moz: superreview+
asa: approval1.8b5+
Details | Diff | Splinter Review

Description Jaime Mitchell (use bugmail@jaimem.org.uk for email) 2005-08-03 11:37:29 PDT
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] 2005-08-05 01:47:19 PDT
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 [:bsmedberg] 2005-08-11 12:39:40 PDT
Created attachment 192407 [details] [diff] [review]
Use "special" localstore in safe mode, rev. 1
Comment 3 Darin Fisher 2005-08-12 11:58:32 PDT
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 [:bsmedberg] 2005-08-12 12:43:27 PDT
I'm happy to prefix with NS_ if that seems useful.
Comment 5 Darin Fisher 2005-08-12 14:53:12 PDT
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] 2005-08-15 23:49:05 PDT
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] 2005-08-22 15:36:58 PDT
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 [:bsmedberg] 2005-08-25 07:51:09 PDT
Created attachment 193816 [details] [diff] [review]
Use "special" localstore in safe mode, rev. 1.1 [checked in on trunk]
Comment 9 Darin Fisher 2005-08-31 13:41:17 PDT
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 [:bsmedberg] 2005-08-31 13:47:25 PDT
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 [:bsmedberg] 2005-09-07 13:24:07 PDT
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 [:bsmedberg] 2005-09-14 07:03:05 PDT
I'm going to morph this bug into the localstore specficially, and let bug 304403
be about the UI.
Comment 13 Benjamin Smedberg [:bsmedberg] 2005-10-05 05:08:33 PDT
*** Bug 311158 has been marked as a duplicate of this bug. ***
Comment 14 neil@parkwaycc.co.uk 2014-08-27 03:49:11 PDT
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.)

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