Bug 8170

Summary:

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

Product:

WebKit

Reporter:

Alexey Proskuryakov <ap>

Component:

SVG

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

joost

Priority:

Keywords:

NeedsReduction

Version:

420+

Hardware:

Mac

OS:

OS X 10.4

Attachments:

Description	Flags
Proposed fix	none
Better yet	hyatt: review-
Complete patch	hyatt: review+

Alexey Proskuryakov

Reported 2006-04-03 22:29:00 PDT

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.

Attachments
Proposed fix (1.97 KB, patch) 2006-04-20 06:54 PDT, Rob Buis	no flags	Details Formatted Diff Diff
Better yet (2.26 KB, patch) 2006-04-20 13:02 PDT, Rob Buis	hyatt: review-	Details Formatted Diff Diff
Complete patch (6.68 KB, patch) 2006-04-21 12:47 PDT, Rob Buis	hyatt: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Rob Buis

Comment 1 2006-04-20 06:54:13 PDT

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

Comment 2 2006-04-20 13:02:14 PDT

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 3 2006-04-20 15:10:22 PDT

Comment on attachment 7854 [details] Better yet Hyatt should review this.

Dave Hyatt

Comment 4 2006-04-20 18:10:35 PDT

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

Comment 5 2006-04-21 11:59:08 PDT

(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

Comment 6 2006-04-21 12:47:09 PDT

Created attachment 7883 [details] Complete patch This time the patch + testcase. Cheers, Rob.

Dave Hyatt

Comment 7 2006-04-21 22:02:20 PDT

Comment on attachment 7883 [details] Complete patch r=me

Eric Seidel (no email)

Comment 8 2006-04-23 15:53:42 PDT

Thanks rob!

Note You need to log in before you can comment on or make changes to this bug.