Bug 570896: Allow separate background-origin and background-clip to be set in the background shorthand. r=bzbarsky
authorL. David Baron <dbaron@dbaron.org>
Thu, 07 Mar 2013 17:59:32 -0800
changeset 124180 2aaba07995ed
parent 124179 501fea96c33a
child 124181 7ebd5b1fa3c3
push id24276
push userdbaron@mozilla.com
push dateFri, 08 Mar 2013 01:59:43 +0000
treeherdermozilla-inbound@7ebd5b1fa3c3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs570896
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 570896: Allow separate background-origin and background-clip to be set in the background shorthand. r=bzbarsky
layout/style/Declaration.cpp
layout/style/nsCSSParser.cpp
layout/style/test/property_database.js
layout/style/test/test_shorthand_property_getters.html
--- a/layout/style/Declaration.cpp
+++ b/layout/style/Declaration.cpp
@@ -456,33 +456,36 @@ Declaration::GetValue(nsCSSProperty aPro
         }
 
         NS_ABORT_IF_FALSE(clip->mValue.GetUnit() == eCSSUnit_Enumerated &&
                           origin->mValue.GetUnit() == eCSSUnit_Enumerated,
                           "should not have inherit/initial within list");
 
         if (clip->mValue.GetIntValue() != NS_STYLE_BG_CLIP_BORDER ||
             origin->mValue.GetIntValue() != NS_STYLE_BG_ORIGIN_PADDING) {
-          // The shorthand only has a single clip/origin value which sets
-          // both properties.  So if they're different (and non-default),
-          // we can't represent the state using the shorthand.
+          MOZ_ASSERT(nsCSSProps::kKeywordTableTable[
+                       eCSSProperty_background_origin] ==
+                     nsCSSProps::kBackgroundOriginKTable);
+          MOZ_ASSERT(nsCSSProps::kKeywordTableTable[
+                       eCSSProperty_background_clip] ==
+                     nsCSSProps::kBackgroundOriginKTable);
           MOZ_STATIC_ASSERT(NS_STYLE_BG_CLIP_BORDER ==
                             NS_STYLE_BG_ORIGIN_BORDER &&
                             NS_STYLE_BG_CLIP_PADDING ==
                             NS_STYLE_BG_ORIGIN_PADDING &&
                             NS_STYLE_BG_CLIP_CONTENT ==
                             NS_STYLE_BG_ORIGIN_CONTENT,
                             "bg-clip and bg-origin style constants must agree");
+          aValue.Append(PRUnichar(' '));
+          origin->mValue.AppendToString(eCSSProperty_background_origin, aValue);
+
           if (clip->mValue != origin->mValue) {
-            aValue.Truncate();
-            return;
+            aValue.Append(PRUnichar(' '));
+            clip->mValue.AppendToString(eCSSProperty_background_clip, aValue);
           }
-
-          aValue.Append(PRUnichar(' '));
-          clip->mValue.AppendToString(eCSSProperty_background_clip, aValue);
         }
 
         image = image->mNext;
         repeat = repeat->mNext;
         attachment = attachment->mNext;
         position = position->mNext;
         clip = clip->mNext;
         origin = origin->mNext;
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -6613,22 +6613,22 @@ CSSParserImpl::ParseBackgroundItem(CSSPa
   nsRefPtr<nsCSSValue::Array> positionArr = nsCSSValue::Array::Create(4);
   aState.mPosition->mValue.SetArrayValue(positionArr, eCSSUnit_Array);
   positionArr->Item(1).SetPercentValue(0.0f);
   positionArr->Item(3).SetPercentValue(0.0f);
   aState.mSize->mXValue.SetAutoValue();
   aState.mSize->mYValue.SetAutoValue();
 
   bool haveColor = false,
-         haveImage = false,
-         haveRepeat = false,
-         haveAttach = false,
-         havePositionAndSize = false,
-         haveOrigin = false,
-         haveSomething = false;
+       haveImage = false,
+       haveRepeat = false,
+       haveAttach = false,
+       havePositionAndSize = false,
+       haveOrigin = false,
+       haveSomething = false;
 
   while (GetToken(true)) {
     nsCSSTokenType tt = mToken.mType;
     UngetToken(); // ...but we'll still cheat and use mToken
     if (tt == eCSSToken_Symbol) {
       // ExpectEndProperty only looks for symbols, and nothing else will
       // show up as one.
       break;
@@ -6693,25 +6693,42 @@ CSSParserImpl::ParseBackgroundItem(CSSPa
         if (haveOrigin)
           return false;
         haveOrigin = true;
         if (!ParseSingleValueProperty(aState.mOrigin->mValue,
                                       eCSSProperty_background_origin)) {
           NS_NOTREACHED("should be able to parse");
           return false;
         }
+
+        // The spec allows a second box value (for background-clip),
+        // immediately following the first one (for background-origin).
+
+        // 'background-clip' and 'background-origin' use the same keyword table
+        MOZ_ASSERT(nsCSSProps::kKeywordTableTable[
+                     eCSSProperty_background_origin] ==
+                   nsCSSProps::kBackgroundOriginKTable);
+        MOZ_ASSERT(nsCSSProps::kKeywordTableTable[
+                     eCSSProperty_background_clip] ==
+                   nsCSSProps::kBackgroundOriginKTable);
         MOZ_STATIC_ASSERT(NS_STYLE_BG_CLIP_BORDER ==
                           NS_STYLE_BG_ORIGIN_BORDER &&
                           NS_STYLE_BG_CLIP_PADDING ==
                           NS_STYLE_BG_ORIGIN_PADDING &&
                           NS_STYLE_BG_CLIP_CONTENT ==
                           NS_STYLE_BG_ORIGIN_CONTENT,
                           "bg-clip and bg-origin style constants must agree");
 
-        aState.mClip->mValue = aState.mOrigin->mValue;
+        if (!ParseSingleValueProperty(aState.mClip->mValue,
+                                      eCSSProperty_background_clip)) {
+          // When exactly one <box> value is set, it is used for both
+          // 'background-origin' and 'background-clip'.
+          // See assertions above showing these values are compatible.
+          aState.mClip->mValue = aState.mOrigin->mValue;
+        }
       } else {
         if (haveColor)
           return false;
         haveColor = true;
         if (!ParseSingleValueProperty(aState.mColor,
                                       eCSSProperty_background_color)) {
           return false;
         }
--- a/layout/style/test/property_database.js
+++ b/layout/style/test/property_database.js
@@ -1320,16 +1320,19 @@ var gCSSProperties = {
 				"0% top url(404.png), url(404.png) 0% top",
 				"fixed repeat-y top left url(404.png), repeat-x green",
 				"url(404.png), -moz-linear-gradient(20px 20px -45deg, blue, green), -moz-element(#a) black",
 				"top left / contain, bottom right / cover",
 				/* test cases with clip+origin in the shorthand */
 				"url(404.png) green padding-box",
 				"url(404.png) border-box transparent",
 				"content-box url(404.png) blue",
+				"url(404.png) green padding-box padding-box",
+				"url(404.png) green padding-box border-box",
+				"content-box border-box url(404.png) blue",
 		],
 		invalid_values: [
 			/* mixes with keywords have to be in correct order */
 			"50% left", "top 50%",
 			/* no quirks mode colors */
 			"-moz-radial-gradient(10% bottom, ffffff, black) scroll no-repeat",
 			/* no quirks mode lengths */
 			"-moz-linear-gradient(10 10px -45deg, red, blue) repeat",
@@ -1346,16 +1349,21 @@ var gCSSProperties = {
 			"url(404.png) rgba(0, 0, 0, 0), url(404.png)",
 			"url(404.png) rgb(255, 0, 0), url(404.png)",
 			"url(404.png) rgba(0, 0, 0, 0), url(404.png) rgba(0, 0, 0, 0)",
 			"url(404.png) rgba(0, 0, 0, 0) rgb(255, 0, 0), url(404.png) rgba(0, 0, 0, 0) rgb(255, 0, 0)",
 			"url(404.png) rgb(255, 0, 0), url(404.png) rgb(255, 0, 0)",
 			/* bug 513395: old syntax for gradients */
 			"-moz-radial-gradient(10% bottom, 30px, 20px 20px, 10px, from(#ffffff), to(black)) scroll no-repeat",
 			"-moz-linear-gradient(10px 10px, 20px 20px, from(red), to(blue)) repeat",
+			/* clip and origin separated in the shorthand */
+			"url(404.png) padding-box green border-box",
+			"url(404.png) padding-box green padding-box",
+			"transparent padding-box url(404.png) border-box",
+			"transparent padding-box url(404.png) padding-box",
 		]
 	},
 	"background-attachment": {
 		domProp: "backgroundAttachment",
 		inherited: false,
 		type: CSS_TYPE_LONGHAND,
 		initial_values: [ "scroll" ],
 		other_values: [ "fixed", "scroll,scroll", "fixed, scroll", "scroll, fixed, scroll", "fixed, fixed" ],
--- a/layout/style/test/test_shorthand_property_getters.html
+++ b/layout/style/test/test_shorthand_property_getters.html
@@ -102,37 +102,40 @@ is(e.style.cssText, "border-style: ridge
 // shorthand syntax are present.
 e.setAttribute("style", "font: medium serif");
 isnot(e.style.font, "", "should have font shorthand");
 e.setAttribute("style", "font: medium serif; font-size-adjust: 0.45");
 is(e.style.font, "", "should not have font shorthand");
 e.setAttribute("style", "font: medium serif; font-stretch: condensed");
 is(e.style.font, "", "should not have font shorthand");
 
-// For background, we can only express the value as a shorthand if
-// origin and clip are both their default, or if they're both the same.
-// ... or at least we will once we support them in the shorthand.
+// Test that all combinations of background-clip and background-origin
+// can be expressed in the shorthand (which wasn't the case previously).
 e.setAttribute("style", "background: red");
 isnot(e.style.background, "", "should have background shorthand");
 e.setAttribute("style", "background: red; background-origin: border-box");
 isnot(e.style.background, "", "should have background shorthand (origin:border-box)");
 e.setAttribute("style", "background: red; background-clip: padding-box");
 isnot(e.style.background, "", "should have background shorthand (clip:padding-box)");
 e.setAttribute("style", "background: red; background-origin: content-box");
-is(e.style.background, "", "should not have background shorthand (origin:content-box)");
+isnot(e.style.background, "", "should have background shorthand (origin:content-box)");
 e.setAttribute("style", "background: red; background-clip: content-box");
-is(e.style.background, "", "should not have background shorthand (clip:content-box)");
+isnot(e.style.background, "", "should have background shorthand (clip:content-box)");
 e.setAttribute("style", "background: red; background-clip: content-box; background-origin: content-box;");
 isnot(e.style.background, "", "should have background shorthand (clip:content-box;origin:content-box)");
+
+// Test background-size in the background shorthand.
 e.setAttribute("style", "background: red; background-size: 100% 100%");
 isnot(e.style.background, "", "should have background shorthand (size:100% 100%)");
 e.setAttribute("style", "background: red; background-size: 100% auto");
 isnot(e.style.background, "", "should have background shorthand (size:100% auto)");
 e.setAttribute("style", "background: red; background-size: auto 100%");
 isnot(e.style.background, "", "should have background shorthand (size:auto 100%)");
+
+// Test background-inline-policy not interacting with the background shorthand.
 e.setAttribute("style", "background: red; -moz-background-inline-policy: each-box");
 isnot(e.style.background, "", "should have background shorthand (-moz-background-inline-policy not relevant)");
 
 // Check that we only serialize background when the lists (of layers) for
 // the subproperties are the same length.
 e.setAttribute("style", "background-clip: border-box, padding-box, border-box; background-origin: border-box, padding-box, padding-box; background-size: cover, auto, contain; background-color: blue; background-image: url(404.png), none, url(404-2.png); background-attachment: fixed, scroll, scroll; background-position: top left, center, 30px 50px; background-repeat: repeat-x, repeat, no-repeat");
 isnot(e.style.background, "", "should have background shorthand (all lists length 3)");
 e.setAttribute("style", "background-clip: border-box, padding-box, border-box, border-box; background-origin: border-box, padding-box, padding-box; background-size: cover, auto, contain; background-color: blue; background-image: url(404.png), none, url(404-2.png); background-attachment: fixed, scroll, scroll; background-position: top left, center, 30px 50px; background-repeat: repeat-x, repeat, no-repeat");