Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-06 14:10:11 +01:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #15685 - absoludity:fix-animation-serialization-15398-pt2, r=upsuper

Browse source 
Fix animation serialization 15398 pt2

Fixes animation serialization so that an animation shorthand is serialized only when the value lists have the same length (part of #15398)

There are two commits here - the first is a refactoring to update PropertyDeclarationBlock::to_css to match the spec (please see the full commit message for perhaps too many details).

The second commit utilizes the changes in the first, and updates the animation serialization itself to write an empty serialization if the longhand value lists have different lengths, as per the original issue.
Interestingly, the refactoring of the first commit simplified quite a lot more than the fix for this bug, in particular, the special case of 'overflow' shorthand serialization is handled by default, without the extra special case code (now removed).

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #15398 (github issue number if applicable).
- [X] There are tests for these changes OR

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15685)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-02-23 20:12:30 -08:00 • committed by GitHub
commit b52288af00
5 changed files with 69 additions and 180 deletions
Download patch file Download diff file Expand all files Collapse all files

74
components/style/properties/declaration_block.rs
View file

@ -128,10 +128,9 @@ impl PropertyDeclarationBlock {
// Step 1.2.3
// We don't print !important when serializing individual properties,
// so we treat this as a normal-importance property
let importance = Importance::Normal;
match shorthand.get_shorthand_appendable_value(list) {
Some(appendable_value) =>
append_declaration_value(dest, appendable_value, importance, false),
append_declaration_value(dest, appendable_value),
None => return Ok(()),
}
}
@ -343,23 +342,26 @@ impl ToCss for PropertyDeclarationBlock {
Importance::Normal
};
// Substep 5
let was_serialized =
try!(
shorthand.serialize_shorthand_to_buffer(
dest,
current_longhands.iter().cloned(),
&mut is_first_serialization,
importance
)
);
// If serialization occured, Substep 7 & 8 will have been completed
// 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 6
if !was_serialized {
continue;
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));
for current_longhand in current_longhands {
// Substep 9
already_serialized.push(current_longhand.id());
@ -413,7 +415,8 @@ pub enum AppendableValue<'a, I>
/// FIXME: This needs more docs, where are the shorthands expanded? We print
/// the property name before-hand, don't we?
DeclarationsForShorthand(ShorthandId, I),
/// A raw CSS string, coming for example from a property with CSS variables.
/// 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)
}
@ -434,9 +437,7 @@ fn handle_first_serialization<W>(dest: &mut W,
/// Append a given kind of appendable value to a serialization.
pub fn append_declaration_value<'a, W, I>(dest: &mut W,
appendable_value: AppendableValue<'a, I>,
importance: Importance,
is_overflow_with_name: bool)
appendable_value: AppendableValue<'a, I>)
-> fmt::Result
where W: fmt::Write,
I: Iterator<Item=&'a PropertyDeclaration>,
@ -447,18 +448,10 @@ pub fn append_declaration_value<'a, W, I>(dest: &mut W,
},
AppendableValue::Declaration(decl) => {
try!(decl.to_css(dest));
},
AppendableValue::DeclarationsForShorthand(shorthand, decls) => {
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"));
},
AppendableValue::DeclarationsForShorthand(shorthand, decls) => {
try!(shorthand.longhands_to_css(decls, dest));
}
}
Ok(())
@ -477,12 +470,6 @@ pub fn append_serialization<'a, W, I, N>(dest: &mut W,
{
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, true);
}
try!(property_name.to_css(dest));
try!(dest.write_char(':'));
@ -496,11 +483,18 @@ pub fn append_serialization<'a, W, I, N>(dest: &mut W,
// for normal parsed values, add a space between key: and value
try!(write!(dest, " "));
}
},
&AppendableValue::DeclarationsForShorthand(..) => try!(write!(dest, " "))
},
// Currently append_serialization is only called with a Css or
// a Declaration AppendableValue.
&AppendableValue::DeclarationsForShorthand(..) => unreachable!()
}
try!(append_declaration_value(dest, appendable_value));
if importance.important() {
try!(write!(dest, " !important"));
}
try!(append_declaration_value(dest, appendable_value, importance, false));
write!(dest, ";")
}

