Bug 1269174: Fix nsSVGViewBox::HasRect() to return false instead of using stale mBaseVal. r=longsonr a=ritu
authorDaniel Holbert <dholbert@cs.stanford.edu>
Tue, 03 May 2016 07:40:26 -0700
changeset 332721 995ecc8dbf0b3a5337784766ee575934cb61df03
parent 332720 314eb11784ea640cb1719d220efe3e588253692b
child 332722 fa189b73fc82a084dde317e644a36a2c143a58cc
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslongsonr, ritu
bugs1269174
milestone48.0a2
Bug 1269174: Fix nsSVGViewBox::HasRect() to return false instead of using stale mBaseVal. r=longsonr a=ritu MozReview-Commit-ID: esNxaomT3D
dom/svg/nsSVGViewBox.cpp
dom/svg/nsSVGViewBox.h
layout/reftests/svg/dynamic-viewBox-change-01.svg
layout/reftests/svg/dynamic-viewBox-change-02.svg
layout/reftests/svg/dynamic-viewBox-change-03.svg
layout/reftests/svg/reftest.list
--- a/dom/svg/nsSVGViewBox.cpp
+++ b/dom/svg/nsSVGViewBox.cpp
@@ -65,16 +65,33 @@ nsSVGAttrTearoffTable<nsSVGViewBox, dom:
 
 void
 nsSVGViewBox::Init()
 {
   mHasBaseVal = false;
   mAnimVal = nullptr;
 }
 
+bool
+nsSVGViewBox::HasRect() const
+{
+  // Check mAnimVal if we have one; otherwise, check mBaseVal if we have one;
+  // otherwise, just return false (we clearly do not have a rect).
+  const nsSVGViewBoxRect* rect = mAnimVal;
+  if (!rect) {
+    if (!mHasBaseVal) {
+      // no anim val, no base val --> no viewbox rect
+      return false;
+    }
+    rect = &mBaseVal;
+  }
+
+  return !rect->none && rect->width >= 0 && rect->height >= 0;
+}
+
 void
 nsSVGViewBox::SetAnimValue(const nsSVGViewBoxRect& aRect,
                            nsSVGElement *aSVGElement)
 {
   if (!mAnimVal) {
     // it's okay if allocation fails - and no point in reporting that
     mAnimVal = new nsSVGViewBoxRect(aRect);
   } else {
--- a/dom/svg/nsSVGViewBox.h
+++ b/dom/svg/nsSVGViewBox.h
@@ -47,21 +47,17 @@ public:
 
   /**
    * Returns true if the corresponding "viewBox" attribute defined a rectangle
    * with finite values and nonnegative width/height.
    * 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, or the width/height are negative.
    */
-  bool HasRect() const
-    {
-      const nsSVGViewBoxRect& rect = GetAnimValue();
-      return !rect.none && rect.width >= 0 && rect.height >= 0;
-    }
+  bool HasRect() const;
 
   /**
    * Returns true if the corresponding "viewBox" attribute either defined a
    * rectangle with finite values or the special "none" value.
    */
   bool IsExplicitlySet() const
     {
       if (mAnimVal || mHasBaseVal) {
new file mode 100644
--- /dev/null
+++ b/layout/reftests/svg/dynamic-viewBox-change-01.svg
@@ -0,0 +1,24 @@
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<svg xmlns="http://www.w3.org/2000/svg" class="reftest-wait"
+     id="target" viewBox="-50 -50 20 20"
+     preserveAspectRatio="none">
+  <script>
+  function doTest() {
+    var target = document.getElementById("target");
+    target.removeAttribute("viewBox");
+    document.documentElement.removeAttribute("class");
+  }
+
+  document.addEventListener("MozReftestInvalidate", doTest, false);
+  </script>
+
+  <!-- Lime background to match pass.svg -->
+  <rect height="100%" width="100%" fill="lime"/>
+
+  <!-- Offscreen red rect - the initial |viewBox| makes this visible,
+       but it shouldn't be visible after we remove that attribute: -->
+  <rect x="-50" y="-50" width="20" height="20" fill="red"/>
+</svg>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/svg/dynamic-viewBox-change-02.svg
@@ -0,0 +1,24 @@
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<svg xmlns="http://www.w3.org/2000/svg" class="reftest-wait"
+     id="target" viewBox="-50 -50 20 20"
+     preserveAspectRatio="none">
+  <script>
+  function doTest() {
+    var target = document.getElementById("target");
+    target.setAttribute("viewBox", "");
+    document.documentElement.removeAttribute("class");
+  }
+
+  document.addEventListener("MozReftestInvalidate", doTest, false);
+  </script>
+
+  <!-- Lime background to match pass.svg -->
+  <rect height="100%" width="100%" fill="lime"/>
+
+  <!-- Offscreen red rect - the initial |viewBox| makes this visible,
+       but it shouldn't be visible after we clear that attribute: -->
+  <rect x="-50" y="-50" width="20" height="20" fill="red"/>
+</svg>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/svg/dynamic-viewBox-change-03.svg
@@ -0,0 +1,24 @@
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<svg xmlns="http://www.w3.org/2000/svg" class="reftest-wait"
+     id="target" viewBox="-50 -50 20 20"
+     preserveAspectRatio="none">
+  <script>
+  function doTest() {
+    var target = document.getElementById("target");
+    target.setAttribute("viewBox", "bogus value");
+    document.documentElement.removeAttribute("class");
+  }
+
+  document.addEventListener("MozReftestInvalidate", doTest, false);
+  </script>
+
+  <!-- Lime background to match pass.svg -->
+  <rect height="100%" width="100%" fill="lime"/>
+
+  <!-- Offscreen red rect - the initial |viewBox| makes this visible,
+       but it shouldn't be visible after we tweak that attribute: -->
+  <rect x="-50" y="-50" width="20" height="20" fill="red"/>
+</svg>
--- a/layout/reftests/svg/reftest.list
+++ b/layout/reftests/svg/reftest.list
@@ -130,16 +130,19 @@ random-if(/^Windows\x20NT\x205\.1/.test(
 == dynamic-use-03.svg pass.svg
 == dynamic-use-04.svg pass.svg
 == dynamic-use-05.svg pass.svg
 == dynamic-use-06.svg pass.svg
 == dynamic-use-07.svg pass.svg
 random == dynamic-use-nested-01a.svg dynamic-use-nested-01-ref.svg
 random == dynamic-use-nested-01b.svg dynamic-use-nested-01-ref.svg
 == dynamic-use-remove-width.svg dynamic-use-remove-width-ref.svg
+== dynamic-viewBox-change-01.svg pass.svg
+== dynamic-viewBox-change-02.svg pass.svg
+== dynamic-viewBox-change-03.svg pass.svg
 == fragmentIdentifier-01.xhtml pass.svg
 == linked-filter-01.svg pass.svg
 == linked-pattern-01.svg pass.svg
 == use-01.svg pass.svg
 == use-01-extref.svg pass.svg
 == use-02-extref.svg use-02-extref-ref.svg
 == use-extref-dataURI-01.svg pass.svg
 == use-children.svg pass.svg