139248 - Issues with the font download dialog in mailnews

Status: VERIFIED FIXED

(MailNews Core :: Internationalization, defect)

Description

[Scott MacGregor]

Using Build ID: 2002042206, win2k.

I have some concerns about the new font download dialog that is now coming up on
windows when we encounter content with a font the user does not have. This
dialog has signficant UE impact for the usability of our mail client. I don't
believe mailnews UE was consulted before it landed so I'm going to cc Jennifer
on this bug too. Frank, do you have a spec you can point us to for how the font
dialog should behave? Can you post the spec? It would give us a good starting
point for discussion.

I'm going to list in this bug a set of issues / comments about the dialog. Let's
have some discussion about them and then I can spin off separate bugs for each
issue we think we need to pursue. Please see the forthcoming attached screen
shot to see what the dialog looks like today.

1) When the dialog comes up, we get a windows title bar with a content area of
zero height, this flashes on the screen then gets replaced by the font dialog.
None of our other xul dialogs do this, I wonder if we are doing something wrong
that's causing this smaller thing to flash onto the screen. This issue is also
partially talked about in Bug #128368

2) Circumstances in which we see the dialog
I suspect this dialog makes a lot of sense for web pages when the user chooses
to view a web page which we don't have a font for. I think it makes sense to
bring up a dialog showing them how to get the font to view that page. I don't
think it makes as much sense for mailnews. A mailnews user gets unsolicated
e-mail that they don't have control over. Clicking the message to delete it
should not cause our application to bring up this dialog. I get tons of I18N
spam with international characters in them that I don't want to read. Yet trying
to delete them them brings up this dialog. I think the model for mail (user
isn't in control of the content) is very different then the model for web
browsing (user controls content). As such, I'd really like to see this dialog
disabled inside of mailnews. 

To complicate matters for e-mail, we get *many* false positives for seeing this
dialog. ASCII messages are frequently sent and received with non ascii charsets
listed in the email headers. This leads to the user seeing this dialog when the
message just contains ascii text. Now just to read some ascii messages, I have
to go through this dialog. 

Assuming I'm in the minority on that decision, here are some concerns I have
about the dialog itself:

3) No window title in the dialog telling you what this dialog is all about. All
our dialogs have window titles. I'm not sure why this one doesn't.

4) The dialog isn't sized correctly. There's a bunch of white space at the
bottom of the dialog. Are we using a fixed height here instead of letting the
dialog intrinsicly resize the width and height based on the content?

5) The cancel button is not in the correct place for a standard dialog. Are we
using the cancel button from the dialog overlay file? That'll make sure your
cancel button is positioned correctly.

6) Why do we have a cancel button at all? I'm not choosing to back out of an
action. The dialog simply provides information telling me to go do something.
Seems like an OK button should be used to dismiss the dialog not a cancel button.

7) Why do we tell the user how to go download the new font?  Instead of a cancel
button we should say "Do you want us to take you to the system font installer?"
If the user hits okay, then launch the windows: control panel / regional
settings / language panel. Since this is a windows only dialog, it's easy to use
the windows shell APIs to automatically launch the windows language panel. We
should do this for the user and not just tell them how to do it in a dialog. I
think this is a pretty important point. Showing an informative alert which
doesn't actually do anything for you every time you click on a message with a
foreign character set seems non helpful. 

8) If we are going to show this dialog every time I click on an email that has a
font I don't have, then we need a check box pref for "never show this dialog
again". We use this technique across the product for these kinds of dialogs
(Ctrl-enter to send, submitting insecure data, etc). This dialog shouldn't be
treated any differently. I should be able to turn it off so I don't see it ever
again.

I think that sums up the list I put together. Feel free to chime in with other
suggestions.

Comment 1

[Scott MacGregor]

Attachment: screen shot of the dialog today

Comment 2

[Scott MacGregor]

nominating. The results of our discussions may have some impact on the beta in
order to provide a good user experience for e-mail.

Comment 3

[Scott MacGregor]

Forgot a comment:

9) the dialog times out and goes away after a while. i.e. if I let it come up
and don't dismiss it myself, I suddenly see it disappear on it's own.

