Bug 731813 - Do not attempt to serialize SVG attributes for mutation events if they do not exist. r=jwatt,bzbarsky
authorCameron McCormack <cam@mcc.id.au>
Mon, 05 Mar 2012 10:38:03 +1100
changeset 88597 a19f6fd4f6f2db4af1be3d44221f3200bf367d84
parent 88596 2ea21a51d22a43b5228d85e1c4a8a0a814c0664f
child 88598 64a62f199dbd1131dac9599cffd65e9338349265
push id591
push usertim.taubert@gmx.de
push dateMon, 12 Mar 2012 08:42:51 +0000
treeherderfx-team@c6f26a8dcd08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt, bzbarsky
bugs731813
milestone13.0a1
Bug 731813 - Do not attempt to serialize SVG attributes for mutation events if they do not exist. r=jwatt,bzbarsky
content/base/src/nsGenericElement.cpp
content/svg/content/src/nsSVGElement.cpp
content/svg/content/test/test_SVGLengthList.xhtml
content/svg/content/test/test_SVGNumberList.xhtml
content/svg/content/test/test_SVGPathSegList.xhtml
content/svg/content/test/test_SVGPointList.xhtml
content/svg/content/test/test_SVGTransformList.xhtml
content/svg/content/test/test_SVGxxxList.xhtml
content/svg/content/test/test_dataTypesModEvents.html
--- a/content/base/src/nsGenericElement.cpp
+++ b/content/base/src/nsGenericElement.cpp
@@ -2731,16 +2731,19 @@ nsGenericElement::SetAttribute(const nsA
 }
 
 nsresult
 nsGenericElement::RemoveAttribute(const nsAString& aName)
 {
   const nsAttrName* name = InternalGetExistingAttrNameFromQName(aName);
 
   if (!name) {
+    // If there is no canonical nsAttrName for this attribute name, then the
+    // attribute does not exist and we can't get its namespace ID and
+    // local name below, so we return early.
     return NS_OK;
   }
 
   // Hold a strong reference here so that the atom or nodeinfo doesn't go
   // away during UnsetAttr. If it did UnsetAttr would be left with a
   // dangling pointer as argument without knowing it.
   nsAttrName tmp(*name);
 
@@ -2884,18 +2887,19 @@ nsresult
 nsGenericElement::RemoveAttributeNS(const nsAString& aNamespaceURI,
                                     const nsAString& aLocalName)
 {
   nsCOMPtr<nsIAtom> name = do_GetAtom(aLocalName);
   PRInt32 nsid =
     nsContentUtils::NameSpaceManager()->GetNameSpaceID(aNamespaceURI);
 
   if (nsid == kNameSpaceID_Unknown) {
-    // Unknown namespace means no attr...
-
+    // If the namespace ID is unknown, it means there can't possibly be an
+    // existing attribute. We would need a known namespace ID to pass into
+    // UnsetAttr, so we return early if we don't have one.
     return NS_OK;
   }
 
   UnsetAttr(nsid, name, true);
 
   return NS_OK;
 }
 
--- a/content/svg/content/src/nsSVGElement.cpp
+++ b/content/svg/content/src/nsSVGElement.cpp
@@ -1461,20 +1461,24 @@ nsSVGElement::MaybeSerializeAttrBeforeRe
 {
   if (!aNotify ||
       !nsContentUtils::HasMutationListeners(this,
                                             NS_EVENT_BITS_MUTATION_ATTRMODIFIED,
                                             this)) {
     return;
   }
 
+  const nsAttrValue* attrValue = mAttrsAndChildren.GetAttr(aName);
+  if (!attrValue)
+    return;
+
   nsAutoString serializedValue;
-  mAttrsAndChildren.GetAttr(aName)->ToString(serializedValue);
-  nsAttrValue attrValue(serializedValue);
-  mAttrsAndChildren.SetAndTakeAttr(aName, attrValue);
+  attrValue->ToString(serializedValue);
+  nsAttrValue oldAttrValue(serializedValue);
+  mAttrsAndChildren.SetAndTakeAttr(aName, oldAttrValue);
 }
 
 /* static */
 nsIAtom* nsSVGElement::GetEventNameForAttr(nsIAtom* aAttr)
 {
   if (aAttr == nsGkAtoms::onload)
     return nsGkAtoms::onSVGLoad;
   if (aAttr == nsGkAtoms::onunload)
--- a/content/svg/content/test/test_SVGLengthList.xhtml
+++ b/content/svg/content/test/test_SVGLengthList.xhtml
@@ -56,16 +56,23 @@ function run_tests()
   eventChecker.expect("modify");
   lengths[0].valueAsString = "10";
   eventChecker.expect("");
   lengths[0].value = 10;
   lengths[0].valueInSpecifiedUnits = 10;
   lengths[0].valueAsString = "10";
   lengths[0].convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER);
   lengths[0].newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 10);
