servo: Merge #14757 - Fix `overflow` serialization with CSS-wide keywords (from canaltinova:overflow); r=Manishearth
authorNazım Can Altınova <canaltinova@gmail.com>
Sat, 31 Dec 2016 11:26:21 -0800
changeset 340454 2096f0339f0afeb6820fc951745db5e6c1dce61c
parent 340453 4308461871cdc0a98b57761d749dc8c1118f08a3
child 340455 5e0c4c1426b65e40625016328414fd1f8d2a707e
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersManishearth
servo: Merge #14757 - Fix `overflow` serialization with CSS-wide keywords (from canaltinova:overflow); r=Manishearth <!-- Please describe your changes on the following line: --> Overflow does not behave like a normal shorthand. CSS-wide keywords were handled in their `to_css_declared` method. So I had to bypass this check to make it work. --- <!-- 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 #14752 (github issue number if applicable). <!-- Either: --> - [X] There are tests for these changes <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 62ff467be855f222829ec961a104a0fdd3e083b0
servo/components/style/properties/declaration_block.rs
servo/components/style/properties/properties.mako.rs
servo/components/style/properties/shorthand/box.mako.rs
--- a/servo/components/style/properties/declaration_block.rs
+++ b/servo/components/style/properties/declaration_block.rs
@@ -106,17 +106,17 @@ impl PropertyDeclarationBlock {
                     return Ok(());
                 }
 
                 // Step 2.3
                 // We don't print !important when serializing individual properties,
                 // so we treat this as a normal-importance property
                 let importance = Importance::Normal;
                 let appendable_value = shorthand.get_shorthand_appendable_value(list).unwrap();
-                append_declaration_value(dest, appendable_value, importance)
+                append_declaration_value(dest, appendable_value, importance, false)
             }
             Err(longhand_or_custom) => {
                 if let Some(&(ref value, _importance)) = self.get(longhand_or_custom) {
                     // Step 3
                     value.to_css(dest)
                 } else {
                     // Step 4
                     Ok(())
@@ -372,28 +372,33 @@ fn handle_first_serialization<W>(dest: &
     }
 
     Ok(())
 }
 
 pub fn append_declaration_value<'a, W, I>
                            (dest: &mut W,
                             appendable_value: AppendableValue<'a, I>,
-                            importance: Importance)
+                            importance: Importance,
+                            is_overflow_with_name: bool)
                             -> fmt::Result
                             where W: fmt::Write, I: Iterator<Item=&'a PropertyDeclaration> {
   match appendable_value {
       AppendableValue::Css(css) => {
           try!(write!(dest, "{}", css))
       },
       AppendableValue::Declaration(decl) => {
           try!(decl.to_css(dest));
        },
        AppendableValue::DeclarationsForShorthand(shorthand, decls) => {
-          try!(shorthand.longhands_to_css(decls, dest));
+          if is_overflow_with_name {
+            try!(shorthand.overflow_longhands_to_css(decls, dest));
+          } else {
+            try!(shorthand.longhands_to_css(decls, dest));
+          }
        }
   }
 
   if importance.important() {
       try!(write!(dest, " !important"));
   }
 
   Ok(())
@@ -409,17 +414,17 @@ pub fn append_serialization<'a, W, I, N>
           I: Iterator<Item=&'a PropertyDeclaration>,
           N: ToCss
 {
     try!(handle_first_serialization(dest, is_first_serialization));
 
     // Overflow does not behave like a normal shorthand. When overflow-x and overflow-y are not of equal
     // values, they no longer use the shared property name "overflow" and must be handled differently
     if shorthands::is_overflow_shorthand(&appendable_value) {
-        return append_declaration_value(dest, appendable_value, importance);
+        return append_declaration_value(dest, appendable_value, importance, true);
     }
 
     try!(property_name.to_css(dest));
     try!(dest.write_char(':'));
 
     // for normal parsed values, add a space between key: and value
     match &appendable_value {
         &AppendableValue::Css(_) => {
@@ -429,17 +434,17 @@ pub fn append_serialization<'a, W, I, N>
             if !decl.value_is_unparsed() {
                 // for normal parsed values, add a space between key: and value
                 try!(write!(dest, " "));
             }
          },
          &AppendableValue::DeclarationsForShorthand(..) => try!(write!(dest, " "))
     }
 
-    try!(append_declaration_value(dest, appendable_value, importance));
+    try!(append_declaration_value(dest, appendable_value, importance, false));
     write!(dest, ";")
 }
 
 pub fn parse_style_attribute(input: &str,
                              base_url: &ServoUrl,
                              error_reporter: StdBox<ParseErrorReporter + Send>,
                              extra_data: ParserContextExtraData)
                              -> PropertyDeclarationBlock {
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -462,16 +462,31 @@ impl ShorthandId {
                         Ok(longhands) => longhands.to_css(dest),
                         Err(_) => Err(fmt::Error)
                     }
                 },
             % endfor
         }
     }
 
