servo: Merge #15733 - Background serialization of multiple values (from absoludity:fix-background-serialization); r=upsuper

Similar to animation, the serialization of multiple backgrounds should
return the longhands serialization if there are an unequal number of
values for the required properties. This is part of #15398

<!-- Please describe your changes on the following line: -->

---
<!-- 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
- [] These changes fix #__ (github issue number if applicable).

<!-- 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: 3086b3d291253a11e83943a34464e21fb1283fba

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : 64aa6e9696ee772e0b76138be96f90a5b26896a0
This commit is contained in:
Michael Nelson 2017-02-26 16:54:11 -08:00
parent 5c7364625b
commit deb9ee27b3
3 changed files with 218 additions and 327 deletions

View File

@ -13,6 +13,8 @@
use properties::longhands::{background_color, background_position_x, background_position_y, background_repeat};
use properties::longhands::{background_attachment, background_image, background_size, background_origin};
use properties::longhands::background_clip;
use properties::longhands::background_clip::single_value::computed_value::T as Clip;
use properties::longhands::background_origin::single_value::computed_value::T as Origin;
use values::specified::position::Position;
use parser::Parse;
@ -134,27 +136,32 @@
_ => None,
}
}
use std::cmp;
let mut len = 0;
% for name in "image position_x position_y repeat size attachment origin clip".split():
len = cmp::max(len, extract_value(self.background_${name}).map(|i| i.0.len())
.unwrap_or(0));
% endfor
let len = extract_value(self.background_image).map(|i| i.0.len()).unwrap_or(0);
// There should be at least one declared value
if len == 0 {
return dest.write_str("")
}
// If a value list length is differs then we don't do a shorthand serialization.
// The exceptions to this is color which appears once only and is serialized
// with the last item.
% for name in "image position_x position_y size repeat origin clip attachment".split():
if len != extract_value(self.background_${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 "image position_x position_y repeat size attachment origin clip".split():
let ${name} = if let DeclaredValue::Value(ref arr) = *self.background_${name} {
arr.0.get(i % arr.0.len())
&arr.0[i]
} else {
None
unreachable!()
};
% endfor
let color = if i == len - 1 {
Some(self.background_color)
} else {
@ -178,75 +185,33 @@
None => ()
};
% for name in "image repeat attachment position_x position_y".split():
try!(${name}.to_css(dest));
try!(write!(dest, " "));
% endfor
if let Some(image) = image {
try!(image.to_css(dest));
} else {
try!(write!(dest, "none"));
}
try!(write!(dest, "/ "));
try!(size.to_css(dest));
try!(write!(dest, " "));
if let Some(repeat) = repeat {
try!(repeat.to_css(dest));
} else {
try!(write!(dest, "repeat"));
}
try!(write!(dest, " "));
if let Some(attachment) = attachment {
try!(attachment.to_css(dest));
} else {
try!(write!(dest, "scroll"));
}
try!(write!(dest, " "));
try!(position_x.unwrap_or(&background_position_x::single_value
::get_initial_position_value())
.to_css(dest));
try!(write!(dest, " "));
try!(position_y.unwrap_or(&background_position_y::single_value
::get_initial_position_value())
.to_css(dest));
if let Some(size) = size {
try!(write!(dest, " / "));
try!(size.to_css(dest));
}
match (origin, clip) {
(Some(origin), Some(clip)) => {
use properties::longhands::background_origin::single_value::computed_value::T as Origin;
use properties::longhands::background_clip::single_value::computed_value::T as Clip;
try!(write!(dest, " "));
match (origin, clip) {
(&Origin::padding_box, &Clip::padding_box) => {
try!(origin.to_css(dest));
},
(&Origin::border_box, &Clip::border_box) => {
try!(origin.to_css(dest));
},
(&Origin::content_box, &Clip::content_box) => {
try!(origin.to_css(dest));
},
_ => {
try!(origin.to_css(dest));
try!(write!(dest, " "));
try!(clip.to_css(dest));
}
}
(&Origin::padding_box, &Clip::padding_box) => {
try!(origin.to_css(dest));
},
_ => {}
(&Origin::border_box, &Clip::border_box) => {
try!(origin.to_css(dest));
},
(&Origin::content_box, &Clip::content_box) => {
try!(origin.to_css(dest));
},
_ => {
try!(origin.to_css(dest));
try!(write!(dest, " "));
try!(clip.to_css(dest));
}
};
}
Ok(())
}
}

