Add "Do Not Edit" warning to prefs.js

VERIFIED FIXED

Status

Core Graveyard
Profile: BackEnd
VERIFIED FIXED
16 years ago
2 years ago

People

(Reporter: Andrew Hagen, Assigned: Conrad Carlen (not reading bugmail))

Tracking

(Blocks: 1 bug, {fixed1.4, relnote})

Trunk
fixed1.4, relnote

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 2 obsolete attachments)

(Reporter)

Description

16 years ago
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. */
(Reporter)

Updated

16 years ago
Blocks: 123929
Keywords: mozilla1.0, nsbeta1
-> Profiles
Assignee: Matti → ccarlen
Component: Browser-General → Profile Manager BackEnd
QA Contact: imajes-qa → ktrina

Comment 2

16 years ago
Created attachment 82525 [details] [diff] [review]
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

16 years ago
*** Bug 142588 has been marked as a duplicate of this bug. ***
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?
(Reporter)

Comment 5

16 years ago
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.
(Reporter)

Comment 6

16 years ago
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.
Keywords: relnote
(Reporter)

Updated

16 years ago
Keywords: mozilla1.0.1

Comment 7

16 years ago
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.
(Reporter)

Comment 8

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

Comment 9

16 years ago
In bug 134676,  steve@steem.com 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 
 */ 
(Reporter)

Comment 10

16 years ago
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.
Keywords: mozilla1.0 → mozilla1.2

Comment 11

16 years ago
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

15 years ago
Created attachment 104440 [details] [diff] [review]
Patch that's more visible and contains more information

Updated

15 years ago
Keywords: patch, review
(Reporter)

Updated

15 years ago
Keywords: mozilla1.0.1, mozilla1.2 → mozilla1.3

Updated

15 years ago
QA Contact: ktrina → gbush

Updated

15 years ago
Attachment #104440 - Flags: review?(ccarlen)
(Assignee)

Comment 13

15 years ago
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

15 years ago
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.
(Assignee)

Comment 15

15 years ago
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. 

Updated

15 years ago
Attachment #104440 - Attachment is obsolete: true
Attachment #104440 - Flags: review?(ccarlen)

Comment 16

15 years ago
Created attachment 117882 [details] [diff] [review]
Less dire patch with NS_LINEBREAK at the end of each line

Updated

15 years ago
Attachment #117882 - Flags: review?(ccarlen)
(Assignee)

Comment 17

15 years ago
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

15 years ago
Created attachment 117897 [details] [diff] [review]
Patch with recommended formatting

Like this?
Attachment #117882 - Attachment is obsolete: true

Updated

15 years ago
Attachment #117897 - Flags: review?(ccarlen)
(Assignee)

Comment 19

15 years ago
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
Attachment #117897 - Flags: review?(ccarlen) → review+

Updated

15 years ago
Attachment #117897 - Flags: superreview?

Updated

15 years ago
Attachment #117882 - Flags: review?(ccarlen)

Updated

15 years ago
Attachment #117897 - Flags: superreview? → superreview?(rbs)

Comment 20

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

sr=rbs
Attachment #117897 - Flags: superreview?(rbs) → superreview+

Comment 21

15 years ago
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

15 years ago
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

15 years ago
Created attachment 120575 [details] [diff] [review]
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".
(Reporter)

Comment 24

15 years ago
Is this checked in?
Flags: blocking1.4b?
Flags: blocking1.4?
Keywords: mozilla1.3
(Assignee)

Comment 25

15 years ago
It's checked in. Thanks, timeless.

http://lxr.mozilla.org/seamonkey/source/modules/libpref/src/nsPrefService.cpp#351
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Updated

15 years ago
Flags: blocking1.4b?
Flags: blocking1.4?

Comment 26

15 years ago
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).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 27

15 years ago
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

15 years ago
Created attachment 121770 [details] [diff] [review]
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.

Updated

15 years ago
Attachment #121770 - Flags: superreview?(rbs)
Attachment #121770 - Flags: review?(ccarlen)

Comment 29

15 years ago
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.

Updated

15 years ago
Attachment #121770 - Flags: superreview?(rbs) → superreview+

Comment 30

15 years ago
adt: nsbeta1-
Keywords: nsbeta1 → nsbeta1-

Updated

15 years ago
Attachment #121770 - Flags: review?(ccarlen) → review+

Comment 31

15 years ago
Fix checked in.
Status: REOPENED → RESOLVED
Last Resolved: 15 years ago15 years ago
Resolution: --- → FIXED

Updated

15 years ago
Attachment #121770 - Flags: approval1.4?

Comment 32

15 years ago
Comment on attachment 121770 [details] [diff] [review]
Spelling correction

a=asa (on behalf of drivers) for checkin to the 1.4 branch.
Attachment #121770 - Flags: approval1.4? → approval1.4+

Comment 33

15 years ago
Fix checked into the 1.4 branch.
Keywords: fixed1.4

Comment 34

15 years ago
build 2003053005
Status: RESOLVED → VERIFIED
Keywords: verified1.4
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.