Bug 553905 - data parser should parse up to a failure and not scrap the whole path. r=jwatt

content/svg/content/src/nsSVGDataParser.cpp

@@ -61,8 +61,6 @@
 nsresult
 nsSVGDataParser::Parse(const nsAString &aValue)
 {
-  nsresult rv = NS_OK;
-
   char *str = ToNewUTF8String(aValue);
   if (!str)
     return NS_ERROR_OUT_OF_MEMORY;
@@ -70,7 +68,7 @@ nsSVGDataParser::Parse(const nsAString &aValue)
   mInputPos = str;
 
   GetNextToken();
-  rv = Match();
+  nsresult rv = Match();
   if (mTokenType != END)
     rv = NS_ERROR_FAILURE; // not all tokens were consumed
 
@@ -85,8 +83,8 @@ nsSVGDataParser::Parse(const nsAString &aValue)
 
 void nsSVGDataParser::GetNextToken()
 {
-  mTokenPos  = mInputPos;
+  mTokenPos = mInputPos;
-  mTokenVal  = *mInputPos;
+  mTokenVal = *mInputPos;
   
   switch (mTokenVal) {
     case '0': case '1': case '2': case '3': case '4':
@@ -129,12 +127,6 @@ void nsSVGDataParser::RewindTo(const char* aPos)
   GetNextToken();
 }
 
-
-nsresult nsSVGDataParser::Match()
-{
-   return NS_OK;
-}
-
 //----------------------------------------------------------------------
 
 nsresult nsSVGDataParser::MatchNonNegativeNumber(float* aX)

content/svg/content/src/nsSVGDataParser.h

@@ -63,7 +63,7 @@ protected:
   // helpers
   void GetNextToken();
   void RewindTo(const char* aPos);
-  virtual nsresult Match();
+  virtual nsresult Match()=0;
 
   nsresult MatchNonNegativeNumber(float* aX);
   PRBool IsTokenNonNegativeNumberStarter();

content/svg/content/src/nsSVGPathDataParser.cpp

@@ -878,10 +878,8 @@ nsSVGPathDataParserToInternal::Parse(const nsAString &aValue)
   mPrevSeg = nsIDOMSVGPathSeg::PATHSEG_UNKNOWN;
 
   nsresult rv = nsSVGPathDataParser::Parse(aValue);
-  NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = PathFini();
+  PathFini();
-  NS_ENSURE_SUCCESS(rv, rv);
 
   return rv;
 }
@@ -1214,7 +1212,7 @@ nsSVGPathDataParserToInternal::PathClose()
   return NS_OK;
 }
 
-nsresult
+void
 nsSVGPathDataParserToInternal::PathFini()
 {
   // We're done adding data to the arrays - copy to a straight array
@@ -1226,7 +1224,7 @@ nsSVGPathDataParserToInternal::PathFini()
   argArraySize = mArguments.Length() * sizeof(float);
   mPathData->mArguments = (float *)malloc(argArraySize + mCommands.Length());
   if (!mPathData->mArguments)
-    return NS_ERROR_OUT_OF_MEMORY;
+    return;
 
   memcpy(mPathData->mArguments, mArguments.Elements(), argArraySize);
   memcpy(mPathData->mArguments + mNumArguments,
@@ -1234,8 +1232,6 @@ nsSVGPathDataParserToInternal::PathFini()
          mCommands.Length());
   mPathData->mNumArguments = mNumArguments;
   mPathData->mNumCommands = mNumCommands;
-
-  return NS_OK;
 }
 
 // ---------------------------------------------------------------

content/svg/content/src/nsSVGPathDataParser.h

@@ -72,7 +72,7 @@ protected:
   virtual nsresult StoreEllipticalArc(PRBool absCoords, float x, float y,
                                       float r1, float r2, float angle,
                                       PRBool largeArcFlag, PRBool sweepFlag) = 0;
-  nsresult  Match();
+  virtual nsresult Match();
  
   nsresult MatchCoordPair(float* aX, float* aY);
   PRBool IsTokenCoordPairStarter();
