Closed
Bug 738043
Opened 13 years ago
Closed 13 years ago
getPref() isn't working in custom.cfg file after upgrading to TB11 (MCD/autoconf)
Categories
(Toolkit :: Preferences, defect)
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: barry.ralphs, Assigned: emk)
References
Details
(Keywords: regression, Whiteboard: [qa-])
Attachments
(2 files, 1 obsolete file)
903 bytes,
patch
|
roc
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
5.58 KB,
patch
|
roc
:
review+
RyanVM
:
checkin+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/535.11 (KHTML, like Gecko) Chrome/17.0.963.79 Safari/535.11
Steps to reproduce:
I upgraded to TB11
Actual results:
getPref() stopped returning correct values. It always returns null after upgrading to TB11.
Expected results:
It should return correct values. e.g.
var listExistingAccounts = getPref("mail.accountmanager.accounts");
should return "account1,account2,account3"
Comment 1•13 years ago
|
||
What did you upgrade from ?
Reporter | ||
Comment 2•13 years ago
|
||
I usually upgrade when each new update comes out. So the previous version was 10.0.2.
Reporter | ||
Comment 3•13 years ago
|
||
So I see 11.0.1 has come out and this issue still hasn't been addressed...?
Reporter | ||
Comment 5•13 years ago
|
||
I have a "custom.cfg" file in the install folder "C:\Program Files (x86)\Mozilla Thunderbird"
& these are set in my "...\defaults\pref\custom.js"
pref("general.config.obscure_value", 0);
pref("general.config.filename", "custom.cfg");
Usul, this could be a Core/toolkit problem with in the custom script support. Could you CC the right people? I doubt getPref is a TB specific function.
Updated•13 years ago
|
Product: Thunderbird → Toolkit
QA Contact: preferences → preferences
Version: 11 → 11 Branch
Just for the record, I have not tried replicating the bug because I would not know how to test it, how to see the error in that custom file. Hopefully someone more familiar with the functionality can see the problem straight.
Assignee | ||
Comment 8•13 years ago
|
||
Probably a regression of bug 451161. AutoConfig can not read per-user settings anymore.
Blocks: 451161
Comment 9•13 years ago
|
||
To clarify, this is all about Mission control desktop/autoconf. I know the lockPref stuff was working in 11 and 12, I don't use the getPref items myself.
Summary: getPref() isn't working in custom.cfg file after upgrading to TB11 → getPref() isn't working in custom.cfg file after upgrading to TB11 (MCD/autoconf)
Comment 10•13 years ago
|
||
Reporter | ||
Comment 11•13 years ago
|
||
You can easily see this by adding the following lines to your custom.cfg file:
var promptService = Components.classes["@mozilla.org/embedcomp/prompt-service;1"]
.getService(Components.interfaces.nsIPromptService);
var listExistingAccounts = getPref("mail.accountmanager.accounts");
promptService.alert(null, "title", listExistingAccounts);
In TB11+ you'll get a blank message box. In TB10- you'll get a list of the accounts.
Comment 12•13 years ago
|
||
Just verified it is broke in Firefox as well.
The getPref function works for default preferences but it no longer works for user set preferences.
This is a big regression.
Severity: normal → critical
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: regression
Comment 13•13 years ago
|
||
For anyone looking, the relevant code is here:
http://mxr.mozilla.org/mozilla-central/source/extensions/pref/autoconfig/src/prefcalls.js#129
Comment 14•13 years ago
|
||
Here's more specific code that used to work in an autoconfig file but doesn't anymore. It works fine elsewhere.
var prefService = Components.classes["@mozilla.org/preferences-service;1"].getService(Components.interfaces.nsIPrefService);
var prefBranch = prefService.getBranch(null);
var prefValue = prefBranch.getCharPref("intl.charsetmenu.browser.cache");
At this point, prefValue is empty.
Comment 15•13 years ago
|
||
Masatoshi - Michael believes that this is a regression from bug 451161. Would you mind taking a look?
Tracking for FF12 and up since this regressed in 11 and will cause pain for deployments.
Assignee: nobody → VYV03354
tracking-firefox12:
--- → +
tracking-firefox13:
--- → +
tracking-firefox14:
--- → +
Assignee | ||
Comment 16•13 years ago
|
||
The reason is obvious. Bug 451161 changed a reading order so that AutoConfig is called before user settings.
Bug 451161 comment 20 explains why the order was reversed. How can I reverse the order again without regressing bug 451161? I have no idea at all.
Assignee | ||
Comment 17•13 years ago
|
||
That said, larger patch will not be suitable for branches.
Attachment #611098 -
Flags: review?(roc)
Attachment #611098 -
Flags: approval-mozilla-beta?
Attachment #611098 -
Flags: approval-mozilla-aurora?
Attachment #611098 -
Flags: review?(roc) → review+
Assignee | ||
Comment 18•13 years ago
|
||
If an user set value is identical to the default one and AutoConfig doesn't change the default, the user set value will be removed from prefs.js on exit anyway.
https://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/prefapi.cpp#331
Attachment #611149 -
Flags: review?(roc)
Comment on attachment 611149 [details] [diff] [review]
Don't ignore user set values from pref files even if values are identical to default
Review of attachment 611149 [details] [diff] [review]:
-----------------------------------------------------------------
::: modules/libpref/src/Preferences.cpp
@@ +467,5 @@
> rv = reader->Open(aFile);
> NS_ENSURE_SUCCESS(rv, rv);
>
> nsCOMPtr<nsIUTF8StringEnumerator> files;
> + rv = reader->FindEntries(nsDependentCString("defaults/preferences/""*.(J|j)(S|s)$"),
I'm not sure what this change is for
::: modules/libpref/src/prefapi.cpp
@@ +172,5 @@
> /* -- Prototypes */
> static nsresult pref_DoCallback(const char* changed_pref);
>
> +enum {
> + kPrefSetDefault = true,
I think 1 instead of true
@@ +745,5 @@
> {
> /* If new value is same as the default value, then un-set the user value.
> Otherwise, set the user value only if it has changed */
> if (!pref_ValueChanged(pref->defaultPref, value, type) &&
> + pref->flags & PREF_HAS_DEFAULT &&
() around the & expression
Assignee | ||
Comment 20•13 years ago
|
||
Attachment #611149 -
Attachment is obsolete: true
Attachment #611149 -
Flags: review?(roc)
Attachment #611324 -
Flags: review?(roc)
Attachment #611324 -
Flags: review?(roc) → review+
Assignee | ||
Updated•13 years ago
|
Keywords: regression
Whiteboard: regression → [autoland-try:-b do -p all -u all -t none]
Assignee | ||
Updated•13 years ago
|
Whiteboard: [autoland-try:-b do -p all -u all -t none]
Assignee | ||
Updated•13 years ago
|
Whiteboard: [autoland-try:611324:-b do -p all -u all -t none]
Updated•13 years ago
|
Whiteboard: [autoland-try:611324:-b do -p all -u all -t none] → [autoland-in-queue]
Comment 21•13 years ago
|
||
Autoland Patchset:
Patches: 611324
Branch: mozilla-central => try
Destination: http://hg.mozilla.org/try/pushloghtml?changeset=148d6b76519a
Try run started, revision 148d6b76519a. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=148d6b76519a
Comment 22•13 years ago
|
||
Try run for 148d6b76519a is complete.
Detailed breakdown of the results available here:
https://tbpl.mozilla.org/?tree=Try&rev=148d6b76519a
Results (out of 224 total builds):
exception: 1
success: 188
warnings: 34
failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-148d6b76519a
Updated•13 years ago
|
Whiteboard: [autoland-in-queue]
Assignee | ||
Updated•13 years ago
|
Keywords: checkin-needed
Whiteboard: [land 611324 to trunk]
Comment 23•13 years ago
|
||
Comment on attachment 611098 [details] [diff] [review]
backout bug 451161 reading order change
[Triage Comment]
Low-risk backout of a regression in FF11. Approved for Aurora 13 and Beta 12.
Attachment #611098 -
Flags: approval-mozilla-beta?
Attachment #611098 -
Flags: approval-mozilla-beta+
Attachment #611098 -
Flags: approval-mozilla-aurora?
Attachment #611098 -
Flags: approval-mozilla-aurora+
Assignee | ||
Updated•13 years ago
|
Whiteboard: [land 611324 to trunk] → [land 611324 to trunk][land 611098 to aurora and beta]
Comment 24•13 years ago
|
||
Attachment 611324 [details] [diff] checked in to inbound.
https://hg.mozilla.org/integration/mozilla-inbound/rev/d754f0fb17e6
Flags: in-testsuite?
Whiteboard: [land 611324 to trunk][land 611098 to aurora and beta] → [land 611098 to aurora and beta]
Target Milestone: --- → mozilla14
Updated•13 years ago
|
Attachment #611324 -
Flags: checkin+
Comment 25•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 26•13 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/57071196be2e
https://hg.mozilla.org/releases/mozilla-aurora/rev/78a6759335b5
status-firefox12:
--- → fixed
status-firefox13:
--- → fixed
Keywords: checkin-needed
OS: Windows 7 → All
Hardware: x86_64 → All
Whiteboard: [land 611098 to aurora and beta]
Updated•13 years ago
|
status-firefox14:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•