diff --git a/components/style/properties/shorthand/background.mako.rs b/components/style/properties/shorthand/background.mako.rs index 857a728a7bb..30c60e977ab 100644 --- a/components/style/properties/shorthand/background.mako.rs +++ b/components/style/properties/shorthand/background.mako.rs @@ -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(()) } } diff --git a/tests/unit/style/properties/serialization.rs b/tests/unit/style/properties/serialization.rs index 2b0085ccf13..db44f486281 100644 --- a/tests/unit/style/properties/serialization.rs +++ b/tests/unit/style/properties/serialization.rs @@ -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 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 property_declaration_block_should_serialize_correctly() { @@ -687,97 +698,23 @@ mod shorthand_serialization { */ 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 = property_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 +725,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 = property_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 +747,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 = property_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 = property_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,19 +1043,6 @@ 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() { @@ -1195,7 +1085,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; \