Skip to content

Commit

Permalink
style: Correctly track whether a longhand contains any variable.
Browse files Browse the repository at this point in the history
  • Loading branch information
emilio committed Mar 29, 2017
1 parent 05b5aab commit 835d95e
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 36 deletions.
81 changes: 51 additions & 30 deletions components/style/properties/declaration_block.rs
Expand Up @@ -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)
}
Expand Down Expand Up @@ -428,25 +430,38 @@ impl ToCss for PropertyDeclarationBlock {
Importance::Normal
};

// Substep 5 - Let value be the result of invoking serialize a CSS
// value of current longhands.
// 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));
}
}
let has_variables = {
let appendable_value =
match shorthand.get_shorthand_appendable_value(current_longhands.iter().cloned()) {
None => continue,
Some(appendable_value) => appendable_value,
};

let has_variables = match appendable_value {
AppendableValue::Css { with_variables, .. } => with_variables,
_ => false,
};

append_declaration_value(&mut value, appendable_value)?;

has_variables
};

// Substep 6
if value.is_empty() {
continue
}

// Substeps 7 and 8
try!(append_serialization::<W, Cloned<slice::Iter< _>>, _>(
dest, &shorthand, AppendableValue::Css(&value), importance,
&mut is_first_serialization));
append_serialization::<_, Cloned<slice::Iter< _>>, _>(
dest,
&shorthand,
AppendableValue::Css { css: &value, with_variables: has_variables },
importance,
&mut is_first_serialization)?;

for current_longhand in current_longhands {
// Substep 9
Expand All @@ -456,8 +471,8 @@ impl ToCss for PropertyDeclarationBlock {
// Substep 10
longhands.remove(index);
}
}
}
}
}
}

// Step 3.3.4
Expand Down Expand Up @@ -503,7 +518,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.
Expand All @@ -513,12 +533,11 @@ fn handle_first_serialization<W>(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.
Expand All @@ -529,18 +548,16 @@ pub fn append_declaration_value<'a, W, I>(dest: &mut W,
I: Iterator<Item=&'a PropertyDeclaration>,
{
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.
Expand All @@ -560,17 +577,21 @@ 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::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::Css(_) => {}
AppendableValue::DeclarationsForShorthand(..) => unreachable!(),
}

try!(append_declaration_value(dest, appendable_value));
Expand Down
18 changes: 12 additions & 6 deletions components/style/properties/properties.mako.rs
Expand Up @@ -561,8 +561,8 @@ impl ShorthandId {
///
/// Returns the optional appendable value.
pub fn get_shorthand_appendable_value<'a, I>(self,
declarations: I)
-> Option<AppendableValue<'a, I::IntoIter>>
declarations: I)
-> Option<AppendableValue<'a, I::IntoIter>>
where I: IntoIterator<Item=&'a PropertyDeclaration>,
I::IntoIter: Clone,
{
Expand All @@ -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;
}
Expand Down

0 comments on commit 835d95e

Please sign in to comment.