diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index 9d76adf1383..4d3da86906c 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -333,7 +333,9 @@ impl PropertyDeclarationBlock { } let iter = self.declarations.iter().map(get_declaration as fn(_) -> _); match shorthand.get_shorthand_appendable_value(iter) { - Some(AppendableValue::Css(css)) => dest.write_str(css), + Some(AppendableValue::Css { css, .. }) => { + dest.write_str(css) + }, Some(AppendableValue::DeclarationsForShorthand(_, decls)) => { shorthand.longhands_to_css(decls, dest) } @@ -409,11 +411,13 @@ impl ToCss for PropertyDeclarationBlock { } } - // Substep 1 - /* Assuming that the PropertyDeclarationBlock contains no duplicate entries, - if the current_longhands length is equal to the properties length, it means - that the properties that map to shorthand are present in longhands */ - if current_longhands.is_empty() || current_longhands.len() != properties.len() { + // Substep 1: + // + // Assuming that the PropertyDeclarationBlock contains no + // duplicate entries, if the current_longhands length is + // equal to the properties length, it means that the + // properties that map to shorthand are present in longhands + if current_longhands.len() != properties.len() { continue; } @@ -428,36 +432,58 @@ impl ToCss for PropertyDeclarationBlock { Importance::Normal }; - // Substep 5 - Let value be the result of invoking serialize a CSS - // value of current longhands. - let mut value = String::new(); - match shorthand.get_shorthand_appendable_value(current_longhands.iter().cloned()) { - None => continue, - Some(appendable_value) => { - try!(append_declaration_value(&mut value, appendable_value)); - } - } + // Substep 5 - Let value be the result of invoking serialize + // a CSS value of current longhands. + let appendable_value = + match shorthand.get_shorthand_appendable_value(current_longhands.iter().cloned()) { + None => continue, + Some(appendable_value) => appendable_value, + }; - // Substep 6 - if value.is_empty() { - continue - } + // We avoid re-serializing if we're already an + // AppendableValue::Css. + let mut value = String::new(); + let value = match appendable_value { + AppendableValue::Css { css, with_variables } => { + debug_assert!(!css.is_empty()); + AppendableValue::Css { + css: css, + with_variables: with_variables, + } + } + other @ _ => { + append_declaration_value(&mut value, other)?; + + // Substep 6 + if value.is_empty() { + continue; + } + + AppendableValue::Css { + css: &value, + with_variables: false, + } + } + }; // Substeps 7 and 8 - try!(append_serialization::>, _>( - dest, &shorthand, AppendableValue::Css(&value), importance, - &mut is_first_serialization)); + append_serialization::<_, Cloned>, _>( + dest, + &shorthand, + value, + importance, + &mut is_first_serialization)?; - for current_longhand in current_longhands { + for current_longhand in ¤t_longhands { // Substep 9 already_serialized.push(current_longhand.id()); - let index_to_remove = longhands.iter().position(|l| l.0 == *current_longhand); + let index_to_remove = longhands.iter().position(|l| l.0 == **current_longhand); if let Some(index) = index_to_remove { // Substep 10 longhands.remove(index); } - } - } + } + } } // Step 3.3.4 @@ -473,12 +499,12 @@ impl ToCss for PropertyDeclarationBlock { // "error: unable to infer enough type information about `_`; // type annotations or generic parameter binding required [E0282]" // Use the same type as earlier call to reuse generated code. - try!(append_serialization::>, _>( + append_serialization::<_, Cloned>, _>( dest, &property, AppendableValue::Declaration(declaration), importance, - &mut is_first_serialization)); + &mut is_first_serialization)?; // Step 3.3.8 already_serialized.push(property); @@ -503,7 +529,12 @@ pub enum AppendableValue<'a, I> DeclarationsForShorthand(ShorthandId, I), /// A raw CSS string, coming for example from a property with CSS variables, /// or when storing a serialized shorthand value before appending directly. - Css(&'a str) + Css { + /// The raw CSS string. + css: &'a str, + /// Whether the original serialization contained variables or not. + with_variables: bool, + } } /// Potentially appends whitespace after the first (property: value;) pair. @@ -513,12 +544,11 @@ fn handle_first_serialization(dest: &mut W, where W: fmt::Write, { if !*is_first_serialization { - try!(write!(dest, " ")); + dest.write_str(" ") } else { *is_first_serialization = false; + Ok(()) } - - Ok(()) } /// Append a given kind of appendable value to a serialization. @@ -529,18 +559,16 @@ pub fn append_declaration_value<'a, W, I>(dest: &mut W, I: Iterator, { match appendable_value { - AppendableValue::Css(css) => { - try!(write!(dest, "{}", css)) + AppendableValue::Css { css, .. } => { + dest.write_str(css) }, AppendableValue::Declaration(decl) => { - try!(decl.to_css(dest)); + decl.to_css(dest) }, AppendableValue::DeclarationsForShorthand(shorthand, decls) => { - try!(shorthand.longhands_to_css(decls, dest)); + shorthand.longhands_to_css(decls, dest) } } - - Ok(()) } /// Append a given property and value pair to a serialization. @@ -560,28 +588,30 @@ pub fn append_serialization<'a, W, I, N>(dest: &mut W, try!(dest.write_char(':')); // for normal parsed values, add a space between key: and value - match &appendable_value { - &AppendableValue::Css(_) => { - try!(write!(dest, " ")) - }, - &AppendableValue::Declaration(decl) => { + match appendable_value { + AppendableValue::Declaration(decl) => { if !decl.value_is_unparsed() { - // for normal parsed values, add a space between key: and value - try!(write!(dest, " ")); + // For normal parsed values, add a space between key: and value. + dest.write_str(" ")? } }, + AppendableValue::Css { with_variables, .. } => { + if !with_variables { + dest.write_str(" ")? + } + } // Currently append_serialization is only called with a Css or // a Declaration AppendableValue. - &AppendableValue::DeclarationsForShorthand(..) => unreachable!() + AppendableValue::DeclarationsForShorthand(..) => unreachable!(), } try!(append_declaration_value(dest, appendable_value)); if importance.important() { - try!(write!(dest, " !important")); + try!(dest.write_str(" !important")); } - write!(dest, ";") + dest.write_char(';') } /// A helper to parse the style attribute of an element, in order for this to be diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 49f88fd31aa..af392618f5e 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -561,8 +561,8 @@ impl ShorthandId { /// /// Returns the optional appendable value. pub fn get_shorthand_appendable_value<'a, I>(self, - declarations: I) - -> Option> + declarations: I) + -> Option> where I: IntoIterator, I::IntoIter: Clone, { @@ -580,15 +580,21 @@ impl ShorthandId { // https://drafts.csswg.org/css-variables/#variables-in-shorthands if let Some(css) = first_declaration.with_variables_from_shorthand(self) { if declarations2.all(|d| d.with_variables_from_shorthand(self) == Some(css)) { - return Some(AppendableValue::Css(css)); - } - return None; + return Some(AppendableValue::Css { + css: css, + with_variables: true, + }); + } + return None; } // Check whether they are all the same CSS-wide keyword. if let Some(keyword) = first_declaration.get_css_wide_keyword() { if declarations2.all(|d| d.get_css_wide_keyword() == Some(keyword)) { - return Some(AppendableValue::Css(keyword.to_str())); + return Some(AppendableValue::Css { + css: keyword.to_str(), + with_variables: false, + }); } return None; } @@ -1191,10 +1197,10 @@ impl PropertyDeclaration { /// the longhand declarations. pub fn may_serialize_as_part_of_shorthand(&self) -> bool { match *self { - PropertyDeclaration::CSSWideKeyword(..) => false, + PropertyDeclaration::CSSWideKeyword(..) | PropertyDeclaration::WithVariables(..) => false, PropertyDeclaration::Custom(..) => - unreachable!("Serialize a custom property as part of shorthand?"), + unreachable!("Serializing a custom property as part of shorthand?"), _ => true, } } diff --git a/tests/wpt/metadata/MANIFEST.json b/tests/wpt/metadata/MANIFEST.json index 139d679d65b..672ef7efbbb 100644 --- a/tests/wpt/metadata/MANIFEST.json +++ b/tests/wpt/metadata/MANIFEST.json @@ -83640,6 +83640,12 @@ {} ] ], + "cssom/serialize-variable-reference.html": [ + [ + "/cssom/serialize-variable-reference.html", + {} + ] + ], "cssom/shorthand-serialization.html": [ [ "/cssom/shorthand-serialization.html", @@ -159941,6 +159947,10 @@ "329fe02cb9e54b1a24a8f9dedcfcf5c0f61c7f24", "testharness" ], + "cssom/serialize-variable-reference.html": [ + "5e83f084efc82184c3052a40bb4a061fd4a1336f", + "testharness" + ], "cssom/shorthand-serialization.html": [ "bd514834dbd48c267c16a4329af6fec7f6cbc081", "testharness" diff --git a/tests/wpt/web-platform-tests/cssom/serialize-variable-reference.html b/tests/wpt/web-platform-tests/cssom/serialize-variable-reference.html new file mode 100644 index 00000000000..d714b81518b --- /dev/null +++ b/tests/wpt/web-platform-tests/cssom/serialize-variable-reference.html @@ -0,0 +1,34 @@ + + +CSSOM - Serialization with variable preserves original serialization. + + +
+
+
+
+