+  // -- Attribute removal
+  eventChecker.expect("remove");
+  text.removeAttribute("x");
+  // -- Non-existent attribute removal
+  eventChecker.expect("");
+  text.removeAttribute("x");
+  text.removeAttributeNS(null, "x");
   eventChecker.finish();
 
   SimpleTest.finish();
 }
 
 window.addEventListener("load", run_tests, false);
 
 ]]>
--- a/content/svg/content/test/test_SVGNumberList.xhtml
+++ b/content/svg/content/test/test_SVGNumberList.xhtml
@@ -45,16 +45,23 @@ function run_tests()
   eventChecker.expect("modify modify");
   numbers[0].value = 15;
   text.setAttribute("rotate", "17 20 30");
   // -- Redundant changes
   eventChecker.expect("");
   numbers[0].value = 17;
   numbers[1].value = 20;
   text.setAttribute("rotate", "17 20 30");
+  // -- Attribute removal
+  eventChecker.expect("remove");
+  text.removeAttribute("rotate");
+  // -- Non-existent attribute removal
+  eventChecker.expect("");
+  text.removeAttribute("rotate");
+  text.removeAttributeNS(null, "rotate");
   eventChecker.finish();
 
   SimpleTest.finish();
 }
 
 window.addEventListener("load", run_tests, false);
 
 ]]>
--- a/content/svg/content/test/test_SVGPathSegList.xhtml
+++ b/content/svg/content/test/test_SVGPathSegList.xhtml
@@ -100,16 +100,25 @@ function run_tests()
   list[0].y = 5;
   path.setAttribute("d", "M20,5 L12,34");
 
   // -- Redundant changes
   eventChecker.expect("");
   list[0].x = 20;
   list[1].y = 34;
   path.setAttribute("d", "M20,5 L12,34");
+
+  // -- Attribute removal
+  eventChecker.expect("remove");
+  path.removeAttribute("d");
+
+  // -- Non-existent attribute removal
+  eventChecker.expect("");
+  path.removeAttribute("d");
+  path.removeAttributeNS(null, "d");
   eventChecker.finish();
 
   SimpleTest.finish();
 }
 
 window.addEventListener("load", run_tests, false);
 
 ]]>
--- a/content/svg/content/test/test_SVGPointList.xhtml
+++ b/content/svg/content/test/test_SVGPointList.xhtml
@@ -45,16 +45,23 @@ function run_tests()
   eventChecker.expect("modify modify");
   points[0].x = 40;
   polyline.setAttribute("points", "30,375 150,380");
   // -- Redundant changes
   eventChecker.expect("");
   points[0].x = 30;
   points[1].y = 380;
   polyline.setAttribute("points", "30,375 150,380");
+  // -- Attribute removal
+  eventChecker.expect("remove");
+  polyline.removeAttribute("points");
+  // -- Non-existent attribute removal
+  eventChecker.expect("");
+  polyline.removeAttribute("points");
+  polyline.removeAttributeNS(null, "points");
   eventChecker.finish();
 
   SimpleTest.finish();
 }
 
 window.addEventListener("load", run_tests, false);
 
 ]]>
--- a/content/svg/content/test/test_SVGTransformList.xhtml
+++ b/content/svg/content/test/test_SVGTransformList.xhtml
@@ -419,16 +419,26 @@ function testMutationEvents(g)
   list[0].matrix.e = 5;
 
   // setAttribute interaction
   eventChecker.expect("modify");
   list[0].setMatrix(mx);
   eventChecker.expect("");
   g.setAttribute("transform", "matrix(1, 0, 0, 1, 0, 0)");
   list[0].setMatrix(mx);
+
+  // Attribute removal
+  eventChecker.expect("remove");
+  g.removeAttribute("transform");
+
+  // Non-existent attribute removal
+  eventChecker.expect("");
+  g.removeAttribute("transform");
+  g.removeAttributeNS(null, "transform");
+
   eventChecker.finish();
 }
 
 window.addEventListener("load", main, false);
 
 ]]>
 </script>
 </pre>
--- a/content/svg/content/test/test_SVGxxxList.xhtml
+++ b/content/svg/content/test/test_SVGxxxList.xhtml
@@ -830,16 +830,17 @@ function run_baseVal_API_tests()
          'an object of the wrong type.');
       eventChecker.ignoreEvents();
     }
 
     // Test removal and addition events
 
     eventChecker.expect("remove add");
     t.element.removeAttribute(t.attr_name);