Comment 4

[Scott MacGregor]

Whoa! I just discovered that this window isn't even a xul dialog. It's an html
web page hosted on:

http://www.mozilla.org/projects/intl/fonts/win/en/package_zh-CN.html

which is made to "look" like a dialog. =(

This really needs to be a skinnable XUL dialog if it's going to be coming up in
mail so we can address the issues I raised above. I'm curious why we are using
an html window for this right now? Seems kinda weird.

Comment 5

[Frank Tang]

It is not a html, it is a xul.
http://www.mozilla.org/projects/intl/fonts/win/en/package_zh-CN.xul


*** This bug has been marked as a duplicate of 128368 ***

Comment 6

[Scott MacGregor]

Re-opening. Frank, I'm not sure I understand why you think this is a dup of
128368. If you read my bug description here, I have a set of 10 separate issues
pertaining to the font dialog which I wanted to discuss in this bug. Only one of
them is partially related to 128368 and I referenced it in that comment. Please
read through my initial list of comments. Thanks!

Comment 7

[Katsuhiko Momoi]

I placed the URL for the Font Downdload engineering specs
in the URL field. This is more of an engineering spec. There should
be a section or a separate document discussing various aspects of User 
Exprience part.

Comment 8

[Frank Tang]

>I don't believe mailnews UE was consulted before it landed so I'm going to cc
Jennifer on this bug too.
What happen is the following:
a. we have font download for the Gecko layout engines in N6.2. The font download
is generic for Gecko layout engine instead of browser. Therefore , the font
download show up when ever the Gecko layout engine detect that a
Japanese/Chinese/Korean page is shown, regardless it is from browser, mail or
composer. The UI specification is documented in
http://client/international/html/6_5/fontdownload_req.htm . sorry, the document
is still netscape internal)

b. in N6.2 time, there are a mail bug which block the language information pass
to the Gecko layout engine. Therefore the mail won't bring up the font download
dialogbox. [e-mail] file a bug against that when she test the font
download feature, and we decide to later that bug (see 
http://bugscape.mcom.com/show_bug.cgi?id=5393
>------- Additional Comments From [e-mail] 2001-05-25 16:29:21 >----
>
>Roy and I just tried the build with the property file on a system w/o >korean 
>fonts. It works for browser, but not for mail. For example, if you >have a korean 
>mail in your mailbox, when mail window comes up, it doesn't pop up >the font 
>download dialog window. And after it fails to pop up, and then you >switch to 
>browser to view a korean page, it doesn't pop up for browser either.
>
>BTW, it should come up once you open the mail folder before you >actually select 
>the mail, since the thread pane already shows the subject of the >korean mail.
.....
>------- Additional Comments From [e-mail] 2001-05-25 >17:01:06 ----
>
>1. the specification say nothing about mail. so it should not >consider a bug.
>(Do we want to ask user download a font while they got spam mail by >those 
>Chinese web sites?)

>------- Additional Comments From [e-mail] 2001-05-25 17:21:30 >----
>
>Filed bug 82815 for mail.

also
http://bugzilla.mozilla.org/show_bug.cgi?id=82815
>Font download dialog has been implemented for browser, we need this >for Mail as 
>well. Besically when the user has a mail in his/her mailbox which >needs extra 
>fonts to view the mail, the font download dialog should be poped up. 

>------- Additional Comment #3 From Jaime Rodriguez, Jr. 2001-05-29 >11:31 -------
>
>Marking as nsbeta1 and requesting for M0.9.2.

>------- Additional Comment #9 From Frank Tang 2001-06-28 15:21 >-------
>
>future

>Can you post the spec?
see link above


>1) When the dialog comes up, we get a windows title bar with
> a content area of zero height, this flashes on the screen then 
>gets replaced by the font dialog.
look at the code in
http://lxr.mozilla.org/seamonkey/source/xpfe/components/intl/nsFontPackageHandler.cpp
anything wrong with the following ?

 88   rv = windowWatch->OpenWindow(nsnull, absUrl, 
 89               "_blank",
 90               "chrome,centerscreen,titlebar,resizable", 
 91               nsnull, 
 92               getter_AddRefs(dialog));

