166919 - [ps] nsAFMObject does not look up fonts correctly

Status: RESOLVED FIXED

(Core :: Printing: Output, defect)

Description

[rods (gone)]

Comment 1

[rods (gone)]

Attachment: patch

As stated in the patch:
Some aFontNames have a more than one font separated by a ","
for example: "arial,serif"

This chops the name up per the comma and checks each font name looking for a
match, if not then it fails.

Comment 2

[rods (gone)]

"when it fails" means it defaults to "times"

Comment 3

[rods (gone)]

Attachment: patch with strtok

Parse for font names, I can use strtok because I am comparing against char*
names anyway.

Comment 4

[Roland Mainz]

rods wrote:
> Parse for font names, I can use strtok because I am comparing against char*
> names anyway.

What about using POSIX |strtok_r()| or |PR_strtok_r()| instead of using nsCRT::
?

Comment 5

[kinmoz]

Comment on attachment 98016 [details] [diff] [review]
patch with strtok

sr=[e-mail]

Comment 6

[Boris Zbarsky [:bzbarsky]]

Um.. http://lxr.mozilla.org/seamonkey/source/gfx/public/nsFont.h#121 
nsFont::EnumerateFamilies.  It seems like it would be a good idea to use that
here and not duplicate the parsing code from nsFont, no?

Comment 7

[dcone (gone)]

Comment on attachment 98016 [details] [diff] [review]
patch with strtok

r=dcone.   I think this is the correct way to fix this.
Just look at what we talked about.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 98016 [details] [diff] [review]
patch with strtok

font-family: Times, "Bookman Antique, Condensed", Helvetica;

I fail to see how parsing this with strtok can possibly be a good idea.

Comment 9

[dcone (gone)]

I don't understand why parsing with strtok is such a bad idea?  Its simple, easy
to maintain, gets the job done with a small amount of code.  A space and a quote
is all that would be needed in the token string to fix the problems in the
example you give.  I am not trying to argue or put any particular fix down.. I
am just curious about why you think the strtok is so bad... is it overlooking
anything that nsFont::EnumerateFamilies is doing and needs for this particular
problem.. if its not.. it seems to be the faster.. less complex solution to this
problem.

Comment 10

[rods (gone)]

Attachment: patch with Enum

Yes, the font def is more complicated than I thought:
http://www.w3.org/TR/REC-CSS2/fonts.html#font-family-prop

This patch uses nsFont::EnumerateFamilies

Comment 11

[dcone (gone)]

Comment on attachment 98441 [details] [diff] [review]
patch with Enum

r=dcone

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 98441 [details] [diff] [review]
patch with Enum

Rod, thanks.  sr=bzbarsky if you add a null-check on the ToNewCString() return
value and stop the enumeration if it returned null...

Don, comment 8 has 3 font families listed.  strtok would parse it as 4 font
families.

Comment 13

[rods (gone)]

fixed