Bug 647885 part 1. Keep better track of whether our computed background-position was specified with percentages. r=dbaron

layout/style/nsComputedDOMStyle.cpp

@@ -1265,7 +1265,7 @@ nsComputedDOMStyle::DoGetBackgroundColor()
 
 
 static void
-SetValueToCalc(nsStyleCoord::Calc *aCalc, nsROCSSPrimitiveValue *aValue)
+SetValueToCalc(const nsStyleCoord::Calc *aCalc, nsROCSSPrimitiveValue *aValue)
 {
   nsRefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue();
   nsAutoString tmp, result;
@@ -1524,28 +1524,24 @@ nsComputedDOMStyle::DoGetBackgroundPosition()
 
     const nsStyleBackground::Position &pos = bg->mLayers[i].mPosition;
 
-    if (pos.mXPosition.mLength == 0) {
-      valX->SetPercent(pos.mXPosition.mPercent);
-    } else if (pos.mXPosition.mPercent == 0.0f) {
+    if (!pos.mXPosition.mHasPercent) {
+      NS_ABORT_IF_FALSE(pos.mXPosition.mPercent == 0.0f,
+                        "Shouldn't have mPercent!");
       valX->SetAppUnits(pos.mXPosition.mLength);
+    } else if (pos.mXPosition.mLength == 0) {
+      valX->SetPercent(pos.mXPosition.mPercent);
     } else {
-      nsStyleCoord::Calc calc;
-      calc.mPercent = pos.mXPosition.mPercent;
-      calc.mLength  = pos.mXPosition.mLength;
-      calc.mHasPercent = PR_TRUE;
-      SetValueToCalc(&calc, valX);
+      SetValueToCalc(&pos.mXPosition, valX);
     }
 
-    if (pos.mYPosition.mLength == 0) {
-      valY->SetPercent(pos.mYPosition.mPercent);
-    } else if (pos.mYPosition.mPercent == 0.0f) {
+    if (!pos.mYPosition.mHasPercent) {
+      NS_ABORT_IF_FALSE(pos.mYPosition.mPercent == 0.0f,
+                        "Shouldn't have mPercent!");
       valY->SetAppUnits(pos.mYPosition.mLength);
+    } else if (pos.mYPosition.mLength == 0) {
+      valY->SetPercent(pos.mYPosition.mPercent);
     } else {
-      nsStyleCoord::Calc calc;
-      calc.mPercent = pos.mYPosition.mPercent;
-      calc.mLength  = pos.mYPosition.mLength;
-      calc.mHasPercent = PR_TRUE;
-      SetValueToCalc(&calc, valY);
+      SetValueToCalc(&pos.mYPosition, valY);
     }
   }
 

layout/style/nsRuleNode.cpp

@@ -4652,12 +4652,14 @@ struct BackgroundItemComputer<nsCSSValuePairList, nsStyleBackground::Position>
       if (eCSSUnit_Percent == specified.GetUnit()) {
         (position.*(axis->result)).mLength = 0;
         (position.*(axis->result)).mPercent = specified.GetPercentValue();
+        (position.*(axis->result)).mHasPercent = PR_TRUE;
       }
       else if (specified.IsLengthUnit()) {
         (position.*(axis->result)).mLength =
           CalcLength(specified, aStyleContext, aStyleContext->PresContext(),
                      aCanStoreInRuleTree);
         (position.*(axis->result)).mPercent = 0.0f;
+        (position.*(axis->result)).mHasPercent = PR_FALSE;
       }
       else if (specified.IsCalcUnit()) {
         LengthPercentPairCalcOps ops(aStyleContext,
@@ -4666,11 +4668,13 @@ struct BackgroundItemComputer<nsCSSValuePairList, nsStyleBackground::Position>
         nsRuleNode::ComputedCalc vals = ComputeCalc(specified, ops);
         (position.*(axis->result)).mLength = vals.mLength;
         (position.*(axis->result)).mPercent = vals.mPercent;
+        (position.*(axis->result)).mHasPercent = ops.mHasPercent;
       }
       else if (eCSSUnit_Enumerated == specified.GetUnit()) {
         (position.*(axis->result)).mLength = 0;
         (position.*(axis->result)).mPercent =
           GetFloatFromBoxPosition(specified.GetIntValue());
+        (position.*(axis->result)).mHasPercent = PR_TRUE;
       } else {
         NS_NOTREACHED("unexpected unit");
       }

layout/style/nsStyleStruct.cpp

@@ -1822,10 +1822,13 @@ PRBool nsStyleBackground::IsTransparent() const
 void
 nsStyleBackground::Position::SetInitialValues()
 {
+  // Initial value is "0% 0%"
   mXPosition.mPercent = 0.0f;
   mXPosition.mLength = 0;
+  mXPosition.mHasPercent = PR_TRUE;
   mYPosition.mPercent = 0.0f;
   mYPosition.mLength = 0;
+  mYPosition.mHasPercent = PR_TRUE;
 }
 
 void

layout/style/nsStyleStruct.h

@@ -351,17 +351,7 @@ struct nsStyleBackground {
   struct Position;
   friend struct Position;
   struct Position {
-    struct PositionCoord {
-      // A 'background-position' can be a linear combination of length
-      // and percent (thanks to calc(), which can combine them).
-      nscoord mLength;
-      float   mPercent;
-
-      bool operator==(const PositionCoord& aOther) const
-        { return mLength == aOther.mLength && mPercent == aOther.mPercent; }
-      bool operator!=(const PositionCoord& aOther) const
-        { return !(*this == aOther); }
-    };
+    typedef nsStyleCoord::Calc PositionCoord;
     PositionCoord mXPosition, mYPosition;
 
     // Initialize nothing

layout/style/test/test_computed_style.html

@@ -115,6 +115,40 @@ var noframe_container = document.getElementById("content");
   p.parentNode.removeChild(p);
 })();
 
+(function test_bug_647885_1() {
+  // Test that various background-position styles round-trip correctly
+  var backgroundPositions = [
+    [ "0 0", "0px 0px", "unitless 0" ],
+    [ "0px 0px", "0px 0px", "0 with units" ],
+    [ "0% 0%", "0% 0%", "0%" ],
+    [ "-moz-calc(0px) 0", "0px 0px", "0 calc with units x" ],
+    [ "0 -moz-calc(0px)", "0px 0px", "0 calc with units y" ],
+    [ "-moz-calc(3px - 3px) 0", "0px 0px", "computed 0 calc with units x" ],
+    [ "0 -moz-calc(3px - 3px)", "0px 0px", "computed 0 calc with units y" ],
+    [ "-moz-calc(0%) 0", "0% 0px", "0% calc x"],
+    [ "0 -moz-calc(0%)", "0px 0%", "0% calc y"],
+    [ "-moz-calc(3px + 2% - 2%) 0", "-moz-calc(3px + 0%) 0px",
+      "computed 0% calc x"],
+    [ "0 -moz-calc(3px + 2% - 2%)", "0px -moz-calc(3px + 0%)",
+      "computed 0% calc y"],
+    [ "-moz-calc(3px - 5px) -moz-calc(6px - 7px)", "-2px -1px",
+      "negative pixel width"],
+    [ "", "0% 0%", "initial value" ],
+  ];
+
+  var p = document.createElement("p");
+  var cs = getComputedStyle(p, "");
+  frame_container.appendChild(p);
+
+  for (var i = 0; i < backgroundPositions.length; ++i) {
+    var test = backgroundPositions[i];
+    p.style.backgroundPosition = test[0];
+    is(cs.backgroundPosition, test[1], "computed value of " + test[2] + " background-position");
+  }
+
+  p.parentNode.removeChild(p);
+})();
+
 </script>
 </pre>
 </body>