352365 - Fix some unicode/character encoding/string issues in IE profile migrator

Status: RESOLVED FIXED

(Firefox :: Migration, defect)

Description

[Jungshik Shin]

There are a few (recently introduced) unicode/character encoding issues in IE profile migrator. I'll fix them together here.

Comment 1

[Jungshik Shin]

Attachment: patch

uploading it here just in case I lose it (again)

Comment 2

[Jungshik Shin]

Comment on attachment 238969 [details] [diff] [review]
patch 

asking for r

I've tested the patch.

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 238969 [details] [diff] [review]
patch 

+      aPrefs->SetComplexValue(aPrefKeyName, NS_GET_IID(nsISupportsString), val);

sucks that you have to do that :/ the code you have commented here works, couldn't you use it?
+    //aPrefs->SetCharPref(aPrefKeyName, NS_ConvertUTF16toUTF8(regValue).get());
I guess it's not strictly speaking correct since the IDL uses "string"...

+  const char   *source = NS_LossyConvertUTF16toASCII(lang).get(),

you can't do that. the ASCII string will be destructed on the next line. you need to keep the string object around.

+  //aPrefs->SetCharPref(prefName.get(), NS_ConvertUTF16toUTF8(font).get());

same commment as above

+regEntry gRegEntries[] = {

hm, seems to me like that should be const, or is that not possible?

+            realm.AppendLiteral(")");

realm.Append(')');

+                              sizeof(gRegEntries)/sizeof(regEntry);

NS_ARRAY_LENGTH?

+  PRUnichar username[sUsernameLengthLimit+2];
+  ::GetEnvironmentVariableW(L"USERNAME", username, sizeof(username));

MSDN says this is the size of the buffer in characters; therefore, using sizeof as here is wrong. you need to divide by sizeof(PRUnichar).

+  username[sizeof(username)-2] = L'\0';
+  wcscat(username, L"@");
+  int usernameLength = wcslen(username);

seems like an odd way to write this... but, not your fault :)

Comment 4

[Jungshik Shin]

Attachment: patch update (addressing biesi's comments)

I used SetCharPref instead of SetComplexValue with nsISupportsString while noting that I'm not supposed to do that according to the IDL. We've done that before in other places.

Comment 5

[Jungshik Shin]

Comment on attachment 245616 [details] [diff] [review]
patch update (addressing biesi's comments)

it's bit-rotten... will upload a new patch.

Comment 6

[Jungshik Shin]

Attachment: patch updated to the trunk

updated to the trunk and tested. 
Three warnings from nsIWindowsRegKey will be taken care of in bug 353802

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

browser/components/migration/src/nsIEProfileMigrator.cpp
+    // XXX Strictly speaking, we have to use SetComplexValue with 
+    // nsISupportsString, but we all know that strings in pref. files
+    // are in UTF-8.
+    aPrefs->SetCharPref(aPrefKeyName, NS_ConvertUTF16toUTF8(regValue).get());

Remove the XXX... there's not really anything to fix here.

+               *sourceEnd = source + lang.Length();

I'd feel better if this was langCstr.Length() (even though this is the same here)

+  // XXX Strictly speaking, we have to use SetComplexValue with 

(as above)

Comment 8

[Jungshik Shin]

Comment on attachment 245648 [details] [diff] [review]
patch updated to the trunk

asking for sr.


review comments were addressed in my tree :
s/lang/langCstr/ was done. 

'XXX ...' was removed.

Comment 9

[Adam Guthrie]

Looks like this was checked in on 2006-11-20 18:03. ->FIXED