266309 - Windows localized installers don't honor CHARSET/FONTSIZE/FONTNAME for the main dialog.

Status: VERIFIED FIXED

(Firefox :: Installer, defect, P1)

Description

[Benjamin Smedberg]

The Windows localized installers are not honoring the CHARSET/FONTSIZE/FONTNAME
directives from install.ini. This means that in many cases improper fonts are
being used to display the installer dialogs.

The directives can be found here:
http://lxr.mozilla.org/aviarybranch/source/toolkit/locales/en-US/installer/windows/install.it#2

However, they don't affect anything useful. Instead, the fonts are taken from
the setup resource DLL
http://lxr.mozilla.org/aviarybranch/source/toolkit/mozapps/installer/windows/wizard/setuprsc/setuprsc.rc#60

The Japanese team has been hacking around this problem by using a resource
editor to change the font name prior to release. But this obviously sucks for
automated releases.

I *think* that a simple solution involves sending a WM_SETFONT message when we
initialize each dialog, but I'm not sure exactly where this call should be made;
I feel like I'm wandering through a spaghetti of win32 API, so if anyone else
understands this code better than I, please step forward.

An alternative hack is to rebuild setuprsrc.dll for each locale, and use
preprocessor defines to hardcode the font settings into the resource DLL. It
might be easier, but it kinda sucks.

As a sidenote that might also need fixing, the title font is hardcoded to
Trebuchet MS Bold at
http://lxr.mozilla.org/aviarybranch/source/toolkit/mozapps/installer/windows/wizard/setup/dialogs.c#226

This should be configurable as well.

Comment 1

[Bernard Alleysson]

Maybe "MS Shell Dlg" is what you want in the resource file, see
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/intl/nls_2np0.asp

Comment 2

[Chase Phillips]

Ben Goodger says he can't reproduce this bug.

What does an instance of this bug look like?  Could someone who's seeing it
attach a screenshot?

Comment 3

[dynamis (Tomoya ASAI)]

Attachment: Screenshot of ja-JP installer without fix on WinXP.

This is the screenshot taken with l10n nightly (as is) on Windows XP.
"Developer Tools" and "Quality Feedback Agent" labels of checkbox have wrong
code.
# of cource they are translated in japanese.

Comment 4

[dynamis (Tomoya ASAI)]

Attachment: Screenshot of ja-JP installer with fix on WinXP.

Screenshot taken with rc1 (fixed by hand using reshacker) on Windows XP.
Whole text are good including the checkbox labels.

Comment 5

[dynamis (Tomoya ASAI)]

Attachment: Screenshot of ja-JP installer without fix on Win98.

This is the screenshot taken with l10n nightly (as is) on Windows 98.
Whole japanese text will be a black bar...

Comment 6

[dynamis (Tomoya ASAI)]

In the installer, each dialog body use the setting written in setuprsrc.dll.
Installer in ja-JP shows funny text because japanese text are shown with 
non-japanese font.
If we change the value of setuprsrc.dll, text will be shown well
# [FONT 8, "MS Sans Serif"] -> [FONT 10, "MS UI Gothic"]
# "MS UI Gothic" is windows default japanese font for UI.

When the specified font don't have the shape data for the text...
[WinNT/2K/XP] just ignore the specified font and use the correct one.
[Win95/98/ME] use the specified font and text will be funny.
So, whole of installer dialog have funny text in win9x.
It seems that WinXP sometimes cannot select correct font and make funny text
partially like the first screenshot.
# Installer have funny text only in the component selecting dialog on WinXP.

The setting written in install.ini is used only in the preparing messege dialog
which is whown before "Welcome to Mozilla Firefox" dialog.


c.f.
Only the body (main part) of dialog use the setting written in setuprsrc.dll.
Frame part of dialog don't use the setting but use the OS setting UI font.
# in the thrid screenshot, "Welcome to Mozilla Firefox" is whown with japanese
# font on win98.

"MS Shell Dlg" works well in win2K, XP (not tested with win9x) but our peer said
that it seems to have some problem.
# I'm asking about the detail now...

Comment 7

[dynamis (Tomoya ASAI)]

Attachment: Screenshot of the dialog before "Welcome to MozFF" on WinXP

This is the dialog shown at the start of the installer.

This is taken with the ja-JP installer without fix on WinXP.
install.ini setting was:
FONTNAME=MS UI Gothic
FONTSIZE=14
# to check where the value of FONTSIZE is used, I setted large font...

Comment 8

[dynamis (Tomoya ASAI)]

