Closed Bug 472621 Opened 15 years ago Closed 15 years ago

Thunderbird fails to use some fonts for the default font of HTML messages, if the font has non-ASCII name.

Categories

(Thunderbird :: Message Compose Window, defect)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b2

People

(Reporter: yuki, Assigned: yuki)

Details

Attachments

(2 files, 4 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; ja; rv:1.9.0.5) Gecko/2008120122 Firefox/3.0.5 Pathtraq/0.9
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; ja; rv:1.8.1.19) Gecko/2008120920 Thunderbird/2.0.0.19

For HTML messages, Thunderbird has an option to specify the default font. The specified font is embedded to the message body like '<font face="..."></font>'. But, if the name of the font is written in non-ASCII characters, Thunderbird possibly breaks the value of "face" of FONT elements. As a result, the font which is specified by the user is ignored, and an alert to change the charset to UTF-8 unexpectedly appears.

Reproducible: Always

Steps to Reproduce:
1. Start Thunderbird.
2. Go to "Tools" => "Options" => "Advanced" => "General" => "Config Editor".
3. Find the key "msgcompose.font_face".
4. Change the value to "MS P明朝" (which is one of major fonts for Japanese on Windows).
5. Close "about:config" and the option dialog.
6. Go to "Tools" => "Account Settings" => (the default account) => "Composition & Addressing".
7. Turn the checkbox "Compose messages in HTML format" on.
8. Click "OK". The accoutn settings dialog closes.
9. Click "Write" button in the main window.
10. Composition window appears. Choose "Options" => "Character Encoding" => "Japanese (ISO-2022-JP)".
11. Input your address to the "to" field.
12. Input "a" to the "subject" field.
13. Input "あ" to the "body" field. (Note: "あ" is one of Japanese characters and it is mapped in the range of ISO-2022-JP.)
14. Click the "Send" button.
Actual Results:  
An confirmation dialog to change the character encoding to UTF-8 appears.

Expected Results:  
The mail is sent and the composition window closes.

On the time, the source of the message body is like following:
---------
<html><head></head><body bgcolor="#ffffff" text="#000000"><font face="ï¼­ï¼³ ï¼°ææ">あ</font></body></html>
---------
"ï¼­ï¼³ ï¼°ææ" is broken version of the string "MS P明朝".

This is caused by the function "loadHTMLMsgPrefs()" defined in "MsgComposeCommands.js".
> fontFace = pref.getCharPref("msgcompose.font_face");
> doStatefulCommand('cmd_fontFace', fontFace);
The returned value of "getCharPref()" is not a Unicode string. We have to convert it before calling doStatefulCommand().
On Shredder, the confirmation dialog to change the character encoding to UTF-8 appears doesn't appear. But, the specified font is still ignored in the got message. I got:
----------------------
Content-Type: text/html; charset=ISO-2022-JP
Content-Transfer-Encoding: 7bit

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>

<meta http-equiv="content-type" content="text/html; charset=ISO-2022-JP">
</head>
<body bgcolor="#ffffff" text="#000000">
<font face="&iuml;&frac14;&shy;&iuml;&frac14;&sup3; &iuml;&frac14;°&aelig;&#152;&#142;&aelig;&#156;&#157;">あ</font>

