Fix regression of background-position serialization. b=258080 r+sr=bzbarsky
authordbaron@dbaron.org
Wed, 18 Apr 2007 13:41:14 -0700
changeset 628 cddd38f3fb291347d80d1c0a5b7311ecf62b7e69
parent 627 e9046fd8f006f6ee9353fe25b1f39e075dcd31b5
child 629 140f26ddf400f22311d504279fd6f9984a6fa06b
push idunknown
push userunknown
push dateunknown
bugs258080
milestone1.9a4pre
Fix regression of background-position serialization. b=258080 r+sr=bzbarsky
layout/base/nsStyleConsts.h
layout/style/nsCSSParser.cpp
layout/style/nsCSSPropList.h
layout/style/nsCSSProps.cpp
layout/style/nsCSSProps.h
layout/style/nsRuleNode.cpp
layout/style/test/test_value_storage.html
--- a/layout/base/nsStyleConsts.h
+++ b/layout/base/nsStyleConsts.h
@@ -226,16 +226,24 @@
 #define NS_STYLE_BG_INLINE_POLICY_BOUNDING_BOX  2
 
 // See nsStyleBackground
 #define NS_STYLE_BG_ORIGIN_BORDER         0
 #define NS_STYLE_BG_ORIGIN_PADDING        1
 #define NS_STYLE_BG_ORIGIN_CONTENT        2
 
 // See nsStyleBackground
+// The parser code depends on |ing these values together.
+#define NS_STYLE_BG_POSITION_CENTER  (1<<0)
+#define NS_STYLE_BG_POSITION_TOP     (1<<1)
+#define NS_STYLE_BG_POSITION_BOTTOM  (1<<2)
+#define NS_STYLE_BG_POSITION_LEFT    (1<<3)
+#define NS_STYLE_BG_POSITION_RIGHT   (1<<4)
+
+// See nsStyleBackground
 #define NS_STYLE_BG_REPEAT_OFF                  0x00
 #define NS_STYLE_BG_REPEAT_X                    0x01
 #define NS_STYLE_BG_REPEAT_Y                    0x02
 #define NS_STYLE_BG_REPEAT_XY                   0x03
 
 // See nsStyleTable
 #define NS_STYLE_BORDER_COLLAPSE                0
 #define NS_STYLE_BORDER_SEPARATE                1
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -4339,36 +4339,24 @@ PRBool CSSParserImpl::ParseProperty(nsre
       }
       // XXX Report errors?
     }
   }
   return PR_FALSE;
 }
 
 // Bits used in determining which background position info we have
-#define BG_CENTER  0x01
-#define BG_TOP     0x02
-#define BG_BOTTOM  0x04
-#define BG_LEFT    0x08
-#define BG_RIGHT   0x10
+#define BG_CENTER  NS_STYLE_BG_POSITION_CENTER
+#define BG_TOP     NS_STYLE_BG_POSITION_TOP
+#define BG_BOTTOM  NS_STYLE_BG_POSITION_BOTTOM
+#define BG_LEFT    NS_STYLE_BG_POSITION_LEFT
+#define BG_RIGHT   NS_STYLE_BG_POSITION_RIGHT
 #define BG_CTB    (BG_CENTER | BG_TOP | BG_BOTTOM)
 #define BG_CLR    (BG_CENTER | BG_LEFT | BG_RIGHT)
 