@@ -144,7 +144,7 @@ class nsSVGPathDataParserToInternal : public nsSVGPathDataParser
 {
 public:
   nsSVGPathDataParserToInternal(nsSVGPathList *data) : mPathData(data) {}
-  virtual nsresult Parse(const nsAString &aValue);
+  nsresult Parse(const nsAString &aValue);
 
 protected:
   virtual nsresult StoreMoveTo(PRBool absCoords, float x, float y);
@@ -187,7 +187,7 @@ private:
   nsresult PathLineTo(float x, float y);
   nsresult PathCurveTo(float x1, float y1, float x2, float y2, float x3, float y3);
   nsresult PathClose();
-  nsresult PathFini();
+  void PathFini();
 };
 
 class nsSVGPathDataParserToDOM : public nsSVGPathDataParser

content/svg/content/src/nsSVGPathElement.cpp

@@ -458,7 +458,10 @@ nsSVGPathElement::BeforeSetAttr(PRInt32 aNamespaceID, nsIAtom* aName,
 
     if (aValue) {
       nsSVGPathDataParserToInternal parser(&mPathData);
-      parser.Parse(*aValue);
+      nsresult rv = parser.Parse(*aValue);
+      if (NS_FAILED(rv)) {
+        ReportAttributeParseFailure(GetOwnerDoc(), aName, *aValue);
+      }
     } else {
       mPathData.Clear();
     }

content/svg/content/src/nsSVGPathSegList.cpp

@@ -132,12 +132,10 @@ NS_INTERFACE_MAP_END
 NS_IMETHODIMP
 nsSVGPathSegList::SetValueString(const nsAString& aValue)
 {
-  nsresult rv;
-  
   WillModify();
   ReleaseSegments(PR_FALSE);
   nsSVGPathDataParserToDOM parser(&mSegments);
-  rv = parser.Parse(aValue);
+  nsresult rv = parser.Parse(aValue);
 
   PRInt32 count = mSegments.Count();
   for (PRInt32 i=0; i<count; ++i) {

content/svg/content/src/nsSVGTransformList.cpp

@@ -172,12 +172,10 @@ nsSVGTransformList::SetValueString(const nsAString& aValue)
   // XXX: we don't implement the _exact_ BNF given in the
   // specs.
 
-  nsresult rv = NS_OK;
-
   // parse transform attribute value
   nsCOMArray<nsIDOMSVGTransform> xforms;
   nsSVGTransformListParser parser(&xforms);
-  rv = parser.Parse(aValue);
+  nsresult rv = parser.Parse(aValue);
 
   if (NS_FAILED(rv)) {
     // there was a parse error.

content/svg/content/src/nsSVGTransformListParser.h

@@ -60,7 +60,7 @@ private:
   nsCOMArray<nsIDOMSVGTransform> *mTransform;
 
   // helpers
-  nsresult Match();
+  virtual nsresult Match();
 
   nsresult MatchNumberArguments(float *aResult,
                                 PRUint32 aMaxNum,

layout/reftests/svg/path-02.svg

@@ -0,0 +1,15 @@
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/licenses/publicdomain/
+-->
+<svg xmlns="http://www.w3.org/2000/svg" width="100%" height="100%">
+  <title>Testcase for path with errors</title>
+
+  <!-- From https://bugzilla.mozilla.org/show_bug.cgi?id=553905 -->
+
+  <rect width="100%" height="100%" fill="lime"/>
+  <g shape-rendering="crispEdges">
+    <path d="M 20 200 H 80" stroke-width="20%" stroke="red"/>
+    <path d="M 20 200 H 80#90" stroke-width="20%" stroke="lime"/>
+  </g>
+</svg>

layout/reftests/svg/reftest.list

@@ -115,6 +115,7 @@ fails == inline-in-xul-basic-01.xul pass.svg
 == opacity-and-gradient-02.svg opacity-and-gradient-02-ref.svg
 == opacity-and-pattern-01.svg pass.svg
 == path-01.svg path-01-ref.svg
+== path-02.svg pass.svg
 == pattern-live-01a.svg pattern-live-01-ref.svg
 == pattern-live-01b.svg pattern-live-01-ref.svg
 == pattern-live-01c.svg pattern-live-01-ref.svg