46487 - GetLocalizedUnicharPref should be moved

Status: VERIFIED FIXED

(Core :: Preferences: Backend, defect, P2)

Description

[Warren Harris]

I think nsPref::GetLocalizedUnicharPref is in the wrong place. Prefs should just 
be a simple "look this attribute up" kind of service. I don't think it should 
load up chrome and resolve urls, etc.

This function is causing a recursive call to initialize the http protocol. Bug 
40159 (nsbeta2+)

This bug was introduced by the checkin to bug 39790 (nsbeta2+). Marking 
nsbeta2.

Comment 1

[Warren Harris]

Removing nsbeta2 nomination since Mitch checked in a work-around for this 
tonight. Bug 46492 is to remove the work-around once other things get fixed. I 
still believe that GetLocalizedUnicharPref should also be (re)moved. See bug 
46491.

Comment 2

[Alec Flett]

where do you suggest we move it?

Comment 3

[Judson Valeski]

I personally think the api makes too many assumptions. I think it should be the 
caller's responsibility to do everything it's doing. Suppose I get one of these 
prefs in the standard way, I suddenly have a useless pref value because I didn't 
have implicit knowledge of the unicodeness of the pref :-/.

Comment 4

[Alec Flett]

We already have other prefs that are "complex"... namely
Get/SetFilePref (uses nsIFile, stores the persistentDescriptor attribute)
Get/SetUnicharPref (converts the string to/from UTF8)

The thing is, there are MANY callers of these routines..My only other thought 
was to provide a pref utility service, but I really think there is valid use for 
having these things in the base API..

One way I've thought about fixing this is to add a flag to the pref itself that 
would indicate what type it was.. this would be stored in prefs.js like
pref_type("some.pref", pref_type_unichar); 

or something.. problem is this would break backwards compatibility with 4.x and 
so I'm not sure what to do about that.

Basically, prefs simply needs to be more complex than simple name/value calls.. 
there are too many prefs consumers that need access to complex prefs...

Comment 5

[tao]

Another way to work around this problem is to examine all the preference
retrieve by GetLocalizedUnicharPref() and see if they really need to be a pref
value? For example, if they are localizable but won't be modified by 
end users nor CCK, then they probably just need to be pulled from property 
files directly.

Comment 6

[Alec Flett]

ok, my plan is to break this out into a third nsIPrefUtils interface.

Comment 7

[Judson Valeski]

this forces embeddors to drag in strres.dll|so . Any chance of reworking this
sooner rather than later?

Comment 8

[Alec Flett]

jud and I talked about this a while back - embedding needs strres.dll after all,
and since prefs doesn't actually load strres until someone calls this method,
this issue isn't quite as hot as we thought it was.

Comment 9

[Brian Nesse (gone)]

Bulk assigning bugs covered by the rewrite of nsPrefs to myself.

Comment 10

[Brian Nesse (gone)]

Bulk updating some fields and accepting

Comment 11

[Brian Nesse (gone)]

GetLocalizedUnicharPref has been rolled into GetComplexValue as part of the work 
being done as part of bug 46863. For that reason, I am marking this as a 
duplicate of 46863. If you feel this is in error, feel free to change it back.


*** This bug has been marked as a duplicate of 46863 ***

Comment 12

[Brian Nesse (gone)]

alecf has requested that I not dupe all these bugs, but rather close them as 
fixed after the work in bug 46863 has landed. Changing again...

Comment 13

[Brian Nesse (gone)]

This was done as a part of the preferences API refactoring. The localized
unichar prefs are now accessible through the ComplexValue methods of PrefBranch.

Comment 14

[sairuh (rarely reading bugmail)]

okay, rubberstamp.