View File

@ -133,16 +133,51 @@ macro_rules! try_parse_one {
impl<'a> LonghandsToSerialize<'a> {
fn to_css_declared<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
try!(self.transition_property.to_css(dest));
try!(write!(dest, " "));
fn extract_value<T>(x: &DeclaredValue<T>) -> Option< &T> {
match *x {
DeclaredValue::Value(ref val) => Some(val),
_ => None,
}
}
try!(self.transition_duration.to_css(dest));
try!(write!(dest, " "));
let len = extract_value(self.transition_property).map(|i| i.0.len()).unwrap_or(0);
// There should be at least one declared value
if len == 0 {
return dest.write_str("")
}
try!(self.transition_timing_function.to_css(dest));
try!(write!(dest, " "));
// If any value list length is differs then we don't do a shorthand serialization
// either.
% for name in "property duration delay timing_function".split():
if len != extract_value(self.transition_${name}).map(|i| i.0.len()).unwrap_or(0) {
return dest.write_str("")
}
% endfor
self.transition_delay.to_css(dest)
let mut first = true;
for i in 0..len {
% for name in "property duration delay timing_function".split():
let ${name} = if let DeclaredValue::Value(ref arr) = *self.transition_${name} {
&arr.0[i]
} else {
unreachable!()
};
% endfor
if first {
first = false;
} else {
try!(write!(dest, ", "));
}
try!(property.to_css(dest));
% for name in "duration timing_function delay".split():
try!(write!(dest, " "));
try!(${name}.to_css(dest));
% endfor
}
Ok(())
}
}
</%helpers:shorthand>

View File