This is the workaround we have been used for ja-JP installer:

  1. with 7-zip utility, extract installer of nightly build.
  2. with reshacker, open the file 'setuprsc.dll'.
     resource hacker we use: http://www.users.on.net/johnson/resourcehacker/
  3. edit only hard-corded font setting.
     editting screenshot: http://skillup.jp/firefox/screenshot/reshacker01.png
  4. save (re-compile) the file.
  5. repeat step 2~4 for UninstallFirefox.exe too (in UninstallFirefox.zip).
  6. re-package installer as written in the mozilla.org document.
     http://www.mozilla.org/projects/firefox/l10n/localize-release.html

Only this is the change of our re-packaged installer.


  ant build script for re-packaging (I use this on WinXP):
http://skillup.jp/firefox/makeinstaller.zip
# this script will do last step only (step 6 above).

  sample of the changed file
http://skillup.jp/firefox/installer/MS_UI_Gothic_9/setuprsc.dll
http://skillup.jp/firefox/installer/MS_UI_Gothic_9/UninstallFirefox.exe
# based on the 2nd round l10n build for 1.0 final.
# - FONT 8, "MS Sans Serif"
# + FONT 9, "MS UI Gothic"

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This creates logistical nightmares when doing releases since we have to deal
with sending modified installers around, waiting for people to make appropriate
edits, etc., so marking blocking-aviary1.1+.

Comment 10

[Bernard Alleysson]

> "MS Shell Dlg" works well in win2K, XP (not tested with win9x) but our peer said
> that it seems to have some problem.
> # I'm asking about the detail now...

Can you elaborate on the problem caused by "MS Shell Dlg" ?

Comment 11

[timeless]

http://msdn.microsoft.com/library/default.asp?url=/library/en-us/intl/nls_4qcn.asp

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.0

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

The same problem of seamonky is bug 286670.

Comment 14

[Daniel Veditz [:dveditz]]

Comment on attachment 177961 [details] [diff] [review]
Patch rv1.0

didn't I review this already?

sr=dveditz

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 177961 [details] [diff] [review]
Patch rv1.0

Japanese l10n team hopes that this patch is checked-in to 1.0.1 aviary branch.

This patch's risk is low. But this problem is important for localization team.

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #14)
> (From update of attachment 177961 [details] [diff] [review] [edit])
> didn't I review this already?
> 
> sr=dveditz
> 

No. Maybe, it is bug 288670 that is seamonkey installer.

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 177961 [details] [diff] [review]
Patch rv1.0

The risk is low. See comment 15.

Comment 18

[Asa Dotzler [:asa]]

Comment on attachment 177961 [details] [diff] [review]
Patch rv1.0

a=asa for checkin to the trunk for 1.1a

Comment 19

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

checked-in to trunk.

Comment 20

[Daniel Veditz [:dveditz]]

We took the seamonkey equivalent for 1.7.7, nominating

Comment 21

[Chase Phillips]

