servo: Merge #15604 - serialize font: to empty on non-default subprops (from zploskey:reset_font_shorthand_serialization); r=SimonSapin
authorZach Ploskey <zach@ploskey.com>
Sun, 19 Mar 2017 08:26:43 -0700
changeset 348379 743677e92ce9a9a31070f90ac749588154cfef5a
parent 348378 24f5c1cd36dbeebfe8f69ca9c5bb5e52a0153c71
child 348380 c6e919aa3aa9d1e73d3e649a1bae7c8488f05554
push id39130
push userservo-vcs-sync@mozilla.com
push dateSun, 19 Mar 2017 16:07:07 +0000
treeherderautoland@743677e92ce9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersSimonSapin
milestone55.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
servo: Merge #15604 - serialize font: to empty on non-default subprops (from zploskey:reset_font_shorthand_serialization); r=SimonSapin Fixes font shorthand serialization so that it serializes to "" when non-default subproperties are defined. These subproperties are those defined in #15033. Adds tests: - font_should_serialize_to_empty_if_there_are_nondefault_subproperties - font_should_serialize_all_available_properties The second test was previously commented out and underwent some cleanup to make it run. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #15036 <!-- Either: --> - [X] There are tests for these changes Source-Repo: https://github.com/servo/servo Source-Revision: fbd561bc2fc2d5529f805c5ae07c04b3c343bc40
servo/components/style/properties/shorthand/font.mako.rs
servo/tests/unit/style/properties/serialization.rs
--- a/servo/components/style/properties/shorthand/font.mako.rs
+++ b/servo/components/style/properties/shorthand/font.mako.rs
@@ -1,28 +1,28 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 <%namespace name="helpers" file="/helpers.mako.rs" />
 
 <%helpers:shorthand name="font" sub_properties="font-style font-variant font-weight font-stretch
                                                 font-size line-height font-family
-                                                ${'font-size-adjust' if product == 'gecko' else ''}
-                                                ${'font-kerning' if product == 'gecko' else ''}
-                                                ${'font-variant-caps' if product == 'gecko' else ''}
-                                                ${'font-variant-position' if product == 'gecko' else ''}
-                                                ${'font-language-override' if product == 'none' else ''}"
+                                                ${'font-size-adjust' if product == 'gecko' or data.testing else ''}
+                                                ${'font-kerning' if product == 'gecko' or data.testing else ''}
+                                                ${'font-variant-caps' if product == 'gecko' or data.testing else ''}
+                                                ${'font-variant-position' if product == 'gecko' or data.testing else ''}
+                                                ${'font-language-override' if data.testing else ''}"
                     spec="https://drafts.csswg.org/css-fonts-3/#propdef-font">
     use properties::longhands::{font_style, font_variant, font_weight, font_stretch};
     use properties::longhands::{font_size, line_height};
-    % if product == "gecko":
+    % if product == "gecko" or data.testing:
     use properties::longhands::{font_size_adjust, font_kerning, font_variant_caps, font_variant_position};
     % endif
-    % if product == "none":
+    % if data.testing:
     use properties::longhands::font_language_override;
     % endif
     use properties::longhands::font_family::SpecifiedValue as FontFamily;
 
     pub fn parse_value(context: &ParserContext, input: &mut Parser) -> Result<Longhands, ()> {
         let mut nb_normals = 0;
         let mut style = None;
         let mut variant = None;
@@ -78,45 +78,59 @@
         };
         let family = FontFamily::parse(input)?;
         Ok(Longhands {
             % for name in "style variant weight stretch size".split():
                 font_${name}: unwrap_or_initial!(font_${name}, ${name}),
             % endfor
             line_height: unwrap_or_initial!(line_height),
             font_family: family,
-            % if product == "gecko":
+            % if product == "gecko" or data.testing:
                 % for name in "size_adjust kerning variant_caps variant_position".split():
                     font_${name}: font_${name}::get_initial_specified_value(),
                 % endfor
             % endif
-            % if product == "none":
+            % if data.testing:
                 font_language_override: font_language_override::get_initial_specified_value(),
             % endif
         })
     }
 
     // This may be a bit off, unsure, possibly needs changes
     impl<'a> ToCss for LonghandsToSerialize<'a>  {
         fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-            self.font_style.to_css(dest)?;
-            dest.write_str(" ")?;
-            self.font_variant.to_css(dest)?;
+
+    % if product == "gecko" or data.testing:
+        % for name in "size_adjust kerning variant_caps variant_position".split():
+            if self.font_${name} != &font_${name}::get_initial_specified_value() {
+                return Ok(());
+            }
+        % endfor
+    % endif
+
+    % if data.testing:
+            if self.font_language_override != &font_language_override::get_initial_specified_value() {
+                return Ok(());
+            }
+    % endif
+
+    % for name in "style variant weight stretch".split():
+            self.font_${name}.to_css(dest)?;
             dest.write_str(" ")?;
-            self.font_weight.to_css(dest)?;
-            dest.write_str(" ")?;
-            self.font_stretch.to_css(dest)?;
-            dest.write_str(" ")?;
+    % endfor
 
             self.font_size.to_css(dest)?;
+
             match *self.line_height {
                 line_height::SpecifiedValue::Normal => {},
                 _ => {
                     dest.write_str("/")?;
                     self.line_height.to_css(dest)?;
                 }
             }
 
             dest.write_str(" ")?;
-            self.font_family.to_css(dest)
+            self.font_family.to_css(dest)?;
+
+            Ok(())
         }
     }
 </%helpers:shorthand>
