142590 - Add "Do Not Edit" warning to prefs.js

Status: VERIFIED FIXED

(Core Graveyard :: Profile: BackEnd, defect)

Description

[Andrew Hagen]

To prevent profile corruption, we need to improve the warning at the top of
prefs.js. Currently, the warning is "generated file." The new warning should
look like this:

# Mozilla User Preferences
/* This is a generated file! Do not edit. To make a manual change to
preferences, create a new text file, user.js, in the same directory as prefs.js.
Entries in user.js will override entries in prefs.js. */

Comment 1

[Matthias Versen [:Matti]]

-> Profiles

Comment 2

[mental]

Attachment: try

Here's a quick stab at it.  If I need to be more careful about line wrapping,
please let me know the right thing to do.

Comment 3

[Bruce Davidson]

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

Comment 4

[[e-mail]]

just a question: some preference entries will only be in prefs.js if
the default gets changed (e.g. display.resolution). So how would you 
set soemthing to the default using user.js alone?

Comment 5

[Andrew Hagen]

If there is a user.js entry, but no corresponding prefs.js entry, the user.js
entry should be honored. I think this is reasonably clear. We could change the
last sentence to: 

Entries in user.js will override entries in prefs.js, if any.

Maybe there is a better wording, but it doesn't have to be perfect.

Comment 6

[Andrew Hagen]

Proposed relnote:  To make a manual change to preferences, do not edit prefs.js.
Create a new text file, user.js, in the same directory as prefs.js. Entries in
user.js will override entries in prefs.js, if necessary.

Comment 7

[Graham]

Are making changes in the Preferences dialog going to be saved in prefs.js or
user.js? There's some real scope for confusion here if the user thinks they've
changed something but it remains overridden by user.js. This is to a degree a
user error, however as Mozilla evolves some preferences which currently have to
be entered in prefs.js may appear in the Preferences dialog and vice-versa.
Keeping preferences in multiple places is an inherently bad idea in my opinion.

Comment 8

[Andrew Hagen]

I'd have to argue that allowing users to possibly munge their profile data
without a warning is a less good idea.

Comment 9

[Akkana Peck]

In bug 134676,  [e-mail] suggests this comment, and I agree that pointing
to customizing.html would be a good idea, since anyone who's gotten as far as
editing the prefs file is probably interested in finding more information.

/* prefs.js - Warning this file WILL get overwritten by mozilla.  You can
 * create a user.js file in this same directory with preferences that will
 * override prefs.js.
 * For more information visit http://www.mozilla.org/unix/customizing.html 
 */ 

Comment 10

[Andrew Hagen]

This bug is not invalid. Fixing it would be a useful addition to Mozilla. We
need to stop advising ordinary users to edit their prefs.js file. Any changes
they want to make should go into user.js.

Comment 11

[Steve Chapel]

Here's a warning that's more visible and gives some more pointers for learning
about preferences:

/* DO NOT EDIT THIS FILE
 *
 * This is a generated file, and manual changes will be overwritten
 * the next time this file is generated.
 *
 * To make a manual change to preferences, create a new text file
 * named user.js in the same directory as prefs.js. Entries in user.js
 * will override entries in prefs.js. For more information, see
 * http://www.mozilla.org/unix/customizing.html#prefs
 * To see a list of all preferences, visit the URL about:config
 */

Comment 12

[Steve Chapel]

Attachment: Patch that's more visible and contains more information

Comment 13

[Conrad Carlen (not reading bugmail)]

This warning seems a bit dire. Shouldn't it just say that, if changed while the
program is running, the changes will be overwritten? For the 2nd para, I think
the sentence "For more information, see
http://www.mozilla.org/unix/customizing.html#prefs" is about enough.

Comment 14

[Steve Chapel]

How about this:

/* Do not edit this file.
 *
 * If you make changes to this file while the browser is running,
 * the changes will be overridden when the browser exits.
 *
 * To make a manual change to preferences, you can visit the URL about:config
 * For more information, see http://www.mozilla.org/unix/customizing.html#prefs
 */

I still have the somewhat dire "Do not edit this file" so we'll get people's
attention. I want to mention the URL about:config because it's probably the
easiest way to edit prefs, and many people will not bother to load the external
URL and read that they should create a user.js file.