>A mailnews user gets unsolicated e-mail that they don't have control over. 
I agree with you. See my 2001-05-25 comment in bugscape bug 5393
>Clicking the message to delete it should not cause our application to bring up
this dialog.
I don't think it will bring up the font download box, if you right click on
thread pane and delete it. 

>Yet trying to delete them them brings up this dialog.
I got similar thing from those X rates spam mail. they open window by using
JavaScript when you view them. This is not worst than that one, right?

Everything have two sides, when you and me don't want to see the content for
korean spam mail. Some Japanese business man may want to see the Japanese email
their relative send them with their US build. 

>ASCII messages are frequently sent and received with non 
>ascii charsets listed in the email headers.
I do not agree with this one. I don't think average users received
non ASCII charset listed in the email header with ASCII only message.

>3) No window title in the dialog telling you what this dialog is all >about.
All our dialogs have window titles. I'm not sure why this one >doesn't.

Not sure what happen there. Must be a bug. We do specify the title of the window
in the xul . We also specifiy titlebar in the OpenWindow call. 

>4) The dialog isn't sized correctly. There's a bunch of white space >at the
>bottom of the dialog. Are we using a fixed height here instead of >letting the
>dialog intrinsicly resize the width and height based on the content?
read http://client/international/html/6_5/fontdownload_req.htm
There are a lot of space in the screenshot of that specification.


>5) The cancel button is not in the correct place for a 
>standard dialog. Are we using the cancel button from the dialog 
>overlay file? That'll make sure your cancel button is positioned >correctly.

No.

First of all, there are two different look and feel will show up now. Depend on
you are running on Win2K/WinXP or other Windows. For other windows, it will give
you a button to click to download a font package to install. for win2k/winxp it
won't since that package won't work on win2k/winxp. It will ask the user to open
"Control Panel:Regional Setting" to install the font package

>6) Why do we have a cancel button at all? I'm not choosing to back 
>out of an action. The dialog simply provides information telling me 
>to go do something. Seems like an OK button should be used to dismiss
> the dialog not a cancel button.
Make sense, will change

>7) Why do we tell the user how to go download the new font? 
> Instead of a cancel button we should say "Do you want us to take
> you to the system font installer?" If the user hits okay, then 
>launch the windows: control panel / regional settings / language >panel. Since
this is a windows only dialog, it's easy to use the 
>windows shell APIs to automatically launch the windows language 
>panel.

As I describe above. The font package currently we point to can only be used by
uses on Win95/98/ME/NT4 but not WinXP nor Win2K. We found that out after we ship
N6.2 so we change the sever side javascript to close the window if they are
running on WinXP and Win2K. Somehow people still don't like that "show a pop up
and close it" effect. So I change the server side javascript code to tell winxp
win2k users how to install the font from control panel . Once they install it,
they won't see the font download dialog box again. 

Is that easy for a xul stored on the server side to launch the windows: control
panel / regional settings / language >panel? I don't think so. 


>8) If we are going to show this dialog every time I click on an email >that has a
>font I don't have, then we need a check box pref for "never show this >dialog
>again". 
No, it won't. it will only show you one per langauge group per session. So if
you launch and don't exit, it will show at most 4 times, one time when it first
hit a Korean page, one time when it first hit the Japanese page, one one time
when it first hit the traditional chinese page, and one time when it first hit
the simplified chinese page. If you quit, then it will give the users a 
chance to do it again. 

I guess, I can add some cookie code in the server side to do that. But that
probably will still have the flashing effect since I need to download the server
side xul / js to run that cookie code. 

>We use this technique across the product for these kinds of dialogs
not really, we don't have such thing for the netscape.com pop up ad, right ? :)
Neither for the blank mail page we load before user select the message in the
mail/news window, right :) Neither for the Print dialog box which the only thing
I will do is to click the "OK" 




 

Comment 9

[jglick]

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

Comment 10

[scottputterman]

