Bug 785606 - Support viewBox=none from SVG 1.2 Tiny r=jwatt
authorRobert Longson <longsonr@gmail.com>
Tue, 26 Feb 2013 16:58:06 +0000
changeset 123031 87e9f84a9fc2c10b7eb29cc822b9064d9f4d880c
parent 123030 6bca0d84ff84359f92befedf62222a8ab853762b
child 123032 41d43416168d32db5683ffe0bccfc727c1a66b6f
push id24372
push useremorley@mozilla.com
push dateWed, 27 Feb 2013 13:22:59 +0000
treeherdermozilla-central@0a91da5f5eab [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt
bugs785606
milestone22.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 785606 - Support viewBox=none from SVG 1.2 Tiny r=jwatt
content/svg/content/src/SVGMarkerElement.cpp
content/svg/content/src/SVGSVGElement.cpp
content/svg/content/src/SVGSVGElement.h
content/svg/content/src/SVGViewBoxSMILType.cpp
content/svg/content/src/nsSVGViewBox.cpp
content/svg/content/src/nsSVGViewBox.h
content/svg/content/test/test_dataTypes.html
content/svg/content/test/test_dataTypesModEvents.html
content/svg/content/test/test_fragments.html
layout/reftests/svg/smil/anim-svg-viewBox-03.svg
layout/reftests/svg/smil/reftest.list
layout/svg/nsSVGInnerSVGFrame.cpp
layout/svg/nsSVGOuterSVGFrame.cpp
--- a/content/svg/content/src/SVGMarkerElement.cpp
+++ b/content/svg/content/src/SVGMarkerElement.cpp
@@ -316,17 +316,17 @@ SVGMarkerElement::GetMarkerTransform(flo
   return gfxMatrix(cos(angle) * scale,   sin(angle) * scale,
                    -sin(angle) * scale,  cos(angle) * scale,
                    aX,                    aY);
 }
 
 nsSVGViewBoxRect
 SVGMarkerElement::GetViewBoxRect()
 {
-  if (mViewBox.IsExplicitlySet()) {
+  if (mViewBox.HasRect()) {
     return mViewBox.GetAnimValue();
   }
   return nsSVGViewBoxRect(
            0, 0,
            mLengthAttributes[MARKERWIDTH].GetAnimValue(mCoordCtx),
            mLengthAttributes[MARKERHEIGHT].GetAnimValue(mCoordCtx));
 }
 
--- a/content/svg/content/src/SVGSVGElement.cpp
+++ b/content/svg/content/src/SVGSVGElement.cpp
@@ -842,22 +842,22 @@ SVGSVGElement::GetCurrentViewElement() c
   }
   return nullptr;
 }
 
 nsSVGViewBoxRect
 SVGSVGElement::GetViewBoxWithSynthesis(
   float aViewportWidth, float aViewportHeight) const
 {
-  // The logic here should match HasViewBox().
+  // The logic here should match HasViewBoxRect().
   SVGViewElement* viewElement = GetCurrentViewElement();
-  if (viewElement && viewElement->mViewBox.IsExplicitlySet()) {
+  if (viewElement && viewElement->mViewBox.HasRect()) {
     return viewElement->mViewBox.GetAnimValue();
   }
-  if (mViewBox.IsExplicitlySet()) {
+  if (mViewBox.HasRect()) {
     return mViewBox.GetAnimValue();
   }
 
   if (ShouldSynthesizeViewBox()) {
     // Special case -- fake a viewBox, using height & width attrs.
     // (Use |this| as context, since if we get here, we're outermost <svg>.)
     return nsSVGViewBoxRect(0, 0,
               ComputeSynthesizedViewBoxDimension(mLengthAttributes[ATTR_WIDTH],
@@ -880,21 +880,21 @@ SVGSVGElement::GetPreserveAspectRatioWit
     const SVGPreserveAspectRatio *pAROverridePtr = GetPreserveAspectRatioProperty();
     if (pAROverridePtr) {
       return *pAROverridePtr;
     }
   }
 
   SVGViewElement* viewElement = GetCurrentViewElement();
 
-  // This check is equivalent to "!HasViewBox() && ShouldSynthesizeViewBox()".
-  // We're just holding onto the viewElement that HasViewBox() would look up,
+  // This check is equivalent to "!HasViewBoxRect() && ShouldSynthesizeViewBox()".
+  // We're just holding onto the viewElement that HasViewBoxRect() would look up,
   // so that we don't have to look it up again later.
-  if (!((viewElement && viewElement->mViewBox.IsExplicitlySet()) ||
-        mViewBox.IsExplicitlySet()) &&
+  if (!((viewElement && viewElement->mViewBox.HasRect()) ||
+        mViewBox.HasRect()) &&
       ShouldSynthesizeViewBox()) {
     // If we're synthesizing a viewBox, use preserveAspectRatio="none";
     return SVGPreserveAspectRatio(SVG_PRESERVEASPECTRATIO_NONE, SVG_MEETORSLICE_SLICE);
   }
 
   if (viewElement && viewElement->mPreserveAspectRatio.IsExplicitlySet()) {
     return viewElement->mPreserveAspectRatio.GetAnimValue();
   }
@@ -907,20 +907,20 @@ SVGSVGElement::GetPreserveAspectRatioWit
 float
 SVGSVGElement::GetLength(uint8_t aCtxType)
 {
   float h, w;
 
   SVGViewElement* viewElement = GetCurrentViewElement();
   const nsSVGViewBoxRect* viewbox = nullptr;
 
-  // The logic here should match HasViewBox().
-  if (viewElement && viewElement->mViewBox.IsExplicitlySet()) {
+  // The logic here should match HasViewBoxRect().
+  if (viewElement && viewElement->mViewBox.HasRect()) {
     viewbox = &viewElement->mViewBox.GetAnimValue();
-  } else if (mViewBox.IsExplicitlySet()) {
+  } else if (mViewBox.HasRect()) {
     viewbox = &mViewBox.GetAnimValue();
   }
 
   if (viewbox) {
     w = viewbox->width;
     h = viewbox->height;
   } else if (IsInner()) {
     SVGSVGElement *ctx = GetCtx();
@@ -1024,29 +1024,29 @@ SVGSVGElement::GetViewBox()
 
 SVGAnimatedPreserveAspectRatio *
 SVGSVGElement::GetPreserveAspectRatio()
 {
   return &mPreserveAspectRatio;
 }
 
 bool
-SVGSVGElement::HasViewBox() const
+SVGSVGElement::HasViewBoxRect() const
 {
   SVGViewElement* viewElement = GetCurrentViewElement();
-  if (viewElement && viewElement->mViewBox.IsExplicitlySet()) {
+  if (viewElement && viewElement->mViewBox.HasRect()) {
     return true;
   }
-  return mViewBox.IsExplicitlySet();
+  return mViewBox.HasRect();
 }
 
 bool
 SVGSVGElement::ShouldSynthesizeViewBox() const
 {
-  NS_ABORT_IF_FALSE(!HasViewBox(),
+  NS_ABORT_IF_FALSE(!HasViewBoxRect(),
                     "Should only be called if we lack a viewBox");
 
   nsIDocument* doc = GetCurrentDoc();
   return doc &&
     doc->IsBeingUsedAsImage() &&
     !mIsPaintingSVGImageElement &&
     !GetParent();
 }
@@ -1105,26 +1105,26 @@ void
 SVGSVGElement::
   SetImageOverridePreserveAspectRatio(const SVGPreserveAspectRatio& aPAR)
 {
 #ifdef DEBUG
   NS_ABORT_IF_FALSE(GetCurrentDoc()->IsBeingUsedAsImage(),
                     "should only override preserveAspectRatio in images");
 #endif
 
-  bool hasViewBox = HasViewBox();
-  if (!hasViewBox && ShouldSynthesizeViewBox()) {
+  bool hasViewBoxRect = HasViewBoxRect();
+  if (!hasViewBoxRect && ShouldSynthesizeViewBox()) {
     // My non-<svg:image> clients will have been painting me with a synthesized
     // viewBox, but my <svg:image> client that's about to paint me now does NOT
     // want that.  Need to tell ourselves to flush our transform.
     mImageNeedsTransformInvalidation = true;
   }
   mIsPaintingSVGImageElement = true;
 
-  if (!hasViewBox) {
+  if (!hasViewBoxRect) {
     return; // preserveAspectRatio irrelevant (only matters if we have viewBox)
   }
 
   if (aPAR.GetDefer() && HasPreserveAspectRatio()) {
     return; // Referring element defers to my own preserveAspectRatio value.
   }
 
   if (SetPreserveAspectRatioProperty(aPAR)) {
@@ -1136,17 +1136,17 @@ void
 SVGSVGElement::ClearImageOverridePreserveAspectRatio()
 {
 #ifdef DEBUG
   NS_ABORT_IF_FALSE(GetCurrentDoc()->IsBeingUsedAsImage(),
                     "should only override image preserveAspectRatio in images");
 #endif
 
   mIsPaintingSVGImageElement = false;
-  if (!HasViewBox() && ShouldSynthesizeViewBox()) {
+  if (!HasViewBoxRect() && ShouldSynthesizeViewBox()) {
     // My non-<svg:image> clients will want to paint me with a synthesized
     // viewBox, but my <svg:image> client that just painted me did NOT
     // use that.  Need to tell ourselves to flush our transform.
     mImageNeedsTransformInvalidation = true;
   }
 
   if (ClearPreserveAspectRatioProperty()) {
     mImageNeedsTransformInvalidation = true;
--- a/content/svg/content/src/SVGSVGElement.h
+++ b/content/svg/content/src/SVGSVGElement.h
@@ -149,29 +149,29 @@ public:
    * has a valid viewBox.
    *
    * Note that this does not check whether we need to synthesize a viewBox,
    * so you must call ShouldSynthesizeViewBox() if you need to check that too.
    *
    * Note also that this method does not pay attention to whether the width or
    * height values of the viewBox rect are positive!
    */
-  bool HasViewBox() const;
+  bool HasViewBoxRect() const;
 
   /**
    * Returns true if we should synthesize a viewBox for ourselves (that is, if
    * we're the root element in an image document, and we're not currently being
    * painted for an <svg:image> element).
    *
-   * Only call this method if HasViewBox() returns false.
+   * Only call this method if HasViewBoxRect() returns false.
    */
   bool ShouldSynthesizeViewBox() const;
 
   bool HasViewBoxOrSyntheticViewBox() const {
-    return HasViewBox() || ShouldSynthesizeViewBox();
+    return HasViewBoxRect() || ShouldSynthesizeViewBox();
   }
 
   gfxMatrix GetViewBoxTransform() const;
 
   bool HasChildrenOnlyTransform() const {
     return mHasChildrenOnlyTransform;
   }
 
--- a/content/svg/content/src/SVGViewBoxSMILType.cpp
+++ b/content/svg/content/src/SVGViewBoxSMILType.cpp
@@ -13,18 +13,17 @@ namespace mozilla {
   
 /*static*/ SVGViewBoxSMILType SVGViewBoxSMILType::sSingleton;
 
 void
 SVGViewBoxSMILType::Init(nsSMILValue& aValue) const
 {
   NS_ABORT_IF_FALSE(aValue.IsNull(), "Unexpected value type");
 
-  nsSVGViewBoxRect* viewBox = new nsSVGViewBoxRect();
-  aValue.mU.mPtr = viewBox;
+  aValue.mU.mPtr = new nsSVGViewBoxRect();
   aValue.mType = this;
 }
 
 void
 SVGViewBoxSMILType::Destroy(nsSMILValue& aValue) const
 {
   NS_PRECONDITION(aValue.mType == this, "Unexpected SMIL value");
   delete static_cast<nsSVGViewBoxRect*>(aValue.mU.mPtr);
@@ -79,16 +78,20 @@ SVGViewBoxSMILType::ComputeDistance(cons
                                     double& aDistance) const
 {
   NS_PRECONDITION(aFrom.mType == aTo.mType,"Trying to compare different types");
   NS_PRECONDITION(aFrom.mType == this, "Unexpected source type");
 
   const nsSVGViewBoxRect* from = static_cast<const nsSVGViewBoxRect*>(aFrom.mU.mPtr);
   const nsSVGViewBoxRect* to = static_cast<const nsSVGViewBoxRect*>(aTo.mU.mPtr);
 
+  if (from->none || to->none) {
+    return NS_ERROR_FAILURE;
+  }
+
   // We use the distances between the edges rather than the difference between
   // the x, y, width and height for the "distance". This is necessary in
   // order for the "distance" result that we calculate to be the same for a
   // given change in the left side as it is for an equal change in the opposite
   // side. See https://bugzilla.mozilla.org/show_bug.cgi?id=541884#c12
 
   float dLeft = to->x - from->x;
   float dTop = to->y - from->y;
@@ -106,19 +109,24 @@ SVGViewBoxSMILType::Interpolate(const ns
                                 double aUnitDistance,
                                 nsSMILValue& aResult) const
 {
   NS_PRECONDITION(aStartVal.mType == aEndVal.mType,
                   "Trying to interpolate different types");
   NS_PRECONDITION(aStartVal.mType == this,
                   "Unexpected types for interpolation");
   NS_PRECONDITION(aResult.mType == this, "Unexpected result type");
-  
+
   const nsSVGViewBoxRect* start = static_cast<const nsSVGViewBoxRect*>(aStartVal.mU.mPtr);
   const nsSVGViewBoxRect* end = static_cast<const nsSVGViewBoxRect*>(aEndVal.mU.mPtr);
+
+  if (start->none || end->none) {
+    return NS_ERROR_FAILURE;
+  }
+
   nsSVGViewBoxRect* current = static_cast<nsSVGViewBoxRect*>(aResult.mU.mPtr);
 
   float x = (start->x + (end->x - start->x) * aUnitDistance);
   float y = (start->y + (end->y - start->y) * aUnitDistance);
   float width = (start->width + (end->width - start->width) * aUnitDistance);
   float height = (start->height + (end->height - start->height) * aUnitDistance);
 
   *current = nsSVGViewBoxRect(x, y, width, height);
--- a/content/svg/content/src/nsSVGViewBox.cpp
+++ b/content/svg/content/src/nsSVGViewBox.cpp
@@ -20,20 +20,22 @@ using namespace mozilla;
 /* Implementation of nsSVGViewBoxRect methods */
 
 bool
 nsSVGViewBoxRect::operator==(const nsSVGViewBoxRect& aOther) const
 {
   if (&aOther == this)
     return true;
 
-  return x == aOther.x &&
-    y == aOther.y &&
-    width == aOther.width &&
-    height == aOther.height;
+  return (none && aOther.none) ||
+    (!none && !aOther.none &&
+     x == aOther.x &&
+     y == aOther.y &&
+     width == aOther.width &&
+     height == aOther.height);
 }
 
 /* Cycle collection macros for nsSVGViewBox */
 
 NS_SVG_VAL_IMPL_CYCLE_COLLECTION(nsSVGViewBox::DOMBaseVal, mSVGElement)
 NS_SVG_VAL_IMPL_CYCLE_COLLECTION(nsSVGViewBox::DOMAnimVal, mSVGElement)
 NS_SVG_VAL_IMPL_CYCLE_COLLECTION(nsSVGViewBox::DOMAnimatedRect, mSVGElement)
 
@@ -74,43 +76,46 @@ static nsSVGAttrTearoffTable<nsSVGViewBo
   sAnimSVGViewBoxTearoffTable;
 
 
 /* Implementation of nsSVGViewBox methods */
 
 void
 nsSVGViewBox::Init()
 {
-  mBaseVal = nsSVGViewBoxRect();
+  mHasBaseVal = false;
   mAnimVal = nullptr;
-  mHasBaseVal = false;
 }
 
 void
-nsSVGViewBox::SetAnimValue(float aX, float aY, float aWidth, float aHeight,
+nsSVGViewBox::SetAnimValue(const nsSVGViewBoxRect& aRect,
                            nsSVGElement *aSVGElement)
 {
   if (!mAnimVal) {
     // it's okay if allocation fails - and no point in reporting that
-    mAnimVal = new nsSVGViewBoxRect(aX, aY, aWidth, aHeight);
+    mAnimVal = new nsSVGViewBoxRect(aRect);
   } else {
-    nsSVGViewBoxRect rect(aX, aY, aWidth, aHeight);
-    if (rect == *mAnimVal) {
+    if (aRect == *mAnimVal) {
       return;
     }
-    *mAnimVal = rect;
+    *mAnimVal = aRect;
   }
   aSVGElement->DidAnimateViewBox();
 }
 
 void
 nsSVGViewBox::SetBaseValue(const nsSVGViewBoxRect& aRect,
                            nsSVGElement *aSVGElement)
 {
-  if (mHasBaseVal && mBaseVal == aRect) {
+  if (!mHasBaseVal || mBaseVal == aRect) {
+    // This method is used to set a single x, y, width
+    // or height value. It can't create a base value
+    // as the other components may be undefined. We record
+    // the new value though, so as not to lose data.
+    mBaseVal = aRect;
     return;
   }
 
   nsAttrValue emptyOrOldValue = aSVGElement->WillChangeViewBox();
 
   mBaseVal = aRect;
   mHasBaseVal = true;
 
@@ -118,16 +123,21 @@ nsSVGViewBox::SetBaseValue(const nsSVGVi
   if (mAnimVal) {
     aSVGElement->AnimationNeedsResample();
   }
 }
 
 static nsresult
 ToSVGViewBoxRect(const nsAString& aStr, nsSVGViewBoxRect *aViewBox)
 {
+  if (aStr.EqualsLiteral("none")) {
+    aViewBox->none = true;
+    return NS_OK;
+  }
+
   nsCharSeparatedTokenizerTemplate<IsSVGWhitespace>
     tokenizer(aStr, ',',
               nsCharSeparatedTokenizer::SEPARATOR_OPTIONAL);
   float vals[NUM_VIEWBOX_COMPONENTS];
   uint32_t i;
   for (i = 0; i < NUM_VIEWBOX_COMPONENTS && tokenizer.hasMoreTokens(); ++i) {
     NS_ConvertUTF16toUTF8 utf8Token(tokenizer.nextToken());
     const char *token = utf8Token.get();
@@ -147,48 +157,59 @@ ToSVGViewBoxRect(const nsAString& aStr, 
       tokenizer.lastTokenEndedWithSeparator()) {  // Trailing comma.
     return NS_ERROR_DOM_SYNTAX_ERR;
   }
 
   aViewBox->x = vals[0];
   aViewBox->y = vals[1];
   aViewBox->width = vals[2];
   aViewBox->height = vals[3];
+  aViewBox->none = false;
 
   return NS_OK;
 }
 
 nsresult
 nsSVGViewBox::SetBaseValueString(const nsAString& aValue,
                                  nsSVGElement *aSVGElement,
                                  bool aDoSetAttr)
 {
   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;
+
+  nsresult rv = ToSVGViewBoxRect(aValue, &viewBox);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  if (viewBox == mBaseVal) {
+    return NS_OK;
+  }
 
-    if (aDoSetAttr) {
-      aSVGElement->DidChangeViewBox(emptyOrOldValue);
-    }
-    if (mAnimVal) {
-      aSVGElement->AnimationNeedsResample();
-    }
+  nsAttrValue emptyOrOldValue;
+  if (aDoSetAttr) {
+    emptyOrOldValue = aSVGElement->WillChangeViewBox();
   }
-  return res;
+  mHasBaseVal = true;
+  mBaseVal = viewBox;
+
+  if (aDoSetAttr) {
+    aSVGElement->DidChangeViewBox(emptyOrOldValue);
+  }
+  if (mAnimVal) {
+    aSVGElement->AnimationNeedsResample();
+  }
+  return NS_OK;
 }
 
 void
 nsSVGViewBox::GetBaseValueString(nsAString& aValue) const
 {
+  if (mBaseVal.none) {
+    aValue.AssignLiteral("none");
+    return;
+  }
   PRUnichar buf[200];
   nsTextFormatter::snprintf(buf, sizeof(buf)/sizeof(PRUnichar),
                             NS_LITERAL_STRING("%g %g %g %g").get(),
                             (double)mBaseVal.x, (double)mBaseVal.y,
                             (double)mBaseVal.width, (double)mBaseVal.height);
   aValue.Assign(buf);
 }
 
@@ -210,16 +231,20 @@ nsSVGViewBox::ToDOMAnimatedRect(nsIDOMSV
 nsSVGViewBox::DOMAnimatedRect::~DOMAnimatedRect()
 {
   sSVGAnimatedRectTearoffTable.RemoveTearoff(mVal);
 }
 
 nsresult
 nsSVGViewBox::ToDOMBaseVal(nsIDOMSVGRect **aResult, nsSVGElement *aSVGElement)
 {
+  if (!mHasBaseVal || mBaseVal.none) {
+    *aResult = nullptr;
+    return NS_OK;
+  }
   nsRefPtr<DOMBaseVal> domBaseVal =
     sBaseSVGViewBoxTearoffTable.GetTearoff(this);
   if (!domBaseVal) {
     domBaseVal = new DOMBaseVal(this, aSVGElement);
     sBaseSVGViewBoxTearoffTable.AddTearoff(this, domBaseVal);
   }
 
   domBaseVal.forget(aResult);
@@ -229,16 +254,21 @@ nsSVGViewBox::ToDOMBaseVal(nsIDOMSVGRect
 nsSVGViewBox::DOMBaseVal::~DOMBaseVal()
 {
   sBaseSVGViewBoxTearoffTable.RemoveTearoff(mVal);
 }
 
 nsresult
 nsSVGViewBox::ToDOMAnimVal(nsIDOMSVGRect **aResult, nsSVGElement *aSVGElement)
 {
+  if ((mAnimVal && mAnimVal->none) ||
+      (!mAnimVal && (!mHasBaseVal || mBaseVal.none))) {
+    *aResult = nullptr;
+    return NS_OK;
+  }
   nsRefPtr<DOMAnimVal> domAnimVal =
     sAnimSVGViewBoxTearoffTable.GetTearoff(this);
   if (!domAnimVal) {
     domAnimVal = new DOMAnimVal(this, aSVGElement);
     sAnimSVGViewBoxTearoffTable.AddTearoff(this, domAnimVal);
   }
 
   domAnimVal.forget(aResult);
@@ -250,48 +280,44 @@ nsSVGViewBox::DOMAnimVal::~DOMAnimVal()
   sAnimSVGViewBoxTearoffTable.RemoveTearoff(mVal);
 }
 
 NS_IMETHODIMP
 nsSVGViewBox::DOMBaseVal::SetX(float aX)
 {
   nsSVGViewBoxRect rect = mVal->GetBaseValue();
   rect.x = aX;
-  mVal->SetBaseValue(rect.x, rect.y, rect.width, rect.height,
-                     mSVGElement);
+  mVal->SetBaseValue(rect, mSVGElement);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSVGViewBox::DOMBaseVal::SetY(float aY)
 {
   nsSVGViewBoxRect rect = mVal->GetBaseValue();
   rect.y = aY;
-  mVal->SetBaseValue(rect.x, rect.y, rect.width, rect.height,
-                     mSVGElement);
+  mVal->SetBaseValue(rect, mSVGElement);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSVGViewBox::DOMBaseVal::SetWidth(float aWidth)
 {
   nsSVGViewBoxRect rect = mVal->GetBaseValue();
   rect.width = aWidth;
-  mVal->SetBaseValue(rect.x, rect.y, rect.width, rect.height,
-                     mSVGElement);
+  mVal->SetBaseValue(rect, mSVGElement);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSVGViewBox::DOMBaseVal::SetHeight(float aHeight)
 {
   nsSVGViewBoxRect rect = mVal->GetBaseValue();
   rect.height = aHeight;
-  mVal->SetBaseValue(rect.x, rect.y, rect.width, rect.height,
-                     mSVGElement);
+  mVal->SetBaseValue(rect, mSVGElement);
   return NS_OK;
 }
 
 nsISMILAttr*
 nsSVGViewBox::ToSMILAttr(nsSVGElement *aSVGElement)
 {
   return new SMILViewBox(this, aSVGElement);
 }
@@ -335,12 +361,12 @@ nsSVGViewBox::SMILViewBox::ClearAnimValu
 
 nsresult
 nsSVGViewBox::SMILViewBox::SetAnimValue(const nsSMILValue& aValue)
 {
   NS_ASSERTION(aValue.mType == &SVGViewBoxSMILType::sSingleton,
                "Unexpected type to assign animated value");
   if (aValue.mType == &SVGViewBoxSMILType::sSingleton) {
     nsSVGViewBoxRect &vb = *static_cast<nsSVGViewBoxRect*>(aValue.mU.mPtr);
-    mVal->SetAnimValue(vb.x, vb.y, vb.width, vb.height, mSVGElement);
+    mVal->SetAnimValue(vb, mSVGElement);
   }
   return NS_OK;
 }
--- a/content/svg/content/src/nsSVGViewBox.h
+++ b/content/svg/content/src/nsSVGViewBox.h
@@ -17,22 +17,23 @@
 
 class nsISMILAnimationElement;
 class nsSMILValue;
 
 struct nsSVGViewBoxRect
 {
   float x, y;
   float width, height;
+  bool none;
 
-  nsSVGViewBoxRect() : x(0), y(0), width(0), height(0) {}
+  nsSVGViewBoxRect() : none(true) {}
   nsSVGViewBoxRect(float aX, float aY, float aWidth, float aHeight) :
-    x(aX), y(aY), width(aWidth), height(aHeight) {}
+    x(aX), y(aY), width(aWidth), height(aHeight), none(false) {}
   nsSVGViewBoxRect(const nsSVGViewBoxRect& rhs) :
-    x(rhs.x), y(rhs.y), width(rhs.width), height(rhs.height) {}
+    x(rhs.x), y(rhs.y), width(rhs.width), height(rhs.height), none(rhs.none) {}
   bool operator==(const nsSVGViewBoxRect& aOther) const;
 };
 
 class nsSVGViewBox
 {
 
 public:
 
@@ -43,29 +44,34 @@ public:
    * with finite values. Returns false if the viewBox was set to an invalid
    * string, or if any of the four rect values were too big to store in a
    * float.
    *
    * This method does not check whether the width or height values are
    * positive, so callers must check whether the viewBox rect is valid where
    * necessary!
    */
+  bool HasRect() const
+    { return (mAnimVal && !mAnimVal->none) ||
+             (!mAnimVal && mHasBaseVal && !mBaseVal.none); }
+
+  /**
+   * Returns true if the corresponding "viewBox" attribute either defined a
+   * rectangle with finite values or the special "none" value.
+   */
   bool IsExplicitlySet() const
-    { return (mHasBaseVal || mAnimVal); }
+    { return mAnimVal || mHasBaseVal; }
 
   const nsSVGViewBoxRect& GetBaseValue() const
     { return mBaseVal; }
   void SetBaseValue(const nsSVGViewBoxRect& aRect,
                     nsSVGElement *aSVGElement);
-  void SetBaseValue(float aX, float aY, float aWidth, float aHeight,
-                    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,
+  void SetAnimValue(const nsSVGViewBoxRect& aRect,
                     nsSVGElement *aSVGElement);
 
   nsresult SetBaseValueString(const nsAString& aValue,
                               nsSVGElement *aSVGElement,
                               bool aDoSetAttr);
   void GetBaseValueString(nsAString& aValue) const;
 
   nsresult ToDOMAnimatedRect(nsIDOMSVGAnimatedRect **aResult,
--- a/content/svg/content/test/test_dataTypes.html
+++ b/content/svg/content/test/test_dataTypes.html
@@ -243,29 +243,18 @@ function runTests()
      "empty preserveAspectRatio attribute");
   marker.removeAttribute("preserveAspectRatio");
   ok(marker.getAttribute("preserveAspectRatio") === null,
      "removed preserveAspectRatio attribute");
 
   // viewBox attribute
   var baseViewBox = marker.viewBox.baseVal;
   var animViewBox = marker.viewBox.animVal;
-  is(baseViewBox.x, 0, "viewBox baseVal");
-  is(animViewBox.x, 0, "viewBox baseVal");
-  is(baseViewBox.y, 0, "viewBox baseVal");
-  is(animViewBox.y, 0, "viewBox baseVal");
-  is(baseViewBox.width, 0, "viewBox baseVal");
-  is(animViewBox.width, 0, "viewBox baseVal");
-  is(baseViewBox.height, 0, "viewBox baseVal");
-  is(animViewBox.height, 0, "viewBox baseVal");
-  baseViewBox.x = 10;
-  baseViewBox.y = 11;
-  baseViewBox.width = 12;
-  baseViewBox.height = 13;
-  is(marker.getAttribute("viewBox"), "10 11 12 13", "viewBox attribute");
+  is(baseViewBox, null, "viewBox baseVal");
+  is(animViewBox, null, "viewBox animVal");
 
   marker.setAttribute("viewBox", "1 2 3 4");
   is(marker.viewBox.baseVal.x, 1, "viewBox.x baseVal");
   is(marker.viewBox.animVal.x, 1, "viewBox.x animVal");
   is(marker.viewBox.baseVal.y, 2, "viewbox.y baseVal");
   is(marker.viewBox.animVal.y, 2, "viewbox.y animVal");
   is(marker.viewBox.baseVal.width, 3, "viewbox.width baseVal");
   is(marker.viewBox.animVal.width, 3, "viewbox.width animVal");
@@ -275,21 +264,38 @@ function runTests()
   is(marker.viewBox.animVal.x, 5, "viewBox.x animVal");
   marker.viewBox.baseVal.y = 6;
   is(marker.viewBox.animVal.y, 6, "viewBox.y animVal");
   marker.viewBox.baseVal.width = 7;
   is(marker.viewBox.animVal.width, 7, "viewBox.width animVal");
   marker.viewBox.baseVal.height = 8;
   is(marker.viewBox.animVal.height, 8, "viewBox.height animVal");
   is(marker.getAttribute("viewBox"), "5 6 7 8", "viewBox attribute");
+  var storedViewBox = marker.viewBox.baseVal;
   marker.removeAttribute("viewBox");
   is(marker.hasAttribute("viewBox"), false, "viewBox hasAttribute");
   ok(marker.getAttribute("viewBox") === null, "removed viewBox attribute");
+  is(marker.viewBox.baseVal, null, "viewBox baseVal");
+  is(marker.viewBox.animVal, null, "viewBox animVal");
+
+  is(storedViewBox.width, 7, "Should not lose values");
+  storedViewBox.width = 200;
+  is(storedViewBox.width, 200, "Should be able to change detached viewBox rect");
+  is(marker.hasAttribute("viewBox"), false, "viewBox hasAttribute should still be false");
+  ok(marker.getAttribute("viewBox") === null, "viewBox attribute should still be null");
+  is(marker.viewBox.baseVal, null, "viewBox baseVal");
+  is(marker.viewBox.animVal, null, "viewBox animVal");
+
+  marker.setAttribute("viewBox", "none");
+  is(marker.hasAttribute("viewBox"), true, "viewBox hasAttribute");
+  is(marker.viewBox.baseVal, null, "viewBox baseVal");
+  is(marker.viewBox.animVal, null, "viewBox animVal");
 
   marker.setAttribute("viewBox", "");
+  is(marker.hasAttribute("viewBox"), true, "viewBox hasAttribute");
   ok(marker.getAttribute("viewBox") === "", "empty viewBox attribute");
 
   SimpleTest.finish();
 }
 
 window.addEventListener("load", runTests, false);
 </script>
 </pre>
--- a/content/svg/content/test/test_dataTypesModEvents.html
+++ b/content/svg/content/test/test_dataTypesModEvents.html
@@ -231,17 +231,18 @@ function runTests()
   // 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;
+  marker.setAttribute("viewBox", "none");
+  marker.setAttribute("viewBox", "none");
 
   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");
 
--- a/content/svg/content/test/test_fragments.html
+++ b/content/svg/content/test/test_fragments.html
@@ -34,16 +34,17 @@ function runTests()
   var svg = $("svg");
   var doc = svg.contentWindow.document;
   
   var tests = [
       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(viewBox(none))", true, "none", null, null),
       new Test("svgView(zoomAndPan(disable))", true, null, null, "disable"),
       new Test("svgView(transform(translate(-10,-20) scale(2) rotate(45) translate(5,10)))", true, null, null, null),
       // No duplicates allowed
       new Test("svgView(zoomAndPan(disable);zoomAndPan(disable))", false, null, null, null),
       new Test("svgView(viewBox(0,0,200,200);viewBox(0,0,200,200))", false, null, null, null),
       new Test("svgView(preserveAspectRatio(xMaxYMin);preserveAspectRatio(xMaxYMin))", false, null, null, null),
       new Test("svgView(transform(translate(0,200));transform(translate(0,200)))", false, null, null, null),
       // No invalid values allowed
new file mode 100644
--- /dev/null
+++ b/layout/reftests/svg/smil/anim-svg-viewBox-03.svg
@@ -0,0 +1,18 @@
+<svg xmlns="http://www.w3.org/2000/svg"
+     xmlns:xlink="http://www.w3.org/1999/xlink"
+     class="reftest-wait"
+     onload="setTimeAndSnapshot(12, true)">
+  <title>Test discrete animation of the "viewBox" attribute on the "svg" element</title>
+  <script xlink:href="smil-util.js" type="text/javascript"/>
+  <rect width="100%" height="100%" fill="lime"/>
+
+  <svg width="200" height="200" viewBox="200 0 150 50">
+    <animate attributeName="viewBox"
+             calcMode="discrete"
+             begin="10s" dur="4s"
+             values="200 0 150 50; none; 200 0 150 50"
+             fill="freeze"/>
+    <rect x="-100" y="-100" width="1000" height="1000" fill="red"/>
+    <rect width="200" height="200" fill="lime"/>
+  </svg>
+</svg>
--- a/layout/reftests/svg/smil/reftest.list
+++ b/layout/reftests/svg/smil/reftest.list
@@ -134,16 +134,17 @@ skip-if(B2G) == anim-feComposite-operato
 == anim-filter-filterUnits-01.svg lime.svg
 
 # animate some boolean attributes:
 skip-if(B2G) == anim-feConvolveMatrix-preserveAlpha-01.svg lime.svg # bug 773482
 
 # animate some viewBox attributes
 == anim-svg-viewBox-01.svg lime.svg
 == anim-svg-viewBox-02.svg lime.svg
+== anim-svg-viewBox-03.svg lime.svg
 == anim-view-01.svg#view lime.svg
 
 # animate some preserveAspectRatio attributes
 skip-if(B2G) == anim-feImage-preserveAspectRatio-01.svg lime.svg
 == anim-svg-preserveAspectRatio-01.svg lime.svg
 
 # animate some string attributes:
 == anim-filter-href-01.svg lime.svg
--- a/layout/svg/nsSVGInnerSVGFrame.cpp
+++ b/layout/svg/nsSVGInnerSVGFrame.cpp
@@ -131,21 +131,21 @@ nsSVGInnerSVGFrame::NotifySVGChanged(uin
     }
 
     // Coordinate context changes affect mCanvasTM if we have a
     // percentage 'x' or 'y', or if we have a percentage 'width' or 'height' AND
     // a 'viewBox'.
 
     if (!(aFlags & TRANSFORM_CHANGED) &&
         (xOrYIsPercentage ||
-         (widthOrHeightIsPercentage && svg->HasViewBox()))) {
+         (widthOrHeightIsPercentage && svg->HasViewBoxRect()))) {
       aFlags |= TRANSFORM_CHANGED;
     }
 
-    if (svg->HasViewBox() || !widthOrHeightIsPercentage) {
+    if (svg->HasViewBoxRect() || !widthOrHeightIsPercentage) {
       // Remove COORD_CONTEXT_CHANGED, since we establish the coordinate
       // context for our descendants and this notification won't change its
       // dimensions:
       aFlags &= ~COORD_CONTEXT_CHANGED;
 
       if (!aFlags) {
         return; // No notification flags left
       }
--- a/layout/svg/nsSVGOuterSVGFrame.cpp
+++ b/layout/svg/nsSVGOuterSVGFrame.cpp
@@ -282,19 +282,19 @@ nsSVGOuterSVGFrame::GetIntrinsicRatio()
     }
     return ratio;
   }
 
   SVGViewElement* viewElement = content->GetCurrentViewElement();
   const nsSVGViewBoxRect* viewbox = nullptr;
 
   // The logic here should match HasViewBox().
-  if (viewElement && viewElement->mViewBox.IsExplicitlySet()) {
+  if (viewElement && viewElement->mViewBox.HasRect()) {
     viewbox = &viewElement->mViewBox.GetAnimValue();
-  } else if (content->mViewBox.IsExplicitlySet()) {
+  } else if (content->mViewBox.HasRect()) {
     viewbox = &content->mViewBox.GetAnimValue();
   }
 
   if (viewbox) {
     float viewBoxWidth = viewbox->width;
     float viewBoxHeight = viewbox->height;
 
     if (viewBoxWidth < 0.0f) {
@@ -749,17 +749,17 @@ nsSVGOuterSVGFrame::NotifyViewportOrTran
   // No point in doing anything when were not init'ed yet:
   if (!mViewportInitialized) {
     return;
   }
 
   SVGSVGElement *content = static_cast<SVGSVGElement*>(mContent);
 
   if (aFlags & COORD_CONTEXT_CHANGED) {
-    if (content->HasViewBox()) {
+    if (content->HasViewBoxRect()) {
       // Percentage lengths on children resolve against the viewBox rect so we
       // don't need to notify them of the viewport change, but the viewBox
       // transform will have changed, so we need to notify them of that instead.
       aFlags = TRANSFORM_CHANGED;
     }
     else if (content->ShouldSynthesizeViewBox()) {
       // In the case of a synthesized viewBox, the synthetic viewBox's rect
       // changes as the viewport changes. As a result we need to maintain the