To do my dirty deeds, it'd be good if we could drop needing a charset for updater, so that we can actually localizer that UI in all languages. See http://groups.google.com/group/mozilla.dev.apps.firefox/browse_thread/thread/90322a3043958f91/3c6e83c1b059a999?lnk=raot#3c6e83c1b059a999 for reference.

This blocks blocker bug 405871.

Not blocking on this bug for final ship. Would take a safe enough patch if one comes through.

This bug is Windows-specific, right? As far as I could tell, there is nothing non-Unicode in the Windows progress UI implementation. _UNICODE and UNICODE are defined on the command line, so the Unicode APIs are used. This patch only makes sure that the updater.ini file is generated in UTF-16LE on Windows. I tested this by sticking a |ShowProgressUI(); return 0;| in NS_main in updater.cpp, and it can successfully show a Unicode Persian string. With this patch applied on top of the one in bug 305039, we wouldn't need toolkit/locales/en-US/installer/windows/charset.mk any more, so this patch drops that as well.

Comment on attachment 352361 [details] [diff] [review] Patch (v1) With the removal of charset.mk Seamonkey, Thunderbird, and Sunbird will all need to be updated at the same time.

Hmmm, so I guess I will need to pull comm-central anyway and patch them up. :-)

I can do it if it is a pain for you to do so... the patches will need to be reviewed by a reviewer for each app as well and I've got a comm-central repo. I'm going to focus on bug 305039 first anyways.

Yeah, that would be great if you can handle it. Thanks!

Requesting blocking since all of the required patches are attached to this bug and I'd like to get pre-approval to land this along with the Unicode installer patches

Comment on attachment 355680 [details] [diff] [review] comm-central patch btw: this patch requires the utf16-le-bom.bin from the patch in bug 305039

Comment on attachment 352361 [details] [diff] [review] Patch (v1) The locale is stored in the updater.ini and it appears that nsIINIParser.getString is failing when the file is UTF-16LE Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIINIParser.getString]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: file:///C:/moz/tmp-mozilla-central/ff-opt-static/dist/bin/components/nsUpdateService.js :: getLocale :: line 549" data: no] >diff --git a/browser/locales/Makefile.in b/browser/locales/Makefile.in >--- a/browser/locales/Makefile.in >+++ b/browser/locales/Makefile.in >... >@@ -349,7 +348,8 @@ ifeq ($(OS_ARCH),WINNT) > ifeq ($(OS_ARCH),WINNT) > cat $< $(srcdir)/updater_append.ini $(srcdir)/../installer/windows/nsis/updater_append.ini | \ > sed -e "s/%AB_CD%/$(AB_CD)/" | \ >- iconv -f UTF-8 -t $(WIN_INSTALLER_CHARSET) > $(FINAL_TARGET)/updater.ini >+ iconv -f UTF-8 -t UTF-16LE | \ >+ cat $(srcdir)/../../toolkit/mozapps/installer/windows/nsis/utf16-le-bom.bin - > $(FINAL_TARGET)/updater.ini Please use the following since the rest of this file does $(topsrcdir)/toolkit/mozapps/installer/windows/nsis/

Comment on attachment 355680 [details] [diff] [review] comm-central patch I'm going to hold off on the comm-central reviews until the ini parser issue is resolved.

One way to resolve this... I believe it should be possible to do something similar to what is done for Linux and Mac OS X for reading the updater UI strings and then use MultiByteToWideChar to convert the string from UTF-8 to UTF-16LE.

Hey Ehsan, what do you think of this alternate approach?

Comment on attachment 355735 [details] [diff] [review] patch - alternate approach rev2 Looks very good. Thanks for the patch!

Comment on attachment 355735 [details] [diff] [review] patch - alternate approach rev2 a191=beltzner

Note: I am going to leave charset.mk and the associated include statements in the Makefiles when I check this in since they are still used by the installer. I'll remove them when I check in bug 305039

Axel, it looks like the following locales don't have a translated updater.ini and should be able to translate the updater.ini after this bug is fixed ml as gu-IN si bn-IN hi-IN ka mr kn pa-IN te

Pushed to mozilla-central (everything but the removal of the en-US charset.mk which I will remove from mozilla-central in bug 305039). http://hg.mozilla.org/mozilla-central/rev/177e11abd9d9 I'll push to mozilla-1.9.1 and comm-central after one cycle. I won't be removing the en-US charset.mk since that would be a string change.

Pushed to mozilla-1.9.1 http://hg.mozilla.org/releases/mozilla-1.9.1/rev/881fdb2a350f Pushed to comm-central http://hg.mozilla.org/comm-central/rev/05d7c1308855

Verified fix on Mozilla/5.0 (Windows; U; Windows NT 6.1; zh-CN; rv:1.9.1b3pre) Gecko/20090211 Shiretoko/3.1b3pre, Mozilla/5.0 (Windows; U; Windows NT 6.1; pa-IN; rv:1.9.1b3pre) Gecko/20090211 Shiretoko/3.1b3pre. On m-c, it looks like the fix is there also. However, firefox fails to startup after installing (for unicode builds it seems). I'll track down a regression and see if this bug is related, and comment here later.

(In reply to comment #23) >... > On m-c, it looks like the fix is there also. However, firefox fails to startup > after installing (for unicode builds it seems). I'll track down a regression > and see if this bug is related, and comment here later. When you say "for unicode builds it seems" what specifically do you mean? All builds use the same code and this will help understand what might be failing.

still diagnosing the problem. but here's what i have so far: on 1.9.1branch-l10n --- ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-1.9.1/: locales like zh-CN, ja, pa-IN - installs and opens firefox in their language correctly with a new profile. CHECK. on Mozilla-central-l10n --- ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-central-l10n/: locales like zh-CN and ja will install in their language, but will not fire up firefox. instead, you are thrown with this attached error. FAIL. - However, if you opened up fr build, firefox will open correctly. This could be an entirely different bug than the fix here, but i'm still working on a regression range.

Hey Tony, those types of errors are almost always due to the locale files themselves and is definitely not due to this fix.

yeah thought so. okay i'll track this issue in a new bug. thanks rob.

I don't think this is worth filing a new bug, since it's caused by untranslated strings. You can see the dashboard <http://l10n.mozilla.org/dashboard/> for any locale to see if it's green before testing a build.

Yeah, we had a separate email thread and someone pointed me to the dashboard page. Looks like a known issue for trunk, so the subject is now moot. Thanks for clarifying.