See http://samples.msdn.microsoft.com/ietestcenter/svg112/svg_harness.htm?url=./svg112/svg/chapter_08.2.svg

I think we should definitely send a warning to the console about problems in the path data. Can you add: // ReportToConsole comments at appropriate places so that when someone comes to implement that function properly they notice to use it there. It probably means changing some of the return values from void back to nsresult.

You and your ReportToConsole obsession ;-)

:-P So I may be missing something, but where are the ReportToConsole comments in the patch?

In one place it's + nsresult rv = parser.Parse(*aValue); + if (NS_FAILED(rv)) { + ReportAttributeParseFailure(GetOwnerDoc(), aName, *aValue); + } In the other cases the call stack traces back to here... http://mxr.mozilla.org/mozilla-central/source/content/svg/content/src/nsSVGElement.cpp#299 So when they fail you get the effect you want.

And ReportAttributeParseFailure is just a wrapper for ReportToConsole

Ah, sorry, when I did a quick search for 'ReportToConsole' and got nothing I thought you must have lost the comments from your diff. One other question: why the changes to nsSVGTransformList::SetValueString? I know we're changing the handling for path data because of Appendix F.5, but I'm not aware of a similar requirement for the 'transform' attribute. Other than that it looks good I think.

We're compatible with Opera if we change everything, but I can't find the transform requirement either. Oh for consistency in the specification. I'll do another patch that just has the path change in it and we can raise the other parsing with w3c.

Comment on attachment 435403 [details] [diff] [review] with ReportToConsole r=jwatt with the transform changes dropped, and could you add a comment like the following for the OOM return in PathFini: // mArguments, mNumCommands and mNumArguments are left in a consistent state

pushed http://hg.mozilla.org/mozilla-central/rev/f6c53a95a98a