Bug 1140623 - Correct mochitest failures that occur when the layout.css.scroll-snap.enabled preference is enabled (V2 Patch). r=heycam
☠☠ backed out by 8fe7716a118b ☠ ☠
authorKearwood (Kip) Gilbert <kgilbert@mozilla.com>
Thu, 12 Mar 2015 11:22:00 +0100
changeset 233527 ef3881f8d1a3f2895d3f431db619c2f08962744a
parent 233526 c2e29988051b167486f1ebb7affa9639b548e1c2
child 233528 855a9c8ad7e4ec9375fa93fb1f7ccd26d25a1613
push id28417
push userryanvm@gmail.com
push dateFri, 13 Mar 2015 19:52:44 +0000
treeherdermozilla-central@977add19414a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1140623, 657143
milestone39.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 1140623 - Correct mochitest failures that occur when the layout.css.scroll-snap.enabled preference is enabled (V2 Patch). r=heycam - CSS scroll snapping related attributes are now sorted correctly - Now passes mochitests with CSS scroll snapping enabled: - layout/style/test/test_bug657143.html - layout/style/test/test_compute_data_with_start_struct.html - layout/style/test/test_property_syntax_errors.html - layout/style/test/test_style_struct_copy_constructors.html - layout/style/test/test_value_computation.html
layout/style/nsCSSParser.cpp
layout/style/nsCSSPropList.h
layout/style/nsComputedDOMStylePropertyList.h
layout/style/nsRuleNode.cpp
layout/style/test/property_database.js
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -15105,16 +15105,17 @@ CSSParserImpl::ParseScrollSnapPoints(nsC
       SkipUntil(')');
       return false;
     }
     nsRefPtr<nsCSSValue::Array> functionArray =
       aValue.InitFunction(eCSSKeyword_repeat, 1);
     functionArray->Item(1) = lengthValue;
     return true;
   }