Comment 15

[Conrad Carlen (not reading bugmail)]

I like that better :-) Also, I think the code would read better if you put the
NS_LINEBREAK on the end of each line rather than its own line. 

Comment 16

[Steve Chapel]

Attachment: Less dire patch with NS_LINEBREAK at the end of each line

Comment 17

[Conrad Carlen (not reading bugmail)]

Comment on attachment 117882 [details] [diff] [review]
Less dire patch with NS_LINEBREAK at the end of each line

>--- cvsroot/mozilla/modules/libpref/src/nsPrefService.cpp.orig	2003-03-19 00:31:22.000000000 -0500
>+++ cvsroot/mozilla/modules/libpref/src/nsPrefService.cpp	2003-03-20 14:24:12.000000000 -0500
>@@ -348,10 +348,16 @@
> 
> nsresult nsPrefService::WritePrefFile(nsIFile* aFile)
> {
>-  const char                outHeader[] = "# Mozilla User Preferences" 
>-                                          NS_LINEBREAK 
>-                                          "// This is a generated file!" 
>-                                          NS_LINEBREAK 
>+  const char                outHeader[] =
      "# Mozilla User Preferences" NS_LINEBREAK
>+    NS_LINEBREAK
>+    "/* Do not edit this file." NS_LINEBREAK
>+    " *" NS_LINEBREAK

If the strings start so far to the right, the lines become too wide. Wrap and
indent it like this. Sorry, I was wrong about the NS_LINEBREAK at the end of
each line. Now that there's less text to begin with, maybe it is better to put
those on their own line.

Comment 18

[Steve Chapel]

Attachment: Patch with recommended formatting

Like this?

Comment 19

[Conrad Carlen (not reading bugmail)]

Comment on attachment 117897 [details] [diff] [review]
Patch with recommended formatting

Stick a new line after the last
+    NS_LINEBREAK;
and I'm OK with it. No need for a new patch on that change. r=ccarlen

Comment 20

[rbs]

Comment on attachment 117897 [details] [diff] [review]
Patch with recommended formatting

sr=rbs

Comment 21

[Steve Chapel]

Could someone check this in for me? I don't have cvs access. Don't forget the
extra newline mentioned in comment #19!

Comment 22

[Malcolm Rowe]

Comment on attachment 117897 [details] [diff] [review]
Patch with recommended formatting

Please - "changes will be overwritten when the browser exits", not "changes
will be overridden when the browser exits".  It sounds like broken English at
the moment.

Comment 23

[Steve Chapel]

Attachment: Patch with spelling correction and added newline

This is the patch to check in. It has the requested extra newline and the
change from "overridden" to "overwritten".

Comment 24

[Andrew Hagen]

Is this checked in?

Comment 25

[Conrad Carlen (not reading bugmail)]

It's checked in. Thanks, timeless.

http://lxr.mozilla.org/seamonkey/source/modules/libpref/src/nsPrefService.cpp#351

Comment 26

[Malcolm Rowe]

Reopened. The wrong patch was checked in.

Attachment 117897 [details] [diff] was checked in (with r/sr), but attachment 120575 [details] [diff] [review] was the
intended patch ('this is the patch to check in' - see comment 23).

Comment 27

[Steve Chapel]

Arg! The patch was checked in the day before the misspelling was first noticed.
I suggest leaving this bug marked fixed, and open a new bug on the change from
overriding to overwriting.

Comment 28

[Malcolm Rowe]

Attachment: Spelling correction

I already reopened the bug, I'm afraid.  Here's the patch that fixes the
spelling mistake.

It appears that the extra newline was added during checkin.

Comment 29

[rbs]

For obvious changes like that, just keep the same bug. If my memory serves me
right, I think brendan has granted a de facto catch-all sr=brendan for
embarassing typos in comments.

Comment 30

[Samir Gehani]

adt: nsbeta1-

Comment 31

[[e-mail]]

Fix checked in.

Comment 32

[Asa Dotzler [:asa]]

Comment on attachment 121770 [details] [diff] [review]
Spelling correction

a=asa (on behalf of drivers) for checkin to the 1.4 branch.

Comment 33

[[e-mail]]

Fix checked into the 1.4 branch.

Comment 34

[Grace Bush]

build 2003053005