--- Comment #4 from Vincent Hennebert <> 2009-09-21 
04:38:54 PDT ---
Hi Jonathan,

Thank you for your contribution to fixing this issue.

I think the whole parsing of this property needs to be refactored, because it's
unnecessarily complicated and doesn't work. I don't really get the purpose of
the "while (!fontFamilyParsed)" loop but it seems to me that it's doing a job
that should be left over to the FontFamilyProperty parser. Also, it doesn't
seem to handle the case where there are more than one comma (i.e., at least 3
families specified). If more than one font-style/-variant/-weight is specified,
it will try to parse the second value (say, font-variant) as a font family.
Also, it tries to apply a special treatment when system fonts (caption, icon,
menu...) are specified, whereas it is explicitly stated in the Recommendation
that they don't apply to XSL-FO.

I think the best way to handle that is to write a parser that follows the
grammar given by the Recommendation. Hopefully a simple LL parser will do [1],
but given that all of the sub-properties accept the 'inherit' keyword it will
probably not be LL(1). If an LL parser can't be used, then we will have to use
an LR one [2], which is slightly more problematic since it cannot be easily
coded by hand, and we will have to rely on an external parsing library.

I suggest you to look if an LL parser can be written to properly parse this
property, and report back here if it's not the case, and if we must consider
other solutions.



(In reply to comment #1)
> Created an attachment (id=24277)
 --> ( [details]
> Allow case when no font-family is quoted and no comma-separated list of font
> families
> Allow case when no font-family is quoted and no comma-separated list of font
> families.
> The code in FontShorthandProperty is looking for the beginning of the
> font-family.  If the font-family contains commas then the code works as 
> before.
>  If the font-family begins with a quote the code works as before.  There was
> one case missing - when the code is looking for the beginning of the font
> family but there are no commas and no quotes.
> The font-family can be quite complex containing commas, single quoted strings,
> double quote strings, and strings without quotes but the parsing of this
> complex expression is taken care of by FontFamilyProperty.  All
> FontShorthandProperty does is look for where the font-family begins.  It 
> almost
> did this correctly (before this patch) when the font-family contained commas 
> or
> consisted of a quoted string.  Now there is the case when the font-family
> contains no quotes and no commas.
> However, there is a fly in the ointment.  In testing this change (and in
> testing the previous unchanged code), I added the following test case, which
> fails in both cases.
>      <fo:block font="xx-large/1.4 Arial, 'Times New Roman', sans-serif">
>         <test:assert property="font-family" expected="[Arial, Times New Roman,
> sans-serif]"/>
>         <test:assert property="font-size" expected="20736mpt"/>
>         <test:assert property="font-weight" expected="400" />
>         <test:assert property="font-style" expected="NORMAL" />
>         <test:assert property="line-height.optimum" expected="29030mpt" />
>         <test:assert property="font-variant" expected="NORMAL" />
>         Test font shorthand
>       </fo:block> 
> There seems to be a problem when the quote follows a comma.  I will add a 
> patch
> to which demonstrates this problem, and will continue 
> to
> diagnose the code to see what further problems in the code are causing this
> problem.

Configure bugmail:
------- You are receiving this mail because: -------
You are the assignee for the bug.

Reply via email to