philippe (part-time) Reporter
	Description • 18 years ago

All text in webpages and chrome of Minefield is displayed using 'Helvetica'.
Styling of text on Webpages using CSS: font-family: XXX (where XXX is the font name) is not applied.

Behaviour changed between the 2006122119 build and the 2006122120
works OK
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a2pre) Gecko/20061221 Minefield/3.0a2pre ID:2006122119 [cairo]
Fails
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a2pre) Gecko/20061221 Minefield/3.0a2pre ID:2006122120 [cairo]

--> bug 362665

	philippe (part-time) Reporter
	Comment 1 • 18 years ago

Each paragraph should be styled as specified.
All of them are styled with 'Helvetica' except one: 'georgia'

	philippe (part-time) Reporter
	Comment 2 • 18 years ago

Side by Side
Minefield (build 2006122120) on the left.
Camino trunk build (build 2006122117 (1.2+)) on the right. 

That Camino build predates the Landing of the patch in bug 362665 by a few hours

	philippe (part-time) Reporter
	Comment 3 • 18 years ago

Note the incorrect alignment of text on the bookmark bar

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 4 • 18 years ago

On my environment, I cannot reproduce this bug.
Please wait.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 5 • 18 years ago

Would you attach the log by this patch?

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 6 • 18 years ago

Oops, sorry. I attached the wrong file.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 7 • 18 years ago

Wow, I can reproduce with changing the page charset to western in the testcase.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 8 • 18 years ago

I think that the resolver works fine.
The cause may be font switching by ATSUI on gfxAtsuiFont.cpp

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 9 • 18 years ago

The previous comment is wrong. I find a easy mistake.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 10 • 18 years ago

I forgot to cache the postscript name that is using for finding the atsui font id in |FindFromSystem|.
But I don't know why it works fine with non-western encodings...

# I need to leave my desk. After this review, please check-in this patch if anybody can do it.

	philippe (part-time) Reporter
	Comment 11 • 18 years ago

I'm a bit late/slow... :-)
Requested log. As you guessed, can't find those fonts

(In reply to comment #10)
>
Glad you found it. I'll try this as soon as possible

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 12 • 18 years ago

(In reply to comment #11)
> Created an attachment (id=249421) [edit]
> log per comment 6
> 
> I'm a bit late/slow... :-)
> Requested log. As you guessed, can't find those fonts

Wow, your list doesn't contain the 'Times' and 'Lucida Grande'. Did you uninstall the fonts??

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 13 • 18 years ago

Or, your these fonts don't have the postscript name.
I cannot find the fonts they are not having the postscript name, I may need to care the fonts.(This bug cannot be fixed by the patch.)

	philippe (part-time) Reporter
	Comment 14 • 18 years ago

(In reply to comment #12)
> (In reply to comment #11)
> > Created an attachment (id=249421) [edit]

> 
> Wow, your list doesn't contain the 'Times' and 'Lucida Grande'. Did you
> uninstall the fonts??
> 
No, all fonts are there in their default location.

(10.4.8 PPC)

I loaded an other page from my development server. That page uses most of the roman fonts available on OS X/10.4. The log shows that Minefield can't find any of them.

	philippe (part-time) Reporter
	Comment 15 • 18 years ago

(In reply to comment #13)
> Or, your these fonts don't have the postscript name.
> I cannot find the fonts they are not having the postscript name, I may need to
> care the fonts.(This bug cannot be fixed by the patch.)
> 

According to FontBook.app, the postscript name for Lucida Grande is 'LucidaGrande', the bold face is 'LucidaGrande-Bold'.
Times is 'Times-Roman'

I had a script somewhere to output a list of all fonts. I'll try to find it back.

	philippe (part-time) Reporter
	Comment 16 • 18 years ago

(In reply to comment #15)

> I had a script somewhere to output a list of all fonts. I'll try to find it
> back.
> 

Found it, Font Tools, form Apple
http://developer.apple.com/textfonts/download/

I'll attach the list next.

	philippe (part-time) Reporter
	Comment 17 • 18 years ago

Result of ftxinstalledfonts -fpq > ~/Desktop/installedfontsPSQD.txt

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 18 • 18 years ago

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

we need to care the non-postscript font(?).

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 19 • 18 years ago

philippe:

Would you test this patch? And would you attach the log?

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 20 • 18 years ago

I understand what happens. Because I changed the system locale to English, I can reproduce the bug of failing to list up the all fonts.

The converting by CFString doesn't work fine. I don't know the reason. Now, I use TCE for converting. Because old gfx uses this.

And I found another bug. If the (n)th font has font family name that is same as the name of (n+1)th or later fonts, the resolver returns (n)th font for the name. I add the new member mFamilyNames that is linking to postscript name. See for the detail of the members.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 21 • 18 years ago

(In reply to comment #20)
> the name of (n+1)th or later fonts, the resolver returns (n)th font for the
> name. I add the new member mFamilyNames that is linking to postscript name. See
> for the detail of the members.

"See the header file for the detail of the members."
     ^^^^^^^^^^^^^^^

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 22 • 18 years ago

Oops, there is a unused variable.

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)
	Comment 23 • 18 years ago

Comment on attachment 249458 [details] [diff] [review]
Patch rv2.0.1

Looks good again, I think; though once again, I didn't test.  Btw, "localized" in your comments, not "localzed" :)

	philippe (part-time) Reporter
	Comment 24 • 18 years ago

Patch rv2.0.1
In a  quick first testing: this works fine. !!
The test case attached here displays correctly. The text in the chrome part of the browser displays correctly (bookmarks bar, Preference window, and more).

Thank you Masayuki-san.

However, some Japanese sites (like http://mozillazine.jp/) still don't display correctly from my end. But I think that is another bug (maybe reopen 357555 for that ?).

	philippe (part-time) Reporter
	Comment 25 • 18 years ago

from http://mozillazine.jp/
patched Minefield, English OS 10.4.8
I see the same problem in a tab title when it contains Japanese characters

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 26 • 18 years ago

Thank you.
# I added a new condition for an assertion.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 27 • 18 years ago

checked-in, thank you.

philippe:

Thank you for your testing! I will file a bug for the font switching. If you find the font switching bug, please note the URLs. Thanks.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 28 • 18 years ago

Oops, I forgot to type a diff command.