I think at the least mscott's point #8 needs to be fixed for rtm.  Getting asked
about this once per session is too much for people who are hitting this because
of spam.  As Peter mention's in a recently duped bug, this makes Spam that much
worse.  Adding adt2 which I hope will be followed by an nsbeta1+.

Comment 11

[Peter Trudelle]

IMO, this could significantly affect adoption of Mail for people trying out the
beta. Since it was not part of any spec approved by the UI designer for Mail,
why shouldn't it just be backed out until it passes review? 

Comment 12

[Frank Tang]

peter-
read http://client/international/html/6_5/fontdownload_req.htm 
it is a spec in 6.2 for gecko engine. not for browesr nor mail.

>Since it was not part of any spec approved by the UI designer
That spec was approved by ue team. Ask jaimejr. He run that spec thorugh last year. 

It does not affect mail because some other bug blocking that happen. After that
bug got fixed this feature show up. see my comment 

Comment 13

[Frank Tang]

>8) If we are going to show this dialog every time I click on an email that has 
>a font I don't have, then we need a check box pref for "never show this 
>dialog again".
Agree, reassign to yokoyama and mark it as nsbeta1+. Pleaset add that pref in
per language group based in MachV only code (in nsFontPackageHandler) which
won't impact embeding customer's font package handler. Also make sure use
differen pref for japanese, Korena, simplified chinese and traditional chinese. 

mscott- any sample code about the "never show" dialog box we can copy ?

Comment 14

[Scott MacGregor]

was it approved by the mail UE team? It's great that you have a spec against the
gecko engine but by adding it to gecko we added some very non pleasant side
effects to mail which I don't think were considered. I'm pretty sure mail UE
didn't know about this until after we started seeing this dialog come up for so
many messages....

Comment 15

[Scott MacGregor]

Frank, what was the bug fix that caused this to start working for mail? I tried
going through the check in logs since December and I couldn't find the change
that made this start showing up. I was curious to see why it wasn't showing up
before. Thanks!

Comment 16

[jglick]

This dialog is new to me and I agree it could use some tweaking. Some 
suggestions if it is decided this dialog is necessary for Mail as well as 
Browser:

1. Deleting a mail message should not cause this dialog to appear. Only Opening 
the message should prompt the dialog if necessary.

2. Try to eliminate the "false positives" for seeing this dialog. Maybe non 
ASCII charsets in the email headers could be ignored and just the text in the 
body could be considered? (From mscott: messages are frequently sent and 
received with non ascii charsets listed in the email headers. This leads to the 
user seeing this dialog when the message just contains ascii text.) 

3. Dialog needs a title. I'd recommend the product name (Netscape/Mozilla) so 
the user knows who is displaying this dialog. 

4. Dialog should be sized correctly and button location should be appropriate to 
OS.

5. If you're just providing info to the user on how to do this, "OK" button is 
more appropriate than "Cancel". But, it would be better if the dialog instead 
gave the user the option to get the necessary language pack. Something like "In 
order to properly display this <message/web page> the <Lang_Pack_Name> language 
pack is needed. Would you like to download this now?" "Yes/No". Since we know 
exactly what language pack is nec, is it possible to just go the correct 
download site and start the download?

6. Agree a "Do not show this dialog again" checkbox would be useful. 

Comment 17

[Roy Yokoyama]

>Frank, what was the bug fix that caused this to start working for mail? 
mscott: nhotta should be able to point you to the patch.  He said it's something 
        to do with passing the language attribute to gfx. 
        nhotta?

Comment 18

[nhottanscp]

bug 102623 - added a lang attribute so that the right font can be selected for
the message view

Comment 19

[Frank Tang]

mscott:
>was it approved by the mail UE team?

As I understand, the spec is produced by the UE team before n6.2 and the mail
impact is not a plan efforts but by a fix of a bug. Ask jaimejr, we simply
implement what jaimejr hand over to us from UE team. 
The side effect of triggering in mail is not planned, but discussed during that
time. We didn't have major change after n6.2 (except alecf and yokoyama move
that code around but without major algorithm change. the triggering part is a
side effect.)