@ -2,16 +2,27 @@
* 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/. */
pub use std::sync::Arc;
pub use style::computed_values::display::T::inline_block;
pub use style::properties::{DeclaredValue, PropertyDeclaration, PropertyDeclarationBlock, Importance, PropertyId};
pub use style::values::specified::{BorderStyle, BorderWidth, CSSColor, Length, NoCalcLength};
pub use style::values::specified::{LengthOrPercentage, LengthOrPercentageOrAuto, LengthOrPercentageOrAutoOrContent};
pub use style::properties::longhands::outline_color::computed_value::T as ComputedColor;
pub use style::properties::longhands::outline_style::SpecifiedValue as OutlineStyle;
pub use style::values::{RGBA, Auto};
pub use style::values::specified::url::{UrlExtraData, SpecifiedUrl};
pub use style_traits::ToCss;
use cssparser::Parser;
use media_queries::CSSErrorReporterTest;
use servo_url::ServoUrl;
use style::computed_values::display::T::inline_block;
use style::parser::ParserContext;
use style::properties::{DeclaredValue, PropertyDeclaration, PropertyDeclarationBlock, Importance, PropertyId};
use style::properties::longhands::outline_color::computed_value::T as ComputedColor;
use style::properties::parse_property_declaration_list;
use style::stylesheets::Origin;
use style::values::{RGBA, Auto};
use style::values::specified::{BorderStyle, BorderWidth, CSSColor, Length, NoCalcLength};
use style::values::specified::{LengthOrPercentage, LengthOrPercentageOrAuto, LengthOrPercentageOrAutoOrContent};
use style::values::specified::url::SpecifiedUrl;
use style_traits::ToCss;
fn parse_declaration_block(css_properties: &str) -> PropertyDeclarationBlock {
let url = ServoUrl::parse("http://localhost").unwrap();
let context = ParserContext::new(Origin::Author, &url, Box::new(CSSErrorReporterTest));
let mut parser = Parser::new(css_properties);
parse_property_declaration_list(&context, &mut parser)
}
#[test]
fn property_declaration_block_should_serialize_correctly() {
@ -552,46 +563,6 @@ mod shorthand_serialization {
assert_eq!(serialization, "columns: auto auto;");
}
#[test]
fn transition_should_serialize_all_available_properties() {
use euclid::point::Point2D;
use style::properties::animated_properties::TransitionProperty;
use style::properties::longhands::transition_delay::SpecifiedValue as DelayContainer;
use style::properties::longhands::transition_duration::SpecifiedValue as DurationContainer;
use style::properties::longhands::transition_property::SpecifiedValue as PropertyContainer;
use style::properties::longhands::transition_timing_function;
use style::values::specified::Time as TimeContainer;
let property_name = DeclaredValue::Value(
PropertyContainer(vec![TransitionProperty::MarginLeft])
);
let duration = DeclaredValue::Value(
DurationContainer(vec![TimeContainer(3f32)])
);
let delay = DeclaredValue::Value(
DelayContainer(vec![TimeContainer(4f32)])
);
let timing_function = DeclaredValue::Value(
transition_timing_function::SpecifiedValue(vec![
transition_timing_function::single_value::SpecifiedValue::CubicBezier(
Point2D::new(0f32, 5f32), Point2D::new(5f32, 10f32))
])
);
let mut properties = Vec::new();
properties.push(PropertyDeclaration::TransitionProperty(property_name));
properties.push(PropertyDeclaration::TransitionDelay(delay));
properties.push(PropertyDeclaration::TransitionDuration(duration));
properties.push(PropertyDeclaration::TransitionTimingFunction(timing_function));
let serialization = shorthand_properties_to_string(properties);
assert_eq!(serialization, "transition: margin-left 3s cubic-bezier(0, 5, 5, 10) 4s;");
}
#[test]
fn flex_should_serialize_all_available_properties() {
use style::values::specified::Number as NumberContainer;
@ -676,108 +647,24 @@ mod shorthand_serialization {
}
*/
// TODO: Populate Atom Cache for testing so that the animation shorthand can be tested
/*
#[test]
fn animation_should_serialize_all_available_properties() {
let mut properties = Vec::new();
assert_eq!(serialization, "animation;");
}
*/
mod background {
use style::properties::longhands::background_attachment as attachment;
use style::properties::longhands::background_clip as clip;
use style::properties::longhands::background_image as image;
use style::properties::longhands::background_origin as origin;
use style::properties::longhands::background_position_x as position_x;
use style::properties::longhands::background_position_y as position_y;
use style::properties::longhands::background_repeat as repeat;
use style::properties::longhands::background_size as size;
use style::values::specified::Image;
use style::values::specified::position::{HorizontalPosition, VerticalPosition};
use super::*;
macro_rules! single_vec_value_typedef {
($name:ident, $path:expr) => {
DeclaredValue::Value($name::SpecifiedValue(
vec![$path]
))
};
}
macro_rules! single_vec_value {
($name:ident, $path:expr) => {
DeclaredValue::Value($name::SpecifiedValue(
vec![$name::single_value::SpecifiedValue($path)]
))
};
}
macro_rules! single_vec_keyword_value {
($name:ident, $kw:ident) => {
DeclaredValue::Value($name::SpecifiedValue(
vec![$name::single_value::SpecifiedValue::$kw]
))
};
}
macro_rules! single_vec_variant_value {
($name:ident, $variant:expr) => {
DeclaredValue::Value($name::SpecifiedValue(
vec![$variant]
))
};
}
#[test]
fn background_should_serialize_all_available_properties_when_specified() {
let mut properties = Vec::new();
let block_text = "\
background-color: rgb(255, 0, 0); \
background-image: url(\"http://servo/test.png\"); \
background-repeat: repeat-x; \
background-attachment: scroll; \
background-size: 70px 50px; \
background-position-x: 7px; \
background-position-y: 4px; \
background-origin: border-box; \
background-clip: padding-box;";
let block = parse_declaration_block(block_text);
let color = DeclaredValue::Value(CSSColor {
parsed: ComputedColor::RGBA(RGBA::new(255, 0, 0, 255)),
authored: None
});
let position_x = single_vec_value_typedef!(position_x,
HorizontalPosition {
keyword: None,
position: Some(LengthOrPercentage::Length(NoCalcLength::from_px(7f32))),
}
);
let position_y = single_vec_value_typedef!(position_y,
VerticalPosition {
keyword: None,
position: Some(LengthOrPercentage::Length(NoCalcLength::from_px(4f32))),
}
);
let repeat = single_vec_keyword_value!(repeat, repeat_x);
let attachment = single_vec_keyword_value!(attachment, scroll);
let image = single_vec_value!(image,
Some(Image::Url(SpecifiedUrl::new_for_testing("http://servo/test.png"))));
let size = single_vec_variant_value!(size,
size::single_value::SpecifiedValue::Explicit(
size::single_value::ExplicitSize {
width: LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(70f32)),
height: LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(50f32))
}
)
);
let origin = single_vec_keyword_value!(origin, border_box);
let clip = single_vec_keyword_value!(clip, padding_box);
properties.push(PropertyDeclaration::BackgroundColor(color));
properties.push(PropertyDeclaration::BackgroundPositionX(position_x));
properties.push(PropertyDeclaration::BackgroundPositionY(position_y));
properties.push(PropertyDeclaration::BackgroundRepeat(repeat));
properties.push(PropertyDeclaration::BackgroundAttachment(attachment));
properties.push(PropertyDeclaration::BackgroundImage(image));
properties.push(PropertyDeclaration::BackgroundSize(size));
properties.push(PropertyDeclaration::BackgroundOrigin(origin));
properties.push(PropertyDeclaration::BackgroundClip(clip));
let serialization = shorthand_properties_to_string(properties);
let serialization = block.to_css_string();
assert_eq!(
serialization,
@ -788,56 +675,20 @@ mod shorthand_serialization {
#[test]
fn background_should_combine_origin_and_clip_properties_when_equal() {
let mut properties = Vec::new();
let block_text = "\
background-color: rgb(255, 0, 0); \
background-image: url(\"http://servo/test.png\"); \
background-repeat: repeat-x; \
background-attachment: scroll; \
background-size: 70px 50px; \
background-position-x: 7px; \
background-position-y: 4px; \
background-origin: padding-box; \
background-clip: padding-box;";
let block = parse_declaration_block(block_text);
let color = DeclaredValue::Value(CSSColor {
parsed: ComputedColor::RGBA(RGBA::new(255, 0, 0, 255)),
authored: None
});
let serialization = block.to_css_string();
let position_x = single_vec_value_typedef!(position_x,
HorizontalPosition {
keyword: None,
position: Some(LengthOrPercentage::Length(NoCalcLength::from_px(7f32))),
}
);
let position_y = single_vec_value_typedef!(position_y,
VerticalPosition {
keyword: None,
position: Some(LengthOrPercentage::Length(NoCalcLength::from_px(4f32))),
}
);
let repeat = single_vec_keyword_value!(repeat, repeat_x);
let attachment = single_vec_keyword_value!(attachment, scroll);
let image = single_vec_value!(image,
Some(Image::Url(SpecifiedUrl::new_for_testing("http://servo/test.png"))));
let size = single_vec_variant_value!(size,
size::single_value::SpecifiedValue::Explicit(
size::single_value::ExplicitSize {
width: LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(70f32)),
height: LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(50f32))
}
)
);
let origin = single_vec_keyword_value!(origin, padding_box);
let clip = single_vec_keyword_value!(clip, padding_box);
properties.push(PropertyDeclaration::BackgroundColor(color));
properties.push(PropertyDeclaration::BackgroundPositionX(position_x));
properties.push(PropertyDeclaration::BackgroundPositionY(position_y));
properties.push(PropertyDeclaration::BackgroundRepeat(repeat));
properties.push(PropertyDeclaration::BackgroundAttachment(attachment));
properties.push(PropertyDeclaration::BackgroundImage(image));
properties.push(PropertyDeclaration::BackgroundSize(size));
properties.push(PropertyDeclaration::BackgroundOrigin(origin));
properties.push(PropertyDeclaration::BackgroundClip(clip));
let serialization = shorthand_properties_to_string(properties);
assert_eq!(
serialization,
"background: rgb(255, 0, 0) url(\"http://servo/test.png\") repeat-x \
@ -846,50 +697,52 @@ mod shorthand_serialization {
}
#[test]
fn background_should_always_print_color_and_url_and_repeat_and_attachment_and_position() {
let mut properties = Vec::new();
fn serialize_multiple_backgrounds() {
let block_text = "\
background-color: rgb(0, 0, 255); \
background-image: url(\"http://servo/test.png\"), none; \
background-repeat: repeat-x, repeat-y; \
background-attachment: scroll, scroll; \
background-size: 70px 50px, 20px 30px; \
background-position-x: 7px, 70px; \
background-position-y: 4px, 40px; \
background-origin: border-box, padding-box; \
background-clip: padding-box, padding-box;";
let block = parse_declaration_block(block_text);
let color = DeclaredValue::Value(CSSColor {
parsed: ComputedColor::RGBA(RGBA::new(255, 0, 0, 255)),
authored: None
});
let serialization = block.to_css_string();
let position_x = single_vec_value_typedef!(position_x,
HorizontalPosition {
keyword: None,
position: Some(LengthOrPercentage::Length(NoCalcLength::from_px(0f32))),
}
assert_eq!(
serialization, "background: \
url(\"http://servo/test.png\") repeat-x scroll 7px 4px / 70px 50px border-box padding-box, \
rgb(0, 0, 255) none repeat-y scroll 70px 40px / 20px 30px padding-box;"
);
}
let position_y = single_vec_value_typedef!(position_y,
VerticalPosition {
keyword: None,
position: Some(LengthOrPercentage::Length(NoCalcLength::from_px(0f32))),
}
);
#[test]
fn serialize_multiple_backgrounds_unequal_property_lists() {
// When the lengths of property values are different, the shorthand serialization
// should not be used. Previously the implementation cycled values if the lists were
// uneven. This is incorrect, in that we should serialize to a shorthand only when the
// lists have the same length (this affects background, transition and animation).
// https://github.com/servo/servo/issues/15398 )
// With background, the color is one exception as it should only appear once for
// multiple backgrounds.
// Below, background-position and background-origin only have one value.
let block_text = "\
background-color: rgb(0, 0, 255); \
background-image: url(\"http://servo/test.png\"), none; \
background-repeat: repeat-x, repeat-y; \
background-attachment: scroll, scroll; \
background-size: 70px 50px, 20px 30px; \
background-position: 7px 4px; \
background-origin: border-box; \
background-clip: padding-box, padding-box;";
let block = parse_declaration_block(block_text);
let repeat = single_vec_keyword_value!(repeat, repeat_x);
let attachment = single_vec_keyword_value!(attachment, scroll);
let serialization = block.to_css_string();
let image = single_vec_value!(image, None);
let size = DeclaredValue::Initial;
let origin = DeclaredValue::Initial;
let clip = DeclaredValue::Initial;
properties.push(PropertyDeclaration::BackgroundColor(color));
properties.push(PropertyDeclaration::BackgroundPositionX(position_x));
properties.push(PropertyDeclaration::BackgroundPositionY(position_y));
properties.push(PropertyDeclaration::BackgroundRepeat(repeat));
properties.push(PropertyDeclaration::BackgroundAttachment(attachment));
properties.push(PropertyDeclaration::BackgroundImage(image));
properties.push(PropertyDeclaration::BackgroundSize(size));
properties.push(PropertyDeclaration::BackgroundOrigin(origin));
properties.push(PropertyDeclaration::BackgroundClip(clip));
let serialization = shorthand_properties_to_string(properties);
assert_eq!(serialization, "background: rgb(255, 0, 0) none repeat-x scroll 0px 0px;");
assert_eq!(serialization, block_text);
}
}
@ -1140,23 +993,10 @@ mod shorthand_serialization {
mod animation {
pub use super::*;
use cssparser::Parser;
use media_queries::CSSErrorReporterTest;
use servo_url::ServoUrl;
use style::parser::ParserContext;
use style::properties::{parse_property_declaration_list, PropertyDeclarationBlock};
use style::stylesheets::Origin;
fn property_declaration_block(css_properties: &str) -> PropertyDeclarationBlock {
let url = ServoUrl::parse("http://localhost").unwrap();
let context = ParserContext::new(Origin::Author, &url, Box::new(CSSErrorReporterTest));
let mut parser = Parser::new(css_properties);
parse_property_declaration_list(&context, &mut parser)
}
#[test]
fn serialize_single_animation() {
let block = property_declaration_block("\
let block = parse_declaration_block("\
animation-name: bounce;\
animation-duration: 1s;\
animation-timing-function: ease-in;\
@ -1173,7 +1013,7 @@ mod shorthand_serialization {
#[test]
fn serialize_multiple_animations() {
let block = property_declaration_block("\
let block = parse_declaration_block("\
animation-name: bounce, roll;\
animation-duration: 1s, 0.2s;\
animation-timing-function: ease-in, linear;\
@ -1195,7 +1035,7 @@ mod shorthand_serialization {
// When the lengths of property values are different, the shorthand serialization
// should not be used. Previously the implementation cycled values if the lists were
// uneven. This is incorrect, in that we should serialize to a shorthand only when the
// lists have the same length (both here and for background and transition. See
// lists have the same length (this affects background, transition and animation).
// https://github.com/servo/servo/issues/15398 )
let block_text = "\
animation-name: bounce, roll, flip, jump; \
@ -1206,7 +1046,7 @@ mod shorthand_serialization {
animation-fill-mode: forwards, backwards; \
animation-iteration-count: infinite, 2; \
animation-play-state: paused, running;";
let block = property_declaration_block(block_text);
let block = parse_declaration_block(block_text);
let serialization = block.to_css_string();
@ -1222,7 +1062,58 @@ mod shorthand_serialization {
animation-fill-mode: forwards, backwards; \
animation-iteration-count: infinite, 2; \
animation-play-state: paused, running;";
let block = property_declaration_block(block_text);
let block = parse_declaration_block(block_text);
let serialization = block.to_css_string();
assert_eq!(serialization, block_text);
}
}
mod transition {
pub use super::*;
#[test]
fn transition_should_serialize_all_available_properties() {
let block_text = "transition-property: margin-left; \
transition-duration: 3s; \
transition-delay: 4s; \
transition-timing-function: cubic-bezier(0.2, 5, 0.5, 2);";
let block = parse_declaration_block(block_text);
let serialization = block.to_css_string();
assert_eq!(serialization, "transition: margin-left 3s cubic-bezier(0.2, 5, 0.5, 2) 4s;");
}
#[test]
fn serialize_multiple_transitions() {
let block_text = "transition-property: margin-left, width; \
transition-duration: 3s, 2s; \
transition-delay: 4s, 5s; \
transition-timing-function: cubic-bezier(0.2, 5, 0.5, 2), ease;";
let block = parse_declaration_block(block_text);
let serialization = block.to_css_string();
assert_eq!(serialization, "transition: \
margin-left 3s cubic-bezier(0.2, 5, 0.5, 2) 4s, \
width 2s ease 5s;");
}
#[test]
fn serialize_multiple_transitions_unequal_property_lists() {
// When the lengths of property values are different, the shorthand serialization
// should not be used. Previously the implementation cycled values if the lists were
// uneven. This is incorrect, in that we should serialize to a shorthand only when the
// lists have the same length (this affects background, transition and animation).
// https://github.com/servo/servo/issues/15398 )
// The duration below has 1 extra value.
let block_text = "transition-property: margin-left, width; \
transition-duration: 3s, 2s, 4s; \
transition-delay: 4s, 5s; \
transition-timing-function: cubic-bezier(0.2, 5, 0.5, 2), ease;";
let block = parse_declaration_block(block_text);
let serialization = block.to_css_string();