--- a/servo/tests/unit/style/properties/serialization.rs
+++ b/servo/tests/unit/style/properties/serialization.rs
@@ -603,61 +603,68 @@ mod shorthand_serialization {
 
         properties.push(PropertyDeclaration::FlexDirection(direction));
         properties.push(PropertyDeclaration::FlexWrap(wrap));
 
         let serialization = shorthand_properties_to_string(properties);
         assert_eq!(serialization, "flex-flow: row wrap;");
     }
 
-    // TODO: Populate Atom Cache for testing so that the font shorthand can be tested
-    /*
     mod font {
         use super::*;
-        use style::properties::longhands::font_family::computed_value::T as FamilyContainer;
-        use style::properties::longhands::font_family::computed_value::FontFamily;
-        use style::properties::longhands::font_style::computed_value::T as FontStyle;
-        use style::properties::longhands::font_variant::computed_value::T as FontVariant;
-        use style::properties::longhands::font_weight::SpecifiedValue as FontWeight;
-        use style::properties::longhands::font_size::SpecifiedValue as FontSizeContainer;
-        use style::properties::longhands::font_stretch::computed_value::T as FontStretch;
-        use style::properties::longhands::line_height::SpecifiedValue as LineHeight;
+
+        #[test]
+        fn font_should_serialize_to_empty_if_there_are_nondefault_subproperties() {
+            // Test with non-default font-kerning value
+            let block_text = "font-style: italic; \
+                              font-variant: normal; \
+                              font-weight: bolder; \
+                              font-stretch: expanded; \
+                              font-size: 4px; \
+                              line-height: 3; \
+                              font-family: serif; \
+                              font-size-adjust: none; \
+                              font-variant-caps: normal; \
+                              font-variant-position: normal; \
+                              font-language-override: normal; \
+                              font-kerning: none";
+
+            let block = parse_declaration_block(block_text);
+
+            let mut s = String::new();
+            let id = PropertyId::parse("font".into()).unwrap();
+            let x = block.property_value_to_css(&id, &mut s);
+
+            assert_eq!(x.is_ok(), true);
+            assert_eq!(s, "");
+        }
 
         #[test]
         fn font_should_serialize_all_available_properties() {
-            let mut properties = Vec::new();
-
-
-            let font_family = DeclaredValue::Value(
-                FamilyContainer(vec![FontFamily::Generic(atom!("serif"))])
-            );
-
+            let block_text = "font-style: italic; \
+                              font-variant: normal; \
+                              font-weight: bolder; \
+                              font-stretch: expanded; \
+                              font-size: 4px; \
+                              line-height: 3; \
+                              font-family: serif; \
+                              font-size-adjust: none; \
+                              font-kerning: auto; \
+                              font-variant-caps: normal; \
+                              font-variant-position: normal; \
+                              font-language-override: normal;";
 
-            let font_style = DeclaredValue::Value(FontStyle::italic);
-            let font_variant = DeclaredValue::Value(FontVariant::normal);
-            let font_weight = DeclaredValue::Value(FontWeight::Bolder);
-            let font_size = DeclaredValue::Value(FontSizeContainer(
-                LengthOrPercentage::Length(NoCalcLength::from_px(4f32)))
-            );
-            let font_stretch = DeclaredValue::Value(FontStretch::expanded);
-            let line_height = DeclaredValue::Value(LineHeight::Number(3f32));
+            let block = parse_declaration_block(block_text);
+
+            let serialization = block.to_css_string();
 
-            properties.push(PropertyDeclaration::FontFamily(font_family));
-            properties.push(PropertyDeclaration::FontStyle(font_style));
-            properties.push(PropertyDeclaration::FontVariant(font_variant));
-            properties.push(PropertyDeclaration::FontWeight(font_weight));
-            properties.push(PropertyDeclaration::FontSize(font_size));
-            properties.push(PropertyDeclaration::FontStretch(font_stretch));
-            properties.push(PropertyDeclaration::LineHeight(line_height));
+            assert_eq!(serialization, "font: italic normal bolder expanded 4px/3 serif;");
+        }
 
-            let serialization = shorthand_properties_to_string(properties);
-            assert_eq!(serialization, "font:;");
-        }
     }
-    */
 
     mod background {
         use super::*;
 
         #[test]
         fn background_should_serialize_all_available_properties_when_specified() {
             let block_text = "\
                 background-color: rgb(255, 0, 0); \