Comment on attachment 8931547 Bug 440908 - Allow preference files to set locked prefs.
https://reviewboard.mozilla.org/r/202672/#review208008 As per our IRC discussion, I think this is a good feature, and the patch does a good job of implementing it. But before doing this I'd like to officially split the prefs file format in two -- one format for default prefs, and one for user prefs -- and then only support `locked_pref` in default prefs files. And I want to need to think about that for a bit, just to make sure it's a good idea. ::: modules/libpref/Preferences.cpp:238 (Diff revision 1) > enum > { > kPrefSetDefault = 1, > kPrefForceSet = 2, > kPrefSticky = 4, > + kPrefLocked = 8, The patches in bug 1394578 are going to bitrot this again. Sorry :( ::: modules/libpref/Preferences.cpp:949 (Diff revision 1) > char* mLbEnd; // line buffer end > char* mVb; // value buffer (ptr into mLb) > Maybe<PrefType> mVtype; // pref value type > bool mIsDefault; // true if (default) pref > bool mIsSticky; // true if (sticky) pref > + bool mIsLocked; // true if (locked) pref It would be nice to replace these three bools with a single enum that has the following alternatives: Default, StickyDefault, LockedDefault, User. That would simplify some of the code below. ::: modules/libpref/test/unit/test_lockedprefs.js:20 (Diff revision 1) > +} > + > +add_test(function notChangedFromAPI() { > + resetAndLoad(["data/testPrefLocked.js"]); > + Assert.strictEqual(ps.getBoolPref("testPref.unlocked.bool"), true); > + Assert.strictEqual(ps.getBoolPref("testPref.locked.bool"), false); Keeping track of the true and false values here is tricky. If you changed it to an int you could make the "testPref.locked.int" use 100 and 101, and "testPref.unlocked.int" use 200 and 201, or something like that, and it would be a bit easier to read. ::: modules/libpref/test/unit/test_lockedprefs.js:24 (Diff revision 1) > + Assert.strictEqual(ps.getBoolPref("testPref.unlocked.bool"), true); > + Assert.strictEqual(ps.getBoolPref("testPref.locked.bool"), false); > + > + ps.setBoolPref("testPref.unlocked.bool", false); > + Assert.ok(ps.prefHasUserValue("testPref.unlocked.bool"), > + "should be able to set an unlocked pref"); This string comment isn't quite right, because you can set a locked pref. ::: modules/libpref/test/unit/test_lockedprefs.js:29 (Diff revision 1) > + "should be able to set an unlocked pref"); > + Assert.strictEqual(ps.getBoolPref("testPref.unlocked.bool"), false); > + > + ps.setBoolPref("testPref.locked.bool", true); > + Assert.ok(ps.prefHasUserValue("testPref.locked.bool"), > + "somehow, the user value is still set"); This comment string is a bit weird too. -- You received this bug notification because you are a member of Desktop Packages, which is subscribed to thunderbird in Ubuntu. https://bugs.launchpad.net/bugs/623844 Title: lockpref not honored in /etc/thunderbird/pref/thunderbird.js Status in Mozilla Thunderbird: Fix Released Status in thunderbird package in Ubuntu: New Bug description: Binary package hint: thunderbird appended /etc/thunderbird/pref/thunderbird.js with the following line: lockpref("app.update.enabled", false); When starting Thunderbird and checking the config app.update.enabled is set to false, but can be edited by the user. ProblemType: Bug DistroRelease: Ubuntu 10.04 Package: thunderbird 3.0.6+build2+nobinonly-0ubuntu0.10.04.1 ProcVersionSignature: Ubuntu 2.6.32-24.39-generic 2.6.32.15+drm33.5 Uname: Linux 2.6.32-24-generic x86_64 Architecture: amd64 Date: Wed Aug 25 09:58:43 2010 InstallationMedia: Ubuntu 10.04 LTS "Lucid Lynx" - Release amd64 (20100429) ProcEnviron: PATH=(custom, no user) LANG=de_DE.utf8 SHELL=/bin/bash SourcePackage: thunderbird To manage notifications about this bug go to: https://bugs.launchpad.net/thunderbird/+bug/623844/+subscriptions -- Mailing list: https://launchpad.net/~desktop-packages Post to : desktop-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~desktop-packages More help : https://help.launchpad.net/ListHelp