Let's focus, what is the absoulte necessary part for rtm ? I guess it is the
"never show me again" part, right ?

Comment 20

[Roy Yokoyama]

There is a browser bug 86525 which is requesting "never show me again" option too.
I guess we can kill two birds with one stone.

Comment 21

[Peter Trudelle]

IMO, when the checkbox is added, the alert should be changed to no longer
disappear on its own.  This will give people the chance to read it, decide how
to handle it, and disable it if they so choose.  This is especially true if we
offer them the ability to download the correct font from the alert, which I
think would be an enormous improvement over telling them to dig around in prefs.

Comment 22

[Roy Yokoyama]

Attachment: Create new Pref setting for fontdownload

Introduce a new "intl.fontdownload.enabled" Pref setting for enable/disable
the fontdwnld dlgbx.

We used to use the browser's "browser.display.use_document_fonts" to control;
but we now should use generic setting for all the app. ("browser.xxxx" is 
browser specific.)

I need help from UI people to add the checkmark in the Pref dlg.
nhotta may be able to help for mail/news?
smontagu may be able to help for browser?

I also need help from ftang to change the behavior of the fontdownload
dlgbox (ie. alert should no longer disappear on its own and add little
wordings where the pref setting for this). Though I am bit surprised
of this behavior. I wasn't aware of having this kind of _intelligent_
behavior.

Comment 23

[Scott MacGregor]

Roy, is there anyway to disable the dialog for mail windows? I spent a couple
days trying to investiage that but didn't get very far. The font selection code
is so far below all the usual layers that I had no way of knowing what kind of
window we were dealing with. I believe the code where we decide to bring up this
dialog lies underneath the native nsWindow code.

Comment 24

[Roy Yokoyama]

