fixed parsing of line-height in font shorthand

[ju1ius]

fixes #16

[sabberworm]

@GaryJones My commit also handles the border-radius case but the counter-argument will be the same as with issue #12: While the input is accepted and the output looks the same, the parsed structure is counter-intuitive in that the 5px / 20px part will appear as one element, next to the 10px part. The only way to rectify this is to look at the property name to deduce whether the slash is meant to discern two CSSValues or two completely different rule values. I’m not prepared to be this specific right now.

[ju1ius]

Representing 5px / 20px as a CSSSlashedValue seems incorrect because there's no such thing as a "slashed value" in CSS. I would say that this example is: two primitive values inside a value list, inside a rule, inside a declaration.
Instead of creating a CSSSlashedValue class, I would go for implementing CSSValueList, along with a parseValueList method that could smartly handle the well-known special cases, such as shorthand properties, "slashed" lists separators, or multiple backgrounds/box-shadows, etc...

[sabberworm]

It not only seems incorrect, it is incorrect. But there’s always a trade-off. I’ve always been proud of the way the CSSParser handles all possible properties well enough without having to make an exception on a single property. This also makes for a quite future-proof parser as long as new specifications adhere to existing markup (more or less the only change for this parser to support all of CSS 3 instead of just 2.1 is the addition of the CSSFunction class and its parsing rules). The downside of this is of course that this CSS parser will assign proper wrapper classes to most encountered identifiers but will never be able to understand CSS. Sure it would be great if you could call $oDeclarationBlock->getRule('line-height') and be given the line height even if it was declared using font: 12px/1.2… and while you’re at it, any shorthand-property should be accessible using the more specific notation. But a CSS parser like this would be much higher-maintenance than I could ever be prepared to invest. This project is perfectly suited for all the use-cases I ever had to deal with and almost all use cases I can come up with off the top of my head. It was never intended to be a parser that understood CSS, just one that made it possible to change structure (not meaning) easily without having to resort to RegEx hacking. If you’re willing to invest the time necessary to build a parser that understands the meaning of CSS and not just the structure, be my guest (though I still have my reservations about being to maintain such a specific set of rules which have to be updated for every new iteration of the spec).

[sabberworm]

I’ve had a little time to think about your proposed changes and now I’ve come to the conclusion that they’re actually making a lot of sense. I’ve added the CSSValueList class and it is currently used as a superclass for CSSColor, CSSFunction and CSSSlashedValue. It would make a great deal of sense to change the CSSRule class to only store one CSSValue object which may be a CSSValueList (or a nested set thereof) if there are multiple values per rule.

CSSParser could implement a method getListSeparatorPrecedence($sRule = null) which would, by default, return array('/', ' ', ',') (meaning comma binds the tightest, slash the loosest) but could return array(' ', '/', ',') for $sRule =~ /^font-?$/.

The only downside of this would be breaking backwards-compatibility for clients who expect to get a doubly-nested array on CSSRule->getValues().

[ju1ius]

Sure it would be great if you could call $oDeclarationBlock->getRule('line-height') and be given the line height even if it was declared using font: 12px/1.2… and while you’re at it, any shorthand-property should be accessible using the more specific notation.

I'm currently working on implementing this.
I'll look at your changes and try to submit a pull request soon.