Bug 539697 - valueAsString returns an incorrect failure code. r=jwatt

content/svg/content/src/nsSVGAngle.cpp

@@ -76,13 +76,11 @@ public:
   NS_IMETHOD NewValueSpecifiedUnits(PRUint16 unitType,
                                     float valueInSpecifiedUnits)
     {
-      NS_ENSURE_FINITE(valueInSpecifiedUnits, NS_ERROR_ILLEGAL_VALUE);
-      mVal.NewValueSpecifiedUnits(unitType, valueInSpecifiedUnits, nsnull);
-      return NS_OK;
+      return mVal.NewValueSpecifiedUnits(unitType, valueInSpecifiedUnits, nsnull);
     }
 
   NS_IMETHOD ConvertToSpecifiedUnits(PRUint16 unitType)
-    { mVal.ConvertToSpecifiedUnits(unitType, nsnull); return NS_OK; }
+    { return mVal.ConvertToSpecifiedUnits(unitType, nsnull); }
 
 private:
   nsSVGAngle mVal;
@@ -205,7 +203,7 @@ GetValueFromString(const nsAString &aValueAsString,
   const char *str = value.get();
 
   if (NS_IsAsciiWhitespace(*str))
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_DOM_SYNTAX_ERR;
   
   char *rest;
   *aValue = float(PR_strtod(str, &rest));
@@ -216,7 +214,7 @@ GetValueFromString(const nsAString &aValueAsString,
     }
   }
   
-  return NS_ERROR_FAILURE;
+  return NS_ERROR_DOM_SYNTAX_ERR;
 }
 
 float
@@ -244,31 +242,35 @@ nsSVGAngle::SetBaseValueInSpecifiedUnits(float aValue,
   aSVGElement->DidChangeAngle(mAttrEnum, PR_TRUE);
 }
 
-void
+nsresult
 nsSVGAngle::ConvertToSpecifiedUnits(PRUint16 unitType,
                                     nsSVGElement *aSVGElement)
 {
   if (!IsValidUnitType(unitType))
-    return;
+    return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
 
   float valueInUserUnits = mBaseVal / GetUnitScaleFactor();
   mSpecifiedUnitType = PRUint8(unitType);
   SetBaseValue(valueInUserUnits, aSVGElement);
+  return NS_OK;
 }
 
-void
+nsresult
 nsSVGAngle::NewValueSpecifiedUnits(PRUint16 unitType,
                                    float valueInSpecifiedUnits,
                                    nsSVGElement *aSVGElement)
 {
+  NS_ENSURE_FINITE(valueInSpecifiedUnits, NS_ERROR_ILLEGAL_VALUE);
+
   if (!IsValidUnitType(unitType))
-    return;
+    return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
 
   mBaseVal = mAnimVal = valueInSpecifiedUnits;
   mSpecifiedUnitType = PRUint8(unitType);
   if (aSVGElement) {
     aSVGElement->DidChangeAngle(mAttrEnum, PR_TRUE);
   }
+  return NS_OK;
 }
 
 nsresult

content/svg/content/src/nsSVGBoolean.cpp

@@ -61,7 +61,7 @@ nsSVGBoolean::SetBaseValueString(const nsAString &aValueAsString,
   else if (aValueAsString.EqualsLiteral("false"))
     val = PR_FALSE;
   else
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_DOM_SYNTAX_ERR;
 
   mBaseVal = mAnimVal = val;
   return NS_OK;

content/svg/content/src/nsSVGEnum.cpp

@@ -79,7 +79,7 @@ nsSVGEnum::SetBaseValueString(const nsAString& aValue,
 
   // only a warning since authors may mistype attribute values
   NS_WARNING("unknown enumeration key");
-  return NS_ERROR_FAILURE;
+  return NS_ERROR_DOM_SYNTAX_ERR;
 }
 
 void
@@ -112,7 +112,7 @@ nsSVGEnum::SetBaseValue(PRUint16 aValue,
     }
     tmp++;
   }
-  return NS_ERROR_FAILURE;
+  return NS_ERROR_DOM_SYNTAX_ERR;
 }
 
 nsresult

content/svg/content/src/nsSVGInteger.cpp

