109902 - Localized Pref cannot be unlocalized

Status: RESOLVED WONTFIX

(Core :: Preferences: Backend, defect)

Description

[Ben Bucksch (:BenB)]

Description:
Localized prefs have an odd URL, pointing to the localized properties file, in
their field in the default prefs. I would expect that this is optional, that I
can still set a normal (non-localized) value in the default prefs.
Unfortunately, this doesn't seem to work.

Relevance:
This is important, if
1. I do not want the pref to be localized
2. I want to simply override the pref (isn't that the point of prefs?)

More argumentation:
If this isn't supposed to work, why are these prefs at all?

Reproduction:
1. Set
     pref("general.useragent.locale", "foo");
   instead of
     pref("general.useragent.locale", "chrome://...../...properties");
   in default/pref/all.js2. Open about:
3. Look at the UA string

Actual result:
E.g. Mozilla/5.0 (X11; U; Linux i686; ; Preview) Gecko/20011031 Beonex/0.7

Expected result:
E.g. Mozilla/5.0 (X11; U; Linux i686; foo; Preview) Gecko/20011031 Beonex/0.7


Additional Comments:
To exclude that this is not a problem specific to that pref, here is the
pref-reading code:
       nsCOMPtr<nsIPrefLocalizedString> pls;
       prefs->GetComplexValue(UA_PREF("locale"),
                                NS_GET_IID(nsIPrefLocalizedString),
                                getter_AddRefs(pls));
       if (pls) {
            nsXPIDLString uval;
            pls->ToString(getter_Copies(uval));
nsHttpHandler.cpp, line 1051+

Comment 1

[Brian Nesse (gone)]

The default value of a LocalizedUnicharPref comes from a properties file. It is
not optional, that is simply where it comes from. As far as I know, you are only
supposed to be able to override the value of a LocalizedUnicharPref by setting a
user value to override it, or by replacing the value in the properties file.

cc'ing alecf who probably knows more about the origins of LocalizedUnicharPref
than I do...

Comment 2

[Alec Flett]

you are supposed to be able to store non-chromeURLs in a _users_ prefs.js, but
in order to properly detect these overridden values, we don't allow them in the
default prefs... This is an "as designed" feature - you should distribute a new
.properties file if you want to override this pref.

The point of this kind of pref is one that is overridden with language packs -
i.e. where the default should vary with the installed locale.

This should be marked INVALID.

Comment 3

[Brian Nesse (gone)]

Resolving INVALID as per alecf.

Comment 4

[Ben Bucksch (:BenB)]

First off, this is certainly a valid request. It is in no case invalid. It is at
worst wontfix. REOPENing.

> you should distribute a new
> .properties file if you want to override this pref.

Which usually means that I have to distribute a whole jar file, just to change a
default pref.

> This is an "as designed" feature

Then it is very badly designed.

As I said in the initial description, the point of prefs is that it should be
easy to change them (because they are likely to be changed), *both* for users
and for distributors.

In this case, it is for distributors very hard to change the value:
1. I have to touch lots of file scattered all over the source tree. I lose the
oversight, get merge conflicts and so on.
2. In order for the change to take effect, I have to rebuilt the zip file. This
increases the turn-around cycle seriouly.

All that could be avoided, if you just use the value in the defaults pref file,
if it isn't a URI. I don't know, what you want to allow as properties file URIs,
but the test should be fairly easy.

I might even fix this bug myself.

Comment 5

[Alec Flett]

ok, fix the bug yourself, but this feature is as designed for the goal it was
trying to achieve (providing prefs whose defaults change based on the installed
locale) - you are trying to use this API for some other purpose for which this
API was never intended. 

I welcome patches, but at least get some perspective. I think you will find that
fixing this is not as obvious as you claim. It was done this way for a reason.
you'll see the logic goes (psuedo code)

if (has_user_value(pref))
  return getUnicharPref(pref);
else
  return valueFromStringBundle();

this allows the user value to be a URL, and allows the pref code to have no
special knowledge of chrome urls (because prefs are used in an embedding
context, and embedding may or may not have chrome)
As long as you can meet both these requirements, I'll be satisfied.

Comment 6

[Ben Bucksch (:BenB)]

Alec, thank you.

> you are trying to use this API for some other purpose for which this
> API was never intended. 

Things are not so separated. Some former prefs are converted to localized prefs.
Anyways...

> return valueFromStringBundle();

This function has to read the default prefs as well, to know, which properties
file to look in. What I envisioned was:

getDefaultValue (formerly valueFromStringBundle()):
if has_def_pref(pref)
  value = get_def_pref(pref);
  if isURI(pref)
     return getValueFromStringBundle(bundle = value, name = pref);
  else
     return value;
else 
  err;
>  allows the pref code to have no pecial knowledge of chrome urls (because
> prefs are used in an embedding context, and embedding may or may not have
> chrome)

So, the problem is to properly code the isURI function. Do you have suggestions?

How is that done for embedding? (or is there a webpage descrivbing it?)

Comment 7

[Alec Flett]

uris should ALWAYS be parsed using the nsIURIParser service, or nsIURI.

I don't understand the "how is this done for embedding" comment. my comment was
that due to embedding requirements, the pref code should have no special
knowledge of chrome URIs.

Your solution changes the intended semantics of the interface, and that is part
of my concern: the original interface should return an error if the default pref
does not contain a loadable URL. This prevents people from arbitrarily using
GetComplexPref with nsIPrefLocalizedString when they just want a unichar string
of some kind... in bnesse's recent sweep through prefs, he found a few incidents
where people were already misusing this API..

I'm really leaning back towards this being INVALID, because the intent of the
API is to throw an error when there is no loadable URL, and that's exactly what
you want to change.

If you're willing to change all.js, why can't you just change the .properties
file instead? You're really forcing an entire API to change so that you can
change the .js file instead of the .properties file.

Comment 8

[Ben Bucksch (:BenB)]

> in bnesse's recent sweep through prefs, he found a few incidents
> where people were already misusing this API..

This is easily done in my suggestiong by inserting code between "else" and
"return value;".

> I'm really leaning back towards this being INVALID, because the intent of the
> API is to throw an error when there is no loadable URL

I could even live with a warning on the console during runtime, if that helps you.

> If you're willing to change all.js, why can't you just change the properties
> file instead?

Discussed per email. Much of the point of default prefs is to easily change the
differences between distributions. Changing .properties files is not easy. ALso,
as said, properties files are scattered all over the source tree, while all.js
has it all in one place. Esp. since the tendency of Netscape is to change more
and more normal prefs to localized prefs.

Comment 9

[Benjamin Smedberg]

Localized prefs are really weird, but we're not going to fix them piecemeal, so I'm going to WONTFIX this bug.