Jesse Ruderman Assignee
	Description • 18 years ago

###!!! ASSERTION: invalid length type: 'Error', file /Users/admin/trunk/mozilla/content/svg/content/src/nsSVGLength2.cpp, line 228

Invalid documents shouldn't cause assertions.

	Robert Longson [:longsonr]
	Comment 3 • 18 years ago

I think we should log something to the Error Console in such circumstances. We do that with other content errors. 

Since we haven't done this up to now in the svg subsystem, perhaps some kind of note in the code saying we should do this at these points would be sufficient to remind when we implement console logging.

	Jesse Ruderman Assignee
	Comment 4 • 18 years ago

I prefer bugs to source code "todo" comments.

	Robert Longson [:longsonr]
	Comment 5 • 18 years ago

You could add something like this in nsSVGElement::ParseAttribute to report the error to the console...

     // Check for nsSVGLength2 attribute
     LengthAttributesInfo lengthInfo = GetLengthInfo();
     PRUint32 i;
     for (i = 0; i < lengthInfo.mLengthCount; i++) {
       if (aAttribute == *lengthInfo.mLengthInfo[i].mName) {
         nsresult rv = lengthInfo.mLengths[i].SetBaseValueString(aValue, this,
                                                                 PR_FALSE);
         if (NS_FAILED(rv)) {
+          nsAutoString attributeName;
+          aAttribute->ToString(attributeName);
+          const nsAFlatString& attributeValue = PromiseFlatString(aValue);
+          const PRUnichar *strings[] = { attributeName.get(), attributeValue.ge
t() };
+          nsSVGUtils::ReportToConsole(GetOwnerDoc(),
+                                      "AttributeParseWarning",
+                                      strings, NS_ARRAY_LENGTH(strings));
           return PR_FALSE;
         }
         aResult.SetTo(aValue);
         return PR_TRUE;
       }
     }

     // Check for nsSVGNumber2 attribute

and the same for nsSVGNumber2.

	Robert Longson [:longsonr]
	Comment 6 • 18 years ago

I've checked in logging code for nsSVGLength2 and nsSVGNumber2 so we will get console logging now. 

As far my 2c is worth the asserts can just go.

	Jonathan Watt [:jwatt]
	Comment 7 • 18 years ago

Since we don't have logging for nsSVGAngle, it would be better to make the assertion a warning in that instance rather than remove it I think. I also think when an attempt is made to set an invalid value, that value should be give it's default. Please open a new bug if that doesn't get resolved here.

	Jesse Ruderman Assignee
	Comment 8 • 17 years ago

jwatt, I think you'll have to file the bug about resetting when someone tries to set a length to an invalid value.

	Jonathan Watt [:jwatt]
	Comment 9 • 17 years ago

Thanks. I've filed bug 378860.

	Jesse Ruderman Assignee
	Comment 10 • 17 years ago

Crashtest checked in.