@@ -59,12 +59,12 @@ nsSVGInteger::SetBaseValueString(const nsAString &aValueAsString,
   const char *str = value.get();
 
   if (NS_IsAsciiWhitespace(*str))
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_DOM_SYNTAX_ERR;
   
   char *rest;
   PRInt32 val = strtol(str, &rest, 10);
   if (rest == str || *rest != '\0') {
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_DOM_SYNTAX_ERR;
   }
 
   mBaseVal = mAnimVal = val;

content/svg/content/src/nsSVGLength.cpp

@@ -396,12 +396,12 @@ nsSVGLength::SetValueAsString(const nsAString & aValueAsString)
         DidModify();
       } else { // parse error
         // not a valid unit type
-        rv = NS_ERROR_FAILURE;
+        rv = NS_ERROR_DOM_SYNTAX_ERR;
       }
     }
     else { // parse error
       // no number
-      rv = NS_ERROR_FAILURE;
+      rv = NS_ERROR_DOM_SYNTAX_ERR;
     }
   }
 
@@ -417,7 +417,7 @@ nsSVGLength::NewValueSpecifiedUnits(PRUint16 unitType, float valueInSpecifiedUni
   NS_ENSURE_FINITE(valueInSpecifiedUnits, NS_ERROR_ILLEGAL_VALUE);
 
   if (!IsValidUnitType(unitType))
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
 
   WillModify();
   mValueInSpecifiedUnits = valueInSpecifiedUnits;
@@ -432,7 +432,7 @@ NS_IMETHODIMP
 nsSVGLength::ConvertToSpecifiedUnits(PRUint16 unitType)
 {
   if (!IsValidUnitType(unitType))
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
 
   WillModify();
   float valueInUserUnits;

content/svg/content/src/nsSVGLength2.cpp

@@ -170,7 +170,7 @@ GetValueFromString(const nsAString &aValueAsString,
   const char *str = value.get();
 
   if (NS_IsAsciiWhitespace(*str))
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_DOM_SYNTAX_ERR;
   
   char *rest;
   *aValue = float(PR_strtod(str, &rest));
@@ -181,7 +181,7 @@ GetValueFromString(const nsAString &aValueAsString,
     }
   }
   
-  return NS_ERROR_FAILURE;
+  return NS_ERROR_DOM_SYNTAX_ERR;
 }
 
 float
@@ -340,26 +340,30 @@ nsSVGLength2::SetBaseValueInSpecifiedUnits(float aValue,
 #endif
 }
 
-void
+nsresult
 nsSVGLength2::ConvertToSpecifiedUnits(PRUint16 unitType,
                                       nsSVGElement *aSVGElement)
 {
   if (!IsValidUnitType(unitType))
-    return;
+    return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
 
   float valueInUserUnits = 
     mBaseVal / GetUnitScaleFactor(aSVGElement, mSpecifiedUnitType);
   mSpecifiedUnitType = PRUint8(unitType);
   SetBaseValue(valueInUserUnits, aSVGElement);
+
+  return NS_OK;
 }
 
-void
+nsresult
 nsSVGLength2::NewValueSpecifiedUnits(PRUint16 unitType,
                                      float valueInSpecifiedUnits,
                                      nsSVGElement *aSVGElement)
 {
+  NS_ENSURE_FINITE(valueInSpecifiedUnits, NS_ERROR_ILLEGAL_VALUE);
+
   if (!IsValidUnitType(unitType))
-    return;
+    return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
 
   mBaseVal = mAnimVal = valueInSpecifiedUnits;
   mSpecifiedUnitType = PRUint8(unitType);
@@ -370,6 +374,7 @@ nsSVGLength2::NewValueSpecifiedUnits(PRUint16 unitType,
     aSVGElement->AnimationNeedsResample();
   }
 #endif
+  return NS_OK;
 }
 
 nsresult

content/svg/content/src/nsSVGLengthList.cpp

@@ -151,7 +151,7 @@ nsSVGLengthList::SetValueString(const nsAString& aValue)
     nsCOMPtr<nsISVGLength> length;
     NS_NewSVGLength(getter_AddRefs(length), NS_ConvertASCIItoUTF16(token));
     if (!length) {
-      rv = NS_ERROR_FAILURE;
+      rv = NS_ERROR_DOM_SYNTAX_ERR;
       break;
     }
     AppendElement(length);

content/svg/content/src/nsSVGMarkerElement.cpp

@@ -114,7 +114,7 @@ nsSVGOrientType::SetBaseValue(PRUint16 aValue,
       PR_TRUE);
     return NS_OK;
   }
