Closed
Bug 738043
Opened 12 years ago
Closed 12 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•12 years ago
|
||
What did you upgrade from ?
Reporter | ||
Comment 2•12 years ago
|
||
I usually upgrade when each new update comes out. So the previous version was 10.0.2.
Reporter | ||
Comment 3•12 years ago
|
||
So I see 11.0.1 has come out and this issue still hasn't been addressed...?
Reporter | ||
Comment 5•12 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•12 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•12 years ago
|
||
Probably a regression of bug 451161. AutoConfig can not read per-user settings anymore.
Blocks: 451161
Comment 9•12 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•12 years ago
|
||
aceman: if you're curious how to test this: http://mike.kaply.com/2012/03/16/customizing-firefox-autoconfig-files/ http://mike.kaply.com/2012/03/20/customizing-firefox-autoconfig-files-continued/ http://mike.kaply.com/2012/03/22/customizing-firefox-advanced-autoconfig-files/
Reporter | ||
Comment 11•12 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•12 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•12 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•12 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•12 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•12 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•12 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•12 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•12 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•12 years ago
|
Keywords: regression
Whiteboard: regression → [autoland-try:-b do -p all -u all -t none]
Assignee | ||
Updated•12 years ago
|
Whiteboard: [autoland-try:-b do -p all -u all -t none]
Assignee | ||
Updated•12 years ago
|
Whiteboard: [autoland-try:611324:-b do -p all -u all -t none]
Updated•12 years ago
|
Whiteboard: [autoland-try:611324:-b do -p all -u all -t none] → [autoland-in-queue]
Comment 21•12 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•12 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•12 years ago
|
Whiteboard: [autoland-in-queue]
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Whiteboard: [land 611324 to trunk]
Comment 23•12 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•12 years ago
|
Whiteboard: [land 611324 to trunk] → [land 611324 to trunk][land 611098 to aurora and beta]
Comment 24•12 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•12 years ago
|
Attachment #611324 -
Flags: checkin+
Comment 25•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d754f0fb17e6
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 26•12 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•12 years ago
|
status-firefox14:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•