From 0fc38f79d64f217b7a9421ffbd40da6d763b9eb1 Mon Sep 17 00:00:00 2001 From: Ting-Yu Lin Date: Mon, 24 Oct 2016 12:46:40 +0800 Subject: [PATCH] Use enum BorderWidth as SpecifiedValue Use enum BorderWidth instead of a tuple-like struct to store the specified value. BorderWidth is needed to be used in both longhand and shorthand border width properties, so I put it in `specified` module. Fixed #13869. --- components/script/dom/element.rs | 17 ++--- .../style/properties/longhand/border.mako.rs | 35 ++-------- .../style/properties/shorthand/border.mako.rs | 19 ++--- components/style/values/specified/mod.rs | 69 +++++++++++++++++++ tests/unit/style/properties/serialization.rs | 57 ++++++++------- tests/unit/style/properties/viewport.rs | 4 +- 6 files changed, 122 insertions(+), 79 deletions(-) diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index cd4c3db5209..28ab35979ba 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -88,7 +88,7 @@ use style::matching::{common_style_affecting_attributes, rare_style_affecting_at use style::parser::ParserContextExtraData; use style::properties::{DeclaredValue, Importance}; use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, parse_style_attribute}; -use style::properties::longhands::{self, background_image, border_spacing, font_family, font_size, overflow_x}; +use style::properties::longhands::{background_image, border_spacing, font_family, font_size, overflow_x}; use style::selector_impl::{NonTSPseudoClass, ServoSelectorImpl}; use style::selector_matching::ApplicableDeclarationBlock; use style::sink::Push; @@ -574,19 +574,16 @@ impl LayoutElementHelpers for LayoutJS { }; if let Some(border) = border { - let width_value = specified::Length::Absolute(Au::from_px(border as i32)); + let width_value = specified::BorderWidth::from_length( + specified::Length::Absolute(Au::from_px(border as i32))); hints.push(from_declaration( - PropertyDeclaration::BorderTopWidth(DeclaredValue::Value( - longhands::border_top_width::SpecifiedValue(width_value))))); + PropertyDeclaration::BorderTopWidth(DeclaredValue::Value(width_value)))); hints.push(from_declaration( - PropertyDeclaration::BorderLeftWidth(DeclaredValue::Value( - longhands::border_left_width::SpecifiedValue(width_value))))); + PropertyDeclaration::BorderLeftWidth(DeclaredValue::Value(width_value)))); hints.push(from_declaration( - PropertyDeclaration::BorderBottomWidth(DeclaredValue::Value( - longhands::border_bottom_width::SpecifiedValue(width_value))))); + PropertyDeclaration::BorderBottomWidth(DeclaredValue::Value(width_value)))); hints.push(from_declaration( - PropertyDeclaration::BorderRightWidth(DeclaredValue::Value( - longhands::border_right_width::SpecifiedValue(width_value))))); + PropertyDeclaration::BorderRightWidth(DeclaredValue::Value(width_value)))); } } diff --git a/components/style/properties/longhand/border.mako.rs b/components/style/properties/longhand/border.mako.rs index 52819633091..40892a79ace 100644 --- a/components/style/properties/longhand/border.mako.rs +++ b/components/style/properties/longhand/border.mako.rs @@ -27,27 +27,14 @@ use cssparser::ToCss; use std::fmt; use values::HasViewportPercentage; + use values::specified::BorderWidth; - impl ToCss for SpecifiedValue { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - self.0.to_css(dest) - } - } + pub type SpecifiedValue = BorderWidth; #[inline] pub fn parse(_context: &ParserContext, input: &mut Parser) - -> Result { - specified::parse_border_width(input).map(SpecifiedValue) - } - #[derive(Debug, Clone, PartialEq)] - #[cfg_attr(feature = "servo", derive(HeapSizeOf))] - pub struct SpecifiedValue(pub specified::Length); - - impl HasViewportPercentage for SpecifiedValue { - fn has_viewport_percentage(&self) -> bool { - let &SpecifiedValue(length) = self; - length.has_viewport_percentage() - } + -> Result { + BorderWidth::parse(input) } pub mod computed_value { @@ -57,20 +44,6 @@ #[inline] pub fn get_initial_value() -> computed_value::T { Au::from_px(3) // medium } - - impl ToComputedValue for SpecifiedValue { - type ComputedValue = computed_value::T; - - #[inline] - fn to_computed_value(&self, context: &Context) -> computed_value::T { - self.0.to_computed_value(context) - } - - #[inline] - fn from_computed_value(computed: &computed_value::T) -> Self { - SpecifiedValue(ToComputedValue::from_computed_value(computed)) - } - } % endfor diff --git a/components/style/properties/shorthand/border.mako.rs b/components/style/properties/shorthand/border.mako.rs index c29a6a2e7fb..ef04ab474f0 100644 --- a/components/style/properties/shorthand/border.mako.rs +++ b/components/style/properties/shorthand/border.mako.rs @@ -16,22 +16,19 @@ ${helpers.four_sides_shorthand("border-style", "border-%s-style", pub fn parse_value(context: &ParserContext, input: &mut Parser) -> Result { let _unused = context; - let (top, right, bottom, left) = try!(parse_four_sides(input, specified::parse_border_width)); + let (top, right, bottom, left) = try!(parse_four_sides(input, specified::BorderWidth::parse)); Ok(Longhands { % for side in ["top", "right", "bottom", "left"]: - ${to_rust_ident('border-%s-width' % side)}: - Some(longhands::${to_rust_ident('border-%s-width' % side)}::SpecifiedValue(${side})), + ${to_rust_ident('border-%s-width' % side)}: Some(${side}), % endfor }) } impl<'a> LonghandsToSerialize<'a> { fn to_css_declared(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - // extract tuple container values so that the different border widths - // can be compared via partial eq % for side in ["top", "right", "bottom", "left"]: let ${side} = match self.border_${side}_width { - &DeclaredValue::Value(ref value) => DeclaredValue::Value(value.0), + &DeclaredValue::Value(ref value) => DeclaredValue::Value(*value), &DeclaredValue::WithVariables { css: ref a, first_token_type: ref b, base_url: ref c, from_shorthand: ref d } => DeclaredValue::WithVariables { @@ -52,7 +49,7 @@ ${helpers.four_sides_shorthand("border-style", "border-%s-style", pub fn parse_border(context: &ParserContext, input: &mut Parser) -> Result<(Option, Option, - Option), ()> { + Option), ()> { use values::specified; let _unused = context; let mut color = None; @@ -75,7 +72,7 @@ pub fn parse_border(context: &ParserContext, input: &mut Parser) } } if width.is_none() { - if let Ok(value) = input.try(specified::parse_border_width) { + if let Ok(value) = input.try(specified::BorderWidth::parse) { width = Some(value); any = true; continue @@ -97,8 +94,7 @@ pub fn parse_border(context: &ParserContext, input: &mut Parser) Ok(Longhands { border_${side}_color: color, border_${side}_style: style, - border_${side}_width: - width.map(longhands::${to_rust_ident('border-%s-width' % side)}::SpecifiedValue), + border_${side}_width: width }) } @@ -128,8 +124,7 @@ pub fn parse_border(context: &ParserContext, input: &mut Parser) % for side in ["top", "right", "bottom", "left"]: border_${side}_color: color.clone(), border_${side}_style: style, - border_${side}_width: - width.map(longhands::${to_rust_ident('border-%s-width' % side)}::SpecifiedValue), + border_${side}_width: width, % endfor }) } diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 436dc65d7d9..aad327ca1d3 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -1304,6 +1304,75 @@ pub fn parse_border_width(input: &mut Parser) -> Result { }) } +#[derive(Clone, PartialEq, Copy, Debug)] +#[cfg_attr(feature = "servo", derive(HeapSizeOf))] +pub enum BorderWidth { + Thin, + Medium, + Thick, + Width(Length), +} + +impl BorderWidth { + pub fn from_length(length: Length) -> Self { + BorderWidth::Width(length) + } + + pub fn parse(input: &mut Parser) -> Result { + match input.try(Length::parse_non_negative) { + Ok(length) => Ok(BorderWidth::Width(length)), + Err(_) => match_ignore_ascii_case! { try!(input.expect_ident()), + "thin" => Ok(BorderWidth::Thin), + "medium" => Ok(BorderWidth::Medium), + "thick" => Ok(BorderWidth::Thick), + _ => Err(()) + } + } + } +} + +impl ToCss for BorderWidth { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + match *self { + BorderWidth::Thin => dest.write_str("thin"), + BorderWidth::Medium => dest.write_str("medium"), + BorderWidth::Thick => dest.write_str("thick"), + BorderWidth::Width(length) => length.to_css(dest) + } + } +} + +impl HasViewportPercentage for BorderWidth { + fn has_viewport_percentage(&self) -> bool { + match *self { + BorderWidth::Thin | BorderWidth::Medium | BorderWidth::Thick => false, + BorderWidth::Width(length) => length.has_viewport_percentage() + } + } +} + +impl ToComputedValue for BorderWidth { + type ComputedValue = Au; + + #[inline] + fn to_computed_value(&self, context: &Context) -> Self::ComputedValue { + // We choose the pixel length of the keyword values the same as both spec and gecko. + // Spec: https://drafts.csswg.org/css-backgrounds-3/#line-width + // Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1312155#c0 + match *self { + BorderWidth::Thin => Length::from_px(1.).to_computed_value(context), + BorderWidth::Medium => Length::from_px(3.).to_computed_value(context), + BorderWidth::Thick => Length::from_px(5.).to_computed_value(context), + BorderWidth::Width(length) => length.to_computed_value(context) + } + } + + #[inline] + fn from_computed_value(computed: &Self::ComputedValue) -> Self { + BorderWidth::Width(ToComputedValue::from_computed_value(computed)) + } +} + // The integer values here correspond to the border conflict resolution rules in CSS 2.1 ยง // 17.6.2.1. Higher values override lower values. define_numbered_css_keyword_enum! { BorderStyle: diff --git a/tests/unit/style/properties/serialization.rs b/tests/unit/style/properties/serialization.rs index 1d644be305a..20c4d1eba07 100644 --- a/tests/unit/style/properties/serialization.rs +++ b/tests/unit/style/properties/serialization.rs @@ -6,7 +6,7 @@ pub use cssparser::ToCss; pub use std::sync::Arc; pub use style::computed_values::display::T::inline_block; pub use style::properties::{DeclaredValue, PropertyDeclaration, PropertyDeclarationBlock, Importance}; -pub use style::values::specified::{BorderStyle, CSSColor, Length}; +pub use style::values::specified::{BorderStyle, BorderWidth, CSSColor, Length}; pub use style::values::specified::{LengthOrPercentage, LengthOrPercentageOrAuto, LengthOrPercentageOrAutoOrContent}; pub use style::properties::longhands::outline_color::computed_value::T as ComputedColor; pub use style::values::RGBA; @@ -192,18 +192,13 @@ mod shorthand_serialization { #[test] fn border_width_should_serialize_correctly() { - use style::properties::longhands::border_bottom_width::SpecifiedValue as BottomContainer; - use style::properties::longhands::border_left_width::SpecifiedValue as LeftContainer; - use style::properties::longhands::border_right_width::SpecifiedValue as RightContainer; - use style::properties::longhands::border_top_width::SpecifiedValue as TopContainer; - let mut properties = Vec::new(); - let top_px = DeclaredValue::Value(TopContainer(Length::from_px(10f32))); - let bottom_px = DeclaredValue::Value(BottomContainer(Length::from_px(10f32))); + let top_px = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(10f32))); + let bottom_px = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(10f32))); - let right_px = DeclaredValue::Value(RightContainer(Length::from_px(15f32))); - let left_px = DeclaredValue::Value(LeftContainer(Length::from_px(15f32))); + let right_px = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(15f32))); + let left_px = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(15f32))); properties.push(PropertyDeclaration::BorderTopWidth(top_px)); properties.push(PropertyDeclaration::BorderRightWidth(right_px)); @@ -214,6 +209,24 @@ mod shorthand_serialization { assert_eq!(serialization, "border-width: 10px 15px;"); } + #[test] + fn border_width_with_keywords_should_serialize_correctly() { + let mut properties = Vec::new(); + + let top_px = DeclaredValue::Value(BorderWidth::Thin); + let right_px = DeclaredValue::Value(BorderWidth::Medium); + let bottom_px = DeclaredValue::Value(BorderWidth::Thick); + let left_px = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(15f32))); + + properties.push(PropertyDeclaration::BorderTopWidth(top_px)); + properties.push(PropertyDeclaration::BorderRightWidth(right_px)); + properties.push(PropertyDeclaration::BorderBottomWidth(bottom_px)); + properties.push(PropertyDeclaration::BorderLeftWidth(left_px)); + + let serialization = shorthand_properties_to_string(properties); + assert_eq!(serialization, "border-width: thin medium thick 15px;"); + } + #[test] fn border_color_should_serialize_correctly() { let mut properties = Vec::new(); @@ -257,10 +270,6 @@ mod shorthand_serialization { mod border_shorthands { - use style::properties::longhands::border_bottom_width::SpecifiedValue as BottomContainer; - use style::properties::longhands::border_left_width::SpecifiedValue as LeftContainer; - use style::properties::longhands::border_right_width::SpecifiedValue as RightContainer; - use style::properties::longhands::border_top_width::SpecifiedValue as TopContainer; use super::*; // we can use border-top as a base to test out the different combinations @@ -270,7 +279,7 @@ mod shorthand_serialization { fn directional_border_should_show_all_properties_when_values_are_set() { let mut properties = Vec::new(); - let width = DeclaredValue::Value(TopContainer(Length::from_px(4f32))); + let width = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(4f32))); let style = DeclaredValue::Value(BorderStyle::solid); let color = DeclaredValue::Value(CSSColor { parsed: ComputedColor::RGBA(RGBA { red: 1f32, green: 0f32, blue: 0f32, alpha: 1f32 }), @@ -289,7 +298,7 @@ mod shorthand_serialization { fn directional_border_with_no_specified_style_will_show_style_as_none() { let mut properties = Vec::new(); - let width = DeclaredValue::Value(TopContainer(Length::from_px(4f32))); + let width = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(4f32))); let style = DeclaredValue::Initial; let color = DeclaredValue::Value(CSSColor { parsed: ComputedColor::RGBA(RGBA { red: 1f32, green: 0f32, blue: 0f32, alpha: 1f32 }), @@ -308,7 +317,7 @@ mod shorthand_serialization { fn directional_border_with_no_specified_color_will_not_show_color() { let mut properties = Vec::new(); - let width = DeclaredValue::Value(TopContainer(Length::from_px(4f32))); + let width = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(4f32))); let style = DeclaredValue::Value(BorderStyle::solid); let color = DeclaredValue::Initial; @@ -324,7 +333,7 @@ mod shorthand_serialization { fn border_right_should_serialize_correctly() { let mut properties = Vec::new(); - let width = DeclaredValue::Value(RightContainer(Length::from_px(4f32))); + let width = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(4f32))); let style = DeclaredValue::Value(BorderStyle::solid); let color = DeclaredValue::Initial; @@ -340,7 +349,7 @@ mod shorthand_serialization { fn border_bottom_should_serialize_correctly() { let mut properties = Vec::new(); - let width = DeclaredValue::Value(BottomContainer(Length::from_px(4f32))); + let width = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(4f32))); let style = DeclaredValue::Value(BorderStyle::solid); let color = DeclaredValue::Initial; @@ -356,7 +365,7 @@ mod shorthand_serialization { fn border_left_should_serialize_correctly() { let mut properties = Vec::new(); - let width = DeclaredValue::Value(LeftContainer(Length::from_px(4f32))); + let width = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(4f32))); let style = DeclaredValue::Value(BorderStyle::solid); let color = DeclaredValue::Initial; @@ -372,7 +381,7 @@ mod shorthand_serialization { fn border_should_serialize_correctly() { let mut properties = Vec::new(); - let top_width = DeclaredValue::Value(TopContainer(Length::from_px(4f32))); + let top_width = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(4f32))); let top_style = DeclaredValue::Value(BorderStyle::solid); let top_color = DeclaredValue::Initial; @@ -380,7 +389,7 @@ mod shorthand_serialization { properties.push(PropertyDeclaration::BorderTopStyle(top_style)); properties.push(PropertyDeclaration::BorderTopColor(top_color)); - let right_width = DeclaredValue::Value(RightContainer(Length::from_px(4f32))); + let right_width = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(4f32))); let right_style = DeclaredValue::Value(BorderStyle::solid); let right_color = DeclaredValue::Initial; @@ -388,7 +397,7 @@ mod shorthand_serialization { properties.push(PropertyDeclaration::BorderRightStyle(right_style)); properties.push(PropertyDeclaration::BorderRightColor(right_color)); - let bottom_width = DeclaredValue::Value(BottomContainer(Length::from_px(4f32))); + let bottom_width = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(4f32))); let bottom_style = DeclaredValue::Value(BorderStyle::solid); let bottom_color = DeclaredValue::Initial; @@ -396,7 +405,7 @@ mod shorthand_serialization { properties.push(PropertyDeclaration::BorderBottomStyle(bottom_style)); properties.push(PropertyDeclaration::BorderBottomColor(bottom_color)); - let left_width = DeclaredValue::Value(LeftContainer(Length::from_px(4f32))); + let left_width = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(4f32))); let left_style = DeclaredValue::Value(BorderStyle::solid); let left_color = DeclaredValue::Initial; diff --git a/tests/unit/style/properties/viewport.rs b/tests/unit/style/properties/viewport.rs index 4964373c7d8..221e7d568bb 100644 --- a/tests/unit/style/properties/viewport.rs +++ b/tests/unit/style/properties/viewport.rs @@ -12,14 +12,14 @@ use style::values::specified::{Length, ViewportPercentageLength}; fn has_viewport_percentage_for_specified_value() { //TODO: test all specified value with a HasViewportPercentage impl let pvw = PropertyDeclaration::BorderTopWidth( - DeclaredValue::Value(border_top_width::SpecifiedValue( + DeclaredValue::Value(border_top_width::SpecifiedValue::from_length( Length::ViewportPercentage(ViewportPercentageLength::Vw(100.)) )) ); assert!(pvw.has_viewport_percentage()); let pabs = PropertyDeclaration::BorderTopWidth( - DeclaredValue::Value(border_top_width::SpecifiedValue( + DeclaredValue::Value(border_top_width::SpecifiedValue::from_length( Length::Absolute(Au(100)) )) );