33668 - PLS move preferences (*.js) files to chrome/.../locale

Status: VERIFIED FIXED

(SeaMonkey :: Preferences, defect, P3)

Description

[tao]

As of now all preferences (*.js) files reside in defaults/prefs/.
These preferences/config files do contain localizable resources 
such as 

  pref("general.useragent.locale", "en-US");     -->"ja-JP" for Ja build 
  pref("intl.accept_languages", "en");  -->"ja-JP"    -->what else? 
  intl.charset.default --> ? 
  pref("intl.charset_menu.static","iso-2022-jp, shift_jis,euc-jp,iso-8859-1");
  pref("intl.charset.detector","japsm"); 
  pref("intl.charset_menu.cache", ""); ->? 
  pref("mailnews.send_default_charset","iso-2022-jp");  -- send charset for
          Japanese 
  pref("mailnews.view_default_charset","iso-2022-jp");  -- msg view chraset
          default for Japanese 
  intl.font_charset 
  intl.font_spec_list 
  font.name.serif.x-western -? 
  font.size.variable.ja- ? 
  font.size.fixed.x-unicode -



Thanks

Comment 1

[Fergus Sullivan]

Adding self to cclist.

Comment 2

[tao]

Hi, folks:


Unless there is an objection, please resolve this bug in M16 so we can get our
beta2 radar clear and focus on other tasks.


Thanks.

Comment 3

[Alec Flett]

so I've been thinking about this. I think the basic problem is that these are
PREFERENCES. They are not localizeable strings, and any place that we're putting
localized strings in prefs, we're doing the wrong thing. 

we should be using string bundles in those places. Prefs are not a place to
store user-visible strings, it's just a mis-use of preferences. Preferences do
not change with locales, and the default value of the prefs (such as all.js) is
dependant on the installation and profile, not the current locale.

If we're putting special language-specific URLs (like making www.netscape.jp the
default japanese homepage) then these still belong in string bundles.... 

Comment 4

[tao]

Hi,Alec:


My concern is in creating locale-sensitive profile in which the default
prefs values derived from the *.js such as all.js in defaults/pref/.

I am not targeting at switching preferences values per-locale as you suggested.
Let me know if I can clarify it further.



Thanks

Comment 5

[Alec Flett]

this is what I am referring to, and I think it's wrong.

I think any place where we are storing locale-sensitive data in the default
prefs files like all.js is simply incorrect, and we should fix the code to use
string bundles instead.

Comment 6

[tao]

While preference values are modifiable in runtime, resources managed by 
stringbundles are not.

I propose leaving all.js and others in defaults/pref but externalizing 
localizable prefs into a third file, sas "intl.js", and place it under 
chrome/pref/locale/en-US.


How does this sound?

Comment 7

[Daniel Veditz [:dveditz]]

So how do we handle any 3rd party app with "localized" prefs? They aren't 
Netscape so they can't add their prefs to our intl.js

Or what if AIM start slipping so we decide to go ahead and ship it later -- 
same problem.

So now you have to invent a mechanism for components to register themselves as 
a "default pref provider" just in case some of those are localized. We could 
probably use categories for that, but it seems way overkill.

I agree completely with Alec, localized prefs are evil: put such data in string 
bundles. The code can be set to determine if a pref exists or not. If it does 
then use it, if not get the string bundle value. If the user ever explicitly 
changes it then save the pref.

Comment 8

[Alec Flett]

I'd like to know what localizable string you need to have set at runtime. I have
this feeling that 90% of time it's just a misuse of prefs, changing a value that
will never be changed by the user.

It seems like a more appropriate solution would be:
- put a blank string in prefs.
- when the code who calls GetUnicharPref() gets a blank string, then they pull
the string out of a string bundle.

Tao, can you list the preferences here that you'd like to make localizable? I'll
bet we can find better solutions for most of them.

Comment 9

[tao]

Alec:


Look at the top of the description section of this bug for what prefs values
are being localized.


I'll reply to Dan's comments later on.


Thanks

Comment 10

[Alec Flett]

it really looks like the ONLY one that might be tricky is the first one. The
rest should be derivable from string bundles... i.e. all of those values should
exist in a string bundle in chrome://global/locale/locale-defaults.bundle

And then it seems like you could derive general.useragent.locale at the first
startup.

Comment 11

[tao]

> So how do we handle any 3rd party app with "localized" prefs? They aren't 
> Netscape so they can't add their prefs to our intl.js

How does the current scheme support 3rd party app with "localizable" prefs?