(In reply to comment #20)
> We took the seamonkey equivalent for 1.7.7, nominating

This bug is seeking approval for a much larger patch which adds (or readds?)
general support for other fonts in the installer.  The spot fix we took on the
1.7 branch was a 2-line fix:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=MozillaTinderboxAll&branch=MOZILLA_1_7_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-03-23&maxdate=2005-03-25&cvsroot=%2Fcvsroot.

Since the patch is large, how safe is it?  Is it just readding code that was at
one time on the 1.7 or aviary1.0 branches?

Comment 22

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #21)
> Since the patch is large, how safe is it?  Is it just readding code that was at
> one time on the 1.7 or aviary1.0 branches?

The risk is same as seamonkey's patch. It is low.
Firefox and thunderbird installer ignore font settings on every text label.
Therefore this patch has many lines.
In case of seamonkey, the problem is two labels only. Therefore seamonkey's
patch is two lines only.
In ohter words, number of lines in patch is same as number of having problem labels.

Comment 23

[Benjamin Smedberg]

The only benefit of this patch is that the Japanese installer bits will not need
manual resouce-hack munging. It's only vaguely risky, so it should be up to
Chase IMO, balancing the risk against the pains of manual repackaging.

Comment 24

[Chase Phillips]

Comment on attachment 177961 [details] [diff] [review]
Patch rv1.0

Now that Firefox 1.0.3 has been released and the pressure is off to constantly
have the branch in a can-ship state, we have a great opportunity to try this
patch out there.

You have approval to land this patch there.  If it destabilizes the branch with
no quick fix for that available, you'll have to back the patch out.  Please
help us mobilize build testing of various locales after the patch lands to
ensure that no regressions exist as a result.

Comment 25

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch(checked-in to 1.0.1 aviary)

Comment 26

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Checking in toolkit/mozapps/installer/windows/wizard/setup/dialogs.c;
/cvsroot/mozilla/toolkit/mozapps/installer/windows/wizard/setup/dialogs.c,v  <--
 dialogs.c
new revision: 1.22.2.1.4.4.2.3; previous revision: 1.22.2.1.4.4.2.2
done
connect: Connection refused at /cvsroot/CVSROOT/dolog.pl line 244.

Comment 27

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I found a regression that is bug 291143.
I backed out the patch from 1.0.1 aviary branch.

Comment 28

[Daniel Veditz [:dveditz]]

Existing patch backed out, not blocking a security release

Comment 29

[Chase Phillips]

This patch caused a regression when it landed on the aviary1.0.1 branch.  Has a
fix for that regression been developed?  If so, I'd like for us to consider this
patch again for a 1.0.x release if it means we can land a modified version of
this patch that fixes the regression it caused, as well.

Also, this patch landed on the trunk before the mozilla1.8 branch was created so
it should be on that branch, as well.  Masayuki, can you take a look at the
ja-JP win32 installer that is being built by the l10n Windows build system and
ensure that this is fixed in the post-1.5b1 packages?

Comment 30

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

See bug 291143. We need to change intl resouce.
And we need bug 291909 too.

Comment 31

[Chase Phillips]

(In reply to comment #30)
> See bug 291143. We need to change intl resouce.
> And we need bug 291909 too.

Do you have cycles to create a new patch that contains the original patch for
aviary1.0.1 along with the fixes it needs to land (addressing the regressions
mentioned in bug 291143 and bug 291909)?

I cannot guarantee I can get you an audience in a 1.0.x release post-1.0.7 but
it would help my case if we had a patch in-hand, verified in advance, and ready
to land.

Comment 32

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

> Masayuki, can you take a look at the
> ja-JP win32 installer that is being built by the l10n Windows build system and
> ensure that this is fixed in the post-1.5b1 packages?

I don't know all problems are gone. The all text length is different by language.
I don't know current static text objects have enough size for all language.

Comment 33

[Chase Phillips]

(In reply to comment #32)
> I don't know all problems are gone. The all text length is different by language.
> I don't know current static text objects have enough size for all language.

Since I am recommending to the org we only redistribute ja-JP win32 installers
that are built by our build systems locally, I suggest any problems with the
ja-JP win32 installers be addressed ASAP.  What we have in the mozilla1.8
nightlies is what we're going to ship in the release.

Comment 34

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

O.K. I will create the patch for 1.0.x. But I'm already working some regressions
on 1.8 branch. I need have a time. Please wait few days.

Comment 35

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Oops... I find a bug.

-> REOPEN.

Comment 36

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

The new patch and for 1.0.x patch will come tomorrow.

Comment 37

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch for Trunk/1.8branch

The 3 windows are not modified font settings.
1. The license field.
2. The install path of selecting path dialog.
3. The install path of installing options summary dialog.

Comment 38

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch for 1.0.x branch

This patch marged attachment 177961 [details] [diff] [review](this bug), attachment 181425 [details] [diff] [review](bug 291143),
attachment 181851 [details] [diff] [review](bug 291909) and previous patch(attachment 197267 [details] [diff] [review], this bug).

Comment 39

[Benjamin Smedberg]

Comment on attachment 197268 [details] [diff] [review]
Patch for 1.0.x branch

Chase, this is a pretty large change for the 1.0.x branch, and it will require
changes to all the locales' install.it; I'm not sure it's worth it.

Comment 40

[Benjamin Smedberg]

Comment on attachment 197267 [details] [diff] [review]
Patch for Trunk/1.8branch

oops, I did not mean to mark approval+

Comment 41

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

checked-in to trunk.

Comment 42

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

-> FIXED

Comment 43

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

-> FIXED

Comment 44

[Mike Schroepfer]

Comment on attachment 197267 [details] [diff] [review]
Patch for Trunk/1.8branch

Approved per 9/26 bug triage meeting.

Comment 45

[Scott MacGregor]

Please get the 1.8branch version of the patch checked in on the branch ASAP.

Comment 46

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Oh, sorry. I forgot this check-in. But I will sleep soon. I will can check-in
after 14 hours. If anyone can check-in the patch, please check-in to 1.8branch.

Comment 47

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

checked-in to 1.8 branch.

Comment 48

[Daniel Veditz [:dveditz]]

out of scope for the 1.0 branch, approval denied per drivers. New customers will get 1.5, old customers have already seen this brokenness in earlier 1.0 versions.