57
components/style/properties/properties.mako.rs
View file

@ -13,7 +13,7 @@
use std::borrow::Cow;
use std::boxed::Box as StdBox;
use std::collections::HashSet;
use std::fmt::{self, Write};
use std::fmt;
use std::sync::Arc;
use app_units::Au;
@ -549,59 +549,10 @@ impl ShorthandId {
}
}
/// 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".
/// Finds and returns an appendable value for the given declarations.
///
/// We use this function as a special-case for that.
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,
is_first_serialization: &mut bool,
importance: Importance)
-> Result<bool, fmt::Error>
where W: Write,
I: IntoIterator<Item=&'a PropertyDeclaration>,
I::IntoIter: Clone,
{
match self.get_shorthand_appendable_value(declarations) {
None => Ok(false),
Some(appendable_value) => {
append_serialization(
dest,
&self,
appendable_value,
importance,
is_first_serialization
).and_then(|_| Ok(true))
}
}
}
fn get_shorthand_appendable_value<'a, I>(self,
/// Returns the optional appendable value.
pub fn get_shorthand_appendable_value<'a, I>(self,
declarations: I)
-> Option<AppendableValue<'a, I::IntoIter>>
where I: IntoIterator<Item=&'a PropertyDeclaration>,

68
components/style/properties/shorthand/box.mako.rs
View file

@ -34,42 +34,6 @@
}
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, "; "));
try!(write!(dest, "overflow-y: "));
try!(self.overflow_y.to_css(dest));
}
write!(dest, ";")
}
}
</%helpers:shorthand>
@ -315,27 +279,27 @@ macro_rules! try_parse_one {
}
}
// TODO: When the lengths are different, shorthand shouldn't be serialized
// at all.
use std::cmp;
let mut len = 0;
% for name in "duration timing_function delay direction fill_mode iteration_count play_state name".split():
len = cmp::max(len, extract_value(self.animation_${name}).map(|i| i.0.len())
.unwrap_or(0));
% endfor
let len = extract_value(self.animation_name).map(|i| i.0.len()).unwrap_or(0);
// There should be at least one declared value
if len == 0 {
return dest.write_str("")
}
// If any value list length is differs then we don't do a shorthand serialization
// either.
% for name in "duration timing_function delay direction fill_mode iteration_count play_state".split():
if len != extract_value(self.animation_${name}).map(|i| i.0.len()).unwrap_or(0) {
return dest.write_str("")
}
% endfor
let mut first = true;
for i in 0..len {
% for name in "duration timing_function delay direction fill_mode iteration_count play_state name".split():
let ${name} = if let DeclaredValue::Value(ref arr) = *self.animation_${name} {
arr.0.get(i % arr.0.len())
&arr.0[i]
} else {
None
unreachable!()
};
% endfor
@ -346,15 +310,11 @@ macro_rules! try_parse_one {
}
% for name in "duration timing_function delay direction fill_mode iteration_count play_state".split():
if let Some(${name}) = ${name} {
try!(${name}.to_css(dest));
try!(write!(dest, " "));
}
try!(${name}.to_css(dest));
try!(write!(dest, " "));
% endfor
if let Some(name) = name {
try!(name.to_css(dest));
}
try!(name.to_css(dest));
}
Ok(())
}

16
components/style/properties/shorthand/serialize.mako.rs
View file

@ -2,7 +2,7 @@
* 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/. */
use properties::{AppendableValue, DeclaredValue, PropertyDeclaration, ShorthandId};
use properties::DeclaredValue;
use style_traits::ToCss;
use values::specified::{BorderStyle, CSSColor};
use std::fmt;
@ -91,17 +91,3 @@ fn serialize_directional_border<W, I,>(dest: &mut W,
_ => Ok(())
}
}
#[allow(missing_docs)]
pub fn is_overflow_shorthand<'a, I>(appendable_value: &AppendableValue<'a, I>) -> bool
where I: Iterator<Item=&'a PropertyDeclaration>
{
if let AppendableValue::DeclarationsForShorthand(shorthand, _) = *appendable_value {
if let ShorthandId::Overflow = shorthand {
return true;
}
}
false
}