> Or what if AIM start slipping so we decide to go ahead and ship it later -- 
> same problem.

How does the current scheme resolve this issue?

> So now you have to invent a mechanism for components to register themselves as 
> a "default pref provider" just in case some of those are localized. 

I am not following you here... The idea of moving these localizable prefs to
chrome/ is to take the advantage of the "chrome" type protocol so that we
can create locale-sensitve profiles.

> We could 
> probably use categories for that, but it seems way overkill.

What's the "category" here? Is it an xpinstaller or smartupdate term? What
does it mean?

>I agree completely with Alec, localized prefs are evil: put such data in string 
>bundles. The code can be set to determine if a pref exists or not. If it does 
>then use it, if not get the string bundle value. If the user ever explicitly 
>changes it then save the pref.

As long as we can move those "prefs" into locale-sensitve place and package
them by lcoale, I have no objection. It's evil to see locale-sensitve resources
being packaged into a locale-neutral file (jar, xpi) and thus increase
un-necessary complexity to the localization process, packaging, download,
installation, smart update, etc...

It seems to me externalizing them into a chrome/pref/locale/en-US/intl.js
is the simplest way to achieve the goal.

Since Alec is cleaning up the prefs, this might be a good time to address
this issue, too.


my 2c

Comment 12

[tao]

Hi, Alec/Dan:


Your suggestion seems to solve the problem here. If Matt also agrees, that
settles it.

There are some other values in the prefs that used to be localized in 4.x
time, but obsolete in 6. Why don't remove them at the same time? I'll
get my group to provide the list.



Thanks.

Comment 13

[tao]

These bugs are prerequisites of our beta2 localization plan. Add "beta2" 
to keyword.

Comment 14

[rchen]

I would like to point out more strings which need localization in the file 
mailnews.js:

pref("ldap_1.directory1.description", "Personal Address Book");
pref("ldap_1.directory6.searchBase", "c=US");
pref("ldap_1.directory6.attributes.telephoneNumber", "Phone Number:homephone");
pref("ldap_1.directory6.attributes.street", "State:st");
pref("ldap_2.servers.pab.description",  "Personal Address Book");
pref("ldap_2.servers.history.description",	"Collected Addresses");
pref("ldap_2.servers.netcenter.description",	"Netcenter Member Directory");
pref("ldap_2.servers.netcenter.customDisplayUrl",		
"http://dirsearch.netscape.com/cgi-bin/member_lookup.cgi?dn=%s");

Comment 15

[tao]

Per discussion w/ alecf, reassign to alecf and mark it M16.

