Bug 8170 – SVG CSS property values with extra items do not get treated as invalid (they should)

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Alexey Proskuryakov]

SVG CSS properties that have too many elements aren't invalidated, like it used to be the case with HTML properties, see bug 7118. 

The fix should be very similar, but there is no test case to verify it yet. Actually, SVGCSSParser already does the invalidation when handling the valid_primitive case, but not in some other cases.

[Rob Buis]

Created attachment 7849 [details]
Proposed fix

This is my first attempt. From the few small tests I did it handles the parsedValue
cases ok (for stroke and clip-path). In khtml/css/cssparser.cpp the code checks
shorthands, but for svg there only seems to be the marker property, which gets
handled in parseShorthand.
Cheers,

Rob.

[Rob Buis]

Created attachment 7854 [details]
Better yet

After feedback from ap, this one is much better.
It removes my faulty cleanup. Also it now handles initial and inherit
cases, and shorthands (marker) works too. IMHO all that is lacking
is a testcase, which I am working on, but will take some time.
Cheers,

Rob.

[Eric Seidel (no email)]

Comment on attachment 7854 [details]
Better yet

Hyatt should review this.

[Dave Hyatt]

Comment on attachment 7854 [details]
Better yet

You're missing the inShorthand check when you check to see if value->current() is null.  If you're inside an SVG shorthand there could still be properties that will be parsed afterwards.

[Rob Buis]

(In reply to comment #4)
> (From update of attachment 7854 [details] [edit])
> You're missing the inShorthand check when you check to see if value->current()
> is null.  If you're inside an SVG shorthand there could still be properties
> that will be parsed afterwards.
> 

Hi,

I think there is such a call in the last patch.
If you still think it lacks, please let me know, I
am working on wrapping this patch up including
testcase :)
Cheers,

Rob.

[Rob Buis]

Created attachment 7883 [details]
Complete patch

This time the patch + testcase.
Cheers,

Rob.

[Dave Hyatt]

Comment on attachment 7883 [details]
Complete patch

r=me

[Eric Seidel (no email)]

Thanks rob!