114891 - Incorrect charset used to migrate prefs.js entries

Status: VERIFIED FIXED

(Core Graveyard :: Profile: Migration, defect)

Description

[Mike Kaply [:mkaply]]

In nsPrefMigration.cpp, the following prefs.js entries are migrated:

ldap_2.server.*.description
intl.font*.fixed_font
intl.font*.prop_font

The migration code uses kPlatformCharsetSel_FileName to determine with which 
charset the values are encoded in the file prefs.js.

This is wrong. kPlatformCharset_FileName is used for mapping filenames on the 
harddrive. There should be a new kPlatformCharset like kPlatformCharset_prefs_js 
for migrating prefs.js.

This will be a bug on any 4.x browser that uses a different codepage for the 
file system vs. things in prefs.js. For instance, on OS/2, we store all our 
values in prefs.js in ISO-8859-1 on 4.x, but our filesystem is 850.

Comment 1

[scottputterman]

Naoki this sounds like we might want someone on i18n looking at this.

Comment 2

[Mike Kaply [:mkaply]]

*** Bug 122754 has been marked as a duplicate of this bug. ***

Comment 3

[Mike Kaply [:mkaply]]

Attachment: Add new enum for prefs.js charset

this patch adds a new enum for the codepage of the prefs.js file and uses it.

The other (simpler) option is just to use 4xBookmarksFile in pref migration.
That appears to cover this for all platforms.

Comment 4

[nhottanscp]

The new selector is used in XP code nsPrefMigration.cpp, so it has to be
implemented for all platforms.
Mike, did you check GetCharset for other platforms? Are they all implemented to
return a same value for kPlatformCharsetSel_FileName and the new one
kPlatformCharsetSel_4xPrefsJS?

Comment 5

[Mike Kaply [:mkaply]]

Actually, no platforms handle any of the kPlatformCharsetSel values except OS/2.

They take the default charset (mCharset)

See:

http://lxr.mozilla.org/seamonkey/ident?i=GetCharset

The only platform that does anything special with GetCharset at all is OS/2 
which is why this change will work.

Comment 6

[nhottanscp]

Comment on attachment 67294 [details] [diff] [review]
Add new enum for prefs.js charset

r=nhotta

Comment 7

[nhottanscp]

>no platforms handle any of the kPlatformCharsetSel values except OS/2
FYI, I am implementing kPlatformCharsetSel_KeyboardInput, bug 141248

Comment 8

[Christopher Blizzard (:blizzard)]

Comment on attachment 67294 [details] [diff] [review]
Add new enum for prefs.js charset

sr=blizzard

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 67294 [details] [diff] [review]
Add new enum for prefs.js charset

a=[e-mail] for branch checkin.	Make sure it's in the trunk as well

Comment 10

[Mike Kaply [:mkaply]]

Fix checked in.

Comment 11

[K'Trina Medina]

Verified code fix