Bug 1519944 - [css-logical] Implement the margin-block/inline shorthands. r=emilio
authorMats Palmgren <mats@mozilla.com>
Tue, 15 Jan 2019 02:27:44 +0100
changeset 510956 382bf54cf7cfe27ba2efe75e8bc2a5f46cd64534
parent 510955 62f90a1062e5d87c38c5431c17af6fbdd8c7b059
child 510957 28294d20a81b2eb48e6536574810f5e0c8713f58
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1519944
milestone66.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 1519944 - [css-logical] Implement the margin-block/inline shorthands. r=emilio
devtools/shared/css/generated/properties-db.js
layout/style/nsCSSProps.h
layout/style/test/property_database.js
servo/components/style/properties/shorthands/margin.mako.rs
servo/ports/geckolib/glue.rs
testing/web-platform/meta/css/css-logical/animation-001.html.ini
testing/web-platform/meta/css/css-logical/logical-box-margin.html.ini
--- a/devtools/shared/css/generated/properties-db.js
+++ b/devtools/shared/css/generated/properties-db.js
@@ -6838,16 +6838,30 @@ exports.CSS_PROPERTIES = {
     "supports": [],
     "values": [
       "auto",
       "inherit",
       "initial",
       "unset"
     ]
   },
+  "margin-block": {
+    "isInherited": false,
+    "subproperties": [
+      "margin-block-start",
+      "margin-block-end"
+    ],
+    "supports": [],
+    "values": [
+      "auto",
+      "inherit",
+      "initial",
+      "unset"
+    ]
+  },
   "margin-block-end": {
     "isInherited": false,
     "subproperties": [
       "margin-block-end"
     ],
     "supports": [],
     "values": [
       "auto",
@@ -6877,16 +6891,30 @@ exports.CSS_PROPERTIES = {
     "supports": [],
     "values": [
       "auto",
       "inherit",
       "initial",
       "unset"
     ]
   },
+  "margin-inline": {
+    "isInherited": false,
+    "subproperties": [
+      "margin-inline-start",
+      "margin-inline-end"
+    ],
+    "supports": [],
+    "values": [
+      "auto",
+      "inherit",
+      "initial",
+      "unset"
+    ]
+  },
   "margin-inline-end": {
     "isInherited": false,
     "subproperties": [
       "margin-inline-end"
     ],
     "supports": [],
     "values": [
       "auto",
--- a/layout/style/nsCSSProps.h
+++ b/layout/style/nsCSSProps.h
@@ -160,16 +160,17 @@ class nsCSSProps {
   static bool PropHasFlags(nsCSSPropertyID aProperty, Flags aFlags) {
     MOZ_ASSERT(0 <= aProperty && aProperty < eCSSProperty_COUNT,
                "out of range");
     return (nsCSSProps::kFlagsTable[aProperty] & aFlags) == aFlags;
   }
 
   static nsCSSPropertyID Physicalize(nsCSSPropertyID aProperty,
                                      const mozilla::ComputedStyle& aStyle) {
+    MOZ_ASSERT(!IsShorthand(aProperty));
     if (PropHasFlags(aProperty, Flags::IsLogical)) {
       return Servo_ResolveLogicalProperty(aProperty, &aStyle);
     }
     return aProperty;
   }
 
  private:
   // A table for shorthand properties.  The appropriate index is the
--- a/layout/style/test/property_database.js
+++ b/layout/style/test/property_database.js
@@ -1987,16 +1987,32 @@ var gCSSProperties = {
   "-moz-image-region": {
     domProp: "MozImageRegion",
     inherited: true,
     type: CSS_TYPE_LONGHAND,
     initial_values: [ "auto" ],
     other_values: [ "rect(3px 20px 15px 4px)", "rect(17px, 21px, 33px, 2px)" ],
     invalid_values: [ "rect(17px, 21px, 33, 2px)" ]
   },
+  "margin-inline": {
+    domProp: "marginInline",
+    inherited: false,
+    type: CSS_TYPE_TRUE_SHORTHAND,
+    subproperties: [ "margin-inline-start", "margin-inline-end" ],
+    initial_values: [ "0", "0px 0em" ],
+    other_values: [ "1px", "3em 1%", "5%",
+      "calc(2px) 1%",
+      "calc(-2px) 1%",
+      "calc(50%) 1%",
+      "calc(3*25px) calc(2px)",
+      "calc(25px*3) 1em",
+      "calc(3*25px + 50%) calc(3*25px - 50%)",
+    ],
+    invalid_values: [ "5", "..25px", ".+5px", ".px", "-.px", "++5px", "-+4px", "+-3px", "--7px", "+-.6px", "-+.5px", "++.7px", "--.4px" ],
+  },
   "margin-inline-end": {
     domProp: "marginInlineEnd",
     inherited: false,
     type: CSS_TYPE_LONGHAND,
     applies_to_first_letter: true,
     logical: true,
     /* no subproperties */
     /* auto may or may not be initial */
@@ -6209,16 +6225,32 @@ var gCSSProperties = {
       "calc(2px)",
       "calc(50%)",
       "calc(3*25px)",
       "calc(25px*3)",
       "calc(3*25px + 50%)",
     ],
     invalid_values: [ "none" ],
   },
+  "margin-block": {
+    domProp: "marginBlock",
+    inherited: false,
+    type: CSS_TYPE_TRUE_SHORTHAND,
+    subproperties: [ "margin-block-start", "margin-block-end" ],
+    initial_values: [ "0", "0px 0em" ],
+    other_values: [ "1px", "3em 1%", "5%",
+      "calc(2px) 1%",
+      "calc(-2px) 1%",
+      "calc(50%) 1%",
+      "calc(3*25px) calc(2px)",
+      "calc(25px*3) 1em",
+      "calc(3*25px + 50%) calc(3*25px - 50%)",
+    ],
+    invalid_values: [ "5", "..25px", ".+5px", ".px", "-.px", "++5px", "-+4px", "+-3px", "--7px", "+-.6px", "-+.5px", "++.7px", "--.4px" ],
+  },
   "margin-block-end": {
     domProp: "marginBlockEnd",
     inherited: false,
     type: CSS_TYPE_LONGHAND,
     applies_to_first_letter: true,
     logical: true,
     /* XXX testing auto has prerequisites */
     initial_values: [ "0", "0px", "0%", "calc(0pt)", "calc(0% + 0px)" ],
--- a/servo/components/style/properties/shorthands/margin.mako.rs
+++ b/servo/components/style/properties/shorthands/margin.mako.rs
@@ -3,8 +3,51 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 <%namespace name="helpers" file="/helpers.mako.rs" />
 
 ${helpers.four_sides_shorthand("margin", "margin-%s", "specified::LengthPercentageOrAuto::parse",
                                spec="https://drafts.csswg.org/css-box/#propdef-margin",
                                allowed_in_page_rule=True,
                                allow_quirks=True)}
+
+% for axis in ["block", "inline"]:
+    <%
+        spec = "https://drafts.csswg.org/css-logical/#propdef-margin-%s" % axis
+    %>
+    <%helpers:shorthand
+        name="margin-${axis}"
+        sub_properties="${' '.join(
+            'margin-%s-%s' % (axis, side)
+            for side in ['start', 'end']
+        )}"
+        spec="${spec}">
+
+        use crate::parser::Parse;
+        use crate::values::specified::length::LengthPercentageOrAuto;
+        pub fn parse_value<'i, 't>(
+            context: &ParserContext,
+            input: &mut Parser<'i, 't>,
+        ) -> Result<Longhands, ParseError<'i>> {
+            let start_value = LengthPercentageOrAuto::parse(context, input)?;
+            let end_value =
+                input.try(|input| LengthPercentageOrAuto::parse(context, input)).unwrap_or_else(|_| start_value.clone());
+
+            Ok(expanded! {
+                margin_${axis}_start: start_value,
+                margin_${axis}_end: end_value,
+            })
+        }
+
+        impl<'a> ToCss for LonghandsToSerialize<'a>  {
+            fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
+                self.margin_${axis}_start.to_css(dest)?;
+
+                if self.margin_${axis}_end != self.margin_${axis}_start {
+                    dest.write_str(" ")?;
+                    self.margin_${axis}_end.to_css(dest)?;
+                }
+
+                Ok(())
+            }
+        }
+    </%helpers:shorthand>
+% endfor
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -1045,17 +1045,17 @@ pub extern "C" fn Servo_ComputedValues_E
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_ResolveLogicalProperty(
     property_id: nsCSSPropertyID,
     style: ComputedStyleBorrowed,
 ) -> nsCSSPropertyID {
     let longhand = LonghandId::from_nscsspropertyid(property_id)
-        .expect("There are no logical shorthands (yet)");
+        .expect("We shouldn't need to care about shorthands");
 
     longhand
         .to_physical(style.writing_mode)
         .to_nscsspropertyid()
 }
 
 #[no_mangle]
 pub unsafe extern "C" fn Servo_Property_LookupEnabledForAllContent(
--- a/testing/web-platform/meta/css/css-logical/animation-001.html.ini
+++ b/testing/web-platform/meta/css/css-logical/animation-001.html.ini
@@ -1,13 +1,12 @@
 [animation-001.html]
   [Logical shorthands follow the usual prioritization based on number of component longhands]
     expected: FAIL
     bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1289155, https://bugzilla.mozilla.org/show_bug.cgi?id=1370404
 
-  [Physical longhands win over logical shorthands]
+  [Physical shorthands win over logical shorthands]
     expected: FAIL
-    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1370404
+    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1520069
 
-  [Physical shorthands and logical shorthands can be mixed]
+  [Physical shorthands using variables win over logical shorthands]
     expected: FAIL
-    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1370404
-
+    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1520069
--- a/testing/web-platform/meta/css/css-logical/logical-box-margin.html.ini
+++ b/testing/web-platform/meta/css/css-logical/logical-box-margin.html.ini
@@ -1,40 +1,12 @@
 [logical-box-margin.html]
-  [Test that margin-* shorthands set the computed value of both logical and physical longhands, with 'writing-mode: horizontal-tb; direction: ltr; '.]
-    expected: FAIL
-
-  [Test that margin-* shorthands set the computed value of both logical and physical longhands, with 'writing-mode: horizontal-tb; direction: rtl; '.]
-    expected: FAIL
-
-  [Test that margin-* shorthands set the computed value of both logical and physical longhands, with 'writing-mode: vertical-rl; direction: rtl; '.]
-    expected: FAIL
-
-  [Test that margin-* shorthands set the computed value of both logical and physical longhands, with 'writing-mode: sideways-rl; direction: rtl; '.]
-    expected: FAIL
-
-  [Test that margin-* shorthands set the computed value of both logical and physical longhands, with 'writing-mode: vertical-rl; direction: ltr; '.]
-    expected: FAIL
-
-  [Test that margin-* shorthands set the computed value of both logical and physical longhands, with 'writing-mode: sideways-rl; direction: ltr; '.]
-    expected: FAIL
-
-  [Test that margin-* shorthands set the computed value of both logical and physical longhands, with 'writing-mode: vertical-lr; direction: rtl; '.]
-    expected: FAIL
-
-  [Test that margin-* shorthands set the computed value of both logical and physical longhands, with 'writing-mode: sideways-lr; direction: ltr; '.]
-    expected: FAIL
-
-  [Test that margin-* shorthands set the computed value of both logical and physical longhands, with 'writing-mode: vertical-lr; direction: ltr; '.]
-    expected: FAIL
-
-  [Test that margin-* shorthands set the computed value of both logical and physical longhands, with 'writing-mode: sideways-lr; direction: rtl; '.]
-    expected: FAIL
-
   [Test that margin-block shorthand sets longhands and serializes correctly.]
     expected: FAIL
+    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=137688
 
   [Test that margin-inline shorthand sets longhands and serializes correctly.]
     expected: FAIL
+    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=137688
 
   [Test that margin shorthand sets longhands and serializes correctly.]
     expected: FAIL
-
+    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=137688