>The font selection code is so far below all the usual layers that I had no way 
>of knowing what kind of window we were dealing with
You are correct.  The font detection part is deep inside gfx. 
(see the patch in #22)  Unfortunately, there is no way of knowing 
which window we are dealing with.  

>is there anyway to disable the dialog for mail windows?
Sorry, but only way I can think of is to use the Pref setting.

Comment 25

[Roy Yokoyama]

over to smontagu for load balancing

Comment 26

[Frank Tang]

chat with putterman and jaimejr about this issue in adt. putterman favor us to
turn this feature off for mail only. mscott- can you help us to figure out how
can we tell in nsIFontMetrics::Init  that such request is coming from mail display ?
or maybe we should turn this off for UTF-8 and UTF-16 display. Since mail is
passing UTF-8 to the gecko, that will stop it. 

Comment 27

[Simon Montagu :smontagu]

Attachment: Don't offer to download in mail

This is a bit clumsy, but seems to work. Will changes be needed to the mac
build system, or does nsFontPackageService not get built there?

Comment 28

[nhottanscp]

+      rv = domDocument->GetDocumentElement(getter_AddRefs(domElement));
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      rv = domElement->GetAttribute(NS_LITERAL_STRING("windowtype"), windowType);
GetDocumentElement, it may return success when domElement is null (see comments
in bug 144735).

Comment 29

[Roy Yokoyama]

Are we ok with making nsLocale.dll depend on dom and appshell?
( btw, we have 'string' included in makefile.win and makefile.in)

Comment 30

[Simon Montagu :smontagu]

Attachment: Patch v.2, addressing comments 28 and 29

ftang and I thought it was better to make intl/locale depend on dom and
appshell than doing the same to gfx.

Comment 31

[Simon Montagu :smontagu]

By the way, it is possible to fool this patch by clicking on a Japanese mail
message and immediately moving a navigator window in front of the mail window,
or vice versa by entering a Japanese URL in the location bar and moving a mail
window in front of the navigator window while it loads.

Comment 32

[Frank Tang]

should we have this dependency in locale or
xpfe/components/intl/nsFontPackageHandler.cpp ?

Comment 33

[Simon Montagu :smontagu]

From the point of view of the dependency itself, I think it would be better in
xpfe/components/intl, but that would introduce a complication.

At the moment |nsFontPackageService::CallDownload| sets |aOutState| to
|eDownload| before calling |nsFontPackageHandler::NeedFontPackage|, which will
prevent it from calling again. So, if a user opens a Japanese mail message and
then opens a Japanese web page later in the same session, the font download
dialog will not appear for the web page.

A way round that would be to change the patch to return NS_ERROR_FAILURE when
called from a mail window, and only reset |aOutState| if
|nsFontPackageHandler::NeedFontPackage| returns NS_OK.

Comment 34

[Simon Montagu :smontagu]

... and the trouble with *that* is that it will cause an assertion in
CheckFontLangGroup in nsFontMetricsWin.cpp:

    res = gFontPackageProxy->NeedFontPackage(fontpackageid);
    NS_ASSERTION(NS_SUCCEEDED(res), "cannot notify missing font package ");

which would probably be even more annoying than the download, for the section of
the community who use debug builds.

Comment 35

[Simon Montagu :smontagu]

Attachment: Patch v.3

This moves the dependency on appshell into xpfe/components/intl, and avoids the
problem described in comment #34 by returning NS_ERROR_ABORT

Comment 36

[nhottanscp]

It was found that GlobalWindowImpl::GetDocument also returns null.
I ses nsASDOMWindowEnumerator::GetNext has the same issue.
Please add error checks.

Comment 37

[Simon Montagu :smontagu]

Attachment: Patch v.4 - more error checking per nhotta's last comment

Comment 38

[Simon Montagu :smontagu]

nhotta, would you like to r= attachment 83927 [details] [diff] [review]?

Comment 39

[nhottanscp]

Comment on attachment 83927 [details] [diff] [review]
Patch v.4 - more error checking per nhotta's last comment

r=nhotta

Comment 40

[Scott MacGregor]

Comment on attachment 83927 [details] [diff] [review]
Patch v.4 - more error checking per nhotta's last comment

sr=mscott

I know it's a hack but I can't think of a better way for us to suppress this
dialog in mail. Thanks so much for doing this Simon.

Comment 41

[Simon Montagu :smontagu]

I won't be able to check this in before Monday 5/17. If time is critical for
getting this in to RC3, maybe Roy can take it back?

Comment 42

[Roy Yokoyama]

I can check it in for you, simon.

I have one question:
what will happen if Gecko embedder would like to suppress
the fontdlg?  I dont' think this patch addresses a generic
"never show me again" (bug 86525).

Comment 43

[Frank Tang]

this is not needed for rc3. 
simon is off today. 

roy- please help to land this into trunk first. and mark it as fixed. 
IQA, please verify it is fixed on the trunk and mark it as verified. 

Comment 44

[Roy Yokoyama]

checked into the trunk.

Comment 45

[Frank Tang]

can someone verify this on trunk so we can ask adt approval ?

Comment 46

[Scott MacGregor]

Using windows 2k: 2002052008. This is now working for me. If I click on a
message that caused the font dialog to come up, I no longer get the dialog!

Comment 47

[ji]

Verified as fixed with 05/21 trunk build. I don't see the dialog when viewing a
mail which requires extra fonts for that language, and I do see the dialog when
viewing a web page which needs the extra fonts.

Comment 48

[scottputterman]

adding adt1.0.0+.  Please get drivers approval and then check into the 1.0 branch.

Comment 49

[Simon Montagu :smontagu]

drivers replied:

  denying this request for now. 
  please re-submit this request after mozilla 1.0 has shipped.

Comment 50

[scottputterman]

changing to adt1.0.1+ for checkin to the 1.0 branch for the Mozilla1.0.1
milestone.  Please get drivers approval before checking in.

Comment 51

[Scott Collins]

Comment on attachment 83927 [details] [diff] [review]
Patch v.4 - more error checking per nhotta's last comment

a=scc for checkin to mozilla1.0.1

Comment 52

[Simon Montagu :smontagu]

Checked in to branch

Comment 53

[nhottanscp]

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

Comment 54

[lchiang]

ji - can you verify this bug fix in 1.01 branch?  When verified, pls replace
fixed1.0.1 keyword with verified1.0.1.  Thanks.