Closed
Bug 142590
Opened 22 years ago
Closed 21 years ago
Add "Do Not Edit" warning to prefs.js
Categories
(Core Graveyard :: Profile: BackEnd, defect)
Core Graveyard
Profile: BackEnd
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: contact2009, Assigned: ccarlen)
References
Details
(Keywords: fixed1.4, relnote)
Attachments
(4 files, 2 obsolete files)
1.15 KB,
patch
|
Details | Diff | Splinter Review | |
1.36 KB,
patch
|
ccarlen
:
review+
rbs
:
superreview+
|
Details | Diff | Splinter Review |
1.36 KB,
patch
|
Details | Diff | Splinter Review | |
627 bytes,
patch
|
neil
:
review+
rbs
:
superreview+
asa
:
approval1.4+
|
Details | Diff | Splinter Review |
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•22 years ago
|
Blocks: profile-corrupt
Keywords: mozilla1.0,
nsbeta1
Comment 1•22 years ago
|
||
-> Profiles
Assignee: Matti → ccarlen
Component: Browser-General → Profile Manager BackEnd
QA Contact: imajes-qa → ktrina
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•22 years ago
|
||
*** Bug 142588 has been marked as a duplicate of this bug. ***
Comment 4•22 years ago
|
||
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•22 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•22 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•22 years ago
|
Keywords: mozilla1.0.1
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•22 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•22 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•22 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•22 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•22 years ago
|
||
Updated•22 years ago
|
Reporter | ||
Updated•22 years ago
|
Updated•22 years ago
|
QA Contact: ktrina → gbush
Updated•21 years ago
|
Attachment #104440 -
Flags: review?(ccarlen)
Assignee | ||
Comment 13•21 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•21 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•21 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•21 years ago
|
Attachment #104440 -
Attachment is obsolete: true
Attachment #104440 -
Flags: review?(ccarlen)
Comment 16•21 years ago
|
||
Updated•21 years ago
|
Attachment #117882 -
Flags: review?(ccarlen)
Assignee | ||
Comment 17•21 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.
Updated•21 years ago
|
Attachment #117897 -
Flags: review?(ccarlen)
Assignee | ||
Comment 19•21 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•21 years ago
|
Attachment #117897 -
Flags: superreview?
Updated•21 years ago
|
Attachment #117882 -
Flags: review?(ccarlen)
Attachment #117897 -
Flags: superreview? → superreview?(rbs)
Comment 20•21 years ago
|
||
Comment on attachment 117897 [details] [diff] [review] Patch with recommended formatting sr=rbs
Attachment #117897 -
Flags: superreview?(rbs) → superreview+
Comment 21•21 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•21 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•21 years ago
|
||
This is the patch to check in. It has the requested extra newline and the change from "overridden" to "overwritten".
Reporter | ||
Comment 24•21 years ago
|
||
Is this checked in?
Assignee | ||
Comment 25•21 years ago
|
||
It's checked in. Thanks, timeless. http://lxr.mozilla.org/seamonkey/source/modules/libpref/src/nsPrefService.cpp#351
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Updated•21 years ago
|
Flags: blocking1.4b?
Flags: blocking1.4?
Comment 26•21 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•21 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•21 years ago
|
||
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•21 years ago
|
Attachment #121770 -
Flags: superreview?(rbs)
Attachment #121770 -
Flags: review?(ccarlen)
Comment 29•21 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.
Attachment #121770 -
Flags: superreview?(rbs) → superreview+
Updated•21 years ago
|
Attachment #121770 -
Flags: review?(ccarlen) → review+
Comment 31•21 years ago
|
||
Fix checked in.
Status: REOPENED → RESOLVED
Closed: 21 years ago → 21 years ago
Resolution: --- → FIXED
Updated•21 years ago
|
Attachment #121770 -
Flags: approval1.4?
Comment 32•21 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+
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•