17.10 KB, image/jpeg
905 bytes, patch
|Details | Diff | Splinter Review|
4.47 KB, patch
|Details | Diff | Splinter Review|
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.
nominating. The results of our discussions may have some impact on the beta in order to provide a good user experience for e-mail.
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.
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.
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 ***
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → DUPLICATE
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!
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
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.
Status: REOPENED → ASSIGNED
*** Bug 142535 has been marked as a duplicate of this bug. ***
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+.
Whiteboard: [adt2 rtm]
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?
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
>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 ?
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....
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!
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.
>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?
Status: NEW → ASSIGNED
bug 102623 - added a lang attribute so that the right font can be selected for the message view
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 ?
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.
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.
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.
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.
>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.
over to smontagu for load balancing
Assignee: yokoyama → smontagu
Status: ASSIGNED → NEW
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.
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?
+ 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).
Are we ok with making nsLocale.dll depend on dom and appshell? ( btw, we have 'string' included in makefile.win and makefile.in)
ftang and I thought it was better to make intl/locale depend on dom and appshell than doing the same to gfx.
Attachment #83630 - Attachment is obsolete: true
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.
should we have this dependency in locale or xpfe/components/intl/nsFontPackageHandler.cpp ?
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.
... 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.
This moves the dependency on appshell into xpfe/components/intl, and avoids the problem described in comment #34 by returning NS_ERROR_ABORT
Attachment #83783 - Attachment is obsolete: true
It was found that GlobalWindowImpl::GetDocument also returns null. I ses nsASDOMWindowEnumerator::GetNext has the same issue. Please add error checks.
Comment on attachment 83927 [details] [diff] [review] Patch v.4 - more error checking per nhotta's last comment r=nhotta
Attachment #83927 - Flags: review+
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.
Attachment #83927 - Flags: superreview+
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?
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).
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.
checked into the trunk.
Status: NEW → RESOLVED
Closed: 17 years ago → 17 years ago
Resolution: --- → FIXED
can someone verify this on trunk so we can ask adt approval ?
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!
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.
Status: RESOLVED → VERIFIED
adding adt1.0.0+. Please get drivers approval and then check into the 1.0 branch.
drivers replied: denying this request for now. please re-submit this request after mozilla 1.0 has shipped.
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 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
Attachment #83927 - Flags: approval+
*** Bug 161159 has been marked as a duplicate of this bug. ***
ji - can you verify this bug fix in 1.01 branch? When verified, pls replace fixed1.0.1 keyword with verified1.0.1. Thanks.
You need to log in before you can comment on or make changes to this bug.