206716 - Composer can not use Chinese font.

Status: RESOLVED FIXED

(SeaMonkey :: Composer, defect)

Description

[Yeh You-Ying]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4b) Gecko/20030507
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4b) Gecko/20030507

We choose Chinese font while composing, but no effect.
If Western font chosen, it's all ok !

Reproducible: Always

Steps to Reproduce:
1. Open Composer or Open New Message(MailNews)
2. Choose Chinese font and then edit.


Actual Results:  
While choosing Chinese font, Mozilla Composer should work as choosing Western Font.

Comment 1

[Yeh You-Ying]

Attachment: This is an example that I compse a message with different fonts!

All western fonts work, but Chinese fonts fail.

Comment 2

[Yeh You-Ying]

Bug still exists in 1.4 RC1

Comment 3

[Chien-Pang Kuo]

It still can't use in 1.4 RC3.
Only useful in English font.

Comment 4

[Mike Kaply [:mkaply]]

I'm assuming you mean a font with a Chinese name?

Comment 5

[Mike Kaply [:mkaply]]

Attachment: Switch editor APIs to use String, not CString

OK, the main fix I did for this was in ComposerCommands.js, but looking at
ComposerCommands.cpp, these change should be everywhere.

In these cases, they were working with a wide string anyway, so the extra
conversions were unnecessary.

Comment 6

[Mike Kaply [:mkaply]]

My change is too far reaching - it breaks stuff.

To really fix just this problem. we just need one change to composerCommands.js

I'm continuing to investigate.

Comment 7

[Mike Kaply [:mkaply]]

Attachment: Just fix ComposerCommands.js

In looking at this patch, the only thing that needs fixing is
ComposerCommands.js in two places.

The right stuff is happening inside Composer.

When I tried to change ComposerComands.cpp, I ended up breaking Midas commands.


So I'm focusing on the problem at hand.

Comment 8

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

This patch worries me a bit because of the nsComposerCommands.cpp counterpart.
I don't see why something like nsParagraphsStateCommand still relies on
a getCStringValue() in the c++ code while the JS calls a setStringValue()
through a doStatefulCommand(), and relies on a getStringValue() in
pokeMultiStateUI()... Can you please explain me how things were broken with
your cpp changes ?

brade: what do you think ?

Comment 9

[Mike Kaply [:mkaply]]

With my CPP changes, things in the Midas demo like doing an execCommand to set a
URL failed.

I didn't debug it yet.

Comment 10

[Kathleen :Brade]

Comment on attachment 149341 [details] [diff] [review]
Just fix ComposerCommands.js

these are probably the right fixes but I'd like to see more context please... 
:-)

Comment 11

[Mike Kaply [:mkaply]]

Attachment: More context

More context

Comment 12

[Kathleen :Brade]

Comment on attachment 149351 [details] [diff] [review]
More context

r=brade

Comment 13

[kinmoz]

Comment on attachment 149351 [details] [diff] [review]
More context

sr=[e-mail]

Comment 14

[WADA:World Anti-bad-Duping Agency]

2004-06-15-07(trunk)/Win-2K used font of Japanese Font Name successufully in
both HTML mail composition and HTML editing by Composer.
Original problem of this bug has been resolved.

However, there are problems in font-family specification in HTML source.

(1) Font name was always converted to lower case.
    For example, "Times New Roman" was converted to "times new roman".
    This conversion was done even on DBCS alphabet letters of Japanese Font Name.
    Is this conversion to lower case is intentional?

    This did not cause font use problem by Mozilla on MS Windows.
    However, this may cause font use problem by Mailer/Browser on other systems.
    Won't this cause problem? (especially on Unix)

(2) If font name contains space characters, font name was not enclosed by quotes.
    HTML became <SPAN STYLE="font-family: times new roman;">
    This should be <SPAN STYLE="font-family: 'times new roman';">
    Workaround:
    After font selection from Format/Fonts menu, add single quotes manually
    through "Advanced Properties/Inline Styles" of the tag.

    This problem is not so small in Japan because most popular Japanese Fonts,
     "MS Mincho" and "MS Gothic" on MS Wondows by Microsoft, 
     contain space in the font name.

Comment 15

[Mike Kaply [:mkaply]]

The lower casing happens here:

http://lxr.mozilla.org/seamonkey/source/editor/libeditor/html/nsHTMLCSSUtils.cpp#873

glazou: how can we be more selective about our lower casing? And shouldn't we be
quoting stuff?

Comment 16

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Attachment: in answer to mkaply's question; this does not deal with the quotes problem

Comment 17

[Mike Kaply [:mkaply]]

Comment on attachment 151362 [details] [diff] [review]
in answer to mkaply's question; this does not deal with the quotes problem

r=mkaply

Comment 18

[Kathleen :Brade]

Reopen bug since it has an active patch.

Daniel--can you please wrap the || to a new line?  Thanks!

Comment 19

[kinmoz]

Comment on attachment 151362 [details] [diff] [review]
in answer to mkaply's question; this does not deal with the quotes problem

sr=[e-mail]

Comment 20

[Mike Kaply [:mkaply]]

Checked in to trunk.

I did not wrap the || per brade because it would have made the code look very
strange.

I don't think we need to fix the quote problem, since CSS says the quotes are
unnecessary.

Comment 21

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #20)

> I don't think we need to fix the quote problem, since CSS says the quotes are
> unnecessary.

You are right.
CSS says that quoting is required only when leading/trailing space(s) or
consecutive spaces are included in font name.
Therefore "not quoted" is not problem when "Times New Roman", "MS Mincho" and
"MS Gothic".
Sorry for my mis-understanding.

Comment 22

[Stephen Donner [:stephend] Not actively reading bugmail]

This fix caused bug 249682.  Who is going to step up and fix it for 1.8?

Comment 23

[Mike Kaply [:mkaply]]

stephen, I was just about to start debugging that.

Comment 24

[Kathleen :Brade]

this bug is fixed; please move discussion to bug 249682

Comment 25

[Boris Zbarsky [:bzbarsky]]

The first patch for this bug caused bug 289625.  Shouldn't _all_ getters/setters
of state_attribute been updated to get/setStringValue off the bat instead of
waiting for regressions to crop up?  Some are now using CStringValue while
others use StringValue, which presumably causes the problem....

Comment 26

[Kathleen :Brade]

(Disclaimer: I don't have a tree to test this theory right now)
The problem with ComposerCommands.js is that goUpdateCommandState() can call
pokeMultiStateUI() and cmdParams may have a cstring or a string.

doStatefulCommand() also calls pokeMultiStateUI() but cmdParams is always
consistent there.

One solution would be to make all commands pass the same string type (get rid of
all cstrings).

Another option is to make pokeMultiStateUI() able to handle either type (instead
of calling getStringValue() it could call getValueType() and then call the
appropriate string getter method).

Given that this bug is resolved fixed, I will put this comment also in bug
289625 since I assume that is where the fix/patch will be posted.