+    // Overflow does not behave like a normal shorthand. When overflow-x and overflow-y are not of equal
+    // values, they no longer use the shared property name "overflow".
+    pub fn overflow_longhands_to_css<'a, W, I>(&self, declarations: I, dest: &mut W) -> fmt::Result
+        where W: fmt::Write, I: Iterator<Item=&'a PropertyDeclaration> {
+        match *self {
+            ShorthandId::Overflow => {
+                match shorthands::overflow::LonghandsToSerialize::from_iter(declarations) {
+                    Ok(longhands) => longhands.to_css_declared_with_name(dest),
+                    Err(_) => Err(fmt::Error)
+                }
+            },
+            _ => Err(fmt::Error)
+        }
+    }
+
     /// Serializes the possible shorthand name with value to input buffer given
     /// a list of longhand declarations.
     ///
     /// On success, returns true if the shorthand value is written, or false if
     /// no shorthand value is present.
     pub fn serialize_shorthand_to_buffer<'a, W, I>(self,
                                                    dest: &mut W,
                                                    declarations: I,
--- a/servo/components/style/properties/shorthand/box.mako.rs
+++ b/servo/components/style/properties/shorthand/box.mako.rs
@@ -10,34 +10,55 @@
     pub fn parse_value(context: &ParserContext, input: &mut Parser) -> Result<Longhands, ()> {
         let overflow = try!(overflow_x::parse(context, input));
         Ok(Longhands {
             overflow_x: Some(overflow),
             overflow_y: Some(overflow_y::SpecifiedValue(overflow)),
         })
     }
 
-
-    // Overflow does not behave like a normal shorthand. When overflow-x and overflow-y are not of equal
-    // values, they no longer use the shared property name "overflow".
-    // Other shorthands do not include their name in the to_css method
     impl<'a> LonghandsToSerialize<'a>  {
         fn to_css_declared<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
             let x_and_y_equal = match (self.overflow_x, self.overflow_y) {
                 (&DeclaredValue::Value(ref x_value), &DeclaredValue::Value(ref y_container)) => {
                     *x_value == y_container.0
                 },
                 (&DeclaredValue::WithVariables { .. }, &DeclaredValue::WithVariables { .. }) => true,
                 (&DeclaredValue::Initial, &DeclaredValue::Initial) => true,
                 (&DeclaredValue::Inherit, &DeclaredValue::Inherit) => true,
                 (&DeclaredValue::Unset, &DeclaredValue::Unset) => true,
                 _ => false
             };
 
             if x_and_y_equal {
+                try!(self.overflow_x.to_css(dest));
+            }
+            Ok(())
+        }
+
+        // Overflow does not behave like a normal shorthand. When overflow-x and overflow-y are not of equal
+        // values, they no longer use the shared property name "overflow".
+        // Other shorthands do not include their name in the to_css method
+        pub fn to_css_declared_with_name<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+            let x_and_y_equal = match (self.overflow_x, self.overflow_y) {
+                (&DeclaredValue::Value(ref x_value), &DeclaredValue::Value(ref y_container)) => {
+                    *x_value == y_container.0
+                },
+                (_, &DeclaredValue::WithVariables { .. }) |
+                (&DeclaredValue::WithVariables { .. }, _) => {
+                    // We don't serialize shorthands with variables
+                    return dest.write_str("");
+                },
+                (&DeclaredValue::Initial, &DeclaredValue::Initial) => true,
+                (&DeclaredValue::Inherit, &DeclaredValue::Inherit) => true,
+                (&DeclaredValue::Unset, &DeclaredValue::Unset) => true,
+                _ => false
+            };
+
+            if x_and_y_equal {
                 try!(write!(dest, "overflow"));
                 try!(write!(dest, ": "));
                 try!(self.overflow_x.to_css(dest));
             } else {
                 try!(write!(dest, "overflow-x"));
                 try!(write!(dest, ": "));
                 try!(self.overflow_x.to_css(dest));
                 try!(write!(dest, "; "));