locked preferences values shouldn't be stored in user preferences

RESOLVED FIXED in mozilla1.9.3a5

Status

()

RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

Trunk
mozilla1.9.3a5
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

10 years ago
Created attachment 390442 [details] [diff] [review]
patch

One of the consequences of this is debian bug #512111 ( http://bugs.debian.org/512111 ).

What can happen in other contexts, is an administrator replacing a locked preference with an unlocked pref with a different value. Users would then not get the modified value.

The attached patch doesn't save locked preferences at all when it was not already in the prefs file, but will keep any user value if it was set before the pref was locked. Which means in the case the administrator replaces the lockes pref with an unlocked pref with a different value, the user won't get it, as he had a custom value in the first place. I think this is a reasonable drawback, if that be called so.

Note that bug 330590 might be related to this one, though I haven't verified.

PS: No owner listed on http://www.mozilla.org/owners.html, so I'm requesting review to the last person who reviewed last commit to libpref.
Attachment #390442 - Flags: review?(dveditz)
(Assignee)

Comment 1

10 years ago
(In reply to comment #0)
> Note that bug 330590 might be related to this one, though I haven't verified.

Except that the behaviour described in comment #2 out there would still happen with the patch from here applied.
Assignee: nobody → mh+mozilla
Attachment #390442 - Flags: review?(benjamin)

Updated

10 years ago
Attachment #390442 - Flags: review?(dveditz)
Attachment #390442 - Flags: review?(benjamin)
Attachment #390442 - Flags: review-

Comment 2

10 years ago
Comment on attachment 390442 [details] [diff] [review]
patch

I can't accept this without a test; I'm a little confused about the scenario to begin with: the administrator is setting a locked preference via what means? Is it a default pref or a userpref?
(Assignee)

Comment 3

10 years ago
(In reply to comment #2)
> (From update of attachment 390442 [details] [diff] [review])
> I can't accept this without a test; I'm a little confused about the scenario to
> begin with: the administrator is setting a locked preference via what means? Is
> it a default pref or a userpref?

On Debian and Ubuntu (iirc), the administrator would be setting a locked preference in the standard preferences, cf. bug 440908. On other cases, he/she would be setting the locked preference in the general.config.filename pointed file. cf. https://developer.mozilla.org/en/Automatic_Mozilla_Configurator/Locked_config_settings

For a test, I have absolutely no idea how this can be tested through the test framework...
(Assignee)

Comment 4

9 years ago
Created attachment 437278 [details] [diff] [review]
Same patch, with a test

The test is derived from modules/libpref/test/unit/test_bug345529.js so I took the license text (public domain) from there.
Attachment #390442 - Attachment is obsolete: true
Attachment #437278 - Flags: review?(benjamin)

Comment 5

9 years ago
Comment on attachment 437278 [details] [diff] [review]
Same patch, with a test

Woohoo, somebody other than me willing to review libpref stuff.
Attachment #437278 - Flags: review?(benjamin) → review?(dwitte)

Comment 6

9 years ago
Comment on attachment 437278 [details] [diff] [review]
Same patch, with a test

Looks OK. I'm really not sure how this will interact with existing uses, but the change in behavior seems sane.

r=dwitte
Attachment #437278 - Flags: review?(dwitte) → review+
(Assignee)

Comment 7

9 years ago
http://hg.mozilla.org/mozilla-central/rev/b723f72611ea
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
You need to log in before you can comment on or make changes to this bug.