Alec proposes to add a new type of pref:

 1. In defaults/prefs/*.js, change the pref entry, say 
   
    pref("mailnews.send_default_charset","iso-2022-jp"); 
    ->
    pref("mailnews.send_default_charset",
          "chrome://messenger/locale/xxx.properties");

 2. In "chrome://messenger/locale/xxx.properties", insert a line

    mailnews.send_default_charset=iso-2022-jp

 3. In runtime, the pref module will retrieve the pref value via the
    stringBundle.

 4. The pref module will be smart enough to detect values of chrome URL type
    and convert the data types (int, char, etc..) automatically so the 
    application module do not neeed to change the calling sequence.

 5. The stringBundle caching mechanism needs to be modified to use the
    converted URL (from chrome URL) as the hash key instead of the combination
    of language, country, and chrome URL. I will log a new bug about this.

Many thanks to Alec's great idea.

Comment 16

[tao]

The bug for changing stringBundle caching is 38113.

Comment 17

[Alec Flett]

Actually, I wasn't going to make the pref code smart enough to detect the type,
but I could. For now I'll just do strings, which will be only unicode. The new
function will be (in IDL)
  wstring getLocalizedUnicharPref(in string value);

(and yes, I know it's inconsistent with CopyUnicharPref(), but I'm going to
change CopyUnicharPref() and CopyCharPref to getCharPref and getUnicharPref at
some point)

Comment 18

[Daniel Veditz [:dveditz]]

I just wanted to clarify something between Tao's and Alec's last comments: you 
aren't planning to always look up resourced data for stuff that looks like a 
chrome: URL, right?  Because chrome: URLs could be perfectly legitimate things 
to want to have in a pref.

It looks like Alec was proposing the StringBundle resource would be retrieved 
only if the client explicitly requested that the pref be treated that way, just 
wanted to make sure I understood that correctly.

Comment 19

[tao]

Hi, Dan:

Yes, that's the way I understood it. 


Hi, Alec:

Regarding your previous comment, I think we are OK since most of the localizable
prefs in Seamonkey are char array.




Thanks

Comment 20

[Alec Flett]

right, this is how it will work (with some tricks to make sure we aren't
actually allocating too many strings, and so forth)

nsPref::GetLocalizedUnicharPref(const char* prefname, PRUnichar**value)
{
  CopyCharPref(prefname, bundlename);
  bundle = bundleService->GetBundle(bundlename);
  bundle->GetStringFromName(bundle, value);
}

Comment 21

[Alec Flett]

just an update on what I'm about to checkin:
I also realized that if someone calls SetCharPref or SetUnicharPref on these
special prefs, such as the user's homepage, that we want that string to override
the string that's in the stringbundle... so basically what I do is if the pref
has been set in the user's profile, then it overrides any stringbundle string...
otherwise it just does what it says above.

Comment 22

[Alec Flett]

ok, fix is in.

Comment 23

[tao]

well done, thanks!


Guess now we need to move those prefs value to their respective property files.
Shall we create 

   communicator/locale/pref/
             pref-all.properties
             pref-mailnews.properties
             pref-all-ns.properties
             pref-mailnews-ns.properties
   aim/locale/
             pref-aim.properties

Or we just stash them in existing properties of each top level components?


I prefer the latter to avoid loading of extra files.

Comments?

Comment 24

[Alec Flett]

I'd vote for seperate properties files, and call them pref-defaults.properties
(just to be clear that they are not prefwindow-related pref strings, but rather
the pref defaults)

Comment 25

[tao]

Are you suggesting just add one new file pref-defaults.properties or 
one (pref-defaults-xxx.properties) for each existing *.js file?

Comment 26

[Alec Flett]

sorry, multiple, but with the same name
chrome://messenger/locale/pref-defaults.properties
chrome://navigator/locale/pref-defaults.properties
and so forth.

Comment 27

[tao]

Hmmm.. I don't like the idea of same name but different directories. 
In the future, we might move them to same directory. In fact, mailnews.js
and mailnews-ns.js might have name collision in commercial build...

But, multiple files it is.

Comment 28

[Alec Flett]

Well, we can name them whatever, but we already have multiple properties files 
with the same names across different chrome directories (prefs.properties is one 
example, which are strings for the pref window) ... that's kind of the point of 
having seperate chrome directories for each component - so we don't have to 
worry about collision.

And we designed the mailnews.js/ns-mailnews.js system on purpose, there is no 
collision there.

Comment 29

[tao]

What I meant about mailnews.js and mailnews-ns.js is that pref property files
for both would become

  chrome://messenger/locale/pref-defaults.properties

Unless we separate them as

  chrome://messenger/locale/pref-defaults.properties
  chrome://messenger/locale/pref-defaults-ns.properties

But, as I said earlier, we migth put pref-defaults.properties for all UI 
components into one single package such as

  chrome://regional/locale/pref-defaults-nav.properties
  chrome://regional/locale/pref-defaults-mailnews.properties
  chrome://regional/locale/pref-defaults-editor.properties

So that URLs are separated from the rest of localizable resources...


Anyway, it depends on how much we can do in PR2..

BTW, I am planning to merge config.js into all.js since only one pref in 
config.js would survive if we remove those obsolete ones. Is this on anyone's
plate? In moving URLs to the above pref-defaults, we can clean the pref 
files at all once.

Comments/objections/taker?

Comment 30

[Daniel Veditz [:dveditz]]

A fair number of these "localizable prefs" appear to be the kind of thing we 
need to open our first window -- do we really want to be opening yet more 
separate files at startup? Especially small properties files which might 
contribute to bumping bigger files out of the stringbundle cache?

Your proposal lets the default pref name which stringbundle to use -- 
personally I'd stick it into stringbundles we know we will have to open 
already. Just a thought.

Comment 31

[Alec Flett]

Dan, you've won me over, I didn't even think about all that. :)
I now also think they should go into existing files.

Comment 32

[leger]

Putting on [nsbeta2+] radar for beta2 fix.

Comment 33

[Alec Flett]

I tell ya, this bug has gotten filled up with too much stuff. I've done the work 
to add the prefs API, so let's mark this fixed and open a new bug (or bugs?) 
against moving the default prefs into properties files.

Comment 34

[Alec Flett]

oops, actually marking fixed this time.

Comment 35

[sairuh (rarely reading bugmail)]

->teruko to verify.

Comment 36

[msanz]

changing QA contact to Tao

Comment 37

[tao]

Yes, I have been using the new API. It works fine for me. Thanks to Alecf.