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 88593 a19f6fd4f6f2db4af1be3d44221f3200bf367d84
parent 88592 2ea21a51d22a43b5228d85e1c4a8a0a814c0664f
child 88594 64a62f199dbd1131dac9599cffd65e9338349265
push id22208
push usermak77@bonardo.net
push dateFri, 09 Mar 2012 12:34:50 +0000
treeherdermozilla-central@ead9016b4102 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt, bzbarsky
bugs731813
milestone13.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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");