344888 - Bogus assertion: "ASSERTION: invalid length type"

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jesse Ruderman]

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

Invalid documents shouldn't cause assertions.

Comment 1

[Jesse Ruderman]

Attachment: testcase

Comment 2

[Jesse Ruderman]

Attachment: patch removing the assertion from several files

Comment 3

[Robert Longson [:longsonr]]

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.

Comment 4

[Jesse Ruderman]

I prefer bugs to source code "todo" comments.

Comment 5

[Robert Longson [:longsonr]]

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.

Comment 6

[Robert Longson [:longsonr]]

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.

Comment 7

[Jonathan Watt [:jwatt]]

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.

Comment 8

[Jesse Ruderman]

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

Comment 9

[Jonathan Watt [:jwatt]]

Thanks. I've filed bug 378860.

Comment 10

[Jesse Ruderman]

Crashtest checked in.