-// Note: Don't change this table unless you update
-// parseBackgroundPosition!
-
-static const PRInt32 kBackgroundXYPositionKTable[] = {
-  eCSSKeyword_center, BG_CENTER,
-  eCSSKeyword_top, BG_TOP,
-  eCSSKeyword_bottom, BG_BOTTOM,
-  eCSSKeyword_left, BG_LEFT,
-  eCSSKeyword_right, BG_RIGHT,
-  -1,
-};
-
 PRBool CSSParserImpl::ParseSingleValueProperty(nsresult& aErrorCode,
                                                nsCSSValue& aValue,
                                                nsCSSProperty aPropID)
 {
   switch (aPropID) {
   case eCSSProperty_UNKNOWN:
   case eCSSProperty_background:
   case eCSSProperty_background_position:
@@ -4823,35 +4811,35 @@ PRBool CSSParserImpl::ParseAzimuth(nsres
     return PR_TRUE;
   }
   return PR_FALSE;
 }
 
 static nsCSSValue
 BackgroundPositionMaskToCSSValue(PRInt32 aMask, PRBool isX)
 {
-  PRInt32 pct = 50;
+  PRInt32 val = NS_STYLE_BG_POSITION_CENTER;
   if (isX) {
     if (aMask & BG_LEFT) {
-      pct = 0;
+      val = NS_STYLE_BG_POSITION_LEFT;
     }
     else if (aMask & BG_RIGHT) {
-      pct = 100;
+      val = NS_STYLE_BG_POSITION_RIGHT;
     }
   }
   else {
     if (aMask & BG_TOP) {
-      pct = 0;
+      val = NS_STYLE_BG_POSITION_TOP;
     }
     else if (aMask & BG_BOTTOM) {
-      pct = 100;
-    }
-  }
-
-  return nsCSSValue(pct, eCSSUnit_Enumerated);
+      val = NS_STYLE_BG_POSITION_BOTTOM;
+    }
+  }
+
+  return nsCSSValue(val, eCSSUnit_Enumerated);
 }
 
 PRBool CSSParserImpl::ParseBackground(nsresult& aErrorCode)
 {
   nsAutoParseCompoundProperty compound(this);
 
   // Fill in the values that the shorthand will set if we don't find
   // other values.
@@ -4951,17 +4939,17 @@ PRBool CSSParserImpl::ParseBackground(ns
           return PR_FALSE;
         haveRepeat = PR_TRUE;
         if (!ParseSingleValueProperty(aErrorCode, mTempData.mColor.mBackRepeat,
                                       eCSSProperty_background_repeat)) {
           NS_NOTREACHED("should be able to parse");
           return PR_FALSE;
         }
       } else if (nsCSSProps::FindKeyword(keyword,
-                   kBackgroundXYPositionKTable, dummy)) {
+                   nsCSSProps::kBackgroundPositionKTable, dummy)) {
         if (havePosition)
           return PR_FALSE;
         havePosition = PR_TRUE;
         if (!ParseBackgroundPositionValues(aErrorCode)) {
           return PR_FALSE;
         }
       } else {
         if (haveColor)
@@ -5025,17 +5013,17 @@ PRBool CSSParserImpl::ParseBackgroundPos
     }
     // We have one percentage/length. Get the optional second
     // percentage/length/keyword.
     if (ParseVariant(aErrorCode, yValue, VARIANT_LP, nsnull)) {
       // We have two numbers
       return PR_TRUE;
     }
 
