servo: Merge #14920 - Correctly handle unserializable shorthand (from upsuper:shorthand-variable); r=emilio

get_shorthand_appendable_value doesn't always return a serializable value. This change makes it handle that case correctly.

This change also updates step number in property_value_to_css to reflect the latest spec.

---
<!-- 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 stylo [bug 1329533](https://bugzilla.mozilla.org/show_bug.cgi?id=1329533)

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- 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: 867df7f3d7a89b48de76106601cbc5e474acb9dd
This commit is contained in:
Xidorn Quan 2017-01-09 20:10:32 -08:00
parent d8164c36b4
commit 3120858b70

View File

@ -86,21 +86,21 @@ impl PropertyDeclarationBlock {
pub fn property_value_to_css<W>(&self, property: &PropertyId, dest: &mut W) -> fmt::Result pub fn property_value_to_css<W>(&self, property: &PropertyId, dest: &mut W) -> fmt::Result
where W: fmt::Write, where W: fmt::Write,
{ {
// Step 1: done when parsing a string to PropertyId // Step 1.1: done when parsing a string to PropertyId
// Step 2 // Step 1.2
match property.as_shorthand() { match property.as_shorthand() {
Ok(shorthand) => { Ok(shorthand) => {
// Step 2.1 // Step 1.2.1
let mut list = Vec::new(); let mut list = Vec::new();
let mut important_count = 0; let mut important_count = 0;
// Step 2.2 // Step 1.2.2
for &longhand in shorthand.longhands() { for &longhand in shorthand.longhands() {
// Step 2.2.1 // Step 1.2.2.1
let declaration = self.get(PropertyDeclarationId::Longhand(longhand)); let declaration = self.get(PropertyDeclarationId::Longhand(longhand));
// Step 2.2.2 & 2.2.3 // Step 1.2.2.2 & 1.2.2.3
match declaration { match declaration {
Some(&(ref declaration, importance)) => { Some(&(ref declaration, importance)) => {
list.push(declaration); list.push(declaration);
@ -112,26 +112,28 @@ impl PropertyDeclarationBlock {
} }
} }
// Step 3.3.2.4
// If there is one or more longhand with important, and one or more // If there is one or more longhand with important, and one or more
// without important, we don't serialize it as a shorthand. // without important, we don't serialize it as a shorthand.
if important_count > 0 && important_count != list.len() { if important_count > 0 && important_count != list.len() {
return Ok(()); return Ok(());
} }
// Step 2.3 // Step 1.2.3
// We don't print !important when serializing individual properties, // We don't print !important when serializing individual properties,
// so we treat this as a normal-importance property // so we treat this as a normal-importance property
let importance = Importance::Normal; let importance = Importance::Normal;
let appendable_value = shorthand.get_shorthand_appendable_value(list).unwrap(); match shorthand.get_shorthand_appendable_value(list) {
append_declaration_value(dest, appendable_value, importance, false) Some(appendable_value) =>
append_declaration_value(dest, appendable_value, importance, false),
None => return Ok(()),
}
} }
Err(longhand_or_custom) => { Err(longhand_or_custom) => {
if let Some(&(ref value, _importance)) = self.get(longhand_or_custom) { if let Some(&(ref value, _importance)) = self.get(longhand_or_custom) {
// Step 3 // Step 2
value.to_css(dest) value.to_css(dest)
} else { } else {
// Step 4 // Step 3
Ok(()) Ok(())
} }
} }