back out 8247f7b038ce because it landed with the wrong bug # in commit message (said bug 785017, should've said bug 783915)
authorDaniel Holbert <dholbert@cs.stanford.edu>
Sat, 25 Aug 2012 10:01:48 -0700
changeset 105472 4b46b309c12631fc1412cef7526fcc5dca5cb86e
parent 105471 4a40ae179ea2d665a48979b7daaf741edcc54982
child 105473 9245897374c1a8647716680c89b4fdf49dc8c01c
push id55
push usershu@rfrn.org
push dateThu, 30 Aug 2012 01:33:09 +0000
bugs785017, 783915
milestone17.0a1
backs out8247f7b038ced1dcbe73187e650dd612fe8ab8b1
back out 8247f7b038ce because it landed with the wrong bug # in commit message (said bug 785017, should've said bug 783915)
content/svg/content/src/SVGAnimatedPreserveAspectRatio.cpp
content/svg/content/src/SVGAnimatedPreserveAspectRatio.h
content/svg/content/src/SVGFragmentIdentifier.cpp
content/svg/content/src/nsSVGElement.cpp
content/svg/content/src/nsSVGViewBox.cpp
content/svg/content/src/nsSVGViewBox.h
content/svg/content/test/test_fragments.html
--- a/content/svg/content/src/SVGAnimatedPreserveAspectRatio.cpp
+++ b/content/svg/content/src/SVGAnimatedPreserveAspectRatio.cpp
@@ -196,41 +196,36 @@ ToPreserveAspectRatio(const nsAString &a
   }
 
   *aValue = val;
   return NS_OK;
 }
 
 nsresult
 SVGAnimatedPreserveAspectRatio::SetBaseValueString(
-  const nsAString &aValueAsString, nsSVGElement *aSVGElement, bool aDoSetAttr)
+  const nsAString &aValueAsString, nsSVGElement *aSVGElement)
 {
   SVGPreserveAspectRatio val;
   nsresult res = ToPreserveAspectRatio(aValueAsString, &val);
   if (NS_FAILED(res)) {
     return res;
   }
 
-  nsAttrValue emptyOrOldValue;
-  if (aDoSetAttr) {
-    emptyOrOldValue = aSVGElement->WillChangePreserveAspectRatio();
-  }
-
   mBaseVal = val;
   mIsBaseSet = true;
-
   if (!mIsAnimated) {
     mAnimVal = mBaseVal;
   }
-  if (aDoSetAttr) {
-    aSVGElement->DidChangePreserveAspectRatio(emptyOrOldValue);
-  }
-  if (mIsAnimated) {
+  else {
     aSVGElement->AnimationNeedsResample();
   }
+
+  // We don't need to call DidChange* here - we're only called by
+  // nsSVGElement::ParseAttribute under nsGenericElement::SetAttr,
+  // which takes care of notifying.
   return NS_OK;
 }
 
 void
 SVGAnimatedPreserveAspectRatio::GetBaseValueString(
   nsAString& aValueAsString) const
 {
   nsAutoString tmpString;
--- a/content/svg/content/src/SVGAnimatedPreserveAspectRatio.h
+++ b/content/svg/content/src/SVGAnimatedPreserveAspectRatio.h
@@ -87,18 +87,17 @@ public:
     mBaseVal.mMeetOrSlice = nsIDOMSVGPreserveAspectRatio::SVG_MEETORSLICE_MEET;
     mBaseVal.mDefer = false;
     mAnimVal = mBaseVal;
     mIsAnimated = false;
     mIsBaseSet = false;
   }
 
   nsresult SetBaseValueString(const nsAString& aValue,
-                              nsSVGElement *aSVGElement,
-                              bool aDoSetAttr);
+                              nsSVGElement *aSVGElement);
   void GetBaseValueString(nsAString& aValue) const;
 
   void SetBaseValue(const SVGPreserveAspectRatio &aValue,
                     nsSVGElement *aSVGElement);
   nsresult SetBaseAlign(uint16_t aAlign, nsSVGElement *aSVGElement) {
     if (aAlign < nsIDOMSVGPreserveAspectRatio::SVG_PRESERVEASPECTRATIO_NONE ||
         aAlign > nsIDOMSVGPreserveAspectRatio::SVG_PRESERVEASPECTRATIO_XMAXYMAX) {
       return NS_ERROR_FAILURE;
--- a/content/svg/content/src/SVGFragmentIdentifier.cpp
+++ b/content/svg/content/src/SVGFragmentIdentifier.cpp
@@ -1,36 +1,31 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "SVGFragmentIdentifier.h"
+#include "mozilla/CharTokenizer.h"
 #include "nsIDOMSVGDocument.h"
 #include "nsSVGSVGElement.h"
 #include "nsSVGViewElement.h"
 
 using namespace mozilla;
 
 static bool
 IsMatchingParameter(const nsAString &aString, const nsAString &aParameterName)
 {
   // The first two tests ensure aString.Length() > aParameterName.Length()
   // so it's then safe to do the third test
   return StringBeginsWith(aString, aParameterName) &&
          aString.Last() == ')' &&
          aString.CharAt(aParameterName.Length()) == '(';
 }
 
-inline bool
-IgnoreWhitespace(PRUnichar aChar)
-{
-  return false;
-}
-
 static nsSVGViewElement*
 GetViewElement(nsIDocument *aDocument, const nsAString &aId)
 {
   dom::Element* element = aDocument->GetElementById(aId);
   return (element && element->IsSVG(nsGkAtoms::view)) ?
             static_cast<nsSVGViewElement*>(element) : nullptr;
 }
 
@@ -94,30 +89,29 @@ SVGFragmentIdentifier::RestoreOldZoomAnd
 bool
 SVGFragmentIdentifier::ProcessSVGViewSpec(const nsAString &aViewSpec,
                                           nsSVGSVGElement *root)
 {
   if (!IsMatchingParameter(aViewSpec, NS_LITERAL_STRING("svgView"))) {
     return false;
   }
 
-  // SVGViewAttributes may occur in any order, but each type may only occur
-  // at most one time in a correctly formed SVGViewSpec.
-  // If we encounter any attribute more than once or get any syntax errors
-  // we're going to return false and cancel any changes.
-  
-  bool viewBoxFound = false;
-  bool preserveAspectRatioFound = false;
-  bool zoomAndPanFound = false;
+  // SVGViewAttribute may occur in any order, but each type may only occur at most one time
+  // in a correctly formed SVGViewSpec.
+  // If we encounter any element more than once or get any syntax errors we're going to
+  // return without updating the root element
+
+  const nsAString *viewBoxParams = nullptr;
+  const nsAString *preserveAspectRatioParams = nullptr;
+  const nsAString *zoomAndPanParams = nullptr;
 
   // Each token is a SVGViewAttribute
   int32_t bracketPos = aViewSpec.FindChar('(');
-  uint32_t lengthOfViewSpec = aViewSpec.Length() - bracketPos - 2;
-  nsCharSeparatedTokenizerTemplate<IgnoreWhitespace> tokenizer(
-    Substring(aViewSpec, bracketPos + 1, lengthOfViewSpec), ';');
+  CharTokenizer<';'>tokenizer(
+    Substring(aViewSpec, bracketPos + 1, aViewSpec.Length() - bracketPos - 2));
 
   if (!tokenizer.hasMoreTokens()) {
     return false;
   }
   do {
 
     nsAutoString token(tokenizer.nextToken());
 
@@ -126,72 +120,60 @@ SVGFragmentIdentifier::ProcessSVGViewSpe
       // invalid SVGViewAttribute syntax
       return false;
     }
 
     const nsAString &params =
       Substring(token, bracketPos + 1, token.Length() - bracketPos - 2);
 
     if (IsMatchingParameter(token, NS_LITERAL_STRING("viewBox"))) {
-      if (viewBoxFound ||
-          NS_FAILED(root->mViewBox.SetBaseValueString(
-                      params, root, true))) {
-        return false;
-      }
-      viewBoxFound = true;
-    } else if (IsMatchingParameter(token, NS_LITERAL_STRING("preserveAspectRatio"))) {
-      if (preserveAspectRatioFound ||
-          NS_FAILED(root->mPreserveAspectRatio.SetBaseValueString(
-                      params, root, true))) {
-        return false;
-      }
-      preserveAspectRatioFound = true;
-    } else if (IsMatchingParameter(token, NS_LITERAL_STRING("zoomAndPan"))) {
-      if (zoomAndPanFound) {
+      if (viewBoxParams) {
         return false;
       }
-      nsIAtom *valAtom = NS_GetStaticAtom(params);
-      if (!valAtom) {
+      viewBoxParams = &params;
+    } else if (IsMatchingParameter(token, NS_LITERAL_STRING("preserveAspectRatio"))) {
+      if (preserveAspectRatioParams) {
         return false;
       }
-      const nsSVGEnumMapping *mapping = nsSVGSVGElement::sZoomAndPanMap;
-      while (mapping->mKey) {
-        if (valAtom == *(mapping->mKey)) {
-          // If we've got a valid zoomAndPan value, then set it on our root element.
-          if (NS_FAILED(root->mEnumAttributes[nsSVGSVGElement::ZOOMANDPAN].SetBaseValue(
-                          mapping->mVal, root))) {
-            return false;
-          }
-          break;
-        }
-        mapping++;
+      preserveAspectRatioParams = &params;
+    } else if (IsMatchingParameter(token, NS_LITERAL_STRING("zoomAndPan"))) {
+      if (zoomAndPanParams) {
+        return false;
       }
-      if (!mapping->mKey) {
-          // Unrecognised zoomAndPan value
-          return false;
-      }
-      zoomAndPanFound = true;
+      zoomAndPanParams = &params;
     } else {
       // We don't support transform or viewTarget currently
       return false;
     }
   } while (tokenizer.hasMoreTokens());
 
-  if (root->mUseCurrentView) {
-    // A previous SVGViewSpec may have overridden some attributes.
-    // If they are no longer overridden we need to restore the old values.
-    if (!viewBoxFound) {
-      RestoreOldViewBox(root);
+  if (viewBoxParams) {
+    root->mViewBox.SetBaseValueString(*viewBoxParams, root);
+  } else {
+    RestoreOldViewBox(root);
+  }
+
+  if (preserveAspectRatioParams) {
+    root->mPreserveAspectRatio.SetBaseValueString(*preserveAspectRatioParams, root);
+  } else {
+    RestoreOldPreserveAspectRatio(root);
+  }
+
+  if (zoomAndPanParams) {
+    nsCOMPtr<nsIAtom> valAtom = do_GetAtom(*zoomAndPanParams);
+    const nsSVGEnumMapping *mapping = root->sZoomAndPanMap;
+    while (mapping->mKey) {
+      if (valAtom == *(mapping->mKey)) {
+        root->mEnumAttributes[nsSVGSVGElement::ZOOMANDPAN].SetBaseValue(mapping->mVal, root);
+        break;
+      }
+      mapping++;
     }
-    if (!preserveAspectRatioFound) {
-      RestoreOldPreserveAspectRatio(root);
-    }
-    if (!zoomAndPanFound) {
-      RestoreOldZoomAndPan(root);
-    }
+  } else {
+    RestoreOldZoomAndPan(root);
   }
 
   return true;
 }
 
 bool
 SVGFragmentIdentifier::ProcessFragmentIdentifier(nsIDocument *aDocument,
                                                  const nsAString &aAnchorName)
--- a/content/svg/content/src/nsSVGElement.cpp
+++ b/content/svg/content/src/nsSVGElement.cpp
@@ -516,31 +516,31 @@ nsSVGElement::ParseAttribute(int32_t aNa
       }
     }
 
     if (!foundMatch) {
       // Check for nsSVGViewBox attribute
       if (aAttribute == nsGkAtoms::viewBox) {
         nsSVGViewBox* viewBox = GetViewBox();
         if (viewBox) {
-          rv = viewBox->SetBaseValueString(aValue, this, false);
+          rv = viewBox->SetBaseValueString(aValue, this);
           if (NS_FAILED(rv)) {
             viewBox->Init();
           } else {
             aResult.SetTo(*viewBox, &aValue);
             didSetResult = true;
           }
           foundMatch = true;
         }
       // Check for SVGAnimatedPreserveAspectRatio attribute
       } else if (aAttribute == nsGkAtoms::preserveAspectRatio) {
         SVGAnimatedPreserveAspectRatio *preserveAspectRatio =
           GetPreserveAspectRatio();
         if (preserveAspectRatio) {
-          rv = preserveAspectRatio->SetBaseValueString(aValue, this, false);
+          rv = preserveAspectRatio->SetBaseValueString(aValue, this);
           if (NS_FAILED(rv)) {
             preserveAspectRatio->Init();
           } else {
             aResult.SetTo(*preserveAspectRatio, &aValue);
             didSetResult = true;
           }
           foundMatch = true;
         }
--- a/content/svg/content/src/nsSVGViewBox.cpp
+++ b/content/svg/content/src/nsSVGViewBox.cpp
@@ -143,35 +143,30 @@ ToSVGViewBoxRect(const nsAString& aStr, 
   aViewBox->width = vals[2];
   aViewBox->height = vals[3];
 
   return NS_OK;
 }
 
 nsresult
 nsSVGViewBox::SetBaseValueString(const nsAString& aValue,
-                                 nsSVGElement *aSVGElement,
-                                 bool aDoSetAttr)
+                                 nsSVGElement *aSVGElement)
 {
   nsSVGViewBoxRect viewBox;
   nsresult res = ToSVGViewBoxRect(aValue, &viewBox);
   if (NS_SUCCEEDED(res)) {
-    nsAttrValue emptyOrOldValue;
-    if (aDoSetAttr) {
-      emptyOrOldValue = aSVGElement->WillChangeViewBox();
-    }
     mBaseVal = nsSVGViewBoxRect(viewBox.x, viewBox.y, viewBox.width, viewBox.height);
     mHasBaseVal = true;
 
-    if (aDoSetAttr) {
-      aSVGElement->DidChangeViewBox(emptyOrOldValue);
-    }
     if (mAnimVal) {
       aSVGElement->AnimationNeedsResample();
     }
+    // We don't need to call Will/DidChange* here - we're only called by
+    // nsSVGElement::ParseAttribute under nsGenericElement::SetAttr,
+    // which takes care of notifying.
   }
   return res;
 }
 
 void
 nsSVGViewBox::GetBaseValueString(nsAString& aValue) const
 {
   PRUnichar buf[200];
--- a/content/svg/content/src/nsSVGViewBox.h
+++ b/content/svg/content/src/nsSVGViewBox.h
@@ -59,18 +59,17 @@ public:
                     nsSVGElement *aSVGElement)
     { SetBaseValue(nsSVGViewBoxRect(aX, aY, aWidth, aHeight), aSVGElement); }
   const nsSVGViewBoxRect& GetAnimValue() const
     { return mAnimVal ? *mAnimVal : mBaseVal; }
   void SetAnimValue(float aX, float aY, float aWidth, float aHeight,
                     nsSVGElement *aSVGElement);
 
   nsresult SetBaseValueString(const nsAString& aValue,
-                              nsSVGElement *aSVGElement,
-                              bool aDoSetAttr);
+                              nsSVGElement *aSVGElement);
   void GetBaseValueString(nsAString& aValue) const;
 
   nsresult ToDOMAnimatedRect(nsIDOMSVGAnimatedRect **aResult,
                              nsSVGElement *aSVGElement);
   // Returns a new nsISMILAttr object that the caller must delete
   nsISMILAttr* ToSMILAttr(nsSVGElement* aSVGElement);
   
 private:
--- a/content/svg/content/test/test_fragments.html
+++ b/content/svg/content/test/test_fragments.html
@@ -14,69 +14,46 @@ https://bugzilla.mozilla.org/show_bug.cg
 <div id="content" style="display: none"></div>
 
 <iframe id="svg" src="fragments-helper.svg"></iframe>
 
 <pre id="test">
 <script class="testbody" type="application/javascript">
 SimpleTest.waitForExplicitFinish();
 
-function Test(svgFragmentIdentifier, valid, viewBoxString,
-              preserveAspectRatioString, zoomAndPanString)
-{
+function Test(svgFragmentIdentifier, valid) {
     this.svgFragmentIdentifier = svgFragmentIdentifier;
     this.valid = valid;
-    this.viewBoxString = viewBoxString;
-    this.preserveAspectRatioString = preserveAspectRatioString;
-    this.zoomAndPanString = zoomAndPanString;
 }
 
 function runTests()
 {
   var svg = $("svg");
   var doc = svg.contentWindow.document;
   
   var tests = [
-      new Test("view", true, "0 200 100 100", "none", null),
-      new Test("unknown", false, null, null, null),
-      new Test("svgView(viewBox(0,0,200,200))", true, "0 0 200 200", null, null),
-      new Test("svgView(preserveAspectRatio(xMaxYMin slice))", true, null, "xMaxYMin slice", null),
-      new Test("svgView(viewBox(1,2,3,4);preserveAspectRatio(xMinYMax))", true, "1 2 3 4", "xMinYMax meet", null),
-      new Test("svgView(zoomAndPan(disable))", true, null, null, "disable"),
-      new Test("svgView(viewBox(bad)", false, null, null, null),
-      new Test("svgView(preserveAspectRatio(bad))", false, null, null, null),
-      new Test("svgView(zoomAndPan(bad))", false, null, null, null),
-      new Test("svgView", false, null, null, null),
-      new Test("svgView(", false, null, null, null),
-      new Test("svgView()", false, null, null, null),
-      // Be sure we verify that there's a closing paren for svgView()
-      // (and not too many closing parens)
-      new Test("svgView(zoomAndPan(disable)", false, null, null, null),
-      new Test("svgView(zoomAndPan(disable) ", false, null, null, null),
-      new Test("svgView(zoomAndPan(disable)]", false, null, null, null),
-      new Test("svgView(zoomAndPan(disable)))", false, null, null, null)
+      new Test("view", true),
+      new Test("unknown", false),
+      new Test("svgView(viewBox(0,0,200,200)))", true),
+      new Test("svgView(preserveAspectRatio(xMaxYMin slice))", true),
+      new Test("svgView(viewBox(1,2,3,4);preserveAspectRatio(xMaxYMin))", true),
+      new Test("svgView(zoomAndPan(disable))", true),
+      new Test("svgView", false),
+      new Test("svgView(", false),
+      new Test("svgView()", false)
   ];
 
-  var src = svg.getAttribute("src");
-  for (var i = 0; i < tests.length; i++) {
-    var test = tests[i];
-    svg.setAttribute("src", src + "#" + test.svgFragmentIdentifier);
-    is(doc.rootElement.useCurrentView, test.valid,
-       "Expected " + test.svgFragmentIdentifier + " to be " +
-       (test.valid ? "valid" : "invalid"));
-
-    is(doc.rootElement.getAttribute("viewBox"),
-       test.viewBoxString, "unexpected viewBox");
-
-    is(doc.rootElement.getAttribute("preserveAspectRatio"),
-       test.preserveAspectRatioString, "unexpected preserveAspectRatio");
-
-    is(doc.rootElement.getAttribute("zoomAndPan"),
-       test.zoomAndPanString, "unexpected zoomAndPan");
-  }
+ var src = svg.getAttribute("src");
+ for (var i = 0; i < tests.length; i++) {
+   var test = tests[i];
+   svg.setAttribute("src", src + "#" + test.svgFragmentIdentifier);
+   is(doc.rootElement.useCurrentView, test.valid,
+      "Expected " + test.svgFragmentIdentifier + " to be " +
+      (test.valid ? "valid" : "invalid"));
+ }
 
   SimpleTest.finish();
 }
 
 window.addEventListener("load", runTests, false);
 </script>
 </pre>
 </body>