-  return NS_ERROR_FAILURE;
+  return NS_ERROR_DOM_SYNTAX_ERR;
 }
 
 nsresult

content/svg/content/src/nsSVGNumber.cpp

@@ -39,6 +39,7 @@
 #include "nsSVGNumber.h"
 #include "nsTextFormatter.h"
 #include "prdtoa.h"
+#include "nsDOMError.h"
 #include "nsSVGValue.h"
 #include "nsISVGValueUtils.h"
 #include "nsContentUtils.h"
@@ -139,11 +140,10 @@ nsSVGNumber::SetValueString(const nsAString& aValue)
 
       // check to see if there is trailing stuff...
       if (*rest != '\0') {
-        rv = NS_ERROR_FAILURE;
-        NS_ERROR("trailing data in number value");
+        rv = NS_ERROR_DOM_SYNTAX_ERR;
       }
     } else {
-      rv = NS_ERROR_FAILURE;
+      rv = NS_ERROR_DOM_SYNTAX_ERR;
       // no number
     }
   }

content/svg/content/src/nsSVGNumber2.cpp

@@ -93,12 +93,12 @@ nsSVGNumber2::SetBaseValueString(const nsAString &aValueAsString,
   const char *str = value.get();
 
   if (NS_IsAsciiWhitespace(*str))
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_DOM_SYNTAX_ERR;
   
   char *rest;
   float val = float(PR_strtod(str, &rest));
   if (rest == str || *rest != '\0' || !NS_FloatIsFinite(val)) {
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_DOM_SYNTAX_ERR;
   }
 
   mBaseVal = mAnimVal = val;

content/svg/content/src/nsSVGNumberList.cpp

@@ -145,7 +145,7 @@ nsSVGNumberList::SetValueString(const nsAString& aValue)
       nsCOMPtr<nsIDOMSVGNumber> number;
       NS_NewSVGNumber(getter_AddRefs(number), val);
       if (!number) {
-        rv = NS_ERROR_FAILURE;
+        rv = NS_ERROR_DOM_SYNTAX_ERR;
         break;
       }
       AppendElement(number);

content/svg/content/src/nsSVGPointList.cpp

@@ -177,12 +177,12 @@ nsSVGPointList::SetValueString(const nsAString& aValue)
     
     float x = float(PR_strtod(token1, &end));
     if (*end != '\0' || !NS_FloatIsFinite(x)) {
-      rv = NS_ERROR_FAILURE;
+      rv = NS_ERROR_DOM_SYNTAX_ERR;
       break; // parse error
     }
     float y = float(PR_strtod(token2, &end));
     if (*end != '\0' || !NS_FloatIsFinite(y)) {
-      rv = NS_ERROR_FAILURE;
+      rv = NS_ERROR_DOM_SYNTAX_ERR;
       break; // parse error
     }
     
@@ -197,7 +197,7 @@ nsSVGPointList::SetValueString(const nsAString& aValue)
 
   if (token1 || NS_FAILED(rv)) {
     // there was a parse error or we ran out of memory
-    rv = NS_ERROR_FAILURE;
+    rv = NS_ERROR_DOM_SYNTAX_ERR;
   } else {
     WillModify();
     ReleasePoints();

content/svg/content/src/nsSVGPreserveAspectRatio.cpp

@@ -165,12 +165,12 @@ nsSVGPreserveAspectRatio::SetBaseValueString(const nsAString &aValueAsString,
 {
   if (aValueAsString.IsEmpty() ||
       NS_IsAsciiWhitespace(aValueAsString[0])) {
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_DOM_SYNTAX_ERR;
   }
 
   nsWhitespaceTokenizer tokenizer(aValueAsString);
   if (!tokenizer.hasMoreTokens()) {
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_DOM_SYNTAX_ERR;
   }
   const nsAString &token = tokenizer.nextToken();
 
@@ -181,24 +181,28 @@ nsSVGPreserveAspectRatio::SetBaseValueString(const nsAString &aValueAsString,
 
   if (val.mDefer) {
     if (!tokenizer.hasMoreTokens()) {
-      return NS_ERROR_FAILURE;
+      return NS_ERROR_DOM_SYNTAX_ERR;
     }
     rv = val.SetAlign(GetAlignForString(tokenizer.nextToken()));
   } else {
     rv = val.SetAlign(GetAlignForString(token));
   }
 
-  NS_ENSURE_SUCCESS(rv, NS_ERROR_FAILURE);
+  if (NS_FAILED(rv)) {
+    return NS_ERROR_DOM_SYNTAX_ERR;
+  }
 
   if (tokenizer.hasMoreTokens()) {
     rv = val.SetMeetOrSlice(GetMeetOrSliceForString(tokenizer.nextToken()));
-    NS_ENSURE_SUCCESS(rv, NS_ERROR_FAILURE);
+    if (NS_FAILED(rv)) {
+      return NS_ERROR_DOM_SYNTAX_ERR;
+    }
   } else {
     val.mMeetOrSlice = nsIDOMSVGPreserveAspectRatio::SVG_MEETORSLICE_MEET;
   }
 
   if (tokenizer.hasMoreTokens()) {
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_DOM_SYNTAX_ERR;
   }
 
   mAnimVal = mBaseVal = val;

content/svg/content/src/nsSVGTransformList.cpp

@@ -179,7 +179,7 @@ nsSVGTransformList::SetValueString(const nsAString& aValue)
 
   if (NS_FAILED(rv)) {
     // there was a parse error.
-    rv = NS_ERROR_FAILURE;
+    rv = NS_ERROR_DOM_SYNTAX_ERR;
   }
   else {
     WillModify();

content/svg/content/src/nsSVGViewBox.cpp

@@ -125,7 +125,7 @@ nsSVGViewBox::SetBaseValueString(const nsAString& aValue,
   }
   if (i!=4 || nsCRT::strtok(rest, delimiters, &rest)!=0) {
     // there was a parse error.
-    rv = NS_ERROR_FAILURE;
+    rv = NS_ERROR_DOM_SYNTAX_ERR;
   } else {
     SetBaseValue(vals[0], vals[1], vals[2], vals[3], aSVGElement, aDoSetAttr);
   }

content/svg/content/test/Makefile.in

@@ -68,6 +68,7 @@ _TEST_FILES = \
 		test_text.html \
 		text-helper.svg \
 		test_transform.xhtml \
+		test_valueAsString.xhtml \
 		test_valueLeaks.xhtml \
 		viewport-helper.svg \
 		test_viewport.html \

content/svg/content/test/test_valueAsString.xhtml

@@ -0,0 +1,62 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=539697 
+-->
+<head>
+  <title>Test SVGTransform behavior</title>
+  <script type="text/javascript" src="/MochiKit/packed.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=539697">Mozilla Bug 539697</a>
+<p id="display"></p>
+<div id="content">
+
+  <svg xmlns="http://www.w3.org/2000/svg" width="100%" height="1" id="svg">
+    <circle id='c' r='1em' display='none'/>
+    <marker id='m' orient='20rad' display='none'/>
+  </svg>
+
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+<![CDATA[
+
+SimpleTest.waitForExplicitFinish();
+
+function run()
+{
+  var c = document.getElementById('c');
+  var m = document.getElementById('m');
+
+  is(SVGLength.SVG_LENGTHTYPE_EMS, c.r.baseVal.unitType, 'unexpected units');
+  c.r.baseVal.valueAsString = '2px';
+  is(SVGLength.SVG_LENGTHTYPE_PX, c.r.baseVal.unitType, 'unexpected units');
+
+  try {
+    c.r.baseVal.valueAsString = 'rubbish';
+  } catch (e) {
+    is(DOMException.SYNTAX_ERR, e.code, 'syntax error expected');
+  }
+
+  is(SVGAngle.SVG_ANGLETYPE_RAD, m.orientAngle.baseVal.unitType, 'unexpected units');
+  m.orientAngle.baseVal.valueAsString = '2grad';
+  is(SVGAngle.SVG_ANGLETYPE_GRAD, m.orientAngle.baseVal.unitType, 'unexpected units');
+
+  try {
+    m.orientAngle.baseVal.valueAsString = 'rubbish';
+  } catch (e) {
+    is(DOMException.SYNTAX_ERR, e.code, 'syntax error expected');
+  }
+
+  SimpleTest.finish();
+}
+
+window.addEventListener("load", run, false);
+
+]]>
+</script>
+</pre>
+</body>
+</html>