+  UngetToken();
   return false;
 }
 
 
 bool
 CSSParserImpl::ParseScrollSnapDestination(nsCSSValue& aValue)
 {
   if (ParseVariant(aValue, VARIANT_INHERIT, nullptr)) {
--- a/layout/style/nsCSSPropList.h
+++ b/layout/style/nsCSSPropList.h
@@ -3056,41 +3056,40 @@ CSS_PROP_DISPLAY(
     ScrollBehavior,
     CSS_PROPERTY_PARSE_VALUE,
     "layout.css.scroll-behavior.property-enabled",
     VARIANT_HK,
     kScrollBehaviorKTable,
     CSS_PROP_NO_OFFSET,
     eStyleAnimType_None)
 CSS_PROP_DISPLAY(
-    scroll-snap-type-x,
-    scroll_snap_type_x,
-    ScrollSnapTypeX,
-    CSS_PROPERTY_PARSE_VALUE,
+    scroll-snap-coordinate,
+    scroll_snap_coordinate,
+    ScrollSnapCoordinate,
+    CSS_PROPERTY_PARSE_VALUE |
+        CSS_PROPERTY_VALUE_PARSER_FUNCTION |
+        CSS_PROPERTY_VALUE_LIST_USES_COMMAS |
+        CSS_PROPERTY_STORES_CALC,
     "layout.css.scroll-snap.enabled",
-    VARIANT_HK,
-    kScrollSnapTypeKTable,
+    0,
+    kBackgroundPositionKTable,
     CSS_PROP_NO_OFFSET,
     eStyleAnimType_None)
 CSS_PROP_DISPLAY(
-    scroll-snap-type-y,
-    scroll_snap_type_y,
-    ScrollSnapTypeY,
-    CSS_PROPERTY_PARSE_VALUE,
+    scroll-snap-destination,
+    scroll_snap_destination,
+    ScrollSnapDestination,
+    CSS_PROPERTY_PARSE_VALUE |
+        CSS_PROPERTY_VALUE_PARSER_FUNCTION |
+        CSS_PROPERTY_STORES_CALC,
     "layout.css.scroll-snap.enabled",
-    VARIANT_HK,
-    kScrollSnapTypeKTable,
+    0,
+    kBackgroundPositionKTable,
     CSS_PROP_NO_OFFSET,
     eStyleAnimType_None)
-CSS_PROP_SHORTHAND(
-    scroll-snap-type,
-    scroll_snap_type,
-    ScrollSnapType,
-    CSS_PROPERTY_PARSE_FUNCTION,
-    "layout.css.scroll-snap.enabled")
 CSS_PROP_DISPLAY(
     scroll-snap-points-x,
     scroll_snap_points_x,
     ScrollSnapPointsX,
     CSS_PROPERTY_PARSE_VALUE |
         CSS_PROPERTY_VALUE_PARSER_FUNCTION |
         CSS_PROPERTY_STORES_CALC,
     "layout.css.scroll-snap.enabled",
@@ -3105,39 +3104,40 @@ CSS_PROP_DISPLAY(
     CSS_PROPERTY_PARSE_VALUE |
         CSS_PROPERTY_VALUE_PARSER_FUNCTION |
         CSS_PROPERTY_STORES_CALC,
     "layout.css.scroll-snap.enabled",
     0,
     nullptr,
     CSS_PROP_NO_OFFSET,
     eStyleAnimType_None)
+CSS_PROP_SHORTHAND(
+    scroll-snap-type,
+    scroll_snap_type,
+    ScrollSnapType,
+    CSS_PROPERTY_PARSE_FUNCTION,
+    "layout.css.scroll-snap.enabled")
 CSS_PROP_DISPLAY(
-    scroll-snap-destination,
-    scroll_snap_destination,
-    ScrollSnapDestination,
-    CSS_PROPERTY_PARSE_VALUE |
-        CSS_PROPERTY_VALUE_PARSER_FUNCTION |
-        CSS_PROPERTY_STORES_CALC,
+    scroll-snap-type-x,
+    scroll_snap_type_x,
+    ScrollSnapTypeX,
+    CSS_PROPERTY_PARSE_VALUE,
     "layout.css.scroll-snap.enabled",
-    0,
-    kBackgroundPositionKTable,
+    VARIANT_HK,
+    kScrollSnapTypeKTable,
     CSS_PROP_NO_OFFSET,
     eStyleAnimType_None)
 CSS_PROP_DISPLAY(
-    scroll-snap-coordinate,
-    scroll_snap_coordinate,
-    ScrollSnapCoordinate,
-    CSS_PROPERTY_PARSE_VALUE |
-        CSS_PROPERTY_VALUE_PARSER_FUNCTION |
-        CSS_PROPERTY_VALUE_LIST_USES_COMMAS |
-        CSS_PROPERTY_STORES_CALC,
+    scroll-snap-type-y,
+    scroll_snap_type_y,
+    ScrollSnapTypeY,
+    CSS_PROPERTY_PARSE_VALUE,
     "layout.css.scroll-snap.enabled",
-    0,
-    kBackgroundPositionKTable,
+    VARIANT_HK,
+    kScrollSnapTypeKTable,
     CSS_PROP_NO_OFFSET,
     eStyleAnimType_None)
 CSS_PROP_BACKENDONLY(
     size,
     size,
     Size,
     CSS_PROPERTY_PARSE_FUNCTION,
     "",
--- a/layout/style/nsComputedDOMStylePropertyList.h
+++ b/layout/style/nsComputedDOMStylePropertyList.h
@@ -195,22 +195,22 @@ COMPUTED_STYLE_PROP(perspective_origin, 
 COMPUTED_STYLE_PROP(pointer_events,                PointerEvents)
 COMPUTED_STYLE_PROP(position,                      Position)
 COMPUTED_STYLE_PROP(quotes,                        Quotes)
 COMPUTED_STYLE_PROP(resize,                        Resize)
 COMPUTED_STYLE_PROP(right,                         Right)
 COMPUTED_STYLE_PROP(ruby_align,                    RubyAlign)
 COMPUTED_STYLE_PROP(ruby_position,                 RubyPosition)
 COMPUTED_STYLE_PROP(scroll_behavior,               ScrollBehavior)
+COMPUTED_STYLE_PROP(scroll_snap_coordinate,        ScrollSnapCoordinate)
+COMPUTED_STYLE_PROP(scroll_snap_destination,       ScrollSnapDestination)
+COMPUTED_STYLE_PROP(scroll_snap_points_x,          ScrollSnapPointsX)
+COMPUTED_STYLE_PROP(scroll_snap_points_y,          ScrollSnapPointsY)
 COMPUTED_STYLE_PROP(scroll_snap_type_x,            ScrollSnapTypeX)
 COMPUTED_STYLE_PROP(scroll_snap_type_y,            ScrollSnapTypeY)
-COMPUTED_STYLE_PROP(scroll_snap_points_x,          ScrollSnapPointsX)
-COMPUTED_STYLE_PROP(scroll_snap_points_y,          ScrollSnapPointsY)
-COMPUTED_STYLE_PROP(scroll_snap_destination,       ScrollSnapDestination)
-COMPUTED_STYLE_PROP(scroll_snap_coordinate,        ScrollSnapCoordinate)
 //// COMPUTED_STYLE_PROP(size,                     Size)
 COMPUTED_STYLE_PROP(table_layout,                  TableLayout)
 COMPUTED_STYLE_PROP(text_align,                    TextAlign)
 COMPUTED_STYLE_PROP(text_combine_upright,          TextCombineUpright)
 COMPUTED_STYLE_PROP(text_decoration,               TextDecoration)
 COMPUTED_STYLE_PROP(text_decoration_color,         TextDecorationColor)
 COMPUTED_STYLE_PROP(text_decoration_line,          TextDecorationLine)
 COMPUTED_STYLE_PROP(text_decoration_style,         TextDecorationStyle)
--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@@ -5274,20 +5274,21 @@ nsRuleNode::ComputeDisplayData(void* aSt
               canStoreInRuleTree,
               SETDSC_ENUMERATED | SETDSC_UNSET_INITIAL,
               parentDisplay->mScrollSnapTypeY, NS_STYLE_SCROLL_SNAP_TYPE_NONE,
               0, 0, 0, 0);
 
   // scroll-snap-points-x: none, inherit, initial
   const nsCSSValue& scrollSnapPointsX = *aRuleData->ValueForScrollSnapPointsX();
   switch (scrollSnapPointsX.GetUnit()) {
+    case eCSSUnit_Null:
+      break;
     case eCSSUnit_Initial:
     case eCSSUnit_Unset:
     case eCSSUnit_None:
-    case eCSSUnit_Null:
       display->mScrollSnapPointsX.SetNoneValue();
       break;
     case eCSSUnit_Inherit:
       display->mScrollSnapPointsX = parentDisplay->mScrollSnapPointsX;
       canStoreInRuleTree = false;
       break;
     case eCSSUnit_Function: {
       nsCSSValue::Array* func = scrollSnapPointsX.GetArrayValue();
@@ -5308,20 +5309,21 @@ nsRuleNode::ComputeDisplayData(void* aSt
     }
     default:
       NS_NOTREACHED("unexpected unit");
   }
 
   // scroll-snap-points-y: none, inherit, initial
   const nsCSSValue& scrollSnapPointsY = *aRuleData->ValueForScrollSnapPointsY();
   switch (scrollSnapPointsY.GetUnit()) {
+    case eCSSUnit_Null:
+      break;
     case eCSSUnit_Initial:
     case eCSSUnit_Unset:
     case eCSSUnit_None:
-    case eCSSUnit_Null:
       display->mScrollSnapPointsY.SetNoneValue();
       break;
     case eCSSUnit_Inherit:
       display->mScrollSnapPointsY = parentDisplay->mScrollSnapPointsY;
       canStoreInRuleTree = false;
       break;
     case eCSSUnit_Function: {
       nsCSSValue::Array* func = scrollSnapPointsY.GetArrayValue();
@@ -5342,19 +5344,20 @@ nsRuleNode::ComputeDisplayData(void* aSt
     }
     default:
       NS_NOTREACHED("unexpected unit");
   }
 
   // scroll-snap-destination: inherit, initial
   const nsCSSValue& snapDestination = *aRuleData->ValueForScrollSnapDestination();
   switch (snapDestination.GetUnit()) {
+    case eCSSUnit_Null:
+      break;
     case eCSSUnit_Initial:
     case eCSSUnit_Unset:
-    case eCSSUnit_Null:
       display->mScrollSnapDestination.SetInitialZeroValues();
       break;
     case eCSSUnit_Inherit:
       display->mScrollSnapDestination = parentDisplay->mScrollSnapDestination;
       canStoreInRuleTree = false;
       break;
     default: {
         ComputePositionValue(aContext, snapDestination,
@@ -5362,20 +5365,21 @@ nsRuleNode::ComputeDisplayData(void* aSt
       }
   }
 
   // scroll-snap-coordinate: none, inherit, initial
   typedef nsStyleBackground::Position Position;
 
   const nsCSSValue& snapCoordinate = *aRuleData->ValueForScrollSnapCoordinate();
   switch (snapCoordinate.GetUnit()) {
+    case eCSSUnit_Null:
+      break;
     case eCSSUnit_Initial:
     case eCSSUnit_Unset:
     case eCSSUnit_None:
-    case eCSSUnit_Null:
       // Unset and Initial is none, indicated by an empty array
       display->mScrollSnapCoordinate.Clear();
       break;
     case eCSSUnit_Inherit:
       display->mScrollSnapCoordinate = parentDisplay->mScrollSnapCoordinate;
       canStoreInRuleTree = false;
       break;
     case eCSSUnit_List: {
--- a/layout/style/test/property_database.js
+++ b/layout/style/test/property_database.js
@@ -6384,74 +6384,16 @@ if (SpecialPowers.getBoolPref("layout.cs
     type: CSS_TYPE_LONGHAND,
     initial_values: [ "auto" ],
     other_values: [ "smooth" ],
     invalid_values: [ "none",  "1px" ]
   };
 }
 
 if (SpecialPowers.getBoolPref("layout.css.scroll-snap.enabled")) {
-  gCSSProperties["scroll-snap-type-x"] = {
-    domProp: "scrollSnapTypeX",
-    inherited: false,
-    type: CSS_TYPE_LONGHAND,
-    initial_values: [ "none" ],
-    other_values: ["mandatory", "proximity"],
-    invalid_values: [ "auto",  "1px" ]
-  };
-  gCSSProperties["scroll-snap-type-y"] = {
-    domProp: "scrollSnapTypeY",
-    inherited: false,
-    type: CSS_TYPE_LONGHAND,
-    initial_values: [ "none" ],
-    other_values: ["mandatory", "proximity"],
-    invalid_values: [ "auto",  "1px" ]
-  };
-  gCSSProperties["scroll-snap-type"] = {
-    domProp: "scrollSnapType",
-    inherited: false,
-    type: CSS_TYPE_TRUE_SHORTHAND,
-    subproperties: [ "scroll-snap-type-x", "scroll-snap-type-y" ],
-    initial_values: [ "none" ],
-    other_values: [ "mandatory", "proximity" ],
-    invalid_values: [ "auto",  "1px" ]
-  };
-  gCSSProperties["scroll-snap-points-x"] = {
-    domProp: "scrollSnapPointsX",
-    inherited: false,
-    type: CSS_TYPE_LONGHAND,
-    initial_values: [ "none" ],
-    other_values: [ "repeat(100%)", "repeat(120px)", "repeat(calc(3*25px))" ],
-    invalid_values: [ "auto", "1px", "left", "rgb(1,2,3)" ]
-  }
-  gCSSProperties["scroll-snap-points-y"] = {
-    domProp: "scrollSnapPointsY",
-    inherited: false,
-    type: CSS_TYPE_LONGHAND,
-    initial_values: [ "none" ],
-    other_values: [ "repeat(100%)", "repeat(120px)", "repeat(calc(3*25px))" ],
-    invalid_values: [ "auto", "1px", "top", "rgb(1,2,3)" ]
-  }
-  gCSSProperties["scroll-snap-destination"] = {
-    domProp: "scrollSnapDestination",
-    inherited: false,
-    type: CSS_TYPE_LONGHAND,
-    initial_values: [ "0px 0px" ],
-    other_values: [ "25% 25%", "6px 5px", "20% 3em", "0 0", "0in 1in",
-                    "top", "right", "top left", "top right", "center",
-                    "calc(2px)",
-                    "calc(50%)",
-                    "calc(3*25px)",
-                    "calc(3*25px) 5px",
-                    "5px calc(3*25px)",
-                    "calc(20%) calc(3*25px)",
-                    "calc(25px*3)",
-                    "calc(3*25px + 50%)"],
-    invalid_values: [ "auto", "none", "default" ]
-  }
   gCSSProperties["scroll-snap-coordinate"] = {
     domProp: "scrollSnapCoordinate",
     inherited: false,
     type: CSS_TYPE_LONGHAND,
     initial_values: [ "none" ],
     other_values: [ "25% 25%", "top", "0px 100px, 10em 50%",
                     "top left, top right, bottom left, bottom right, center",
                     "calc(2px)",
@@ -6460,16 +6402,74 @@ if (SpecialPowers.getBoolPref("layout.cs
                     "calc(3*25px) 5px",
                     "5px calc(3*25px)",
                     "calc(20%) calc(3*25px)",
                     "calc(25px*3)",
                     "calc(3*25px + 50%)",
                     "calc(20%) calc(3*25px), center"],
     invalid_values: [ "auto", "default" ]
   }
+  gCSSProperties["scroll-snap-destination"] = {
+    domProp: "scrollSnapDestination",
+    inherited: false,
+    type: CSS_TYPE_LONGHAND,
+    initial_values: [ "0px 0px" ],
+    other_values: [ "25% 25%", "6px 5px", "20% 3em", "0in 1in",
+                    "top", "right", "top left", "top right", "center",
+                    "calc(2px)",
+                    "calc(50%)",
+                    "calc(3*25px)",
+                    "calc(3*25px) 5px",
+                    "5px calc(3*25px)",
+                    "calc(20%) calc(3*25px)",
+                    "calc(25px*3)",
+                    "calc(3*25px + 50%)"],
+    invalid_values: [ "auto", "none", "default" ]
+  }
+  gCSSProperties["scroll-snap-points-x"] = {
+    domProp: "scrollSnapPointsX",
+    inherited: false,
+    type: CSS_TYPE_LONGHAND,
+    initial_values: [ "none" ],
+    other_values: [ "repeat(100%)", "repeat(120px)", "repeat(calc(3*25px))" ],
+    invalid_values: [ "auto", "1px", "left", "rgb(1,2,3)" ]
+  }
+  gCSSProperties["scroll-snap-points-y"] = {
+    domProp: "scrollSnapPointsY",
+    inherited: false,
+    type: CSS_TYPE_LONGHAND,
+    initial_values: [ "none" ],
+    other_values: [ "repeat(100%)", "repeat(120px)", "repeat(calc(3*25px))" ],
+    invalid_values: [ "auto", "1px", "top", "rgb(1,2,3)" ]
+  }
+  gCSSProperties["scroll-snap-type"] = {
+    domProp: "scrollSnapType",
+    inherited: false,
+    type: CSS_TYPE_TRUE_SHORTHAND,
+    subproperties: [ "scroll-snap-type-x", "scroll-snap-type-y" ],
+    initial_values: [ "none" ],
+    other_values: [ "mandatory", "proximity" ],
+    invalid_values: [ "auto",  "1px" ]
+  };
+  gCSSProperties["scroll-snap-type-x"] = {
+    domProp: "scrollSnapTypeX",
+    inherited: false,
+    type: CSS_TYPE_LONGHAND,
+    initial_values: [ "none" ],
+    other_values: ["mandatory", "proximity"],
+    invalid_values: [ "auto",  "1px" ]
+  };
+  gCSSProperties["scroll-snap-type-y"] = {
+    domProp: "scrollSnapTypeY",
+    inherited: false,
+    type: CSS_TYPE_LONGHAND,
+    initial_values: [ "none" ],
+    other_values: ["mandatory", "proximity"],
+    invalid_values: [ "auto",  "1px" ]
+  };
 }
 
 if (SpecialPowers.getBoolPref("layout.css.unset-value.enabled")) {
   gCSSProperties["animation-direction"].invalid_values.push("normal, unset");
   gCSSProperties["animation-name"].invalid_values.push("bounce, unset", "unset, bounce");
   gCSSProperties["-moz-border-bottom-colors"].invalid_values.push("red unset", "unset red");
   gCSSProperties["-moz-border-left-colors"].invalid_values.push("red unset", "unset red");
   gCSSProperties["border-radius"].invalid_values.push("unset 2px", "unset / 2px", "2px unset", "2px / unset");