-    if (ParseEnum(aErrorCode, yValue, kBackgroundXYPositionKTable)) {
+    if (ParseEnum(aErrorCode, yValue, nsCSSProps::kBackgroundPositionKTable)) {
       PRInt32 yVal = yValue.GetIntValue();
       if (!(yVal & BG_CTB)) {
         // The second keyword can only be 'center', 'top', or 'bottom'
         return PR_FALSE;
       }
       yValue = BackgroundPositionMaskToCSSValue(yVal, PR_FALSE);
       return PR_TRUE;
     }
@@ -5048,20 +5036,20 @@ PRBool CSSParserImpl::ParseBackgroundPos
 
   // Now try keywords. We do this manually to allow for the first
   // appearance of "center" to apply to the either the x or y
   // position (it's ambiguous so we have to disambiguate). Each
   // allowed keyword value is assigned it's own bit. We don't allow
   // any duplicate keywords other than center. We try to get two
   // keywords but it's okay if there is only one.
   PRInt32 mask = 0;
-  if (ParseEnum(aErrorCode, xValue, kBackgroundXYPositionKTable)) {
+  if (ParseEnum(aErrorCode, xValue, nsCSSProps::kBackgroundPositionKTable)) {
     PRInt32 bit = xValue.GetIntValue();
     mask |= bit;
-    if (ParseEnum(aErrorCode, xValue, kBackgroundXYPositionKTable)) {
+    if (ParseEnum(aErrorCode, xValue, nsCSSProps::kBackgroundPositionKTable)) {
       bit = xValue.GetIntValue();
       if (mask & (bit & ~BG_CENTER)) {
         // Only the 'center' keyword can be duplicated.
         return PR_FALSE;
       }
       mask |= bit;
     }
     else {
--- a/layout/style/nsCSSPropList.h
+++ b/layout/style/nsCSSPropList.h
@@ -272,17 +272,17 @@ CSS_PROP_OUTLINE(-moz-outline-radius-bot
 CSS_PROP_BACKENDONLY(azimuth, azimuth, Azimuth, Aural, mAzimuth, eCSSType_Value, kAzimuthKTable)
 CSS_PROP_SHORTHAND(background, background, Background)
 CSS_PROP_BACKGROUND(background-attachment, background_attachment, BackgroundAttachment, Color, mBackAttachment, eCSSType_Value, kBackgroundAttachmentKTable)
 CSS_PROP_BACKGROUND(-moz-background-clip, _moz_background_clip, MozBackgroundClip, Color, mBackClip, eCSSType_Value, kBackgroundClipKTable)
 CSS_PROP_BACKGROUND(background-color, background_color, BackgroundColor, Color, mBackColor, eCSSType_Value, kBackgroundColorKTable)
 CSS_PROP_BACKGROUND(background-image, background_image, BackgroundImage, Color, mBackImage, eCSSType_Value, nsnull)
 CSS_PROP_BACKGROUND(-moz-background-inline-policy, _moz_background_inline_policy, MozBackgroundInlinePolicy, Color, mBackInlinePolicy, eCSSType_Value, kBackgroundInlinePolicyKTable)
 CSS_PROP_BACKGROUND(-moz-background-origin, _moz_background_origin, MozBackgroundOrigin, Color, mBackOrigin, eCSSType_Value, kBackgroundOriginKTable)
-CSS_PROP_BACKGROUND(background-position, background_position, BackgroundPosition, Color, mBackPosition, eCSSType_ValuePair, nsnull)
+CSS_PROP_BACKGROUND(background-position, background_position, BackgroundPosition, Color, mBackPosition, eCSSType_ValuePair, kBackgroundPositionKTable)
 CSS_PROP_BACKGROUND(background-repeat, background_repeat, BackgroundRepeat, Color, mBackRepeat, eCSSType_Value, kBackgroundRepeatKTable)
 CSS_PROP_DISPLAY(-moz-binding, binding, MozBinding, Display, mBinding, eCSSType_Value, nsnull) // XXX bug 3935
 CSS_PROP_SHORTHAND(border, border, Border)
 CSS_PROP_SHORTHAND(border-bottom, border_bottom, BorderBottom)
 CSS_PROP_BORDER(border-bottom-color, border_bottom_color, BorderBottomColor, Margin, mBorderColor.mBottom, eCSSType_Value, kBorderColorKTable)
 CSS_PROP_BORDER(-moz-border-bottom-colors, border_bottom_colors, MozBorderBottomColors, Margin, mBorderColors.mBottom, eCSSType_ValueList, nsnull)
 CSS_PROP_BORDER(border-bottom-style, border_bottom_style, BorderBottomStyle, Margin, mBorderStyle.mBottom, eCSSType_Value, kBorderStyleKTable)  // on/off will need reflow
 CSS_PROP_BORDER(border-bottom-width, border_bottom_width, BorderBottomWidth, Margin, mBorderWidth.mBottom, eCSSType_Value, kBorderWidthKTable)
--- a/layout/style/nsCSSProps.cpp
+++ b/layout/style/nsCSSProps.cpp
@@ -301,38 +301,36 @@ const PRInt32 nsCSSProps::kBackgroundInl
 
 const PRInt32 nsCSSProps::kBackgroundOriginKTable[] = {
   eCSSKeyword_border,     NS_STYLE_BG_ORIGIN_BORDER,
   eCSSKeyword_padding,    NS_STYLE_BG_ORIGIN_PADDING,
   eCSSKeyword_content,    NS_STYLE_BG_ORIGIN_CONTENT,
   eCSSKeyword_UNKNOWN,-1
 };
 
+// Note: Don't change this table unless you update
+// parseBackgroundPosition!
+
+const PRInt32 nsCSSProps::kBackgroundPositionKTable[] = {
+  eCSSKeyword_center, NS_STYLE_BG_POSITION_CENTER,
+  eCSSKeyword_top, NS_STYLE_BG_POSITION_TOP,
+  eCSSKeyword_bottom, NS_STYLE_BG_POSITION_BOTTOM,
+  eCSSKeyword_left, NS_STYLE_BG_POSITION_LEFT,
+  eCSSKeyword_right, NS_STYLE_BG_POSITION_RIGHT,
+  eCSSKeyword_UNKNOWN,-1
+};
+
 const PRInt32 nsCSSProps::kBackgroundRepeatKTable[] = {
   eCSSKeyword_no_repeat,  NS_STYLE_BG_REPEAT_OFF,
   eCSSKeyword_repeat,     NS_STYLE_BG_REPEAT_XY,
   eCSSKeyword_repeat_x,   NS_STYLE_BG_REPEAT_X,
   eCSSKeyword_repeat_y,   NS_STYLE_BG_REPEAT_Y,
   eCSSKeyword_UNKNOWN,-1
 };
 
-const PRInt32 nsCSSProps::kBackgroundXPositionKTable[] = {
-  eCSSKeyword_left,   0,
-  eCSSKeyword_center, 50,
-  eCSSKeyword_right,  100,
-  eCSSKeyword_UNKNOWN,-1
-};
-
-const PRInt32 nsCSSProps::kBackgroundYPositionKTable[] = {
-  eCSSKeyword_top,    0,
-  eCSSKeyword_center, 50,
-  eCSSKeyword_bottom, 100,
-  eCSSKeyword_UNKNOWN,-1
-};
-
 const PRInt32 nsCSSProps::kBorderCollapseKTable[] = {
   eCSSKeyword_collapse,  NS_STYLE_BORDER_COLLAPSE,
   eCSSKeyword_separate,  NS_STYLE_BORDER_SEPARATE,
   eCSSKeyword_UNKNOWN,-1
 };
 
 const PRInt32 nsCSSProps::kBorderColorKTable[] = {
   eCSSKeyword_transparent, NS_STYLE_COLOR_TRANSPARENT,
--- a/layout/style/nsCSSProps.h
+++ b/layout/style/nsCSSProps.h
@@ -113,19 +113,18 @@ public:
   // Keyword/Enum value tables
   static const PRInt32 kAppearanceKTable[];
   static const PRInt32 kAzimuthKTable[];
   static const PRInt32 kBackgroundAttachmentKTable[];
   static const PRInt32 kBackgroundClipKTable[];
   static const PRInt32 kBackgroundColorKTable[];
   static const PRInt32 kBackgroundInlinePolicyKTable[];
   static const PRInt32 kBackgroundOriginKTable[];
+  static const PRInt32 kBackgroundPositionKTable[];
   static const PRInt32 kBackgroundRepeatKTable[];
-  static const PRInt32 kBackgroundXPositionKTable[];
-  static const PRInt32 kBackgroundYPositionKTable[];
   static const PRInt32 kBorderCollapseKTable[];
   static const PRInt32 kBorderColorKTable[];
   static const PRInt32 kBorderStyleKTable[];
   static const PRInt32 kBorderWidthKTable[];
   static const PRInt32 kBoxAlignKTable[];
   static const PRInt32 kBoxDirectionKTable[];
   static const PRInt32 kBoxOrientKTable[];
   static const PRInt32 kBoxPackKTable[];
--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@@ -3096,17 +3096,30 @@ nsRuleNode::ComputeBackgroundData(nsStyl
   }
   else if (colorData.mBackPosition.mXValue.IsLengthUnit()) {
     bg->mBackgroundXPosition.mCoord = CalcLength(colorData.mBackPosition.mXValue, nsnull, 
                                                  aContext, mPresContext, inherited);
     bg->mBackgroundFlags |= NS_STYLE_BG_X_POSITION_LENGTH;
     bg->mBackgroundFlags &= ~NS_STYLE_BG_X_POSITION_PERCENT;
   }
   else if (eCSSUnit_Enumerated == colorData.mBackPosition.mXValue.GetUnit()) {
-    bg->mBackgroundXPosition.mFloat = (float)colorData.mBackPosition.mXValue.GetIntValue() / 100.0f;
+    switch (colorData.mBackPosition.mXValue.GetIntValue()) {
+      case NS_STYLE_BG_POSITION_LEFT:
+        bg->mBackgroundXPosition.mFloat = 0.0f;
+        break;
+      case NS_STYLE_BG_POSITION_RIGHT:
+        bg->mBackgroundXPosition.mFloat = 1.0f;
+        break;
+      default:
+        NS_NOTREACHED("unexpected value");
+        // fall through
+      case NS_STYLE_BG_POSITION_CENTER:
+        bg->mBackgroundXPosition.mFloat = 0.5f;
+        break;
+    }
     bg->mBackgroundFlags |= NS_STYLE_BG_X_POSITION_PERCENT;
     bg->mBackgroundFlags &= ~NS_STYLE_BG_X_POSITION_LENGTH;
   }
   else if (eCSSUnit_Inherit == colorData.mBackPosition.mXValue.GetUnit()) {
     inherited = PR_TRUE;
     bg->mBackgroundXPosition = parentBG->mBackgroundXPosition;
     bg->mBackgroundFlags &= ~(NS_STYLE_BG_X_POSITION_LENGTH | NS_STYLE_BG_X_POSITION_PERCENT);
     bg->mBackgroundFlags |= (parentFlags & (NS_STYLE_BG_X_POSITION_LENGTH | NS_STYLE_BG_X_POSITION_PERCENT));
@@ -3119,17 +3132,30 @@ nsRuleNode::ComputeBackgroundData(nsStyl
   }
   else if (colorData.mBackPosition.mYValue.IsLengthUnit()) {
     bg->mBackgroundYPosition.mCoord = CalcLength(colorData.mBackPosition.mYValue, nsnull,
                                                  aContext, mPresContext, inherited);
     bg->mBackgroundFlags |= NS_STYLE_BG_Y_POSITION_LENGTH;
     bg->mBackgroundFlags &= ~NS_STYLE_BG_Y_POSITION_PERCENT;
   }
   else if (eCSSUnit_Enumerated == colorData.mBackPosition.mYValue.GetUnit()) {
-    bg->mBackgroundYPosition.mFloat = (float)colorData.mBackPosition.mYValue.GetIntValue() / 100.0f;
+    switch (colorData.mBackPosition.mYValue.GetIntValue()) {
+      case NS_STYLE_BG_POSITION_TOP:
+        bg->mBackgroundYPosition.mFloat = 0.0f;
+        break;
+      case NS_STYLE_BG_POSITION_BOTTOM:
+        bg->mBackgroundYPosition.mFloat = 1.0f;
+        break;
+      default:
+        NS_NOTREACHED("unexpected value");
+        // fall through
+      case NS_STYLE_BG_POSITION_CENTER:
+        bg->mBackgroundYPosition.mFloat = 0.5f;
+        break;
+    }
     bg->mBackgroundFlags |= NS_STYLE_BG_Y_POSITION_PERCENT;
     bg->mBackgroundFlags &= ~NS_STYLE_BG_Y_POSITION_LENGTH;
   }
   else if (eCSSUnit_Inherit == colorData.mBackPosition.mYValue.GetUnit()) {
     inherited = PR_TRUE;
     bg->mBackgroundYPosition = parentBG->mBackgroundYPosition;
     bg->mBackgroundFlags &= ~(NS_STYLE_BG_Y_POSITION_LENGTH | NS_STYLE_BG_Y_POSITION_PERCENT);
     bg->mBackgroundFlags |= (parentFlags & (NS_STYLE_BG_Y_POSITION_LENGTH | NS_STYLE_BG_Y_POSITION_PERCENT));
--- a/layout/style/test/test_value_storage.html
+++ b/layout/style/test/test_value_storage.html
@@ -149,21 +149,16 @@ function xfail_idparseser(property, valu
   if ((property == "cue" || property == "pause") &&
       value.match(/ /) && !value.match(/^(.*) \1$/))
     return true;
 
   // Can't parse "cue: none none"
   if (property == "cue" && value == "none")
     return true;
 
-  // Can't serialize keywords for background-position (bug 258080)
-  if ((property == "background-position" || property == "background") &&
-      value.match(/(top|bottom|center|right|left)/))
-    return true;
-
   return false;
 }
 
 function xfail_idserparse_compute(property, value)
 {
   return false;
 }
 
@@ -176,21 +171,16 @@ function xfail_idsersplitparse_compute(p
       gCSSProperties[subprop].initial_values.indexOf(step1subcomp) == -1)
     return true;
 
   return false;
 }
 
 function xfail_idparsesplitser(property, value)
 {
-  // Can't serialize keywords for background-position (bug 258080)
-  if (property == "background" &&
-      value.match(/(top|bottom|center|right|left)/))
-    return true;
-
   return false;
 }
 
 function xfail_compute(property, value)
 {
   if (property in gBadCompute &&
       gBadCompute[property].indexOf(value) != -1)
     return true;