+    t.element.removeAttributeNS(null, t.attr_name);
     res = t.baseVal.appendItem(item);
     eventChecker.finish();
   }
 }
 
 
 /**
  * This function tests the SVGXxxList API for the anim val list (see also the
--- a/content/svg/content/test/test_dataTypesModEvents.html
+++ b/content/svg/content/test/test_dataTypesModEvents.html
@@ -33,16 +33,17 @@ function runTests()
 
   // class attribute
  
   eventChecker.watchAttr(filter, "class");
   eventChecker.expect("add modify remove add");
   filter.setAttribute("class", "foo");
   filter.className.baseVal = "bar";
   filter.removeAttribute("class");
+  filter.removeAttributeNS(null, "class");
   filter.className.baseVal = "foo";
 
   eventChecker.expect("");
   filter.className.baseVal = "foo";
   filter.setAttribute("class", "foo");
 
   // length attribute
 
@@ -52,16 +53,17 @@ function runTests()
   marker.markerWidth.baseVal.value = 8;
   marker.markerWidth.baseVal.valueInSpecifiedUnits = 9;
   marker.markerWidth.baseVal.valueAsString = "10";
   marker.markerWidth.baseVal.convertToSpecifiedUnits(
     SVGLength.SVG_LENGTHTYPE_CM);
   marker.markerWidth.baseVal.newValueSpecifiedUnits(
     SVGLength.SVG_LENGTHTYPE_MM, 11);
   marker.removeAttribute("markerWidth");
+  marker.removeAttributeNS(null, "markerWidth");
   marker.markerWidth.baseVal.value = 8;
 
   eventChecker.expect("remove add modify");
   marker.removeAttribute("markerWidth");
   console.log(marker.getAttribute("markerWidth"));
   marker.markerWidth.baseVal.convertToSpecifiedUnits(
     SVGLength.SVG_LENGTHTYPE_NUMBER);
   console.log(marker.getAttribute("markerWidth"));
@@ -79,43 +81,46 @@ function runTests()
 
   // number attribute
 
   eventChecker.watchAttr(convolve, "divisor");
   eventChecker.expect("add modify remove add");
   convolve.setAttribute("divisor", "12.5");
   convolve.divisor.baseVal = 6;
   convolve.removeAttribute("divisor");
+  convolve.removeAttributeNS(null, "divisor");
   convolve.divisor.baseVal = 8;
 
   eventChecker.expect("");
   convolve.divisor.baseVal = 8;
   convolve.setAttribute("divisor", "8");
 
   // number-optional-number attribute
 
   eventChecker.watchAttr(blur, "stdDeviation");
   eventChecker.expect("add modify remove add");
   blur.setAttribute("stdDeviation", "5, 6");
   blur.stdDeviationX.baseVal = 8;
   blur.removeAttribute("stdDeviation");
+  blur.removeAttributeNS(null, "stdDeviation");
   blur.setAttribute("stdDeviation", "2, 3");
 
   eventChecker.expect("");
   blur.stdDeviationX.baseVal = 2;
   blur.stdDeviationY.baseVal = 3;
   blur.setAttribute("stdDeviation", "2, 3");
 
   // integer attribute
 
   eventChecker.watchAttr(convolve, "targetX");
   eventChecker.expect("add modify remove add");
   convolve.setAttribute("targetX", "12");
   convolve.targetX.baseVal = 6;
   convolve.removeAttribute("targetX");
+  convolve.removeAttributeNS(null, "targetX");
   convolve.targetX.baseVal = 8;
 
   // Check redundant change when comparing typed value to attribute value
   eventChecker.expect("");
   convolve.setAttribute("targetX", "8");
   // Check redundant change when comparing attribute value to typed value
   eventChecker.expect("remove add");
   convolve.removeAttribute("targetX");
@@ -124,16 +129,17 @@ function runTests()
 
   // integer-optional-integer attribute
 
   eventChecker.watchAttr(filter, "filterRes");
   eventChecker.expect("add modify remove add");
   filter.setAttribute("filterRes", "60, 70");
   filter.filterResX.baseVal = 50;
   filter.removeAttribute("filterRes");
+  filter.removeAttributeNS(null, "filterRes");
   filter.setAttribute("filterRes", "50, 60");
 
   eventChecker.expect("");
   filter.filterResX.baseVal = 50;
   filter.setAttribute("filterRes", "50, 60");
   filter.filterResY.baseVal = 60;
 
   // angle attribute
@@ -144,16 +150,17 @@ function runTests()
   marker.orientAngle.baseVal.value = 12;
   marker.orientAngle.baseVal.valueInSpecifiedUnits = 23;
   marker.orientAngle.baseVal.valueAsString = "34";
   marker.orientAngle.baseVal.newValueSpecifiedUnits(
     SVGAngle.SVG_ANGLETYPE_GRAD, 34);
   marker.orientAngle.baseVal.newValueSpecifiedUnits(
     SVGAngle.SVG_ANGLETYPE_GRAD, 45);
   marker.removeAttribute("orient");
+  marker.removeAttributeNS(null, "orient");
   marker.orientAngle.baseVal.value = 40;
 
   eventChecker.expect("");
   marker.orientAngle.baseVal.value = 40;
   marker.orientAngle.baseVal.valueInSpecifiedUnits = 40;
   marker.orientAngle.baseVal.valueAsString = "40";
   marker.orientAngle.baseVal.convertToSpecifiedUnits(
     SVGAngle.SVG_ANGLETYPE_UNSPECIFIED);
@@ -162,73 +169,78 @@ function runTests()
 
   // boolean attribute
 
   eventChecker.watchAttr(convolve, "preserveAlpha");
   eventChecker.expect("add modify remove add");
   convolve.setAttribute("preserveAlpha", "true");
   convolve.preserveAlpha.baseVal = false;
   convolve.removeAttribute("preserveAlpha");
+  convolve.removeAttributeNS(null, "preserveAlpha");
   convolve.preserveAlpha.baseVal = true;
 
   eventChecker.expect("");
   convolve.preserveAlpha.baseVal = true;
   convolve.setAttribute("preserveAlpha", "true");
 
   // enum attribute
 
   eventChecker.watchAttr(convolve, "edgeMode");
   eventChecker.expect("add modify remove add");
   convolve.setAttribute("edgeMode", "none");
   convolve.edgeMode.baseVal = SVGFEConvolveMatrixElement.SVG_EDGEMODE_WRAP;
   convolve.removeAttribute("edgeMode");
+  convolve.removeAttributeNS(null, "edgeMode");
   convolve.edgeMode.baseVal = SVGFEConvolveMatrixElement.SVG_EDGEMODE_NONE;
 
   eventChecker.expect("");
   convolve.edgeMode.baseVal = SVGFEConvolveMatrixElement.SVG_EDGEMODE_NONE;
   convolve.setAttribute("edgeMode", "none");
   convolve.edgeMode.baseVal = SVGFEConvolveMatrixElement.SVG_EDGEMODE_NONE;
 
   // string attribute
 
   eventChecker.watchAttr(convolve, "result");
   eventChecker.expect("add modify remove add");
   convolve.setAttribute("result", "bar");
   convolve.result.baseVal = "foo";
   convolve.removeAttribute("result");
+  convolve.removeAttributeNS(null, "result");
   convolve.result.baseVal = "bar";
 
   eventChecker.expect("");
   convolve.result.baseVal = "bar";
   convolve.setAttribute("result", "bar");
   convolve.result.baseVal = "bar";
 
   // preserveAspectRatio attribute
 
   eventChecker.watchAttr(marker, "preserveAspectRatio");
   eventChecker.expect("add modify remove add");
   marker.setAttribute("preserveAspectRatio", "xMaxYMid slice");
   marker.preserveAspectRatio.baseVal.align =
     SVGPreserveAspectRatio.SVG_PRESERVEASPECTRATIO_XMAXYMAX;
   marker.removeAttribute("preserveAspectRatio");
+  marker.removeAttributeNS(null, "preserveAspectRatio");
   marker.preserveAspectRatio.baseVal.align =
     SVGPreserveAspectRatio.SVG_PRESERVEASPECTRATIO_XMIDYMIN;
 
   eventChecker.expect("");
   marker.preserveAspectRatio.baseVal.meetOrSlice =
     SVGPreserveAspectRatio.SVG_MEETORSLICE_MEET;
   marker.setAttribute("preserveAspectRatio", "xMidYMin meet");
 
   // viewBox attribute
 
   eventChecker.watchAttr(marker, "viewBox");
   eventChecker.expect("add modify remove add");
   marker.setAttribute("viewBox", "1 2 3 4");
   marker.viewBox.baseVal.height = 5;
   marker.removeAttribute("viewBox");
+  marker.removeAttributeNS(null, "viewBox");
   marker.viewBox.baseVal.height = 4;
 
   eventChecker.ignoreEvents();
   marker.setAttribute("viewBox", "1 2 3 4");
   eventChecker.expect("");
   marker.viewBox.baseVal.height = 4;
   marker.viewBox.baseVal.x = 1;
   marker.setAttribute("viewBox", "1 2 3 4");