</body>
</html>
----------------------
Version: unspecified → 2.0
Attachment #355921 - Attachment description: Patch (ver.1.0) → Patch for Thunderbird 2.0.0.x (ver.1.0)
Attachment #355921 - Flags: superreview?(bienvenu)
Attachment #355921 - Flags: review?(bienvenu)
Attached patch Patch for Shredder (ver.1.0) (obsolete) — Splinter Review
Attachment #355924 - Flags: superreview?(bienvenu)
Attachment #355924 - Flags: review?(bienvenu)
(In reply to comment #0)
> 2. Go to "Tools" => "Options" => "Advanced" => "General" => "Config Editor".

What happens if, instead, you go to Tools - Options - Composition - General, and choose your desired font from the Font: dropdown menu?
(In reply to comment #4)
> (In reply to comment #0)
> > 2. Go to "Tools" => "Options" => "Advanced" => "General" => "Config Editor".
> 
> What happens if, instead, you go to Tools - Options - Composition - General,
> and choose your desired font from the Font: dropdown menu?

They are just same, if you can find the font name from the very long list.
After change of font for composition to "MS P明朝" via UI on Japanese MS Windows (locale=Japan, system charset=Shift_JIS), following was set in prefs.js.
Note: prefs.js was viewed by Seamonkey, with View/Character Encoding = Unicode(UTF-8)
> user_pref("msgcompose.font_face", "MS P明朝");
This entry is dispayed like next, when prefs.js is viewed by Seamonkey with View/Character Encoding=Western(Windows-1252) & Western(ISO-8859-1).
> user_pref("msgcompose.font_face", "ï¼­ï¼³ P明æœ");

> On the time, the source of the message body is like following:
---------
<html><head></head><body bgcolor="#ffffff" text="#000000"><font face="ï¼­ï¼³
ï¼°ææ">あ</font></body></html>
---------

It looks that some codes of Tb mis-handles value of msgcompose.font_face of prefs as Windows-1252(CP932) or ISO-8859-1(or Shift_JIS on Japanese MS Win), even though Tb himslef puts UTF-8 value in msgcompose.font_face of prefs.js.
(In reply to comment #6)
> It looks that some codes of Tb mis-handles value of msgcompose.font_face of
> prefs as Windows-1252(CP932) or ISO-8859-1(or Shift_JIS on Japanese MS Win),
> even though Tb himslef puts UTF-8 value in msgcompose.font_face of prefs.js.

Yes, Thunderbird stores the correct value into the prefs.js.

 * doStatefulCommand() requires an UCS-2 string. Many other functions are also.
 * nsIPrefBranch.getCharPref() returns the value as UTF-8 bytes, not an UCS-2 string.
 * decodeURIComponent(escape(utf8bytes)) converts UTF-8 bytes to an UCS-2 string.

By the way, nsIPrefBranch.getComplexValue() is available to read the value as an UCS-2 string directly. Should I rewrite the patch with this method?
(In reply to comment #7)

> By the way, nsIPrefBranch.getComplexValue() is available to read the value as
> an UCS-2 string directly. Should I rewrite the patch with this method?

Yes, I think that's the right thing to do.
Comment on attachment 355924 [details] [diff] [review]
Patch for Shredder (ver.1.0)

minusing in favor of changing the way the pref is fetched - thx for working on this
Attachment #355924 - Flags: superreview?(bienvenu)
Attachment #355924 - Flags: superreview-
Attachment #355924 - Flags: review?(bienvenu)
Attachment #355924 - Flags: review-
Attachment #355921 - Attachment is obsolete: true
Attachment #355921 - Flags: superreview?(bienvenu)
Attachment #355921 - Flags: review?(bienvenu)
Attached patch Patch for Shredder (ver.2.0) (obsolete) — Splinter Review
I've found a helper function to read Unicode strings.
Attachment #355924 - Attachment is obsolete: true
Attachment #355987 - Flags: superreview?(bienvenu)
Attachment #355987 - Flags: review?(bienvenu)
For 1.8 branch.
Attachment #355988 - Flags: superreview?(bienvenu)
Attachment #355988 - Flags: review?(bienvenu)
Why don't I use the helper function for other prefs?
Attachment #355987 - Attachment is obsolete: true
Attachment #355991 - Flags: superreview?(bienvenu)
Attachment #355991 - Flags: review?(bienvenu)
Attachment #355987 - Flags: superreview?(bienvenu)
Attachment #355987 - Flags: review?(bienvenu)
For 1.8 branch.
Attachment #355988 - Attachment is obsolete: true
Attachment #355992 - Flags: superreview?(bienvenu)
Attachment #355992 - Flags: review?(bienvenu)
Attachment #355988 - Flags: superreview?(bienvenu)
Attachment #355988 - Flags: review?(bienvenu)
Comment on attachment 355991 [details] [diff] [review]
[checked in] Patch for Shredder (ver.2.1)

thx for the patch!
Attachment #355991 - Flags: superreview?(bienvenu)
Attachment #355991 - Flags: superreview+
Attachment #355991 - Flags: review?(bienvenu)
Attachment #355991 - Flags: review+
Comment on attachment 355991 [details] [diff] [review]
[checked in] Patch for Shredder (ver.2.1)

oops, when I go back to the options UI, it doesn't like the font, and shows it blank. So we need to figure out a way of doing this that doesn't break the options ui.
Attachment #355991 - Flags: superreview-
Attachment #355991 - Flags: superreview+
Attachment #355991 - Flags: review-
Attachment #355991 - Flags: review+
Attachment #355991 - Flags: superreview?(bienvenu)
Attachment #355991 - Flags: superreview-
Attachment #355991 - Flags: review?(bienvenu)
Attachment #355991 - Flags: review-
Comment on attachment 355991 [details] [diff] [review]
[checked in] Patch for Shredder (ver.2.1)

actually, how could this break that? It's only changing the way we read the prefs...more investigation needed...
Comment on attachment 355991 [details] [diff] [review]
[checked in] Patch for Shredder (ver.2.1)

I ran a build w/o your patch, and it still wouldn't display the font in the options ui, even after I reverted it to the default and then switched it, so I don't think it has anything to do with your patch.
Attachment #355991 - Flags: superreview?(bienvenu)
Attachment #355991 - Flags: superreview+
Attachment #355991 - Flags: review?(bienvenu)
Attachment #355991 - Flags: review+
Attachment #355992 - Flags: superreview?(bienvenu)
Attachment #355992 - Flags: superreview+
Attachment #355992 - Flags: review?(bienvenu)
Attachment #355992 - Flags: review+
Hiroshi, can you look into the options ui issue and see if you see it as well? Thx!
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → piro
Keywords: checkin-needed
I tested the latest nightly build and I also saw the blank menulist problem.
It is the bug 468774.
Comment on attachment 355991 [details] [diff] [review]
[checked in] Patch for Shredder (ver.2.1)

Checked in: http://hg.mozilla.org/comm-central/rev/3e31d68c0edb
Attachment #355991 - Attachment description: Patch for Shredder (ver.2.1) → [checked in] Patch for Shredder (ver.2.1)
(In reply to comment #20)
> (From update of attachment 355991 [details] [diff] [review])
> Checked in: http://hg.mozilla.org/comm-central/rev/3e31d68c0edb

That should have been: http://hg.mozilla.org/comm-central/rev/c9502995cd11


Marking as fixed as this is now fixed on trunk. To get the patch on branch you'll need to request approval1.9.0.7 (set it to ? no email required), then add checkin-needed again once you get approval. Once it is fixed in there, there is a fixed1.9.0.7 keyword that you can add to the keywords to show that it is fixed on branch as well as trunk.
Status: NEW → RESOLVED
Closed: 15 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0b2
Attachment #355992 - Flags: approval1.8.1.next?
Attachment #355992 - Flags: approval1.8.1.next? → approval1.8.1.next-
Comment on attachment 355992 [details] [diff] [review]
Patch for Thunderbird 2.0.0.x (ver.2.1)

We have too few resources to introduce any risk into the branch for patches which are not security or stability problems.  Unfortunately, this doesn't qualify as either of those.  Apologies.
You need to log in before you can comment on or make changes to this bug.