From a055e8af894129efe1337811a10a012be1dafaca Mon Sep 17 00:00:00 2001 From: Dan Glastonbury Date: Tue, 19 Jun 2018 14:18:33 +1000 Subject: [PATCH 01/32] style: Change nscolor to StyleComplexColor in nsCSSShadowItem. Bug: 1467621 Reviewed-by: xidorn MozReview-Commit-ID: moE2CI7fT8 --- .../sugar/ns_css_shadow_item.rs | 23 ++------------- components/style/values/animated/color.rs | 5 ++-- components/style/values/animated/effects.rs | 6 ++-- components/style/values/computed/effects.rs | 6 ++-- components/style/values/specified/effects.rs | 29 +++++++++++-------- 5 files changed, 27 insertions(+), 42 deletions(-) diff --git a/components/style/gecko_bindings/sugar/ns_css_shadow_item.rs b/components/style/gecko_bindings/sugar/ns_css_shadow_item.rs index 8bdf38d4f84..dfd819bcedb 100644 --- a/components/style/gecko_bindings/sugar/ns_css_shadow_item.rs +++ b/components/style/gecko_bindings/sugar/ns_css_shadow_item.rs @@ -5,9 +5,7 @@ //! Rust helpers for Gecko's `nsCSSShadowItem`. use app_units::Au; -use gecko::values::{convert_nscolor_to_rgba, convert_rgba_to_nscolor}; use gecko_bindings::structs::nsCSSShadowItem; -use values::computed::RGBAColor; use values::computed::effects::{BoxShadow, SimpleShadow}; impl nsCSSShadowItem { @@ -37,31 +35,14 @@ impl nsCSSShadowItem { self.mRadius = shadow.blur.0.to_i32_au(); self.mSpread = 0; self.mInset = false; - if let Some(color) = shadow.color { - self.mHasColor = true; - self.mColor = convert_rgba_to_nscolor(&color); - } else { - // TODO handle currentColor - // https://bugzilla.mozilla.org/show_bug.cgi?id=760345 - self.mHasColor = false; - self.mColor = 0; - } - } - - #[inline] - fn extract_color(&self) -> Option { - if self.mHasColor { - Some(convert_nscolor_to_rgba(self.mColor)) - } else { - None - } + self.mColor = shadow.color.into(); } /// Gets a simple shadow from this item. #[inline] fn extract_simple_shadow(&self) -> SimpleShadow { SimpleShadow { - color: self.extract_color(), + color: self.mColor.into(), horizontal: Au(self.mXOffset).into(), vertical: Au(self.mYOffset).into(), blur: Au(self.mRadius).into(), diff --git a/components/style/values/animated/color.rs b/components/style/values/animated/color.rs index 1b01a86457a..7f6b4cd9d43 100644 --- a/components/style/values/animated/color.rs +++ b/components/style/values/animated/color.rs @@ -186,7 +186,7 @@ impl ComputeSquaredDistance for Color { (Foreground, Foreground) => SquaredDistance::from_sqrt(0.), (Numeric(c1), Numeric(c2)) => c1.compute_squared_distance(&c2)?, (Foreground, Numeric(color)) | (Numeric(color), Foreground) => { - // `computed_squared_distance` is symmetic. + // `computed_squared_distance` is symmetric. color.compute_squared_distance(&RGBA::transparent())? + SquaredDistance::from_sqrt(1.) } @@ -207,7 +207,6 @@ impl ComputeSquaredDistance for Color { impl ToAnimatedZero for Color { #[inline] fn to_animated_zero(&self) -> Result { - // FIXME(nox): This does not look correct to me. - Err(()) + Ok(RGBA::transparent().into()) } } diff --git a/components/style/values/animated/effects.rs b/components/style/values/animated/effects.rs index d0463365fc2..a201ec0549e 100644 --- a/components/style/values/animated/effects.rs +++ b/components/style/values/animated/effects.rs @@ -6,7 +6,7 @@ #[cfg(not(feature = "gecko"))] use values::Impossible; -use values::animated::color::RGBA; +use values::animated::color::Color; use values::computed::{Angle, Number}; use values::computed::length::Length; #[cfg(feature = "gecko")] @@ -17,7 +17,7 @@ use values::generics::effects::Filter as GenericFilter; use values::generics::effects::SimpleShadow as GenericSimpleShadow; /// An animated value for a single `box-shadow`. -pub type BoxShadow = GenericBoxShadow, Length, Length, Length>; +pub type BoxShadow = GenericBoxShadow; /// An animated value for a single `filter`. #[cfg(feature = "gecko")] @@ -28,7 +28,7 @@ pub type Filter = GenericFilter; /// An animated value for the `drop-shadow()` filter. -pub type SimpleShadow = GenericSimpleShadow, Length, Length>; +pub type SimpleShadow = GenericSimpleShadow; impl ComputeSquaredDistance for BoxShadow { #[inline] diff --git a/components/style/values/computed/effects.rs b/components/style/values/computed/effects.rs index 3d804d42605..07ac6441b6c 100644 --- a/components/style/values/computed/effects.rs +++ b/components/style/values/computed/effects.rs @@ -7,7 +7,7 @@ #[cfg(not(feature = "gecko"))] use values::Impossible; use values::computed::{Angle, NonNegativeNumber}; -use values::computed::color::RGBAColor; +use values::computed::color::Color; use values::computed::length::{Length, NonNegativeLength}; #[cfg(feature = "gecko")] use values::computed::url::ComputedUrl; @@ -16,7 +16,7 @@ use values::generics::effects::Filter as GenericFilter; use values::generics::effects::SimpleShadow as GenericSimpleShadow; /// A computed value for a single shadow of the `box-shadow` property. -pub type BoxShadow = GenericBoxShadow, Length, NonNegativeLength, Length>; +pub type BoxShadow = GenericBoxShadow; /// A computed value for a single `filter`. #[cfg(feature = "gecko")] @@ -27,4 +27,4 @@ pub type Filter = GenericFilter; /// A computed value for the `drop-shadow()` filter. -pub type SimpleShadow = GenericSimpleShadow, Length, NonNegativeLength>; +pub type SimpleShadow = GenericSimpleShadow; diff --git a/components/style/values/specified/effects.rs b/components/style/values/specified/effects.rs index f10b53fa788..c3304b05af0 100644 --- a/components/style/values/specified/effects.rs +++ b/components/style/values/specified/effects.rs @@ -17,14 +17,14 @@ use values::generics::effects::BoxShadow as GenericBoxShadow; use values::generics::effects::Filter as GenericFilter; use values::generics::effects::SimpleShadow as GenericSimpleShadow; use values::specified::{Angle, NumberOrPercentage}; -use values::specified::color::RGBAColor; +use values::specified::color::Color; use values::specified::length::{Length, NonNegativeLength}; #[cfg(feature = "gecko")] use values::specified::url::SpecifiedUrl; /// A specified value for a single shadow of the `box-shadow` property. pub type BoxShadow = - GenericBoxShadow, Length, Option, Option>; + GenericBoxShadow, Length, Option, Option>; /// A specified value for a single `filter`. #[cfg(feature = "gecko")] @@ -93,7 +93,7 @@ impl ToComputedValue for Factor { } /// A specified value for the `drop-shadow()` filter. -pub type SimpleShadow = GenericSimpleShadow, Length, Option>; +pub type SimpleShadow = GenericSimpleShadow, Length, Option>; impl Parse for BoxShadow { fn parse<'i, 't>( @@ -135,7 +135,7 @@ impl Parse for BoxShadow { } } if color.is_none() { - if let Ok(value) = input.try(|i| RGBAColor::parse(context, i)) { + if let Ok(value) = input.try(|i| Color::parse(context, i)) { color = Some(value); continue; } @@ -249,16 +249,18 @@ impl Parse for SimpleShadow { context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - let color = input.try(|i| RGBAColor::parse(context, i)).ok(); + let color = input.try(|i| Color::parse(context, i)).ok(); let horizontal = Length::parse(context, input)?; let vertical = Length::parse(context, input)?; let blur = input.try(|i| Length::parse_non_negative(context, i)).ok(); - let color = color.or_else(|| input.try(|i| RGBAColor::parse(context, i)).ok()); + let blur = blur.map(NonNegative::); + let color = color.or_else(|| input.try(|i| Color::parse(context, i)).ok()); + Ok(SimpleShadow { - color: color, - horizontal: horizontal, - vertical: vertical, - blur: blur.map(NonNegative::), + color, + horizontal, + vertical, + blur, }) } } @@ -269,7 +271,10 @@ impl ToComputedValue for SimpleShadow { #[inline] fn to_computed_value(&self, context: &Context) -> Self::ComputedValue { ComputedSimpleShadow { - color: self.color.to_computed_value(context), + color: self.color + .as_ref() + .unwrap_or(&Color::currentcolor()) + .to_computed_value(context), horizontal: self.horizontal.to_computed_value(context), vertical: self.vertical.to_computed_value(context), blur: self.blur @@ -282,7 +287,7 @@ impl ToComputedValue for SimpleShadow { #[inline] fn from_computed_value(computed: &Self::ComputedValue) -> Self { SimpleShadow { - color: ToComputedValue::from_computed_value(&computed.color), + color: Some(ToComputedValue::from_computed_value(&computed.color)), horizontal: ToComputedValue::from_computed_value(&computed.horizontal), vertical: ToComputedValue::from_computed_value(&computed.vertical), blur: Some(ToComputedValue::from_computed_value(&computed.blur)), From ef14e65636ed963b93e5e1f64e907ea40a5247f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 15 Jun 2018 21:23:50 -0700 Subject: [PATCH 02/32] style: Allow parsing operators in media feature expressions. The only bit from the spec which I haven't implemented to my knowledge is the bit that allows you to swap the position of the media feature and the value, because it unnecessarily complicates parsing (we parse the value in terms of the feature), and I don't think it's useful given how easy it is to switch from, e.g., `(500px > width)` to `(width <= 500px)`. I filed https://github.com/w3c/csswg-drafts/issues/2791 about it. Bug: 1422225 Reviewed-by: xidorn MozReview-Commit-ID: 6xrdVl87S9X --- components/style/gecko/media_queries.rs | 208 ++++++++++++++++++++---- 1 file changed, 173 insertions(+), 35 deletions(-) diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index 771f4570b47..fd07595f458 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -7,7 +7,7 @@ use app_units::AU_PER_PX; use app_units::Au; use context::QuirksMode; -use cssparser::{BasicParseErrorKind, Parser, RGBA}; +use cssparser::{BasicParseErrorKind, Parser, RGBA, Token}; use euclid::Size2D; use euclid::TypedScale; use gecko::values::{convert_nscolor_to_rgba, convert_rgba_to_nscolor}; @@ -255,8 +255,41 @@ pub enum Range { Min, /// At most the specified value. Max, - /// Exactly the specified value. +} + +/// The operator that was specified in this media feature. +#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq)] +enum Operator { Equal, + GreaterThan, + GreaterThanEqual, + LessThan, + LessThanEqual, +} + +impl ToCss for Operator { + fn to_css(&self, dest: &mut CssWriter) -> fmt::Result + where + W: fmt::Write, + { + dest.write_str(match *self { + Operator::Equal => "=", + Operator::LessThan => "<", + Operator::LessThanEqual => "<=", + Operator::GreaterThan => ">", + Operator::GreaterThanEqual => ">=", + }) + } +} + +/// Either a `Range` or an `Operator`. +/// +/// Ranged media features are not allowed with operations (that'd make no +/// sense). +#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq)] +enum RangeOrOperator { + Range(Range), + Operator(Operator), } /// A expression for gecko contains a reference to the media feature, the value @@ -265,7 +298,7 @@ pub enum Range { pub struct Expression { feature: &'static nsMediaFeature, value: Option, - range: Range, + range_or_operator: Option, } impl ToCss for Expression { @@ -279,10 +312,12 @@ impl ToCss for Expression { { dest.write_str("-webkit-")?; } - match self.range { - Range::Min => dest.write_str("min-")?, - Range::Max => dest.write_str("max-")?, - Range::Equal => {}, + + if let Some(RangeOrOperator::Range(range)) = self.range_or_operator { + match range { + Range::Min => dest.write_str("min-")?, + Range::Max => dest.write_str("max-")?, + } } // NB: CssStringWriter not needed, feature names are under control. @@ -290,8 +325,15 @@ impl ToCss for Expression { Atom::from_static(*self.feature.mName) })?; - if let Some(ref val) = self.value { + if let Some(RangeOrOperator::Operator(op)) = self.range_or_operator { + dest.write_char(' ')?; + op.to_css(dest)?; + dest.write_char(' ')?; + } else if self.value.is_some() { dest.write_str(": ")?; + } + + if let Some(ref val) = self.value { val.to_css(dest, self)?; } @@ -302,7 +344,7 @@ impl ToCss for Expression { impl PartialEq for Expression { fn eq(&self, other: &Expression) -> bool { self.feature.mName == other.feature.mName && self.value == other.value && - self.range == other.range + self.range_or_operator == other.range_or_operator } } @@ -537,17 +579,53 @@ fn parse_feature_value<'i, 't>( Ok(value) } +/// Consumes an operation or a colon, or returns an error. +fn consume_operation_or_colon( + input: &mut Parser, +) -> Result, ()> { + let first_delim = { + let next_token = match input.next() { + Ok(t) => t, + Err(..) => return Err(()), + }; + + match *next_token { + Token::Colon => return Ok(None), + Token::Delim(oper) => oper, + _ => return Err(()), + } + }; + Ok(Some(match first_delim { + '=' => Operator::Equal, + '>' => { + if input.try(|i| i.expect_delim('=')).is_ok() { + Operator::GreaterThanEqual + } else { + Operator::GreaterThan + } + } + '<' => { + if input.try(|i| i.expect_delim('=')).is_ok() { + Operator::LessThanEqual + } else { + Operator::LessThan + } + } + _ => return Err(()), + })) +} + impl Expression { /// Trivially construct a new expression. fn new( feature: &'static nsMediaFeature, value: Option, - range: Range, + range_or_operator: Option, ) -> Self { Self { feature, value, - range, + range_or_operator, } } @@ -608,12 +686,12 @@ impl Expression { let range = if starts_with_ignore_ascii_case(feature_name, "min-") { feature_name = &feature_name[4..]; - Range::Min + Some(Range::Min) } else if starts_with_ignore_ascii_case(feature_name, "max-") { feature_name = &feature_name[4..]; - Range::Max + Some(Range::Max) } else { - Range::Equal + None }; let atom = Atom::from(string_as_ascii_lowercase(feature_name)); @@ -641,7 +719,7 @@ impl Expression { )); } - if range != Range::Equal && + if range.is_some() && feature.mRangeType != nsMediaFeature_RangeType::eMinMaxAllowed { return Err(location.new_custom_error( @@ -650,17 +728,56 @@ impl Expression { } } - // If there's no colon, this is a media query of the form - // '()', that is, there's no value specified. - // - // Gecko doesn't allow ranged expressions without a value, so just - // reject them here too. - if input.try(|i| i.expect_colon()).is_err() { - if range != Range::Equal { - return Err(input.new_custom_error(StyleParseErrorKind::RangedExpressionWithNoValue)); + let feature_allows_ranges = + feature.mRangeType == nsMediaFeature_RangeType::eMinMaxAllowed; + + let operator = input.try(consume_operation_or_colon); + let operator = match operator { + Err(..) => { + // If there's no colon, this is a media query of the + // form '()', that is, there's no value + // specified. + // + // Gecko doesn't allow ranged expressions without a + // value, so just reject them here too. + if range.is_some() { + return Err(input.new_custom_error( + StyleParseErrorKind::RangedExpressionWithNoValue + )); + } + + return Ok(Expression::new( + feature, + None, + None, + )); } - return Ok(Expression::new(feature, None, range)); - } + Ok(operator) => operator, + }; + + let range_or_operator = match range { + Some(range) => { + if operator.is_some() { + return Err(input.new_custom_error( + StyleParseErrorKind::MediaQueryExpectedFeatureValue + )); + } + Some(RangeOrOperator::Range(range)) + } + None => { + match operator { + Some(operator) => { + if !feature_allows_ranges { + return Err(input.new_custom_error( + StyleParseErrorKind::MediaQueryExpectedFeatureValue + )); + } + Some(RangeOrOperator::Operator(operator)) + } + None => None, + } + } + }; let value = parse_feature_value(feature, feature.mValueType, context, input).map_err(|err| { @@ -668,7 +785,7 @@ impl Expression { .new_custom_error(StyleParseErrorKind::MediaQueryExpectedFeatureValue) })?; - Ok(Expression::new(feature, Some(value), range)) + Ok(Expression::new(feature, Some(value), range_or_operator)) }) } @@ -704,8 +821,8 @@ impl Expression { use std::cmp::Ordering; debug_assert!( - self.range == Range::Equal || - self.feature.mRangeType == nsMediaFeature_RangeType::eMinMaxAllowed, + self.feature.mRangeType == nsMediaFeature_RangeType::eMinMaxAllowed || + self.range_or_operator.is_none(), "Whoops, wrong range" ); @@ -730,7 +847,7 @@ impl Expression { }; // FIXME(emilio): Handle the possible floating point errors? - let cmp = match (required_value, actual_value) { + let cmp = match (actual_value, required_value) { (&Length(ref one), &Length(ref other)) => { computed::Context::for_media_query_evaluation(device, quirks_mode, |context| { one.to_computed_value(&context) @@ -750,11 +867,11 @@ impl Expression { if (*device.pres_context).mOverrideDPPX > 0.0 { self::Resolution::Dppx((*device.pres_context).mOverrideDPPX).to_dpi() } else { - other.to_dpi() + one.to_dpi() } }; - one.to_dpi().partial_cmp(&actual_dpi).unwrap() + actual_dpi.partial_cmp(&other.to_dpi()).unwrap() }, (&Ident(ref one), &Ident(ref other)) => { debug_assert_ne!( @@ -773,10 +890,31 @@ impl Expression { _ => unreachable!(), }; - cmp == Ordering::Equal || match self.range { - Range::Min => cmp == Ordering::Less, - Range::Equal => false, - Range::Max => cmp == Ordering::Greater, + let range_or_op = match self.range_or_operator { + Some(r) => r, + None => return cmp == Ordering::Equal, + }; + + match range_or_op { + RangeOrOperator::Range(range) => { + cmp == Ordering::Equal || match range { + Range::Min => cmp == Ordering::Greater, + Range::Max => cmp == Ordering::Less, + } + } + RangeOrOperator::Operator(op) => { + match op { + Operator::Equal => cmp == Ordering::Equal, + Operator::GreaterThan => cmp == Ordering::Greater, + Operator::GreaterThanEqual => { + cmp == Ordering::Equal || cmp == Ordering::Greater + } + Operator::LessThan => cmp == Ordering::Less, + Operator::LessThanEqual => { + cmp == Ordering::Equal || cmp == Ordering::Less + } + } + } } } } From e7cc548c35dba9d5ce10c15539e7c483b54a3bff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 15 Jun 2018 21:29:57 -0700 Subject: [PATCH 03/32] style: Rename Expression to MediaFeatureExpression. Which is more appropriate, given it represents a `` per spec, and expression is a bit overloaded :) Bug: 1422225 Reviewed-by: xidorn MozReview-Commit-ID: Fed1nJhHxDu --- components/style/gecko/media_queries.rs | 29 +++++++++---------- components/style/media_queries/media_query.rs | 10 +++---- components/style/media_queries/mod.rs | 4 +-- components/style/servo/media_queries.rs | 14 ++++----- .../values/specified/source_size_list.rs | 6 ++-- 5 files changed, 31 insertions(+), 32 deletions(-) diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index fd07595f458..02fc864eb09 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -292,16 +292,16 @@ enum RangeOrOperator { Operator(Operator), } -/// A expression for gecko contains a reference to the media feature, the value -/// the media query contained, and the range to evaluate. +/// A range expression for gecko contains a reference to the media feature, the +/// value the media query contained, and the range to evaluate. #[derive(Clone, Debug, MallocSizeOf)] -pub struct Expression { +pub struct MediaFeatureExpression { feature: &'static nsMediaFeature, value: Option, range_or_operator: Option, } -impl ToCss for Expression { +impl ToCss for MediaFeatureExpression { fn to_css(&self, dest: &mut CssWriter) -> fmt::Result where W: fmt::Write, @@ -341,8 +341,8 @@ impl ToCss for Expression { } } -impl PartialEq for Expression { - fn eq(&self, other: &Expression) -> bool { +impl PartialEq for MediaFeatureExpression { + fn eq(&self, other: &Self) -> bool { self.feature.mName == other.feature.mName && self.value == other.value && self.range_or_operator == other.range_or_operator } @@ -379,7 +379,10 @@ pub enum MediaExpressionValue { } impl MediaExpressionValue { - fn from_css_value(for_expr: &Expression, css_value: &nsCSSValue) -> Option { + fn from_css_value( + for_expr: &MediaFeatureExpression, + css_value: &nsCSSValue, + ) -> Option { // NB: If there's a null value, that means that we don't support the // feature. if css_value.mUnit == nsCSSUnit::eCSSUnit_Null { @@ -437,7 +440,7 @@ impl MediaExpressionValue { } impl MediaExpressionValue { - fn to_css(&self, dest: &mut CssWriter, for_expr: &Expression) -> fmt::Result + fn to_css(&self, dest: &mut CssWriter, for_expr: &MediaFeatureExpression) -> fmt::Result where W: fmt::Write, { @@ -615,7 +618,7 @@ fn consume_operation_or_colon( })) } -impl Expression { +impl MediaFeatureExpression { /// Trivially construct a new expression. fn new( feature: &'static nsMediaFeature, @@ -746,11 +749,7 @@ impl Expression { )); } - return Ok(Expression::new( - feature, - None, - None, - )); + return Ok(Self::new(feature, None, None)); } Ok(operator) => operator, }; @@ -785,7 +784,7 @@ impl Expression { .new_custom_error(StyleParseErrorKind::MediaQueryExpectedFeatureValue) })?; - Ok(Expression::new(feature, Some(value), range_or_operator)) + Ok(Self::new(feature, Some(value), range_or_operator)) }) } diff --git a/components/style/media_queries/media_query.rs b/components/style/media_queries/media_query.rs index 10fa84bd61a..03e1e2de108 100644 --- a/components/style/media_queries/media_query.rs +++ b/components/style/media_queries/media_query.rs @@ -13,7 +13,7 @@ use selectors::parser::SelectorParseErrorKind; use std::fmt::{self, Write}; use str::string_as_ascii_lowercase; use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; -use super::Expression; +use super::MediaFeatureExpression; use values::CustomIdent; /// @@ -65,8 +65,8 @@ pub struct MediaQuery { pub qualifier: Option, /// The media type for this query, that can be known, unknown, or "all". pub media_type: MediaQueryType, - /// The set of expressions that this media query contains. - pub expressions: Vec, + /// The set of range expressions that this media query contains. + pub expressions: Vec, } impl ToCss for MediaQuery { @@ -143,7 +143,7 @@ impl MediaQuery { } // Without a media type, require at least one expression. - expressions.push(Expression::parse(context, input)?); + expressions.push(MediaFeatureExpression::parse(context, input)?); MediaQueryType::All }, @@ -161,7 +161,7 @@ impl MediaQuery { expressions, }); } - expressions.push(Expression::parse(context, input)?) + expressions.push(MediaFeatureExpression::parse(context, input)?) } } } diff --git a/components/style/media_queries/mod.rs b/components/style/media_queries/mod.rs index 8da14fc67e5..8c0722265a7 100644 --- a/components/style/media_queries/mod.rs +++ b/components/style/media_queries/mod.rs @@ -13,6 +13,6 @@ pub use self::media_list::MediaList; pub use self::media_query::{MediaQuery, MediaQueryType, MediaType, Qualifier}; #[cfg(feature = "servo")] -pub use servo::media_queries::{Device, Expression}; +pub use servo::media_queries::{Device, MediaFeatureExpression}; #[cfg(feature = "gecko")] -pub use gecko::media_queries::{Device, Expression}; +pub use gecko::media_queries::{Device, MediaFeatureExpression}; diff --git a/components/style/servo/media_queries.rs b/components/style/servo/media_queries.rs index 336971f98df..14dec24b261 100644 --- a/components/style/servo/media_queries.rs +++ b/components/style/servo/media_queries.rs @@ -170,9 +170,9 @@ pub enum ExpressionKind { /// #[derive(Clone, Debug, PartialEq)] #[cfg_attr(feature = "servo", derive(MallocSizeOf))] -pub struct Expression(pub ExpressionKind); +pub struct MediaFeatureExpression(pub ExpressionKind); -impl Expression { +impl MediaFeatureExpression { /// The kind of expression we're, just for unit testing. /// /// Eventually this will become servo-only. @@ -196,7 +196,7 @@ impl Expression { let name = input.expect_ident_cloned()?; input.expect_colon()?; // TODO: Handle other media features - Ok(Expression(match_ignore_ascii_case! { &name, + Ok(MediaFeatureExpression(match_ignore_ascii_case! { &name, "min-width" => { ExpressionKind::Width(Range::Min(specified::Length::parse_non_negative(context, input)?)) }, @@ -206,7 +206,7 @@ impl Expression { "width" => { ExpressionKind::Width(Range::Eq(specified::Length::parse_non_negative(context, input)?)) }, - _ => return Err(input.new_custom_error(SelectorParseErrorKind::UnexpectedIdent(name.clone()))) + _ => return Err(input.new_custom_error(SelectorParseErrorKind::UnexpectedIdent(name))) })) }) } @@ -228,7 +228,7 @@ impl Expression { } } -impl ToCss for Expression { +impl ToCss for MediaFeatureExpression { fn to_css(&self, dest: &mut CssWriter) -> fmt::Result where W: Write, @@ -246,8 +246,8 @@ impl ToCss for Expression { /// An enumeration that represents a ranged value. /// -/// Only public for testing, implementation details of `Expression` may change -/// for Stylo. +/// Only public for testing, implementation details of `MediaFeatureExpression` +/// may change for Stylo. #[derive(Clone, Copy, Debug, Eq, PartialEq)] #[cfg_attr(feature = "servo", derive(MallocSizeOf))] pub enum Range { diff --git a/components/style/values/specified/source_size_list.rs b/components/style/values/specified/source_size_list.rs index 596c2f73951..f0a0a47e4a9 100644 --- a/components/style/values/specified/source_size_list.rs +++ b/components/style/values/specified/source_size_list.rs @@ -8,7 +8,7 @@ use app_units::Au; use cssparser::{Delimiter, Parser, Token}; #[cfg(feature = "gecko")] use gecko_bindings::sugar::ownership::{HasBoxFFI, HasFFI, HasSimpleFFI}; -use media_queries::{Device, Expression as MediaExpression}; +use media_queries::{Device, MediaFeatureExpression}; use parser::{Parse, ParserContext}; use selectors::context::QuirksMode; use style_traits::ParseError; @@ -21,7 +21,7 @@ use values::specified::{Length, NoCalcLength, ViewportPercentageLength}; pub struct SourceSize { // FIXME(emilio): This should be a `MediaCondition`, and support `and` and // `or`. - condition: MediaExpression, + condition: MediaFeatureExpression, value: Length, } @@ -30,7 +30,7 @@ impl Parse for SourceSize { context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - let condition = MediaExpression::parse(context, input)?; + let condition = MediaFeatureExpression::parse(context, input)?; let value = Length::parse_non_negative(context, input)?; Ok(Self { condition, value }) From 0b49a3701a7717ae9b4662e38bdbe35328394705 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 15 Jun 2018 23:51:07 -0700 Subject: [PATCH 04/32] style: Add code to parse media conditions. Still unused. Bug: 1422225 Reviewed-by: xidorn MozReview-Commit-ID: IQfxObw9BV5 --- components/style/gecko/media_queries.rs | 251 +++++++++--------- .../style/media_queries/media_condition.rs | 109 ++++++++ components/style/media_queries/mod.rs | 2 + components/style/servo/media_queries.rs | 43 +-- 4 files changed, 267 insertions(+), 138 deletions(-) create mode 100644 components/style/media_queries/media_condition.rs diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index 02fc864eb09..a6322842f20 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -651,141 +651,150 @@ impl MediaFeatureExpression { })?; input.parse_nested_block(|input| { - // FIXME: remove extra indented block when lifetimes are non-lexical - let feature; - let range; + Self::parse_in_parenthesis_block(context, input) + }) + } + + /// Parse a media range expression where we've already consumed the + /// parenthesis. + pub fn parse_in_parenthesis_block<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + // FIXME: remove extra indented block when lifetimes are non-lexical + let feature; + let range; + { + let location = input.current_source_location(); + let ident = input.expect_ident().map_err(|err| { + err.location.new_custom_error(match err.kind { + BasicParseErrorKind::UnexpectedToken(t) => { + StyleParseErrorKind::ExpectedIdentifier(t) + }, + _ => StyleParseErrorKind::UnspecifiedError, + }) + })?; + + let mut flags = 0; + + if context.chrome_rules_enabled() || context.stylesheet_origin == Origin::UserAgent { - let location = input.current_source_location(); - let ident = input.expect_ident().map_err(|err| { - err.location.new_custom_error(match err.kind { - BasicParseErrorKind::UnexpectedToken(t) => { - StyleParseErrorKind::ExpectedIdentifier(t) - }, - _ => StyleParseErrorKind::UnspecifiedError, - }) - })?; - - let mut flags = 0; - - if context.chrome_rules_enabled() || context.stylesheet_origin == Origin::UserAgent - { - flags |= structs::nsMediaFeature_RequirementFlags_eUserAgentAndChromeOnly; - } - - let result = { - let mut feature_name = &**ident; - - if unsafe { structs::StaticPrefs_sVarCache_layout_css_prefixes_webkit } && - starts_with_ignore_ascii_case(feature_name, "-webkit-") - { - feature_name = &feature_name[8..]; - flags |= structs::nsMediaFeature_RequirementFlags_eHasWebkitPrefix; - if unsafe { - structs::StaticPrefs_sVarCache_layout_css_prefixes_device_pixel_ratio_webkit - } { - flags |= structs::nsMediaFeature_RequirementFlags_eWebkitDevicePixelRatioPrefEnabled; - } - } - - let range = if starts_with_ignore_ascii_case(feature_name, "min-") { - feature_name = &feature_name[4..]; - Some(Range::Min) - } else if starts_with_ignore_ascii_case(feature_name, "max-") { - feature_name = &feature_name[4..]; - Some(Range::Max) - } else { - None - }; - - let atom = Atom::from(string_as_ascii_lowercase(feature_name)); - match find_feature(|f| atom.as_ptr() == unsafe { *f.mName as *mut _ }) { - Some(f) => Ok((f, range)), - None => Err(()), - } - }; - - match result { - Ok((f, r)) => { - feature = f; - range = r; - }, - Err(()) => { - return Err(location.new_custom_error( - StyleParseErrorKind::MediaQueryExpectedFeatureName(ident.clone()), - )) - }, - } - - if (feature.mReqFlags & !flags) != 0 { - return Err(location.new_custom_error( - StyleParseErrorKind::MediaQueryExpectedFeatureName(ident.clone()), - )); - } - - if range.is_some() && - feature.mRangeType != nsMediaFeature_RangeType::eMinMaxAllowed - { - return Err(location.new_custom_error( - StyleParseErrorKind::MediaQueryExpectedFeatureName(ident.clone()), - )); - } + flags |= structs::nsMediaFeature_RequirementFlags_eUserAgentAndChromeOnly; } - let feature_allows_ranges = - feature.mRangeType == nsMediaFeature_RangeType::eMinMaxAllowed; + let result = { + let mut feature_name = &**ident; - let operator = input.try(consume_operation_or_colon); - let operator = match operator { - Err(..) => { - // If there's no colon, this is a media query of the - // form '()', that is, there's no value - // specified. - // - // Gecko doesn't allow ranged expressions without a - // value, so just reject them here too. - if range.is_some() { - return Err(input.new_custom_error( - StyleParseErrorKind::RangedExpressionWithNoValue - )); + if unsafe { structs::StaticPrefs_sVarCache_layout_css_prefixes_webkit } && + starts_with_ignore_ascii_case(feature_name, "-webkit-") + { + feature_name = &feature_name[8..]; + flags |= structs::nsMediaFeature_RequirementFlags_eHasWebkitPrefix; + if unsafe { + structs::StaticPrefs_sVarCache_layout_css_prefixes_device_pixel_ratio_webkit + } { + flags |= structs::nsMediaFeature_RequirementFlags_eWebkitDevicePixelRatioPrefEnabled; } - - return Ok(Self::new(feature, None, None)); } - Ok(operator) => operator, + + let range = if starts_with_ignore_ascii_case(feature_name, "min-") { + feature_name = &feature_name[4..]; + Some(Range::Min) + } else if starts_with_ignore_ascii_case(feature_name, "max-") { + feature_name = &feature_name[4..]; + Some(Range::Max) + } else { + None + }; + + let atom = Atom::from(string_as_ascii_lowercase(feature_name)); + match find_feature(|f| atom.as_ptr() == unsafe { *f.mName as *mut _ }) { + Some(f) => Ok((f, range)), + None => Err(()), + } }; - let range_or_operator = match range { - Some(range) => { - if operator.is_some() { - return Err(input.new_custom_error( - StyleParseErrorKind::MediaQueryExpectedFeatureValue - )); - } - Some(RangeOrOperator::Range(range)) + match result { + Ok((f, r)) => { + feature = f; + range = r; + }, + Err(()) => { + return Err(location.new_custom_error( + StyleParseErrorKind::MediaQueryExpectedFeatureName(ident.clone()), + )) + }, + } + + if (feature.mReqFlags & !flags) != 0 { + return Err(location.new_custom_error( + StyleParseErrorKind::MediaQueryExpectedFeatureName(ident.clone()), + )); + } + + if range.is_some() && + feature.mRangeType != nsMediaFeature_RangeType::eMinMaxAllowed + { + return Err(location.new_custom_error( + StyleParseErrorKind::MediaQueryExpectedFeatureName(ident.clone()), + )); + } + } + + let feature_allows_ranges = + feature.mRangeType == nsMediaFeature_RangeType::eMinMaxAllowed; + + let operator = input.try(consume_operation_or_colon); + let operator = match operator { + Err(..) => { + // If there's no colon, this is a media query of the + // form '()', that is, there's no value + // specified. + // + // Gecko doesn't allow ranged expressions without a + // value, so just reject them here too. + if range.is_some() { + return Err(input.new_custom_error( + StyleParseErrorKind::RangedExpressionWithNoValue + )); } - None => { - match operator { - Some(operator) => { - if !feature_allows_ranges { - return Err(input.new_custom_error( - StyleParseErrorKind::MediaQueryExpectedFeatureValue - )); - } - Some(RangeOrOperator::Operator(operator)) + + return Ok(Self::new(feature, None, None)); + } + Ok(operator) => operator, + }; + + let range_or_operator = match range { + Some(range) => { + if operator.is_some() { + return Err(input.new_custom_error( + StyleParseErrorKind::MediaQueryExpectedFeatureValue + )); + } + Some(RangeOrOperator::Range(range)) + } + None => { + match operator { + Some(operator) => { + if !feature_allows_ranges { + return Err(input.new_custom_error( + StyleParseErrorKind::MediaQueryExpectedFeatureValue + )); } - None => None, + Some(RangeOrOperator::Operator(operator)) } + None => None, } - }; + } + }; - let value = - parse_feature_value(feature, feature.mValueType, context, input).map_err(|err| { - err.location - .new_custom_error(StyleParseErrorKind::MediaQueryExpectedFeatureValue) - })?; + let value = + parse_feature_value(feature, feature.mValueType, context, input).map_err(|err| { + err.location + .new_custom_error(StyleParseErrorKind::MediaQueryExpectedFeatureValue) + })?; - Ok(Self::new(feature, Some(value), range_or_operator)) - }) + Ok(Self::new(feature, Some(value), range_or_operator)) } /// Returns whether this media query evaluates to true for the given device. diff --git a/components/style/media_queries/media_condition.rs b/components/style/media_queries/media_condition.rs new file mode 100644 index 00000000000..cb3bd73ee20 --- /dev/null +++ b/components/style/media_queries/media_condition.rs @@ -0,0 +1,109 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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/. */ + +//! A media query condition: +//! +//! https://drafts.csswg.org/mediaqueries-4/#typedef-media-condition + +use cssparser::{Parser, Token}; +use parser::ParserContext; +use style_traits::ParseError; + +use super::MediaFeatureExpression; + + +/// A binary `and` or `or` operator. +#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, Parse, ToCss)] +#[allow(missing_docs)] +pub enum Operator { + And, + Or, +} + +/// Represents a media condition. +pub enum MediaCondition { + /// A simple media feature expression, implicitly parenthesized. + Feature(MediaFeatureExpression), + /// A negation of a condition. + Not(Box), + /// A set of joint operations. + Operation(Box<[MediaCondition]>, Operator), + /// A condition wrapped in parenthesis. + InParens(Box), +} + +impl MediaCondition { + /// Parse a single media condition. + pub fn parse<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + let location = input.current_source_location(); + + // FIXME(emilio): This can be cleaner with nll. + let is_negation = match *input.next()? { + Token::ParenthesisBlock => false, + Token::Ident(ref ident) if ident.eq_ignore_ascii_case("not") => true, + ref t => { + return Err(location.new_unexpected_token_error(t.clone())) + } + }; + + if is_negation { + let inner_condition = Self::parse_in_parens(context, input)?; + return Ok(MediaCondition::Not(Box::new(inner_condition))) + } + + // ParenthesisBlock. + let first_condition = Self::parse_paren_block(context, input)?; + let operator = match input.try(Operator::parse) { + Ok(op) => op, + Err(..) => return Ok(first_condition), + }; + + let mut conditions = vec![]; + conditions.push(first_condition); + conditions.push(Self::parse_in_parens(context, input)?); + + let delim = match operator { + Operator::And => "and", + Operator::Or => "or", + }; + + loop { + if input.try(|i| i.expect_ident_matching(delim)).is_err() { + return Ok(MediaCondition::Operation( + conditions.into_boxed_slice(), + operator, + )); + } + + conditions.push(Self::parse_in_parens(context, input)?); + } + } + + /// Parse a media condition in parentheses. + pub fn parse_in_parens<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + input.expect_parenthesis_block()?; + Self::parse_paren_block(context, input) + } + + fn parse_paren_block<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + input.parse_nested_block(|input| { + // Base case. + if let Ok(expr) = input.try(|i| MediaFeatureExpression::parse_in_parenthesis_block(context, i)) { + return Ok(MediaCondition::Feature(expr)); + } + + let inner = Self::parse(context, input)?; + Ok(MediaCondition::InParens(Box::new(inner))) + }) + } +} diff --git a/components/style/media_queries/mod.rs b/components/style/media_queries/mod.rs index 8c0722265a7..d27e33cc64c 100644 --- a/components/style/media_queries/mod.rs +++ b/components/style/media_queries/mod.rs @@ -6,9 +6,11 @@ //! //! [mq]: https://drafts.csswg.org/mediaqueries/ +mod media_condition; mod media_list; mod media_query; +pub use self::media_condition::MediaCondition; pub use self::media_list::MediaList; pub use self::media_query::{MediaQuery, MediaQueryType, MediaType, Qualifier}; diff --git a/components/style/servo/media_queries.rs b/components/style/servo/media_queries.rs index 14dec24b261..3a8235aa65c 100644 --- a/components/style/servo/media_queries.rs +++ b/components/style/servo/media_queries.rs @@ -183,34 +183,43 @@ impl MediaFeatureExpression { /// Parse a media expression of the form: /// /// ``` - /// (media-feature: media-value) + /// media-feature: media-value /// ``` /// - /// Only supports width and width ranges for now. + /// Only supports width ranges for now. pub fn parse<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { input.expect_parenthesis_block()?; input.parse_nested_block(|input| { - let name = input.expect_ident_cloned()?; - input.expect_colon()?; - // TODO: Handle other media features - Ok(MediaFeatureExpression(match_ignore_ascii_case! { &name, - "min-width" => { - ExpressionKind::Width(Range::Min(specified::Length::parse_non_negative(context, input)?)) - }, - "max-width" => { - ExpressionKind::Width(Range::Max(specified::Length::parse_non_negative(context, input)?)) - }, - "width" => { - ExpressionKind::Width(Range::Eq(specified::Length::parse_non_negative(context, input)?)) - }, - _ => return Err(input.new_custom_error(SelectorParseErrorKind::UnexpectedIdent(name))) - })) + Self::parse_in_parenthesis_block(context, input) }) } + /// Parse a media range expression where we've already consumed the + /// parenthesis. + pub fn parse_in_parenthesis_block<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + let name = input.expect_ident_cloned()?; + input.expect_colon()?; + // TODO: Handle other media features + Ok(MediaFeatureExpression(match_ignore_ascii_case! { &name, + "min-width" => { + ExpressionKind::Width(Range::Min(specified::Length::parse_non_negative(context, input)?)) + }, + "max-width" => { + ExpressionKind::Width(Range::Max(specified::Length::parse_non_negative(context, input)?)) + }, + "width" => { + ExpressionKind::Width(Range::Eq(specified::Length::parse_non_negative(context, input)?)) + }, + _ => return Err(input.new_custom_error(SelectorParseErrorKind::UnexpectedIdent(name))) + })) + } + /// Evaluate this expression and return whether it matches the current /// device. pub fn matches(&self, device: &Device, quirks_mode: QuirksMode) -> bool { From d2a1895752f111ad15a04999263cf0f635d569be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 15 Jun 2018 23:58:20 -0700 Subject: [PATCH 05/32] style: Add serialization code for MediaCondition. Bug: 1422225 Reviewed-by: xidorn MozReview-Commit-ID: AxQQottV1hG --- .../style/media_queries/media_condition.rs | 36 ++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/components/style/media_queries/media_condition.rs b/components/style/media_queries/media_condition.rs index cb3bd73ee20..091d7049599 100644 --- a/components/style/media_queries/media_condition.rs +++ b/components/style/media_queries/media_condition.rs @@ -8,7 +8,8 @@ use cssparser::{Parser, Token}; use parser::ParserContext; -use style_traits::ParseError; +use std::fmt::{self, Write}; +use style_traits::{CssWriter, ParseError, ToCss}; use super::MediaFeatureExpression; @@ -33,6 +34,39 @@ pub enum MediaCondition { InParens(Box), } +impl ToCss for MediaCondition { + fn to_css(&self, dest: &mut CssWriter) -> fmt::Result + where + W: fmt::Write, + { + match *self { + // NOTE(emilio): MediaFeatureExpression already includes the + // parenthesis. + MediaCondition::Feature(ref f) => f.to_css(dest), + MediaCondition::Not(ref c) => { + dest.write_str("not ")?; + c.to_css(dest) + } + MediaCondition::InParens(ref c) => { + dest.write_char('(')?; + c.to_css(dest)?; + dest.write_char(')') + } + MediaCondition::Operation(ref list, op) => { + let mut iter = list.iter(); + iter.next().unwrap().to_css(dest)?; + for item in iter { + dest.write_char(' ')?; + op.to_css(dest)?; + dest.write_char(' ')?; + item.to_css(dest)?; + } + Ok(()) + } + } + } +} + impl MediaCondition { /// Parse a single media condition. pub fn parse<'i, 't>( From 3a92fd1cfc1fa87b36d1c1a594ce5394245e06e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 16 Jun 2018 00:59:04 -0700 Subject: [PATCH 06/32] style: Evaluate MediaConditions, and glue it all together. Bug: 1422225 Reviewed-by: xidorn MozReview-Commit-ID: 3MThE2FvfDf --- .../style/media_queries/media_condition.rs | 55 +++++++++++- components/style/media_queries/media_list.rs | 8 +- components/style/media_queries/media_query.rs | 83 +++++++++---------- .../values/specified/source_size_list.rs | 8 +- 4 files changed, 96 insertions(+), 58 deletions(-) diff --git a/components/style/media_queries/media_condition.rs b/components/style/media_queries/media_condition.rs index 091d7049599..c2c29c129ca 100644 --- a/components/style/media_queries/media_condition.rs +++ b/components/style/media_queries/media_condition.rs @@ -7,11 +7,12 @@ //! https://drafts.csswg.org/mediaqueries-4/#typedef-media-condition use cssparser::{Parser, Token}; +use context::QuirksMode; use parser::ParserContext; use std::fmt::{self, Write}; -use style_traits::{CssWriter, ParseError, ToCss}; +use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; -use super::MediaFeatureExpression; +use super::{Device, MediaFeatureExpression}; /// A binary `and` or `or` operator. @@ -22,7 +23,15 @@ pub enum Operator { Or, } +/// Whether to allow an `or` condition or not during parsing. +#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, Parse, ToCss)] +enum AllowOr { + Yes, + No, +} + /// Represents a media condition. +#[derive(Clone, Debug, MallocSizeOf, PartialEq)] pub enum MediaCondition { /// A simple media feature expression, implicitly parenthesized. Feature(MediaFeatureExpression), @@ -72,6 +81,24 @@ impl MediaCondition { pub fn parse<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, + ) -> Result> { + Self::parse_internal(context, input, AllowOr::Yes) + } + + /// Parse a single media condition, disallowing `or` expressions. + /// + /// To be used from the legacy media query syntax. + pub fn parse_disallow_or<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + Self::parse_internal(context, input, AllowOr::No) + } + + fn parse_internal<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + allow_or: AllowOr, ) -> Result> { let location = input.current_source_location(); @@ -96,6 +123,10 @@ impl MediaCondition { Err(..) => return Ok(first_condition), }; + if allow_or == AllowOr::No && operator == Operator::Or { + return Err(location.new_custom_error(StyleParseErrorKind::UnspecifiedError)); + } + let mut conditions = vec![]; conditions.push(first_condition); conditions.push(Self::parse_in_parens(context, input)?); @@ -140,4 +171,24 @@ impl MediaCondition { Ok(MediaCondition::InParens(Box::new(inner))) }) } + + /// Whether this condition matches the device and quirks mode. + pub fn matches(&self, device: &Device, quirks_mode: QuirksMode) -> bool { + match *self { + MediaCondition::Feature(ref f) => f.matches(device, quirks_mode), + MediaCondition::InParens(ref c) => c.matches(device, quirks_mode), + MediaCondition::Not(ref c) => !c.matches(device, quirks_mode), + MediaCondition::Operation(ref conditions, op) => { + let mut iter = conditions.iter(); + match op { + Operator::And => { + iter.all(|c| c.matches(device, quirks_mode)) + } + Operator::Or => { + iter.any(|c| c.matches(device, quirks_mode)) + } + } + } + } + } } diff --git a/components/style/media_queries/media_list.rs b/components/style/media_queries/media_list.rs index a5604b6dc19..f8d15df7257 100644 --- a/components/style/media_queries/media_list.rs +++ b/components/style/media_queries/media_list.rs @@ -73,16 +73,14 @@ impl MediaList { /// Evaluate a whole `MediaList` against `Device`. pub fn evaluate(&self, device: &Device, quirks_mode: QuirksMode) -> bool { - // Check if it is an empty media query list or any queries match (OR condition) + // Check if it is an empty media query list or any queries match. // https://drafts.csswg.org/mediaqueries-4/#mq-list self.media_queries.is_empty() || self.media_queries.iter().any(|mq| { let media_match = mq.media_type.matches(device.media_type()); - // Check if all conditions match (AND condition) + // Check if the media condition match. let query_match = media_match && - mq.expressions - .iter() - .all(|expression| expression.matches(&device, quirks_mode)); + mq.condition.as_ref().map_or(true, |c| c.matches(device, quirks_mode)); // Apply the logical NOT qualifier to the result match mq.qualifier { diff --git a/components/style/media_queries/media_query.rs b/components/style/media_queries/media_query.rs index 03e1e2de108..a2f4f9bea70 100644 --- a/components/style/media_queries/media_query.rs +++ b/components/style/media_queries/media_query.rs @@ -12,10 +12,11 @@ use parser::ParserContext; use selectors::parser::SelectorParseErrorKind; use std::fmt::{self, Write}; use str::string_as_ascii_lowercase; -use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; -use super::MediaFeatureExpression; +use style_traits::{CssWriter, ParseError, ToCss}; +use super::media_condition::MediaCondition; use values::CustomIdent; + /// #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, Parse, PartialEq, ToCss)] pub enum Qualifier { @@ -65,8 +66,9 @@ pub struct MediaQuery { pub qualifier: Option, /// The media type for this query, that can be known, unknown, or "all". pub media_type: MediaQueryType, - /// The set of range expressions that this media query contains. - pub expressions: Vec, + /// The condition that this media query contains. This cannot have `or` + /// in the first level. + pub condition: Option, } impl ToCss for MediaQuery { @@ -86,28 +88,23 @@ impl ToCss for MediaQuery { // // Otherwise, we'd serialize media queries like "(min-width: // 40px)" in "all (min-width: 40px)", which is unexpected. - if self.qualifier.is_some() || self.expressions.is_empty() { + if self.qualifier.is_some() || self.condition.is_none() { dest.write_str("all")?; } }, MediaQueryType::Concrete(MediaType(ref desc)) => desc.to_css(dest)?, } - if self.expressions.is_empty() { - return Ok(()); - } + let condition = match self.condition { + Some(ref c) => c, + None => return Ok(()), + }; if self.media_type != MediaQueryType::All || self.qualifier.is_some() { dest.write_str(" and ")?; } - self.expressions[0].to_css(dest)?; - - for expr in self.expressions.iter().skip(1) { - dest.write_str(" and ")?; - expr.to_css(dest)?; - } - Ok(()) + condition.to_css(dest) } } @@ -118,7 +115,7 @@ impl MediaQuery { Self { qualifier: Some(Qualifier::Not), media_type: MediaQueryType::All, - expressions: vec![], + condition: None, } } @@ -129,40 +126,34 @@ impl MediaQuery { context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - let mut expressions = vec![]; + if let Ok(condition) = input.try(|i| MediaCondition::parse(context, i)) { + return Ok(Self { + qualifier: None, + media_type: MediaQueryType::All, + condition: Some(condition), + }) + } let qualifier = input.try(Qualifier::parse).ok(); - let media_type = match input.try(|i| i.expect_ident_cloned()) { - Ok(ident) => MediaQueryType::parse(&*ident).map_err(|()| { - input.new_custom_error(SelectorParseErrorKind::UnexpectedIdent(ident.clone())) - })?, - Err(_) => { - // Media type is only optional if qualifier is not specified. - if qualifier.is_some() { - return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); - } - - // Without a media type, require at least one expression. - expressions.push(MediaFeatureExpression::parse(context, input)?); - - MediaQueryType::All - }, + let media_type = { + let location = input.current_source_location(); + let ident = input.expect_ident()?; + match MediaQueryType::parse(&ident) { + Ok(t) => t, + Err(..) => return Err(location.new_custom_error( + SelectorParseErrorKind::UnexpectedIdent(ident.clone()) + )) + } }; - // Parse any subsequent expressions - loop { - if input - .try(|input| input.expect_ident_matching("and")) - .is_err() - { - return Ok(MediaQuery { - qualifier, - media_type, - expressions, - }); - } - expressions.push(MediaFeatureExpression::parse(context, input)?) - } + let condition = + if input.try(|i| i.expect_ident_matching("and")).is_ok() { + Some(MediaCondition::parse_disallow_or(context, input)?) + } else { + None + }; + + Ok(Self { qualifier, media_type, condition }) } } diff --git a/components/style/values/specified/source_size_list.rs b/components/style/values/specified/source_size_list.rs index f0a0a47e4a9..d41fc283bcc 100644 --- a/components/style/values/specified/source_size_list.rs +++ b/components/style/values/specified/source_size_list.rs @@ -8,7 +8,7 @@ use app_units::Au; use cssparser::{Delimiter, Parser, Token}; #[cfg(feature = "gecko")] use gecko_bindings::sugar::ownership::{HasBoxFFI, HasFFI, HasSimpleFFI}; -use media_queries::{Device, MediaFeatureExpression}; +use media_queries::{Device, MediaCondition}; use parser::{Parse, ParserContext}; use selectors::context::QuirksMode; use style_traits::ParseError; @@ -19,9 +19,7 @@ use values::specified::{Length, NoCalcLength, ViewportPercentageLength}; /// /// https://html.spec.whatwg.org/multipage/#source-size pub struct SourceSize { - // FIXME(emilio): This should be a `MediaCondition`, and support `and` and - // `or`. - condition: MediaFeatureExpression, + condition: MediaCondition, value: Length, } @@ -30,7 +28,7 @@ impl Parse for SourceSize { context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - let condition = MediaFeatureExpression::parse(context, input)?; + let condition = MediaCondition::parse(context, input)?; let value = Length::parse_non_negative(context, input)?; Ok(Self { condition, value }) From 2d2e84aad59329f6d46b1734c8eefe0a450b4a96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 21 Jun 2018 19:35:35 +0200 Subject: [PATCH 07/32] style: Error reporting fixes for media queries. Do it so that we always try to evaluate the media expression and the modern syntax last, so that the most specific error message comes up. Bug: 1422225 Reviewed-by: xidorn MozReview-Commit-ID: 2tqdAsWh6Kh --- components/style/gecko/media_queries.rs | 31 ++++---------- .../style/media_queries/media_condition.rs | 9 ++--- components/style/media_queries/media_query.rs | 40 +++++++------------ components/style_traits/lib.rs | 4 +- 4 files changed, 28 insertions(+), 56 deletions(-) diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index a6322842f20..9c5ef4d71a1 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -7,7 +7,7 @@ use app_units::AU_PER_PX; use app_units::Au; use context::QuirksMode; -use cssparser::{BasicParseErrorKind, Parser, RGBA, Token}; +use cssparser::{Parser, RGBA, Token}; use euclid::Size2D; use euclid::TypedScale; use gecko::values::{convert_nscolor_to_rgba, convert_rgba_to_nscolor}; @@ -292,8 +292,8 @@ enum RangeOrOperator { Operator(Operator), } -/// A range expression for gecko contains a reference to the media feature, the -/// value the media query contained, and the range to evaluate. +/// A feature expression for gecko contains a reference to the media feature, +/// the value the media query contained, and the range to evaluate. #[derive(Clone, Debug, MallocSizeOf)] pub struct MediaFeatureExpression { feature: &'static nsMediaFeature, @@ -641,21 +641,13 @@ impl MediaFeatureExpression { context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - input.expect_parenthesis_block().map_err(|err| { - err.location.new_custom_error(match err.kind { - BasicParseErrorKind::UnexpectedToken(t) => { - StyleParseErrorKind::ExpectedIdentifier(t) - }, - _ => StyleParseErrorKind::UnspecifiedError, - }) - })?; - + input.expect_parenthesis_block()?; input.parse_nested_block(|input| { Self::parse_in_parenthesis_block(context, input) }) } - /// Parse a media range expression where we've already consumed the + /// Parse a media feature expression where we've already consumed the /// parenthesis. pub fn parse_in_parenthesis_block<'i, 't>( context: &ParserContext, @@ -666,14 +658,7 @@ impl MediaFeatureExpression { let range; { let location = input.current_source_location(); - let ident = input.expect_ident().map_err(|err| { - err.location.new_custom_error(match err.kind { - BasicParseErrorKind::UnexpectedToken(t) => { - StyleParseErrorKind::ExpectedIdentifier(t) - }, - _ => StyleParseErrorKind::UnspecifiedError, - }) - })?; + let ident = input.expect_ident()?; let mut flags = 0; @@ -768,7 +753,7 @@ impl MediaFeatureExpression { Some(range) => { if operator.is_some() { return Err(input.new_custom_error( - StyleParseErrorKind::MediaQueryExpectedFeatureValue + StyleParseErrorKind::MediaQueryUnexpectedOperator )); } Some(RangeOrOperator::Range(range)) @@ -778,7 +763,7 @@ impl MediaFeatureExpression { Some(operator) => { if !feature_allows_ranges { return Err(input.new_custom_error( - StyleParseErrorKind::MediaQueryExpectedFeatureValue + StyleParseErrorKind::MediaQueryUnexpectedOperator )); } Some(RangeOrOperator::Operator(operator)) diff --git a/components/style/media_queries/media_condition.rs b/components/style/media_queries/media_condition.rs index c2c29c129ca..e56f02ff071 100644 --- a/components/style/media_queries/media_condition.rs +++ b/components/style/media_queries/media_condition.rs @@ -163,12 +163,11 @@ impl MediaCondition { ) -> Result> { input.parse_nested_block(|input| { // Base case. - if let Ok(expr) = input.try(|i| MediaFeatureExpression::parse_in_parenthesis_block(context, i)) { - return Ok(MediaCondition::Feature(expr)); + if let Ok(inner) = input.try(|i| Self::parse(context, i)) { + return Ok(MediaCondition::InParens(Box::new(inner))) } - - let inner = Self::parse(context, input)?; - Ok(MediaCondition::InParens(Box::new(inner))) + let expr = MediaFeatureExpression::parse_in_parenthesis_block(context, input)?; + Ok(MediaCondition::Feature(expr)) }) } diff --git a/components/style/media_queries/media_query.rs b/components/style/media_queries/media_query.rs index a2f4f9bea70..089fc9412b2 100644 --- a/components/style/media_queries/media_query.rs +++ b/components/style/media_queries/media_query.rs @@ -9,7 +9,6 @@ use Atom; use cssparser::Parser; use parser::ParserContext; -use selectors::parser::SelectorParseErrorKind; use std::fmt::{self, Write}; use str::string_as_ascii_lowercase; use style_traits::{CssWriter, ParseError, ToCss}; @@ -125,34 +124,23 @@ impl MediaQuery { pub fn parse<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, - ) -> Result> { - if let Ok(condition) = input.try(|i| MediaCondition::parse(context, i)) { - return Ok(Self { - qualifier: None, - media_type: MediaQueryType::All, - condition: Some(condition), - }) - } + ) -> Result> { + let (qualifier, explicit_media_type) = input.try(|input| -> Result<_, ()> { + let qualifier = input.try(Qualifier::parse).ok(); + let ident = input.expect_ident().map_err(|_| ())?; + let media_type = MediaQueryType::parse(&ident)?; + Ok((qualifier, Some(media_type))) + }).unwrap_or_default(); - let qualifier = input.try(Qualifier::parse).ok(); - let media_type = { - let location = input.current_source_location(); - let ident = input.expect_ident()?; - match MediaQueryType::parse(&ident) { - Ok(t) => t, - Err(..) => return Err(location.new_custom_error( - SelectorParseErrorKind::UnexpectedIdent(ident.clone()) - )) - } + let condition = if explicit_media_type.is_none() { + Some(MediaCondition::parse(context, input)?) + } else if input.try(|i| i.expect_ident_matching("and")).is_ok() { + Some(MediaCondition::parse_disallow_or(context, input)?) + } else { + None }; - let condition = - if input.try(|i| i.expect_ident_matching("and")).is_ok() { - Some(MediaCondition::parse_disallow_or(context, input)?) - } else { - None - }; - + let media_type = explicit_media_type.unwrap_or(MediaQueryType::All); Ok(Self { qualifier, media_type, condition }) } } diff --git a/components/style_traits/lib.rs b/components/style_traits/lib.rs index b1eaf4e086f..737450b5e05 100644 --- a/components/style_traits/lib.rs +++ b/components/style_traits/lib.rs @@ -106,12 +106,12 @@ pub enum StyleParseErrorKind<'i> { PropertyDeclarationValueNotExhausted, /// An unexpected dimension token was encountered. UnexpectedDimension(CowRcStr<'i>), - /// Expected identifier not found. - ExpectedIdentifier(Token<'i>), /// Missing or invalid media feature name. MediaQueryExpectedFeatureName(CowRcStr<'i>), /// Missing or invalid media feature value. MediaQueryExpectedFeatureValue, + /// A media feature range operator was not expected. + MediaQueryUnexpectedOperator, /// min- or max- properties must have a value. RangedExpressionWithNoValue, /// A function was encountered that was not expected. From fa7d9bf74a0d435211e08da6e56899c48063a58c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 24 Jun 2018 04:06:49 +0200 Subject: [PATCH 08/32] style: Add namespace bucket for the selector map. After bug 1470163 we have some nasty selectors from mathml.css in every page. We only want to match them against MathML elements. This patch brings the global revalidation selectors from 14 to 2 in about:blank. Also halves the ones from XUL documents. Bug: 1374017 Reviewed-by: heycam MozReview-Commit-ID: nOVyknNcVm --- components/style/selector_map.rs | 51 ++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs index 3b9b6f20184..3b0dcca9525 100644 --- a/components/style/selector_map.rs +++ b/components/style/selector_map.rs @@ -5,7 +5,7 @@ //! A data structure to efficiently index structs containing selectors by local //! name, ids and hash. -use {Atom, LocalName, WeakAtom}; +use {Atom, LocalName, Namespace, WeakAtom}; use applicable_declarations::ApplicableDeclarationList; use context::QuirksMode; use dom::TElement; @@ -102,6 +102,8 @@ pub struct SelectorMap { pub class_hash: MaybeCaseInsensitiveHashMap>, /// A hash from local name to rules which contain that local name selector. pub local_name_hash: PrecomputedHashMap>, + /// A hash from namespace to rules which contain that namespace selector. + pub namespace_hash: PrecomputedHashMap>, /// Rules that don't have ID, class, or element selectors. pub other: SmallVec<[T; 1]>, /// The number of entries in this map. @@ -125,6 +127,7 @@ impl SelectorMap { id_hash: MaybeCaseInsensitiveHashMap::new(), class_hash: MaybeCaseInsensitiveHashMap::new(), local_name_hash: HashMap::default(), + namespace_hash: HashMap::default(), other: SmallVec::new(), count: 0, } @@ -135,6 +138,7 @@ impl SelectorMap { self.id_hash.clear(); self.class_hash.clear(); self.local_name_hash.clear(); + self.namespace_hash.clear(); self.other.clear(); self.count = 0; } @@ -217,6 +221,18 @@ impl SelectorMap { ) } + if let Some(rules) = self.namespace_hash.get(rule_hash_target.namespace()) { + SelectorMap::get_matching_rules( + element, + rules, + matching_rules_list, + context, + flags_setter, + cascade_level, + shadow_cascade_order, + ) + } + SelectorMap::get_matching_rules( element, &self.other, @@ -261,7 +277,8 @@ impl SelectorMap { } impl SelectorMap { - /// Inserts into the correct hash, trying id, class, and localname. + /// Inserts into the correct hash, trying id, class, localname and + /// namespace. pub fn insert( &mut self, entry: T, @@ -298,13 +315,17 @@ impl SelectorMap { .try_entry(name.clone())? .or_insert_with(SmallVec::new) }, + Bucket::Namespace(url) => self.namespace_hash + .try_entry(url.clone())? + .or_insert_with(SmallVec::new), Bucket::Universal => &mut self.other, }; vector.try_push(entry) } - /// Looks up entries by id, class, local name, and other (in order). + /// Looks up entries by id, class, local name, namespace, and other (in + /// order). /// /// Each entry is passed to the callback, which returns true to continue /// iterating entries, or false to terminate the lookup. @@ -319,7 +340,6 @@ impl SelectorMap { E: TElement, F: FnMut(&'a T) -> bool, { - // Id. if let Some(id) = element.id() { if let Some(v) = self.id_hash.get(id, quirks_mode) { for entry in v.iter() { @@ -330,7 +350,6 @@ impl SelectorMap { } } - // Class. let mut done = false; element.each_class(|class| { if !done { @@ -348,7 +367,6 @@ impl SelectorMap { return false; } - // Local name. if let Some(v) = self.local_name_hash.get(element.local_name()) { for entry in v.iter() { if !f(&entry) { @@ -357,7 +375,14 @@ impl SelectorMap { } } - // Other. + if let Some(v) = self.namespace_hash.get(element.namespace()) { + for entry in v.iter() { + if !f(&entry) { + return false; + } + } + } + for entry in self.other.iter() { if !f(&entry) { return false; @@ -425,6 +450,7 @@ enum Bucket<'a> { name: &'a LocalName, lower_name: &'a LocalName, }, + Namespace(&'a Namespace), Universal, } @@ -436,6 +462,8 @@ fn specific_bucket_for<'a>(component: &'a Component) -> Bucket<'a> name: &selector.name, lower_name: &selector.lower_name, }, + Component::Namespace(_, ref url) | + Component::DefaultNamespace(ref url) => Bucket::Namespace(url), // ::slotted(..) isn't a normal pseudo-element, so we can insert it on // the rule hash normally without much problem. For example, in a // selector like: @@ -470,7 +498,7 @@ fn find_bucket<'a>(mut iter: SelectorIter<'a, SelectorImpl>) -> Bucket<'a> { // We basically want to find the most specific bucket, // where: // - // id > class > local name > universal. + // id > class > local name > namespace > universal. // for ss in &mut iter { let new_bucket = specific_bucket_for(ss); @@ -480,10 +508,15 @@ fn find_bucket<'a>(mut iter: SelectorIter<'a, SelectorImpl>) -> Bucket<'a> { current_bucket = new_bucket; }, Bucket::LocalName { .. } => { - if matches!(current_bucket, Bucket::Universal) { + if matches!(current_bucket, Bucket::Universal | Bucket::Namespace(..)) { current_bucket = new_bucket; } }, + Bucket::Namespace(..) => { + if matches!(current_bucket, Bucket::Universal) { + current_bucket = new_bucket; + } + } Bucket::Universal => {}, } } From d307b34be04293b23bdda73266c045215d419ef0 Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Tue, 26 Jun 2018 08:51:13 +0900 Subject: [PATCH 09/32] style: Introduce a constant variable to represents the number of all animatable longhands. We will use this number to cap the pre-allocation AnimationValueMap in the next patch. Bug: 1418806 MozReview-Commit-ID: Iqq9plbD8Vl --- components/style/properties/properties.mako.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index b6a8879e106..b27527d0132 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -100,6 +100,7 @@ pub mod longhands { % for style_struct in data.style_structs: include!("${repr(os.path.join(OUT_DIR, 'longhands/{}.rs'.format(style_struct.name_lower)))[1:-1]}"); % endfor + pub const ANIMATABLE_PROPERTY_COUNT: usize = ${sum(1 for prop in data.longhands if prop.animatable)}; } macro_rules! unwrap_or_initial { From 2274f392d81fd5a575323b163d1b96bc0d857716 Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Tue, 26 Jun 2018 11:08:24 +0900 Subject: [PATCH 10/32] style: Try to allocate possible size for AnimationValueMap before composing. The EffectSet count does not exactly represent the count what we really need for AnimationValueMap, but in most cases it matches. For example; 1) The element has two different keyframes animations @keyframes anim1 { to { opacity: 0; } } @keyframes anim2 { to { transform: rotate(360deg); } } In this case the number matches. 2) The element has two animations but both keyframes have the same CSS property @keyframes anim1 { to { opacity: 0; } } @keyframes anim2 { to { opacity: 0.1; } } In this case the number doesn't match, moreover it results more memory than we ever needed, but this case is presumably less common. 3) The element has an animation having keyframes for two different CSS properties. @keyframes anim { from { opacity: 0; transform: rotate(360deg); } } In this kind of cases, the number doesn't match. But even so, this patch reduces the opportunities that the AnimationValueMap tries to allocate a new memory (i.e. less opportunities on expanding the map). Note that when the hash map is expanded, we do allocate a new RawTable with the new size then replace the old one with the new one [1], so I believe this change will reduce the crash rate to some extent. [1] https://hg.mozilla.org/mozilla-central/file/15c95df467be/servo/components/hashglobe/src/hash_map.rs#l734 Bug: 1418806 Reviewed-by: birtles MozReview-Commit-ID: 6tcF9aqXh7a --- components/style/gecko/wrapper.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 0ce4a45d175..ffac86e974f 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -37,6 +37,7 @@ use gecko_bindings::bindings::Gecko_ElementHasAnimations; use gecko_bindings::bindings::Gecko_ElementHasCSSAnimations; use gecko_bindings::bindings::Gecko_ElementHasCSSTransitions; use gecko_bindings::bindings::Gecko_GetActiveLinkAttrDeclarationBlock; +use gecko_bindings::bindings::Gecko_GetAnimationEffectCount; use gecko_bindings::bindings::Gecko_GetAnimationRule; use gecko_bindings::bindings::Gecko_GetExtraContentStyleDeclarations; use gecko_bindings::bindings::Gecko_GetHTMLPresentationAttrDeclarationBlock; @@ -948,8 +949,16 @@ fn get_animation_rule( cascade_level: CascadeLevel, ) -> Option>> { use gecko_bindings::sugar::ownership::HasSimpleFFI; + use properties::longhands::ANIMATABLE_PROPERTY_COUNT; + + // There's a very rough correlation between the number of effects + // (animations) on an element and the number of properties it is likely to + // animate, so we use that as an initial guess for the size of the + // AnimationValueMap in order to reduce the number of re-allocations needed. + let effect_count = unsafe { Gecko_GetAnimationEffectCount(element.0) }; // Also, we should try to reuse the PDB, to avoid creating extra rule nodes. - let mut animation_values = AnimationValueMap::default(); + let mut animation_values = AnimationValueMap::with_capacity_and_hasher( + effect_count.min(ANIMATABLE_PROPERTY_COUNT), Default::default()); if unsafe { Gecko_GetAnimationRule( element.0, From aca724ec79cd1393987a8fa83215ae44f1be532b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 26 Jun 2018 00:25:58 +0200 Subject: [PATCH 11/32] style: Remove unneeded combinator check in selector-matching. The combinator doesn't change during the loop, no need to check it. Bug: 1471063 Reviewed-by: xidorn MozReview-Commit-ID: KIAt0WiEOtI --- components/selectors/matching.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index c3591ff2727..7c4812cb963 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -576,7 +576,7 @@ where _ => {}, } - if element.is_link() || combinator.is_sibling() { + if element.is_link() { visited_handling = VisitedHandlingMode::AllLinksUnvisited; } From 84280c5203aa1882cbf4b79b156dbbb75c90b2c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 26 Jun 2018 00:30:52 +0200 Subject: [PATCH 12/32] style: Deindent the serialization loop. Bug: 1471063 Reviewed-by: xidorn MozReview-Commit-ID: GPlUAx7YXVb --- components/selectors/parser.rs | 127 +++++++++++++++++---------------- 1 file changed, 64 insertions(+), 63 deletions(-) diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index 3210f1085b0..ac539045115 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -1007,72 +1007,73 @@ impl ToCss for Selector { debug_assert!(!combinators_exhausted); // https://drafts.csswg.org/cssom/#serializing-selectors + if compound.is_empty() { + continue; + } - if !compound.is_empty() { - // 1. If there is only one simple selector in the compound selectors - // which is a universal selector, append the result of - // serializing the universal selector to s. - // - // Check if `!compound.empty()` first--this can happen if we have - // something like `... > ::before`, because we store `>` and `::` - // both as combinators internally. - // - // If we are in this case, after we have serialized the universal - // selector, we skip Step 2 and continue with the algorithm. - let (can_elide_namespace, first_non_namespace) = match &compound[0] { - &Component::ExplicitAnyNamespace | - &Component::ExplicitNoNamespace | - &Component::Namespace(_, _) => (false, 1), - &Component::DefaultNamespace(_) => (true, 1), - _ => (true, 0), - }; - let mut perform_step_2 = true; - if first_non_namespace == compound.len() - 1 { - match (combinators.peek(), &compound[first_non_namespace]) { - // We have to be careful here, because if there is a - // pseudo element "combinator" there isn't really just - // the one simple selector. Technically this compound - // selector contains the pseudo element selector as well - // -- Combinator::PseudoElement, just like - // Combinator::SlotAssignment, don't exist in the - // spec. - (Some(&&Component::Combinator(Combinator::PseudoElement)), _) | - (Some(&&Component::Combinator(Combinator::SlotAssignment)), _) => (), - (_, &Component::ExplicitUniversalType) => { - // Iterate over everything so we serialize the namespace - // too. - for simple in compound.iter() { - simple.to_css(dest)?; - } - // Skip step 2, which is an "otherwise". - perform_step_2 = false; - }, - (_, _) => (), - } - } - - // 2. Otherwise, for each simple selector in the compound selectors - // that is not a universal selector of which the namespace prefix - // maps to a namespace that is not the default namespace - // serialize the simple selector and append the result to s. - // - // See https://github.com/w3c/csswg-drafts/issues/1606, which is - // proposing to change this to match up with the behavior asserted - // in cssom/serialize-namespaced-type-selectors.html, which the - // following code tries to match. - if perform_step_2 { - for simple in compound.iter() { - if let Component::ExplicitUniversalType = *simple { - // Can't have a namespace followed by a pseudo-element - // selector followed by a universal selector in the same - // compound selector, so we don't have to worry about the - // real namespace being in a different `compound`. - if can_elide_namespace { - continue; - } + // 1. If there is only one simple selector in the compound selectors + // which is a universal selector, append the result of + // serializing the universal selector to s. + // + // Check if `!compound.empty()` first--this can happen if we have + // something like `... > ::before`, because we store `>` and `::` + // both as combinators internally. + // + // If we are in this case, after we have serialized the universal + // selector, we skip Step 2 and continue with the algorithm. + let (can_elide_namespace, first_non_namespace) = match compound[0] { + Component::ExplicitAnyNamespace | + Component::ExplicitNoNamespace | + Component::Namespace(..) => (false, 1), + Component::DefaultNamespace(..) => (true, 1), + _ => (true, 0), + }; + let mut perform_step_2 = true; + if first_non_namespace == compound.len() - 1 { + match (combinators.peek(), &compound[first_non_namespace]) { + // We have to be careful here, because if there is a + // pseudo element "combinator" there isn't really just + // the one simple selector. Technically this compound + // selector contains the pseudo element selector as well + // -- Combinator::PseudoElement, just like + // Combinator::SlotAssignment, don't exist in the + // spec. + (Some(&&Component::Combinator(Combinator::PseudoElement)), _) | + (Some(&&Component::Combinator(Combinator::SlotAssignment)), _) => (), + (_, &Component::ExplicitUniversalType) => { + // Iterate over everything so we serialize the namespace + // too. + for simple in compound.iter() { + simple.to_css(dest)?; + } + // Skip step 2, which is an "otherwise". + perform_step_2 = false; + }, + (_, _) => (), + } + } + + // 2. Otherwise, for each simple selector in the compound selectors + // that is not a universal selector of which the namespace prefix + // maps to a namespace that is not the default namespace + // serialize the simple selector and append the result to s. + // + // See https://github.com/w3c/csswg-drafts/issues/1606, which is + // proposing to change this to match up with the behavior asserted + // in cssom/serialize-namespaced-type-selectors.html, which the + // following code tries to match. + if perform_step_2 { + for simple in compound.iter() { + if let Component::ExplicitUniversalType = *simple { + // Can't have a namespace followed by a pseudo-element + // selector followed by a universal selector in the same + // compound selector, so we don't have to worry about the + // real namespace being in a different `compound`. + if can_elide_namespace { + continue; } - simple.to_css(dest)?; } + simple.to_css(dest)?; } } From 07456bbd3d6eb770c5464b35c33d3f2a239ca276 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 26 Jun 2018 00:33:39 +0200 Subject: [PATCH 13/32] style: Simplify selector serialization. Bug: 1471063 Reviewed-by: xidorn MozReview-Commit-ID: 959U7yd5W9j --- components/selectors/parser.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index ac539045115..399f21e6361 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -995,8 +995,7 @@ impl ToCss for Selector { let mut combinators = self.iter_raw_match_order() .rev() - .filter(|x| x.is_combinator()) - .peekable(); + .filter_map(|x| x.as_combinator()); let compound_selectors = self.iter_raw_match_order() .as_slice() .split(|x| x.is_combinator()) @@ -1029,8 +1028,9 @@ impl ToCss for Selector { _ => (true, 0), }; let mut perform_step_2 = true; + let next_combinator = combinators.next(); if first_non_namespace == compound.len() - 1 { - match (combinators.peek(), &compound[first_non_namespace]) { + match (next_combinator, &compound[first_non_namespace]) { // We have to be careful here, because if there is a // pseudo element "combinator" there isn't really just // the one simple selector. Technically this compound @@ -1038,8 +1038,8 @@ impl ToCss for Selector { // -- Combinator::PseudoElement, just like // Combinator::SlotAssignment, don't exist in the // spec. - (Some(&&Component::Combinator(Combinator::PseudoElement)), _) | - (Some(&&Component::Combinator(Combinator::SlotAssignment)), _) => (), + (Some(Combinator::PseudoElement), _) | + (Some(Combinator::SlotAssignment), _) => (), (_, &Component::ExplicitUniversalType) => { // Iterate over everything so we serialize the namespace // too. @@ -1049,7 +1049,7 @@ impl ToCss for Selector { // Skip step 2, which is an "otherwise". perform_step_2 = false; }, - (_, _) => (), + _ => (), } } @@ -1082,7 +1082,7 @@ impl ToCss for Selector { // ">", "+", "~", ">>", "||", as appropriate, followed by another // single SPACE (U+0020) if the combinator was not whitespace, to // s. - match combinators.next() { + match next_combinator { Some(c) => c.to_css(dest)?, None => combinators_exhausted = true, }; From 0d0c6bd29e35139c1d38f4b73899e64cad824c4b Mon Sep 17 00:00:00 2001 From: Xidorn Quan Date: Wed, 27 Jun 2018 15:28:22 +1000 Subject: [PATCH 14/32] style: Use RUSTFMT env in stylo build script. Bug: 1471486 Reviewed-by: emilio MozReview-Commit-ID: JOg0xkmG5Yx --- components/style/build_gecko.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/components/style/build_gecko.rs b/components/style/build_gecko.rs index 95bb1622942..68d160ab5e0 100644 --- a/components/style/build_gecko.rs +++ b/components/style/build_gecko.rs @@ -182,14 +182,21 @@ mod bindings { // Disable rust unions, because we replace some types inside of // them. let mut builder = Builder::default().rust_target(RustTarget::Stable_1_0); - let rustfmt_path = env::var_os("MOZ_AUTOMATION") - .and_then(|_| env::var_os("TOOLTOOL_DIR").or_else(|| env::var_os("MOZ_SRC"))) - .map(PathBuf::from); - builder = match rustfmt_path { - Some(path) => builder.with_rustfmt(path.join("rustc").join("bin").join("rustfmt")), - None => builder.rustfmt_bindings(env::var_os("STYLO_RUSTFMT_BINDINGS").is_some()), - }; + let rustfmt_path = env::var_os("RUSTFMT") + // This can be replaced with + // > .filter(|p| !p.is_empty()).map(PathBuf::from) + // once we can use 1.27+. + .and_then(|p| { + if p.is_empty() { + None + } else { + Some(PathBuf::from(p)) + } + }); + if let Some(path) = rustfmt_path { + builder = builder.with_rustfmt(path); + } for dir in SEARCH_PATHS.iter() { builder = builder.clang_arg("-I").clang_arg(dir.to_str().unwrap()); From 26a9c9f53cbc29821ac52792a3e197a625a72405 Mon Sep 17 00:00:00 2001 From: Xidorn Quan Date: Wed, 27 Jun 2018 10:42:51 +1000 Subject: [PATCH 15/32] style: Remove location from preludes and take it from argument. Bug: 1471104 Reviewed-by: emilio MozReview-Commit-ID: HeJUQvkacaf --- .../stylesheets/font_feature_values_rule.rs | 1 + .../style/stylesheets/keyframes_rule.rs | 38 ++---- components/style/stylesheets/rule_parser.rs | 120 +++++++++--------- 3 files changed, 75 insertions(+), 84 deletions(-) diff --git a/components/style/stylesheets/font_feature_values_rule.rs b/components/style/stylesheets/font_feature_values_rule.rs index 8ce16e2ccf9..c70a46c1c9d 100644 --- a/components/style/stylesheets/font_feature_values_rule.rs +++ b/components/style/stylesheets/font_feature_values_rule.rs @@ -428,6 +428,7 @@ macro_rules! font_feature_values_blocks { fn parse_block<'t>( &mut self, prelude: BlockType, + _location: SourceLocation, input: &mut Parser<'i, 't> ) -> Result> { debug_assert_eq!(self.context.rule_type(), CssRuleType::FontFeatureValues); diff --git a/components/style/stylesheets/keyframes_rule.rs b/components/style/stylesheets/keyframes_rule.rs index a0cb5fa0b1d..85dc00bb9af 100644 --- a/components/style/stylesheets/keyframes_rule.rs +++ b/components/style/stylesheets/keyframes_rule.rs @@ -509,14 +509,8 @@ impl<'a, 'i> AtRuleParser<'i> for KeyframeListParser<'a> { type Error = StyleParseErrorKind<'i>; } -/// A wrapper to wraps the KeyframeSelector with its source location -struct KeyframeSelectorParserPrelude { - selector: KeyframeSelector, - source_location: SourceLocation, -} - impl<'a, 'i> QualifiedRuleParser<'i> for KeyframeListParser<'a> { - type Prelude = KeyframeSelectorParserPrelude; + type Prelude = KeyframeSelector; type QualifiedRule = Arc>; type Error = StyleParseErrorKind<'i>; @@ -525,27 +519,21 @@ impl<'a, 'i> QualifiedRuleParser<'i> for KeyframeListParser<'a> { input: &mut Parser<'i, 't>, ) -> Result> { let start_position = input.position(); - let start_location = input.current_source_location(); - match KeyframeSelector::parse(input) { - Ok(sel) => Ok(KeyframeSelectorParserPrelude { - selector: sel, - source_location: start_location, - }), - Err(e) => { - let location = e.location; - let error = ContextualParseError::InvalidKeyframeRule( - input.slice_from(start_position), - e.clone(), + KeyframeSelector::parse(input).map_err(|e| { + let location = e.location; + let error = ContextualParseError::InvalidKeyframeRule( + input.slice_from(start_position), + e.clone(), ); - self.context.log_css_error(location, error); - Err(e) - }, - } + self.context.log_css_error(location, error); + e + }) } fn parse_block<'t>( &mut self, - prelude: Self::Prelude, + selector: Self::Prelude, + source_location: SourceLocation, input: &mut Parser<'i, 't>, ) -> Result> { let context = ParserContext::new_with_rule_type( @@ -580,9 +568,9 @@ impl<'a, 'i> QualifiedRuleParser<'i> for KeyframeListParser<'a> { // `parse_important` is not called here, `!important` is not allowed in keyframe blocks. } Ok(Arc::new(self.shared_lock.wrap(Keyframe { - selector: prelude.selector, + selector, block: Arc::new(self.shared_lock.wrap(block)), - source_location: prelude.source_location, + source_location, }))) } } diff --git a/components/style/stylesheets/rule_parser.rs b/components/style/stylesheets/rule_parser.rs index 41d87aa07a4..d2f9b691ad4 100644 --- a/components/style/stylesheets/rule_parser.rs +++ b/components/style/stylesheets/rule_parser.rs @@ -146,31 +146,31 @@ pub enum VendorPrefix { /// A rule prelude for at-rule with block. pub enum AtRuleBlockPrelude { /// A @font-face rule prelude. - FontFace(SourceLocation), + FontFace, /// A @font-feature-values rule prelude, with its FamilyName list. - FontFeatureValues(Vec, SourceLocation), + FontFeatureValues(Vec), /// A @counter-style rule prelude, with its counter style name. - CounterStyle(CustomIdent, SourceLocation), + CounterStyle(CustomIdent), /// A @media rule prelude, with its media queries. - Media(Arc>, SourceLocation), + Media(Arc>), /// An @supports rule, with its conditional - Supports(SupportsCondition, SourceLocation), + Supports(SupportsCondition), /// A @viewport rule prelude. Viewport, /// A @keyframes rule, with its animation name and vendor prefix if exists. - Keyframes(KeyframesName, Option, SourceLocation), + Keyframes(KeyframesName, Option), /// A @page rule prelude. - Page(SourceLocation), + Page, /// A @document rule, with its conditional. - Document(DocumentCondition, SourceLocation), + Document(DocumentCondition), } /// A rule prelude for at-rule without block. pub enum AtRuleNonBlockPrelude { /// A @import rule prelude. - Import(CssUrl, Arc>, SourceLocation), + Import(CssUrl, Arc>), /// A @namespace rule prelude. - Namespace(Option, Namespace, SourceLocation), + Namespace(Option, Namespace), } impl<'a, 'i> AtRuleParser<'i> for TopLevelRuleParser<'a> { @@ -184,7 +184,6 @@ impl<'a, 'i> AtRuleParser<'i> for TopLevelRuleParser<'a> { name: CowRcStr<'i>, input: &mut Parser<'i, 't>, ) -> Result, ParseError<'i>> { - let location = input.current_source_location(); match_ignore_ascii_case! { &*name, "import" => { if !self.check_state(State::Imports) { @@ -197,7 +196,7 @@ impl<'a, 'i> AtRuleParser<'i> for TopLevelRuleParser<'a> { let media = MediaList::parse(&self.context, input); let media = Arc::new(self.shared_lock.wrap(media)); - let prelude = AtRuleNonBlockPrelude::Import(url, media, location); + let prelude = AtRuleNonBlockPrelude::Import(url, media); return Ok(AtRuleType::WithoutBlock(prelude)); }, "namespace" => { @@ -215,7 +214,7 @@ impl<'a, 'i> AtRuleParser<'i> for TopLevelRuleParser<'a> { Err(e) => return Err(e.into()), }; let url = Namespace::from(maybe_namespace.as_ref()); - let prelude = AtRuleNonBlockPrelude::Namespace(prefix, url, location); + let prelude = AtRuleNonBlockPrelude::Namespace(prefix, url); return Ok(AtRuleType::WithoutBlock(prelude)); }, // @charset is removed by rust-cssparser if it’s the first rule in the stylesheet @@ -238,24 +237,29 @@ impl<'a, 'i> AtRuleParser<'i> for TopLevelRuleParser<'a> { fn parse_block<'t>( &mut self, prelude: AtRuleBlockPrelude, + location: SourceLocation, input: &mut Parser<'i, 't>, ) -> Result> { - AtRuleParser::parse_block(&mut self.nested(), prelude, input).map(|rule| { + AtRuleParser::parse_block(&mut self.nested(), prelude, location, input).map(|rule| { self.state = State::Body; rule }) } #[inline] - fn rule_without_block(&mut self, prelude: AtRuleNonBlockPrelude) -> CssRule { + fn rule_without_block( + &mut self, + prelude: AtRuleNonBlockPrelude, + source_location: SourceLocation, + ) -> CssRule { match prelude { - AtRuleNonBlockPrelude::Import(url, media, location) => { + AtRuleNonBlockPrelude::Import(url, media) => { let loader = self.loader .expect("Expected a stylesheet loader for @import"); let import_rule = loader.request_stylesheet( url, - location, + source_location, &self.context, &self.shared_lock, media, @@ -264,7 +268,7 @@ impl<'a, 'i> AtRuleParser<'i> for TopLevelRuleParser<'a> { self.state = State::Imports; CssRule::Import(import_rule) }, - AtRuleNonBlockPrelude::Namespace(prefix, url, source_location) => { + AtRuleNonBlockPrelude::Namespace(prefix, url) => { let prefix = if let Some(prefix) = prefix { self.namespaces.prefixes.insert(prefix.clone(), url.clone()); Some(prefix) @@ -284,13 +288,8 @@ impl<'a, 'i> AtRuleParser<'i> for TopLevelRuleParser<'a> { } } -pub struct QualifiedRuleParserPrelude { - selectors: SelectorList, - source_location: SourceLocation, -} - impl<'a, 'i> QualifiedRuleParser<'i> for TopLevelRuleParser<'a> { - type Prelude = QualifiedRuleParserPrelude; + type Prelude = SelectorList; type QualifiedRule = CssRule; type Error = StyleParseErrorKind<'i>; @@ -298,7 +297,7 @@ impl<'a, 'i> QualifiedRuleParser<'i> for TopLevelRuleParser<'a> { fn parse_prelude<'t>( &mut self, input: &mut Parser<'i, 't>, - ) -> Result> { + ) -> Result> { if !self.check_state(State::Body) { return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) } @@ -309,10 +308,16 @@ impl<'a, 'i> QualifiedRuleParser<'i> for TopLevelRuleParser<'a> { #[inline] fn parse_block<'t>( &mut self, - prelude: QualifiedRuleParserPrelude, + prelude: Self::Prelude, + location: SourceLocation, input: &mut Parser<'i, 't>, ) -> Result> { - QualifiedRuleParser::parse_block(&mut self.nested(), prelude, input).map(|result| { + QualifiedRuleParser::parse_block( + &mut self.nested(), + prelude, + location, + input, + ).map(|result| { self.state = State::Body; result }) @@ -373,20 +378,18 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> { name: CowRcStr<'i>, input: &mut Parser<'i, 't>, ) -> Result, ParseError<'i>> { - let location = input.current_source_location(); - match_ignore_ascii_case! { &*name, "media" => { let media_queries = MediaList::parse(self.context, input); let arc = Arc::new(self.shared_lock.wrap(media_queries)); - Ok(AtRuleType::WithBlock(AtRuleBlockPrelude::Media(arc, location))) + Ok(AtRuleType::WithBlock(AtRuleBlockPrelude::Media(arc))) }, "supports" => { let cond = SupportsCondition::parse(input)?; - Ok(AtRuleType::WithBlock(AtRuleBlockPrelude::Supports(cond, location))) + Ok(AtRuleType::WithBlock(AtRuleBlockPrelude::Supports(cond))) }, "font-face" => { - Ok(AtRuleType::WithBlock(AtRuleBlockPrelude::FontFace(location))) + Ok(AtRuleType::WithBlock(AtRuleBlockPrelude::FontFace)) }, "font-feature-values" => { if !cfg!(feature = "gecko") { @@ -394,7 +397,7 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> { return Err(input.new_custom_error(StyleParseErrorKind::UnsupportedAtRule(name.clone()))) } let family_names = parse_family_name_list(self.context, input)?; - Ok(AtRuleType::WithBlock(AtRuleBlockPrelude::FontFeatureValues(family_names, location))) + Ok(AtRuleType::WithBlock(AtRuleBlockPrelude::FontFeatureValues(family_names))) }, "counter-style" => { if !cfg!(feature = "gecko") { @@ -402,7 +405,7 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> { return Err(input.new_custom_error(StyleParseErrorKind::UnsupportedAtRule(name.clone()))) } let name = parse_counter_style_name_definition(input)?; - Ok(AtRuleType::WithBlock(AtRuleBlockPrelude::CounterStyle(name, location))) + Ok(AtRuleType::WithBlock(AtRuleBlockPrelude::CounterStyle(name))) }, "viewport" => { if viewport_rule::enabled() { @@ -426,11 +429,11 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> { } let name = KeyframesName::parse(self.context, input)?; - Ok(AtRuleType::WithBlock(AtRuleBlockPrelude::Keyframes(name, prefix, location))) + Ok(AtRuleType::WithBlock(AtRuleBlockPrelude::Keyframes(name, prefix))) }, "page" => { if cfg!(feature = "gecko") { - Ok(AtRuleType::WithBlock(AtRuleBlockPrelude::Page(location))) + Ok(AtRuleType::WithBlock(AtRuleBlockPrelude::Page)) } else { Err(input.new_custom_error(StyleParseErrorKind::UnsupportedAtRule(name.clone()))) } @@ -443,7 +446,7 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> { } let cond = DocumentCondition::parse(self.context, input)?; - Ok(AtRuleType::WithBlock(AtRuleBlockPrelude::Document(cond, location))) + Ok(AtRuleType::WithBlock(AtRuleBlockPrelude::Document(cond))) }, _ => Err(input.new_custom_error(StyleParseErrorKind::UnsupportedAtRule(name.clone()))) } @@ -452,10 +455,11 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> { fn parse_block<'t>( &mut self, prelude: AtRuleBlockPrelude, + source_location: SourceLocation, input: &mut Parser<'i, 't>, ) -> Result> { match prelude { - AtRuleBlockPrelude::FontFace(location) => { + AtRuleBlockPrelude::FontFace => { let context = ParserContext::new_with_rule_type( self.context, CssRuleType::FontFace, @@ -463,10 +467,10 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> { ); Ok(CssRule::FontFace(Arc::new(self.shared_lock.wrap( - parse_font_face_block(&context, input, location).into(), + parse_font_face_block(&context, input, source_location).into(), )))) }, - AtRuleBlockPrelude::FontFeatureValues(family_names, location) => { + AtRuleBlockPrelude::FontFeatureValues(family_names) => { let context = ParserContext::new_with_rule_type( self.context, CssRuleType::FontFeatureValues, @@ -478,11 +482,11 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> { &context, input, family_names, - location, + source_location, ), )))) }, - AtRuleBlockPrelude::CounterStyle(name, location) => { + AtRuleBlockPrelude::CounterStyle(name) => { let context = ParserContext::new_with_rule_type( self.context, CssRuleType::CounterStyle, @@ -495,19 +499,19 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> { name, &context, input, - location, + source_location, )?.into(), ), ))) }, - AtRuleBlockPrelude::Media(media_queries, source_location) => { + AtRuleBlockPrelude::Media(media_queries) => { Ok(CssRule::Media(Arc::new(self.shared_lock.wrap(MediaRule { media_queries, rules: self.parse_nested_rules(input, CssRuleType::Media), source_location, })))) }, - AtRuleBlockPrelude::Supports(condition, source_location) => { + AtRuleBlockPrelude::Supports(condition) => { let eval_context = ParserContext::new_with_rule_type( self.context, CssRuleType::Style, @@ -535,7 +539,7 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> { ViewportRule::parse(&context, input)?, )))) }, - AtRuleBlockPrelude::Keyframes(name, vendor_prefix, source_location) => { + AtRuleBlockPrelude::Keyframes(name, vendor_prefix) => { let context = ParserContext::new_with_rule_type( self.context, CssRuleType::Keyframes, @@ -555,7 +559,7 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> { }, )))) }, - AtRuleBlockPrelude::Page(source_location) => { + AtRuleBlockPrelude::Page => { let context = ParserContext::new_with_rule_type( self.context, CssRuleType::Page, @@ -569,7 +573,7 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> { source_location, })))) }, - AtRuleBlockPrelude::Document(condition, source_location) => { + AtRuleBlockPrelude::Document(condition) => { if !cfg!(feature = "gecko") { unreachable!() } @@ -586,39 +590,37 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> { } impl<'a, 'b, 'i> QualifiedRuleParser<'i> for NestedRuleParser<'a, 'b> { - type Prelude = QualifiedRuleParserPrelude; + type Prelude = SelectorList; type QualifiedRule = CssRule; type Error = StyleParseErrorKind<'i>; fn parse_prelude<'t>( &mut self, input: &mut Parser<'i, 't>, - ) -> Result> { + ) -> Result> { let selector_parser = SelectorParser { stylesheet_origin: self.stylesheet_origin, namespaces: self.namespaces, url_data: Some(self.context.url_data), }; - - let source_location = input.current_source_location(); - let selectors = SelectorList::parse(&selector_parser, input)?; - - Ok(QualifiedRuleParserPrelude { selectors, source_location, }) + SelectorList::parse(&selector_parser, input) } fn parse_block<'t>( &mut self, - prelude: QualifiedRuleParserPrelude, + selectors: Self::Prelude, + source_location: SourceLocation, input: &mut Parser<'i, 't>, ) -> Result> { let context = ParserContext::new_with_rule_type(self.context, CssRuleType::Style, self.namespaces); let declarations = parse_property_declaration_list(&context, input); + let block = Arc::new(self.shared_lock.wrap(declarations)); Ok(CssRule::Style(Arc::new(self.shared_lock.wrap(StyleRule { - selectors: prelude.selectors, - block: Arc::new(self.shared_lock.wrap(declarations)), - source_location: prelude.source_location, + selectors, + block, + source_location, })))) } } From 9d1ae1840fd5919334fee6f1c48d8faff7626faf Mon Sep 17 00:00:00 2001 From: Xidorn Quan Date: Wed, 27 Jun 2018 10:43:21 +1000 Subject: [PATCH 16/32] style: Upgrade cssparser. Bug: 1471104 MozReview-Commit-ID: 74rBgkJEcYd --- components/malloc_size_of/Cargo.toml | 2 +- components/selectors/Cargo.toml | 2 +- components/style/Cargo.toml | 2 +- components/style_traits/Cargo.toml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/components/malloc_size_of/Cargo.toml b/components/malloc_size_of/Cargo.toml index 92376a6cd61..67718409311 100644 --- a/components/malloc_size_of/Cargo.toml +++ b/components/malloc_size_of/Cargo.toml @@ -24,7 +24,7 @@ servo = [ [dependencies] app_units = "0.6" -cssparser = "0.23.0" +cssparser = "0.24.0" euclid = "0.17" hashglobe = { path = "../hashglobe" } hyper = { version = "0.10", optional = true } diff --git a/components/selectors/Cargo.toml b/components/selectors/Cargo.toml index ee38f2ef276..19adcdd1578 100644 --- a/components/selectors/Cargo.toml +++ b/components/selectors/Cargo.toml @@ -22,7 +22,7 @@ bench = [] [dependencies] bitflags = "1.0" matches = "0.1" -cssparser = "0.23.0" +cssparser = "0.24.0" log = "0.4" fnv = "1.0" phf = "0.7.18" diff --git a/components/style/Cargo.toml b/components/style/Cargo.toml index 4fb50d8ad85..b80b7572bba 100644 --- a/components/style/Cargo.toml +++ b/components/style/Cargo.toml @@ -31,7 +31,7 @@ atomic_refcell = "0.1" bitflags = "1.0" byteorder = "1.0" cfg-if = "0.1.0" -cssparser = "0.23.0" +cssparser = "0.24.0" new_debug_unreachable = "1.0" encoding_rs = {version = "0.7", optional = true} euclid = "0.17" diff --git a/components/style_traits/Cargo.toml b/components/style_traits/Cargo.toml index 67a6c851dce..9b273472774 100644 --- a/components/style_traits/Cargo.toml +++ b/components/style_traits/Cargo.toml @@ -15,7 +15,7 @@ gecko = [] [dependencies] app_units = "0.6" -cssparser = "0.23.0" +cssparser = "0.24.0" bitflags = "1.0" euclid = "0.17" malloc_size_of = { path = "../malloc_size_of" } From ce0496e116387fa29132d25fbada07a09acb770e Mon Sep 17 00:00:00 2001 From: Xidorn Quan Date: Wed, 27 Jun 2018 15:34:29 +1000 Subject: [PATCH 17/32] style: Generate ComputedStyleMap entry list from property data. This changes the order of properties returned from gCS. The old order doesn't make much sense, and other browsers don't agree on an identical order either, so it should be trivial to change it. Also the spec isn't super clear / useful in this case. Several -moz-prefixed properties are excluded from the list due to their being internal. I suspect they are never accessible anyway, so probably nothing gets changed by this. Bug: 1471114 Reviewed-by: xidorn MozReview-Commit-ID: 9LfangjpJ3P --- components/style/properties/properties.mako.rs | 2 ++ components/style/properties/shorthands/background.mako.rs | 1 + components/style/properties/shorthands/box.mako.rs | 1 + components/style/properties/shorthands/font.mako.rs | 1 + components/style/properties/shorthands/svg.mako.rs | 2 ++ components/style/properties/shorthands/text.mako.rs | 1 + 6 files changed, 8 insertions(+) diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index b27527d0132..5ba3619611b 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -836,6 +836,8 @@ bitflags! { * they can be checked in the C++ side via ServoCSSPropList.h. */ /// This property can be animated on the compositor. const CAN_ANIMATE_ON_COMPOSITOR = 0; + /// This shorthand property is accessible from getComputedStyle. + const SHORTHAND_IN_GETCS = 0; } } diff --git a/components/style/properties/shorthands/background.mako.rs b/components/style/properties/shorthands/background.mako.rs index b16c9918a24..dff9319cb30 100644 --- a/components/style/properties/shorthands/background.mako.rs +++ b/components/style/properties/shorthands/background.mako.rs @@ -196,6 +196,7 @@ <%helpers:shorthand name="background-position" + flags="SHORTHAND_IN_GETCS" sub_properties="background-position-x background-position-y" spec="https://drafts.csswg.org/css-backgrounds-4/#the-background-position"> use properties::longhands::{background_position_x, background_position_y}; diff --git a/components/style/properties/shorthands/box.mako.rs b/components/style/properties/shorthands/box.mako.rs index 842dfbd7229..1425f012854 100644 --- a/components/style/properties/shorthands/box.mako.rs +++ b/components/style/properties/shorthands/box.mako.rs @@ -6,6 +6,7 @@ <%helpers:shorthand name="overflow" + flags="SHORTHAND_IN_GETCS" sub_properties="overflow-x overflow-y" spec="https://drafts.csswg.org/css-overflow/#propdef-overflow" > diff --git a/components/style/properties/shorthands/font.mako.rs b/components/style/properties/shorthands/font.mako.rs index a53f899851c..75095bcebc2 100644 --- a/components/style/properties/shorthands/font.mako.rs +++ b/components/style/properties/shorthands/font.mako.rs @@ -285,6 +285,7 @@ <%helpers:shorthand name="font-variant" + flags="SHORTHAND_IN_GETCS" sub_properties="font-variant-caps ${'font-variant-alternates' if product == 'gecko' else ''} ${'font-variant-east-asian' if product == 'gecko' else ''} diff --git a/components/style/properties/shorthands/svg.mako.rs b/components/style/properties/shorthands/svg.mako.rs index aa73c77e46b..03027359dd4 100644 --- a/components/style/properties/shorthands/svg.mako.rs +++ b/components/style/properties/shorthands/svg.mako.rs @@ -5,6 +5,7 @@ <%namespace name="helpers" file="/helpers.mako.rs" /> <%helpers:shorthand name="mask" products="gecko" extra_prefixes="webkit" + flags="SHORTHAND_IN_GETCS" sub_properties="mask-mode mask-repeat mask-clip mask-origin mask-composite mask-position-x mask-position-y mask-size mask-image" spec="https://drafts.fxtf.org/css-masking/#propdef-mask"> @@ -182,6 +183,7 @@ <%helpers:shorthand name="mask-position" products="gecko" extra_prefixes="webkit" + flags="SHORTHAND_IN_GETCS" sub_properties="mask-position-x mask-position-y" spec="https://drafts.csswg.org/css-masks-4/#the-mask-position"> use properties::longhands::{mask_position_x,mask_position_y}; diff --git a/components/style/properties/shorthands/text.mako.rs b/components/style/properties/shorthands/text.mako.rs index 18a0786494d..e83d0b67544 100644 --- a/components/style/properties/shorthands/text.mako.rs +++ b/components/style/properties/shorthands/text.mako.rs @@ -5,6 +5,7 @@ <%namespace name="helpers" file="/helpers.mako.rs" /> <%helpers:shorthand name="text-decoration" + flags="SHORTHAND_IN_GETCS" sub_properties="text-decoration-line ${' text-decoration-style text-decoration-color' if product == 'gecko' else ''}" spec="https://drafts.csswg.org/css-text-decor/#propdef-text-decoration"> From 65953fb15845739ad7a4a6c46d76fd03709a2cf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 26 Jun 2018 20:37:00 +0200 Subject: [PATCH 18/32] style: Rename offset-* logical properties to inset-*. Bug: 1464782 Reviewed-by: xidorn MozReview-Commit-ID: BW44sru99RF --- components/style/properties/helpers.mako.rs | 2 +- components/style/properties/longhands/position.mako.rs | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index 11a1283f1cc..e07c9bd57cb 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -856,7 +856,7 @@ elif len(maybe_size) == 1: size = maybe_size[0] def phys_ident(side, phy_side): - return to_rust_ident(name.replace(side, phy_side).replace("offset-", "")) + return to_rust_ident(name.replace(side, phy_side).replace("inset-", "")) %> % if side is not None: use logical_geometry::PhysicalSide; diff --git a/components/style/properties/longhands/position.mako.rs b/components/style/properties/longhands/position.mako.rs index a99cebfdba9..d7e4e27a3b7 100644 --- a/components/style/properties/longhands/position.mako.rs +++ b/components/style/properties/longhands/position.mako.rs @@ -21,14 +21,15 @@ servo_restyle_damage="reflow_out_of_flow" )} % endfor -// offset-* logical properties, map to "top" / "left" / "bottom" / "right" +// inset-* logical properties, map to "top" / "left" / "bottom" / "right" % for side in LOGICAL_SIDES: ${helpers.predefined_type( - "offset-%s" % side, + "inset-%s" % side, "LengthOrPercentageOrAuto", "computed::LengthOrPercentageOrAuto::Auto", - spec="https://drafts.csswg.org/css-logical-props/#propdef-offset-%s" % side, + spec="https://drafts.csswg.org/css-logical-props/#propdef-inset-%s" % side, flags="GETCS_NEEDS_LAYOUT_FLUSH", + alias="offset-%s" % side, animation_value_type="ComputedValue", logical=True, )} From 06fa3406dea2beacf51f38580a43c181b55cf68e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 27 Jun 2018 11:22:21 +0200 Subject: [PATCH 19/32] style: Put offset-* aliases behind a pref. Bug: 1464782 Reviewed-by: xidorn MozReview-Commit-ID: Hl6Muim3wVH --- components/style/properties/longhands/position.mako.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/properties/longhands/position.mako.rs b/components/style/properties/longhands/position.mako.rs index d7e4e27a3b7..3b34047fc5a 100644 --- a/components/style/properties/longhands/position.mako.rs +++ b/components/style/properties/longhands/position.mako.rs @@ -29,7 +29,7 @@ "computed::LengthOrPercentageOrAuto::Auto", spec="https://drafts.csswg.org/css-logical-props/#propdef-inset-%s" % side, flags="GETCS_NEEDS_LAYOUT_FLUSH", - alias="offset-%s" % side, + alias="offset-%s:layout.css.offset-logical-properties.enabled" % side, animation_value_type="ComputedValue", logical=True, )} From b68e4c2352045ac3303d60e03231d8e420ab2b13 Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Thu, 28 Jun 2018 14:55:45 +0300 Subject: [PATCH 20/32] style: Implement :defined pseudo-class for custom elements. Bug: 1331334 Reviewed-by: emilio --- components/style/element_state.rs | 3 ++- components/style/gecko/non_ts_pseudo_class_list.rs | 1 + components/style/gecko/selector_parser.rs | 3 +++ components/style/gecko/wrapper.rs | 1 + 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/components/style/element_state.rs b/components/style/element_state.rs index 0960ca3d4e1..c57cf4febe6 100644 --- a/components/style/element_state.rs +++ b/components/style/element_state.rs @@ -71,7 +71,8 @@ bitflags! { const IN_OPTIONAL_STATE = 1 << 22; /// const IN_READ_WRITE_STATE = 1 << 22; - /// There is a free bit at 23. + /// + const IN_DEFINED_STATE = 1 << 23; /// const IN_VISITED_STATE = 1 << 24; /// diff --git a/components/style/gecko/non_ts_pseudo_class_list.rs b/components/style/gecko/non_ts_pseudo_class_list.rs index b02e21fbf8a..56e62b6de4b 100644 --- a/components/style/gecko/non_ts_pseudo_class_list.rs +++ b/components/style/gecko/non_ts_pseudo_class_list.rs @@ -47,6 +47,7 @@ macro_rules! apply_non_ts_list { ("visited", Visited, visited, IN_VISITED_STATE, _), ("active", Active, active, IN_ACTIVE_STATE, _), ("checked", Checked, checked, IN_CHECKED_STATE, _), + ("defined", Defined, defined, IN_DEFINED_STATE, _), ("disabled", Disabled, disabled, IN_DISABLED_STATE, _), ("enabled", Enabled, enabled, IN_ENABLED_STATE, _), ("focus", Focus, focus, IN_FOCUS_STATE, _), diff --git a/components/style/gecko/selector_parser.rs b/components/style/gecko/selector_parser.rs index 57a98a89238..7dfc9aa1bbc 100644 --- a/components/style/gecko/selector_parser.rs +++ b/components/style/gecko/selector_parser.rs @@ -187,6 +187,9 @@ impl NonTSPseudoClass { NonTSPseudoClass::Fullscreen => unsafe { mozilla::StaticPrefs_sVarCache_full_screen_api_unprefix_enabled }, + NonTSPseudoClass::Defined => unsafe { + structs::nsContentUtils_sIsCustomElementsEnabled + }, // Otherwise, a pseudo-class is enabled in content when it // doesn't have any enabled flag. _ => !self.has_any_flag( diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index ffac86e974f..47205f7378b 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -2124,6 +2124,7 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { { use selectors::matching::*; match *pseudo_class { + NonTSPseudoClass::Defined | NonTSPseudoClass::Focus | NonTSPseudoClass::Enabled | NonTSPseudoClass::Disabled | From 8688c53f0752361ddc03d989b9d2937b15688e09 Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Thu, 28 Jun 2018 15:58:11 +0300 Subject: [PATCH 21/32] style: Unconditionally enable :defined in chrome. Bug: 1471871 Reviewed-by: emilio --- components/style/gecko/non_ts_pseudo_class_list.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/gecko/non_ts_pseudo_class_list.rs b/components/style/gecko/non_ts_pseudo_class_list.rs index 56e62b6de4b..04f57b73885 100644 --- a/components/style/gecko/non_ts_pseudo_class_list.rs +++ b/components/style/gecko/non_ts_pseudo_class_list.rs @@ -47,7 +47,7 @@ macro_rules! apply_non_ts_list { ("visited", Visited, visited, IN_VISITED_STATE, _), ("active", Active, active, IN_ACTIVE_STATE, _), ("checked", Checked, checked, IN_CHECKED_STATE, _), - ("defined", Defined, defined, IN_DEFINED_STATE, _), + ("defined", Defined, defined, IN_DEFINED_STATE, PSEUDO_CLASS_ENABLED_IN_UA_SHEETS_AND_CHROME), ("disabled", Disabled, disabled, IN_DISABLED_STATE, _), ("enabled", Enabled, enabled, IN_ENABLED_STATE, _), ("focus", Focus, focus, IN_FOCUS_STATE, _), From e6d62b685ba65e0cb55cb49461c39b971f44df1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 28 Jun 2018 10:58:06 +0000 Subject: [PATCH 22/32] style: Make :host() and ::slotted() account for the inner selector's specificity. As resolved in https://github.com/w3c/csswg-drafts/issues/1915. Bug: 1454165 Reviewed-by: xidorn Differential Revision: https://phabricator.services.mozilla.com/D1849 --- components/selectors/builder.rs | 49 +++++++++++++++++++++++++-------- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/components/selectors/builder.rs b/components/selectors/builder.rs index 0be3eb90e0c..29ac822fa03 100644 --- a/components/selectors/builder.rs +++ b/components/selectors/builder.rs @@ -23,7 +23,7 @@ use sink::Push; use smallvec::{self, SmallVec}; use std::cmp; use std::iter; -use std::ops::Add; +use std::ops::{AddAssign, Add}; use std::ptr; use std::slice; @@ -228,6 +228,16 @@ struct Specificity { element_selectors: u32, } + +impl AddAssign for Specificity { + #[inline] + fn add_assign(&mut self, rhs: Self) { + self.id_selectors += rhs.id_selectors; + self.class_like_selectors += rhs.class_like_selectors; + self.element_selectors += rhs.element_selectors; + } +} + impl Add for Specificity { type Output = Specificity; @@ -251,6 +261,7 @@ impl Default for Specificity { } impl From for Specificity { + #[inline] fn from(value: u32) -> Specificity { assert!(value <= MAX_10BIT << 20 | MAX_10BIT << 10 | MAX_10BIT); Specificity { @@ -262,6 +273,7 @@ impl From for Specificity { } impl From for u32 { + #[inline] fn from(specificity: Specificity) -> u32 { cmp::min(specificity.id_selectors, MAX_10BIT) << 20 | cmp::min(specificity.class_like_selectors, MAX_10BIT) << 10 | @@ -298,17 +310,29 @@ where builder, ); } - // FIXME(emilio): Spec doesn't define any particular specificity for - // ::slotted(), so apply the general rule for pseudos per: - // - // https://github.com/w3c/csswg-drafts/issues/1915 - // - // Though other engines compute it dynamically, so maybe we should - // do that instead, eventually. - Component::Slotted(..) | Component::PseudoElement(..) | Component::LocalName(..) => { + Component::PseudoElement(..) | Component::LocalName(..) => { specificity.element_selectors += 1 }, - Component::ID(..) => specificity.id_selectors += 1, + Component::Slotted(ref selector) => { + specificity.element_selectors += 1; + // Note that due to the way ::slotted works we only compete with + // other ::slotted rules, so the above rule doesn't really + // matter, but we do it still for consistency with other + // pseudo-elements. + // + // See: https://github.com/w3c/csswg-drafts/issues/1915 + *specificity += Specificity::from(selector.specificity()); + }, + Component::Host(ref selector) => { + specificity.class_like_selectors += 1; + if let Some(ref selector) = *selector { + // See: https://github.com/w3c/csswg-drafts/issues/1915 + *specificity += Specificity::from(selector.specificity()); + } + } + Component::ID(..) => { + specificity.id_selectors += 1; + }, Component::Class(..) | Component::AttributeInNoNamespace { .. } | Component::AttributeInNoNamespaceExists { .. } | @@ -319,7 +343,6 @@ where Component::Root | Component::Empty | Component::Scope | - Component::Host(..) | Component::NthChild(..) | Component::NthLastChild(..) | Component::NthOfType(..) | @@ -327,7 +350,9 @@ where Component::FirstOfType | Component::LastOfType | Component::OnlyOfType | - Component::NonTSPseudoClass(..) => specificity.class_like_selectors += 1, + Component::NonTSPseudoClass(..) => { + specificity.class_like_selectors += 1; + }, Component::ExplicitUniversalType | Component::ExplicitAnyNamespace | Component::ExplicitNoNamespace | From 8488875a563b37ed83ce4f6283c07f3cdbcf4ad6 Mon Sep 17 00:00:00 2001 From: Gerald Squelart Date: Thu, 28 Jun 2018 14:51:38 +1000 Subject: [PATCH 23/32] style: Removed layout.css.all-shorthand.enabled pref. The 'all' shorthand has shipped a long time ago, so this pref is not needed anymore. Bug: 1459524 Reviewed-by: heycam MozReview-Commit-ID: GND8qSVAfCG --- components/style/properties/properties.mako.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 5ba3619611b..d49eabefe6e 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -178,7 +178,6 @@ pub mod shorthands { data.declare_shorthand( "all", logical_longhands + other_longhands, - gecko_pref="layout.css.all-shorthand.enabled", spec="https://drafts.csswg.org/css-cascade-3/#all-shorthand" ) %> From 6483a89848813bc3ea2c4f5b21c37e26dd887ecc Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Sat, 30 Jun 2018 01:30:37 +0300 Subject: [PATCH 24/32] if ExtendedDOMSlots are used before slots, use FatSlots to have fewer allocations. Bug: 1419661 --- components/style/gecko/wrapper.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 47205f7378b..ee0c134b269 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -651,7 +651,10 @@ impl<'le> GeckoElement<'le> { #[inline] fn extended_slots(&self) -> Option<&structs::FragmentOrElement_nsExtendedDOMSlots> { self.dom_slots().and_then(|s| unsafe { - (s._base.mExtendedSlots.mPtr as *const structs::FragmentOrElement_nsExtendedDOMSlots) + // For the bit usage, see nsContentSlots::GetExtendedSlots. + let e_slots = s._base.mExtendedSlots & + !structs::nsIContent_nsContentSlots_sNonOwningExtendedSlotsFlag; + (e_slots as *const structs::FragmentOrElement_nsExtendedDOMSlots) .as_ref() }) } From 28c9820dd96095c1561695ddd1894c2edf1a747f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 30 Jun 2018 13:45:43 +0000 Subject: [PATCH 25/32] style: Expose logical props in computed style. Bug: 1116638 Reviewed-by: xidorn Differential Revision: https://phabricator.services.mozilla.com/D1862 --- components/style/properties/properties.mako.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index d49eabefe6e..3e51c1a6c71 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -410,6 +410,7 @@ pub struct NonCustomPropertyId(usize); impl NonCustomPropertyId { #[cfg(feature = "gecko")] + #[inline] fn to_nscsspropertyid(self) -> nsCSSPropertyID { static MAP: [nsCSSPropertyID; ${len(data.longhands) + len(data.shorthands) + len(data.all_aliases())}] = [ % for property in data.longhands + data.shorthands + data.all_aliases(): @@ -1710,6 +1711,9 @@ impl PropertyId { } /// Returns a property id from Gecko's nsCSSPropertyID. + /// + /// TODO(emilio): We should be able to make this a single integer cast to + /// `NonCustomPropertyId`. #[cfg(feature = "gecko")] #[allow(non_upper_case_globals)] pub fn from_nscsspropertyid(id: nsCSSPropertyID) -> Result { From 856924f16754cf14f0b0e14e72585ecd477d8ee6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 30 Jun 2018 15:05:15 +0200 Subject: [PATCH 26/32] style: Make StyleContentType an enum class. Most of it is automated by: %s/eStyleContentType_/StyleContentType::/g %s/nsStyleContentType/StyleContentType/g But I removed some parentheses by hand. Bug: 1472443 Reviewed-by: xidorn Differential Revision: https://phabricator.services.mozilla.com/D1900 --- components/style/properties/gecko.mako.rs | 51 +++++++++++------------ 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index f83e0836c91..19a5fb99e8d 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -5489,8 +5489,7 @@ clip-path use values::generics::CounterStyleOrNone; use gecko_bindings::structs::nsStyleContentData; use gecko_bindings::structs::nsStyleContentAttr; - use gecko_bindings::structs::nsStyleContentType; - use gecko_bindings::structs::nsStyleContentType::*; + use gecko_bindings::structs::StyleContentType; use gecko_bindings::bindings::Gecko_ClearAndResizeStyleContents; // Converts a string as utf16, and returns an owned, zero-terminated raw buffer. @@ -5505,19 +5504,19 @@ clip-path fn set_counter_function( data: &mut nsStyleContentData, - content_type: nsStyleContentType, + content_type: StyleContentType, name: &CustomIdent, sep: &str, style: CounterStyleOrNone, device: &Device, ) { - debug_assert!(content_type == eStyleContentType_Counter || - content_type == eStyleContentType_Counters); + debug_assert!(content_type == StyleContentType::Counter || + content_type == StyleContentType::Counters); let counter_func = unsafe { bindings::Gecko_SetCounterFunction(data, content_type).as_mut().unwrap() }; counter_func.mIdent.assign(name.0.as_slice()); - if content_type == eStyleContentType_Counters { + if content_type == StyleContentType::Counters { counter_func.mSeparator.assign_utf8(sep); } style.to_gecko_value(&mut counter_func.mCounterStyle, device); @@ -5538,7 +5537,7 @@ clip-path Gecko_ClearAndResizeStyleContents(&mut self.gecko, 1); *self.gecko.mContents[0].mContent.mString.as_mut() = ptr::null_mut(); } - self.gecko.mContents[0].mType = eStyleContentType_AltContent; + self.gecko.mContents[0].mType = StyleContentType::AltContent; }, Content::Items(items) => { unsafe { @@ -5554,7 +5553,7 @@ clip-path } match *item { ContentItem::String(ref value) => { - self.gecko.mContents[i].mType = eStyleContentType_String; + self.gecko.mContents[i].mType = StyleContentType::String; unsafe { // NB: we share allocators, so doing this is fine. *self.gecko.mContents[i].mContent.mString.as_mut() = @@ -5562,7 +5561,7 @@ clip-path } } ContentItem::Attr(ref attr) => { - self.gecko.mContents[i].mType = eStyleContentType_Attr; + self.gecko.mContents[i].mType = StyleContentType::Attr; unsafe { // NB: we share allocators, so doing this is fine. let maybe_ns = attr.namespace.clone(); @@ -5581,17 +5580,17 @@ clip-path } } ContentItem::OpenQuote - => self.gecko.mContents[i].mType = eStyleContentType_OpenQuote, + => self.gecko.mContents[i].mType = StyleContentType::OpenQuote, ContentItem::CloseQuote - => self.gecko.mContents[i].mType = eStyleContentType_CloseQuote, + => self.gecko.mContents[i].mType = StyleContentType::CloseQuote, ContentItem::NoOpenQuote - => self.gecko.mContents[i].mType = eStyleContentType_NoOpenQuote, + => self.gecko.mContents[i].mType = StyleContentType::NoOpenQuote, ContentItem::NoCloseQuote - => self.gecko.mContents[i].mType = eStyleContentType_NoCloseQuote, + => self.gecko.mContents[i].mType = StyleContentType::NoCloseQuote, ContentItem::Counter(ref name, ref style) => { set_counter_function( &mut self.gecko.mContents[i], - eStyleContentType_Counter, + StyleContentType::Counter, &name, "", style.clone(), @@ -5601,7 +5600,7 @@ clip-path ContentItem::Counters(ref name, ref sep, ref style) => { set_counter_function( &mut self.gecko.mContents[i], - eStyleContentType_Counters, + StyleContentType::Counters, &name, &sep, style.clone(), @@ -5636,7 +5635,7 @@ clip-path pub fn clone_content(&self) -> longhands::content::computed_value::T { use {Atom, Namespace}; use gecko::conversions::string_from_chars_pointer; - use gecko_bindings::structs::nsStyleContentType::*; + use gecko_bindings::structs::StyleContentType; use values::generics::counters::{Content, ContentItem}; use values::computed::url::ComputedImageUrl; use values::{CustomIdent, Either}; @@ -5648,23 +5647,23 @@ clip-path } if self.gecko.mContents.len() == 1 && - self.gecko.mContents[0].mType == eStyleContentType_AltContent { + self.gecko.mContents[0].mType == StyleContentType::AltContent { return Content::MozAltContent; } Content::Items( self.gecko.mContents.iter().map(|gecko_content| { match gecko_content.mType { - eStyleContentType_OpenQuote => ContentItem::OpenQuote, - eStyleContentType_CloseQuote => ContentItem::CloseQuote, - eStyleContentType_NoOpenQuote => ContentItem::NoOpenQuote, - eStyleContentType_NoCloseQuote => ContentItem::NoCloseQuote, - eStyleContentType_String => { + StyleContentType::OpenQuote => ContentItem::OpenQuote, + StyleContentType::CloseQuote => ContentItem::CloseQuote, + StyleContentType::NoOpenQuote => ContentItem::NoOpenQuote, + StyleContentType::NoCloseQuote => ContentItem::NoCloseQuote, + StyleContentType::String => { let gecko_chars = unsafe { gecko_content.mContent.mString.as_ref() }; let string = unsafe { string_from_chars_pointer(*gecko_chars) }; ContentItem::String(string.into_boxed_str()) }, - eStyleContentType_Attr => { + StyleContentType::Attr => { let (namespace, attribute) = unsafe { let s = &**gecko_content.mContent.mAttr.as_ref(); let ns = if s.mNamespaceURL.mRawPtr.is_null() { @@ -5678,7 +5677,7 @@ clip-path }; ContentItem::Attr(Attr { namespace, attribute }) }, - eStyleContentType_Counter | eStyleContentType_Counters => { + StyleContentType::Counter | StyleContentType::Counters => { let gecko_function = unsafe { &**gecko_content.mContent.mCounters.as_ref() }; let ident = CustomIdent(Atom::from(&*gecko_function.mIdent)); @@ -5689,14 +5688,14 @@ clip-path Either::Second(_) => unreachable!("counter function shouldn't have single string type"), }; - if gecko_content.mType == eStyleContentType_Counter { + if gecko_content.mType == StyleContentType::Counter { ContentItem::Counter(ident, style) } else { let separator = gecko_function.mSeparator.to_string(); ContentItem::Counters(ident, separator.into_boxed_str(), style) } }, - eStyleContentType_Image => { + StyleContentType::Image => { unsafe { let gecko_image_request = &**gecko_content.mContent.mImage.as_ref(); From c2d21a84978e4e91796651f648132027f8054f06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 30 Jun 2018 15:16:31 +0200 Subject: [PATCH 27/32] style: Serialize content properties using Servo. Bug: 1472443 Reviewed-by: xidorn Differential Revision: https://phabricator.services.mozilla.com/D1901 --- components/style/properties/gecko.mako.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 19a5fb99e8d..53d0baf9aba 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -5643,7 +5643,7 @@ clip-path use values::specified::Attr; if self.gecko.mContents.is_empty() { - return Content::Normal; + return Content::None; } if self.gecko.mContents.len() == 1 && From fc5dbc1baebd15ab9f28916440d541dda7cb4222 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 30 Jun 2018 17:19:50 +0200 Subject: [PATCH 28/32] style: Remove stray newline. --- components/style/values/generics/counters.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/components/style/values/generics/counters.rs b/components/style/values/generics/counters.rs index 66da2625ffd..779d56d65ee 100644 --- a/components/style/values/generics/counters.rs +++ b/components/style/values/generics/counters.rs @@ -122,7 +122,6 @@ impl Content { pub fn normal() -> Self { Content::Normal } - } /// Items for the `content` property. From e149059608f35b1b371e20decff84b98d698ea1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 1 Jul 2018 00:40:32 +0200 Subject: [PATCH 29/32] style: Fix build / unit tests. --- components/style/servo/media_queries.rs | 2 +- tests/unit/style/Cargo.toml | 2 +- tests/unit/style/lib.rs | 14 - tests/unit/style/media_queries.rs | 398 ------------------------ 4 files changed, 2 insertions(+), 414 deletions(-) delete mode 100644 tests/unit/style/media_queries.rs diff --git a/components/style/servo/media_queries.rs b/components/style/servo/media_queries.rs index 3a8235aa65c..ba31cfcaf4b 100644 --- a/components/style/servo/media_queries.rs +++ b/components/style/servo/media_queries.rs @@ -216,7 +216,7 @@ impl MediaFeatureExpression { "width" => { ExpressionKind::Width(Range::Eq(specified::Length::parse_non_negative(context, input)?)) }, - _ => return Err(input.new_custom_error(SelectorParseErrorKind::UnexpectedIdent(name))) + _ => return Err(input.new_custom_error(SelectorParseErrorKind::UnexpectedIdent(name.clone()))) })) } diff --git a/tests/unit/style/Cargo.toml b/tests/unit/style/Cargo.toml index 23778c73f93..77b0ac34fd1 100644 --- a/tests/unit/style/Cargo.toml +++ b/tests/unit/style/Cargo.toml @@ -12,7 +12,7 @@ doctest = false [dependencies] byteorder = "1.0" app_units = "0.6" -cssparser = "0.23.0" +cssparser = "0.24.0" euclid = "0.17" html5ever = "0.22" parking_lot = "0.5" diff --git a/tests/unit/style/lib.rs b/tests/unit/style/lib.rs index 2c46bb65063..faa15845195 100644 --- a/tests/unit/style/lib.rs +++ b/tests/unit/style/lib.rs @@ -27,7 +27,6 @@ mod attr; mod custom_properties; mod keyframes; mod logical_geometry; -mod media_queries; mod parsing; mod properties; mod rule_tree; @@ -37,16 +36,3 @@ mod str; mod stylesheets; mod stylist; mod viewport; - -mod writing_modes { - use style::logical_geometry::WritingMode; - use style::properties::INITIAL_SERVO_VALUES; - - #[test] - fn initial_writing_mode_is_empty() { - assert_eq!( - WritingMode::new(INITIAL_SERVO_VALUES.get_inherited_box()), - WritingMode::empty(), - ) - } -} diff --git a/tests/unit/style/media_queries.rs b/tests/unit/style/media_queries.rs deleted file mode 100644 index 044dee3db72..00000000000 --- a/tests/unit/style/media_queries.rs +++ /dev/null @@ -1,398 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * 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 euclid::TypedScale; -use euclid::TypedSize2D; -use servo_arc::Arc; -use servo_url::ServoUrl; -use std::borrow::ToOwned; -use style::Atom; -use style::context::QuirksMode; -use style::media_queries::*; -use style::servo::media_queries::*; -use style::shared_lock::SharedRwLock; -use style::stylesheets::{AllRules, Stylesheet, StylesheetInDocument, Origin, CssRule}; -use style::values::{CustomIdent, specified}; -use style_traits::ToCss; - -fn test_media_rule(css: &str, callback: F) -where - F: Fn(&MediaList, &str), -{ - let url = ServoUrl::parse("http://localhost").unwrap(); - let css_str = css.to_owned(); - let lock = SharedRwLock::new(); - let media_list = Arc::new(lock.wrap(MediaList::empty())); - let stylesheet = Stylesheet::from_str( - css, url, Origin::Author, media_list, lock, - None, None, QuirksMode::NoQuirks, 0); - let dummy = Device::new(MediaType::screen(), TypedSize2D::new(200.0, 100.0), TypedScale::new(1.0)); - let mut rule_count = 0; - let guard = stylesheet.shared_lock.read(); - for rule in stylesheet.iter_rules::(&dummy, &guard) { - if let CssRule::Media(ref lock) = *rule { - rule_count += 1; - callback(&lock.read_with(&guard).media_queries.read_with(&guard), css); - } - } - assert!(rule_count > 0, css_str); -} - -fn media_query_test(device: &Device, css: &str, expected_rule_count: usize) { - let url = ServoUrl::parse("http://localhost").unwrap(); - let lock = SharedRwLock::new(); - let media_list = Arc::new(lock.wrap(MediaList::empty())); - let ss = Stylesheet::from_str( - css, url, Origin::Author, media_list, lock, - None, None, QuirksMode::NoQuirks, 0); - let mut rule_count = 0; - ss.effective_style_rules(device, &ss.shared_lock.read(), |_| rule_count += 1); - assert!(rule_count == expected_rule_count, css.to_owned()); -} - -#[test] -fn test_mq_empty() { - test_media_rule("@media { }", |list, css| { - assert!(list.media_queries.len() == 0, css.to_owned()); - }); -} - -#[test] -fn test_mq_screen() { - test_media_rule("@media screen { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == None, css.to_owned()); - assert!(q.media_type == MediaQueryType::Concrete(MediaType::screen()), css.to_owned()); - assert!(q.expressions.len() == 0, css.to_owned()); - }); - - test_media_rule("@media only screen { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == Some(Qualifier::Only), css.to_owned()); - assert!(q.media_type == MediaQueryType::Concrete(MediaType::screen()), css.to_owned()); - assert!(q.expressions.len() == 0, css.to_owned()); - }); - - test_media_rule("@media not screen { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == Some(Qualifier::Not), css.to_owned()); - assert!(q.media_type == MediaQueryType::Concrete(MediaType::screen()), css.to_owned()); - assert!(q.expressions.len() == 0, css.to_owned()); - }); -} - -#[test] -fn test_mq_print() { - test_media_rule("@media print { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == None, css.to_owned()); - assert!(q.media_type == MediaQueryType::Concrete(MediaType::print()), css.to_owned()); - assert!(q.expressions.len() == 0, css.to_owned()); - }); - - test_media_rule("@media only print { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == Some(Qualifier::Only), css.to_owned()); - assert!(q.media_type == MediaQueryType::Concrete(MediaType::print()), css.to_owned()); - assert!(q.expressions.len() == 0, css.to_owned()); - }); - - test_media_rule("@media not print { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == Some(Qualifier::Not), css.to_owned()); - assert!(q.media_type == MediaQueryType::Concrete(MediaType::print()), css.to_owned()); - assert!(q.expressions.len() == 0, css.to_owned()); - }); -} - -#[test] -fn test_mq_unknown() { - test_media_rule("@media fridge { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == None, css.to_owned()); - assert!(q.media_type == MediaQueryType::Concrete( - MediaType(CustomIdent(Atom::from("fridge")))), css.to_owned()); - assert!(q.expressions.len() == 0, css.to_owned()); - }); - - test_media_rule("@media only glass { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == Some(Qualifier::Only), css.to_owned()); - assert!(q.media_type == MediaQueryType::Concrete( - MediaType(CustomIdent(Atom::from("glass")))), css.to_owned()); - assert!(q.expressions.len() == 0, css.to_owned()); - }); - - test_media_rule("@media not wood { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == Some(Qualifier::Not), css.to_owned()); - assert!(q.media_type == MediaQueryType::Concrete( - MediaType(CustomIdent(Atom::from("wood")))), css.to_owned()); - assert!(q.expressions.len() == 0, css.to_owned()); - }); -} - -#[test] -fn test_mq_all() { - test_media_rule("@media all { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == None, css.to_owned()); - assert!(q.media_type == MediaQueryType::All, css.to_owned()); - assert!(q.expressions.len() == 0, css.to_owned()); - }); - - test_media_rule("@media only all { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == Some(Qualifier::Only), css.to_owned()); - assert!(q.media_type == MediaQueryType::All, css.to_owned()); - assert!(q.expressions.len() == 0, css.to_owned()); - }); - - test_media_rule("@media not all { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == Some(Qualifier::Not), css.to_owned()); - assert!(q.media_type == MediaQueryType::All, css.to_owned()); - assert!(q.expressions.len() == 0, css.to_owned()); - }); -} - -#[test] -fn test_mq_or() { - test_media_rule("@media screen, print { }", |list, css| { - assert!(list.media_queries.len() == 2, css.to_owned()); - let q0 = &list.media_queries[0]; - assert!(q0.qualifier == None, css.to_owned()); - assert!(q0.media_type == MediaQueryType::Concrete(MediaType::screen()), css.to_owned()); - assert!(q0.expressions.len() == 0, css.to_owned()); - - let q1 = &list.media_queries[1]; - assert!(q1.qualifier == None, css.to_owned()); - assert!(q1.media_type == MediaQueryType::Concrete(MediaType::print()), css.to_owned()); - assert!(q1.expressions.len() == 0, css.to_owned()); - }); -} - -#[test] -fn test_mq_default_expressions() { - test_media_rule("@media (min-width: 100px) { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == None, css.to_owned()); - assert!(q.media_type == MediaQueryType::All, css.to_owned()); - assert!(q.expressions.len() == 1, css.to_owned()); - match *q.expressions[0].kind_for_testing() { - ExpressionKind::Width(Range::Min(ref w)) => assert!(*w == specified::Length::from_px(100.)), - _ => panic!("wrong expression type"), - } - }); - - test_media_rule("@media (max-width: 43px) { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == None, css.to_owned()); - assert!(q.media_type == MediaQueryType::All, css.to_owned()); - assert!(q.expressions.len() == 1, css.to_owned()); - match *q.expressions[0].kind_for_testing() { - ExpressionKind::Width(Range::Max(ref w)) => assert!(*w == specified::Length::from_px(43.)), - _ => panic!("wrong expression type"), - } - }); -} - -#[test] -fn test_mq_expressions() { - test_media_rule("@media screen and (min-width: 100px) { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == None, css.to_owned()); - assert!(q.media_type == MediaQueryType::Concrete(MediaType::screen()), css.to_owned()); - assert!(q.expressions.len() == 1, css.to_owned()); - match *q.expressions[0].kind_for_testing() { - ExpressionKind::Width(Range::Min(ref w)) => assert!(*w == specified::Length::from_px(100.)), - _ => panic!("wrong expression type"), - } - }); - - test_media_rule("@media print and (max-width: 43px) { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == None, css.to_owned()); - assert!(q.media_type == MediaQueryType::Concrete(MediaType::print()), css.to_owned()); - assert!(q.expressions.len() == 1, css.to_owned()); - match *q.expressions[0].kind_for_testing() { - ExpressionKind::Width(Range::Max(ref w)) => assert!(*w == specified::Length::from_px(43.)), - _ => panic!("wrong expression type"), - } - }); - - test_media_rule("@media print and (width: 43px) { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == None, css.to_owned()); - assert!(q.media_type == MediaQueryType::Concrete(MediaType::print()), css.to_owned()); - assert!(q.expressions.len() == 1, css.to_owned()); - match *q.expressions[0].kind_for_testing() { - ExpressionKind::Width(Range::Eq(ref w)) => assert!(*w == specified::Length::from_px(43.)), - _ => panic!("wrong expression type"), - } - }); - - test_media_rule("@media fridge and (max-width: 52px) { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == None, css.to_owned()); - assert!(q.media_type == MediaQueryType::Concrete( - MediaType(CustomIdent(Atom::from("fridge")))), css.to_owned()); - assert!(q.expressions.len() == 1, css.to_owned()); - match *q.expressions[0].kind_for_testing() { - ExpressionKind::Width(Range::Max(ref w)) => assert!(*w == specified::Length::from_px(52.)), - _ => panic!("wrong expression type"), - } - }); -} - -#[test] -fn test_to_css() { - test_media_rule("@media print and (width: 43px) { }", |list, _| { - let q = &list.media_queries[0]; - let dest = q.to_css_string(); - assert_eq!(dest, "print and (width: 43px)"); - }); -} - -#[test] -fn test_mq_multiple_expressions() { - test_media_rule("@media (min-width: 100px) and (max-width: 200px) { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == None, css.to_owned()); - assert!(q.media_type == MediaQueryType::All, css.to_owned()); - assert!(q.expressions.len() == 2, css.to_owned()); - match *q.expressions[0].kind_for_testing() { - ExpressionKind::Width(Range::Min(ref w)) => assert!(*w == specified::Length::from_px(100.)), - _ => panic!("wrong expression type"), - } - match *q.expressions[1].kind_for_testing() { - ExpressionKind::Width(Range::Max(ref w)) => assert!(*w == specified::Length::from_px(200.)), - _ => panic!("wrong expression type"), - } - }); - - test_media_rule("@media not screen and (min-width: 100px) and (max-width: 200px) { }", |list, css| { - assert!(list.media_queries.len() == 1, css.to_owned()); - let q = &list.media_queries[0]; - assert!(q.qualifier == Some(Qualifier::Not), css.to_owned()); - assert!(q.media_type == MediaQueryType::Concrete(MediaType::screen()), css.to_owned()); - assert!(q.expressions.len() == 2, css.to_owned()); - match *q.expressions[0].kind_for_testing() { - ExpressionKind::Width(Range::Min(ref w)) => assert!(*w == specified::Length::from_px(100.)), - _ => panic!("wrong expression type"), - } - match *q.expressions[1].kind_for_testing() { - ExpressionKind::Width(Range::Max(ref w)) => assert!(*w == specified::Length::from_px(200.)), - _ => panic!("wrong expression type"), - } - }); -} - -#[test] -fn test_mq_malformed_expressions() { - fn check_malformed_expr(list: &MediaList, css: &str) { - assert!(!list.media_queries.is_empty(), css.to_owned()); - for mq in &list.media_queries { - assert!(mq.qualifier == Some(Qualifier::Not), css.to_owned()); - assert!(mq.media_type == MediaQueryType::All, css.to_owned()); - assert!(mq.expressions.is_empty(), css.to_owned()); - } - } - - for rule in &[ - "@media (min-width: 100blah) and (max-width: 200px) { }", - "@media screen and (height: 200px) { }", - "@media (min-width: 30em foo bar) {}", - "@media not {}", - "@media not (min-width: 300px) {}", - "@media , {}", - ] { - test_media_rule(rule, check_malformed_expr); - } -} - -#[test] -fn test_matching_simple() { - let device = Device::new(MediaType::screen(), TypedSize2D::new(200.0, 100.0), TypedScale::new(1.0)); - - media_query_test(&device, "@media not all { a { color: red; } }", 0); - media_query_test(&device, "@media not screen { a { color: red; } }", 0); - media_query_test(&device, "@media not print { a { color: red; } }", 1); - - media_query_test(&device, "@media unknown { a { color: red; } }", 0); - media_query_test(&device, "@media not unknown { a { color: red; } }", 1); - - media_query_test(&device, "@media { a { color: red; } }", 1); - media_query_test(&device, "@media screen { a { color: red; } }", 1); - media_query_test(&device, "@media print { a { color: red; } }", 0); -} - -#[test] -fn test_matching_width() { - let device = Device::new(MediaType::screen(), TypedSize2D::new(200.0, 100.0), TypedScale::new(1.0)); - - media_query_test(&device, "@media { a { color: red; } }", 1); - - media_query_test(&device, "@media (min-width: 50px) { a { color: red; } }", 1); - media_query_test(&device, "@media (min-width: 150px) { a { color: red; } }", 1); - media_query_test(&device, "@media (min-width: 300px) { a { color: red; } }", 0); - - media_query_test(&device, "@media screen and (min-width: 50px) { a { color: red; } }", 1); - media_query_test(&device, "@media screen and (min-width: 150px) { a { color: red; } }", 1); - media_query_test(&device, "@media screen and (min-width: 300px) { a { color: red; } }", 0); - - media_query_test(&device, "@media not screen and (min-width: 50px) { a { color: red; } }", 0); - media_query_test(&device, "@media not screen and (min-width: 150px) { a { color: red; } }", 0); - media_query_test(&device, "@media not screen and (min-width: 300px) { a { color: red; } }", 1); - - media_query_test(&device, "@media (max-width: 50px) { a { color: red; } }", 0); - media_query_test(&device, "@media (max-width: 150px) { a { color: red; } }", 0); - media_query_test(&device, "@media (max-width: 300px) { a { color: red; } }", 1); - - media_query_test(&device, "@media screen and (min-width: 50px) and (max-width: 100px) { a { color: red; } }", 0); - media_query_test(&device, "@media screen and (min-width: 250px) and (max-width: 300px) { a { color: red; } }", 0); - media_query_test(&device, "@media screen and (min-width: 50px) and (max-width: 250px) { a { color: red; } }", 1); - - media_query_test( - &device, "@media not screen and (min-width: 50px) and (max-width: 100px) { a { color: red; } }", 1); - media_query_test( - &device, "@media not screen and (min-width: 250px) and (max-width: 300px) { a { color: red; } }", 1); - media_query_test( - &device, "@media not screen and (min-width: 50px) and (max-width: 250px) { a { color: red; } }", 0); - - media_query_test( - &device, "@media not screen and (min-width: 3.1em) and (max-width: 6em) { a { color: red; } }", 1); - media_query_test( - &device, "@media not screen and (min-width: 16em) and (max-width: 19.75em) { a { color: red; } }", 1); - media_query_test( - &device, "@media not screen and (min-width: 3em) and (max-width: 250px) { a { color: red; } }", 0); -} - -#[test] -fn test_matching_invalid() { - let device = Device::new(MediaType::screen(), TypedSize2D::new(200.0, 100.0), TypedScale::new(1.0)); - - media_query_test(&device, "@media fridge { a { color: red; } }", 0); - media_query_test(&device, "@media screen and (height: 100px) { a { color: red; } }", 0); - media_query_test(&device, "@media not print and (width: 100) { a { color: red; } }", 0); -} From 719209316cbd6ab21351621231837c35b4e2c97b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 1 Jul 2018 00:40:45 +0200 Subject: [PATCH 30/32] Fix servo build. --- Cargo.lock | 28 +++++++++---------- components/canvas/Cargo.toml | 2 +- components/canvas_traits/Cargo.toml | 2 +- components/layout/display_list/builder.rs | 11 ++------ components/script/Cargo.toml | 2 +- .../dom/webidls/CSSStyleDeclaration.webidl | 8 ++++++ components/script_layout_interface/Cargo.toml | 2 +- 7 files changed, 28 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c322fc2329f..ec694c3ddc0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -271,7 +271,7 @@ dependencies = [ "azure 0.29.0 (git+https://github.com/servo/rust-azure)", "canvas_traits 0.0.1", "compositing 0.0.1", - "cssparser 0.23.10 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.17.3 (registry+https://github.com/rust-lang/crates.io-index)", "fnv 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)", "gleam 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -289,7 +289,7 @@ dependencies = [ name = "canvas_traits" version = "0.0.1" dependencies = [ - "cssparser 0.23.10 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.17.3 (registry+https://github.com/rust-lang/crates.io-index)", "gleam 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", "ipc-channel 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -561,7 +561,7 @@ dependencies = [ [[package]] name = "cssparser" -version = "0.23.10" +version = "0.24.0" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "cssparser-macros 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", @@ -569,12 +569,12 @@ dependencies = [ "itoa 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", "matches 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", "phf 0.7.21 (registry+https://github.com/rust-lang/crates.io-index)", - "proc-macro2 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", + "proc-macro2 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "procedural-masquerade 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)", - "quote 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", + "quote 0.6.3 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.66 (registry+https://github.com/rust-lang/crates.io-index)", "smallvec 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", - "syn 0.13.1 (registry+https://github.com/rust-lang/crates.io-index)", + "syn 0.14.2 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -1610,7 +1610,7 @@ name = "malloc_size_of" version = "0.0.1" dependencies = [ "app_units 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)", - "cssparser 0.23.10 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.17.3 (registry+https://github.com/rust-lang/crates.io-index)", "hashglobe 0.1.0", "hyper 0.10.13 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2470,7 +2470,7 @@ dependencies = [ "chrono 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", "cmake 0.1.29 (registry+https://github.com/rust-lang/crates.io-index)", "cookie 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)", - "cssparser 0.23.10 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)", "deny_public_fields 0.0.1", "devtools_traits 0.0.1", "dom_struct 0.0.1", @@ -2547,7 +2547,7 @@ dependencies = [ "app_units 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)", "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "canvas_traits 0.0.1", - "cssparser 0.23.10 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.17.3 (registry+https://github.com/rust-lang/crates.io-index)", "gfx_traits 0.0.1", "html5ever 0.22.3 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2629,7 +2629,7 @@ name = "selectors" version = "0.19.0" dependencies = [ "bitflags 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", - "cssparser 0.23.10 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)", "fnv 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", "matches 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2967,7 +2967,7 @@ dependencies = [ "bitflags 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "byteorder 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "cfg-if 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", - "cssparser 0.23.10 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)", "encoding_rs 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.17.3 (registry+https://github.com/rust-lang/crates.io-index)", "fallible 0.0.1", @@ -3028,7 +3028,7 @@ version = "0.0.1" dependencies = [ "app_units 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)", "byteorder 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", - "cssparser 0.23.10 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.17.3 (registry+https://github.com/rust-lang/crates.io-index)", "html5ever 0.22.3 (registry+https://github.com/rust-lang/crates.io-index)", "parking_lot 0.5.4 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3050,7 +3050,7 @@ version = "0.0.1" dependencies = [ "app_units 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)", "bitflags 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", - "cssparser 0.23.10 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.17.3 (registry+https://github.com/rust-lang/crates.io-index)", "malloc_size_of 0.0.1", "malloc_size_of_derive 0.0.1", @@ -3760,7 +3760,7 @@ dependencies = [ "checksum crossbeam-deque 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "f739f8c5363aca78cfb059edf753d8f0d36908c348f3d8d1503f03d8b75d9cf3" "checksum crossbeam-epoch 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "927121f5407de9956180ff5e936fe3cf4324279280001cd56b669d28ee7e9150" "checksum crossbeam-utils 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "2760899e32a1d58d5abb31129f8fae5de75220bc2176e77ff7c627ae45c918d9" -"checksum cssparser 0.23.10 (registry+https://github.com/rust-lang/crates.io-index)" = "8f1c74d99b0f489cc546336b911452562ebfd4aec034b0c526cb77a3a02d3790" +"checksum cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)" = "495beddc39b1987b8e9f029354eccbd5ef88eb5f1cd24badb764dce338acf2e0" "checksum cssparser-macros 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "f3a5383ae18dbfdeb569ed62019f5bddb2a95cd2d3833313c475a0d014777805" "checksum darling 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "2a78af487e4eb8f4421a1770687b328af6bb4494ca93435210678c6eea875c11" "checksum darling_core 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b315f49c7b6db3708bca6e6913c194581a44ec619b7a39e131d4dd63733a3698" diff --git a/components/canvas/Cargo.toml b/components/canvas/Cargo.toml index 2c2a928e233..e0a6db7fb2c 100644 --- a/components/canvas/Cargo.toml +++ b/components/canvas/Cargo.toml @@ -13,7 +13,7 @@ path = "lib.rs" azure = {git = "https://github.com/servo/rust-azure"} canvas_traits = {path = "../canvas_traits"} compositing = {path = "../compositing"} -cssparser = "0.23.0" +cssparser = "0.24" euclid = "0.17" fnv = "1.0" gleam = "0.5" diff --git a/components/canvas_traits/Cargo.toml b/components/canvas_traits/Cargo.toml index 226acf76dff..8ec09cadbdd 100644 --- a/components/canvas_traits/Cargo.toml +++ b/components/canvas_traits/Cargo.toml @@ -10,7 +10,7 @@ name = "canvas_traits" path = "lib.rs" [dependencies] -cssparser = "0.23.0" +cssparser = "0.24.0" euclid = "0.17" ipc-channel = "0.10" gleam = "0.5.1" diff --git a/components/layout/display_list/builder.rs b/components/layout/display_list/builder.rs index 3887f141f52..9d0de5c3f41 100644 --- a/components/layout/display_list/builder.rs +++ b/components/layout/display_list/builder.rs @@ -1219,11 +1219,7 @@ impl FragmentDisplayListBuilding for Fragment { state.add_display_item(DisplayItem::BoxShadow(Box::new(BoxShadowDisplayItem { base: base, box_bounds: absolute_bounds.to_layout(), - color: box_shadow - .base - .color - .unwrap_or(style.get_color().color) - .to_layout(), + color: style.resolve_color(box_shadow.base.color).to_layout(), offset: LayoutVector2D::new( box_shadow.base.horizontal.px(), box_shadow.base.vertical.px(), @@ -2038,10 +2034,7 @@ impl FragmentDisplayListBuilding for Fragment { base: base.clone(), shadow: webrender_api::Shadow { offset: LayoutVector2D::new(shadow.horizontal.px(), shadow.vertical.px()), - color: shadow - .color - .unwrap_or(self.style().get_color().color) - .to_layout(), + color: self.style.resolve_color(shadow.color).to_layout(), blur_radius: shadow.blur.px(), }, }, diff --git a/components/script/Cargo.toml b/components/script/Cargo.toml index 40dc8d99be7..7f30c842d34 100644 --- a/components/script/Cargo.toml +++ b/components/script/Cargo.toml @@ -37,7 +37,7 @@ canvas_traits = {path = "../canvas_traits"} caseless = "0.2" cookie = "0.10" chrono = "0.4" -cssparser = "0.23.0" +cssparser = "0.24" deny_public_fields = {path = "../deny_public_fields"} devtools_traits = {path = "../devtools_traits"} dom_struct = {path = "../dom_struct"} diff --git a/components/script/dom/webidls/CSSStyleDeclaration.webidl b/components/script/dom/webidls/CSSStyleDeclaration.webidl index 49377cd1044..2841f3e9016 100644 --- a/components/script/dom/webidls/CSSStyleDeclaration.webidl +++ b/components/script/dom/webidls/CSSStyleDeclaration.webidl @@ -350,6 +350,14 @@ partial interface CSSStyleDeclaration { [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString offsetInlineStart; [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString offset-inline-end; [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString offsetInlineEnd; + [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString inset-block-start; + [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString insetBlockStart; + [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString inset-block-end; + [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString insetBlockEnd; + [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString inset-inline-start; + [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString insetInlineStart; + [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString inset-inline-end; + [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString insetInlineEnd; [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString height; [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString minHeight; diff --git a/components/script_layout_interface/Cargo.toml b/components/script_layout_interface/Cargo.toml index 4af1f6237c8..f06ed32526e 100644 --- a/components/script_layout_interface/Cargo.toml +++ b/components/script_layout_interface/Cargo.toml @@ -13,7 +13,7 @@ path = "lib.rs" app_units = "0.6" atomic_refcell = "0.1" canvas_traits = {path = "../canvas_traits"} -cssparser = "0.23.0" +cssparser = "0.24" euclid = "0.17" gfx_traits = {path = "../gfx_traits"} html5ever = "0.22" From 8e5c853fcabe3f4c15a23d3485649c235fce658c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 1 Jul 2018 01:01:46 +0200 Subject: [PATCH 31/32] style: Appease tidy. --- components/style/element_state.rs | 2 +- components/style/media_queries/media_condition.rs | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/components/style/element_state.rs b/components/style/element_state.rs index c57cf4febe6..1652dee672f 100644 --- a/components/style/element_state.rs +++ b/components/style/element_state.rs @@ -71,7 +71,7 @@ bitflags! { const IN_OPTIONAL_STATE = 1 << 22; /// const IN_READ_WRITE_STATE = 1 << 22; - /// + /// const IN_DEFINED_STATE = 1 << 23; /// const IN_VISITED_STATE = 1 << 24; diff --git a/components/style/media_queries/media_condition.rs b/components/style/media_queries/media_condition.rs index e56f02ff071..4b80794af39 100644 --- a/components/style/media_queries/media_condition.rs +++ b/components/style/media_queries/media_condition.rs @@ -6,17 +6,16 @@ //! //! https://drafts.csswg.org/mediaqueries-4/#typedef-media-condition -use cssparser::{Parser, Token}; use context::QuirksMode; +use cssparser::{Parser, Token}; use parser::ParserContext; use std::fmt::{self, Write}; use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; - use super::{Device, MediaFeatureExpression}; /// A binary `and` or `or` operator. -#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, Parse, ToCss)] +#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, Parse, PartialEq, ToCss)] #[allow(missing_docs)] pub enum Operator { And, @@ -24,7 +23,7 @@ pub enum Operator { } /// Whether to allow an `or` condition or not during parsing. -#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, Parse, ToCss)] +#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToCss)] enum AllowOr { Yes, No, From 36495d677f6e5e928c74f8eb372caaae7a303047 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 1 Jul 2018 10:41:19 +0200 Subject: [PATCH 32/32] Remove unused code for parsing a `sizes` attribute. We have `SourceSizeList` in components/style/values/specified/source_size_list.rs which does the job and is tested via WPT in Gecko. --- Cargo.lock | 1 - components/script/dom/htmlimageelement.rs | 70 ---------------- components/script/test.rs | 4 - tests/unit/script/Cargo.toml | 1 - tests/unit/script/htmlimageelement.rs | 97 ----------------------- tests/unit/script/lib.rs | 1 - 6 files changed, 174 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ec694c3ddc0..81d74f5088f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2590,7 +2590,6 @@ dependencies = [ "msg 0.0.1", "script 0.0.1", "servo_url 0.0.1", - "style 0.0.1", ] [[package]] diff --git a/components/script/dom/htmlimageelement.rs b/components/script/dom/htmlimageelement.rs index 61acecda71d..b37199ca0db 100644 --- a/components/script/dom/htmlimageelement.rs +++ b/components/script/dom/htmlimageelement.rs @@ -3,7 +3,6 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use app_units::{Au, AU_PER_PX}; -use cssparser::{Parser, ParserInput}; use document_loader::{LoadType, LoadBlocker}; use dom::activation::Activatable; use dom::attr::Attr; @@ -58,13 +57,7 @@ use std::default::Default; use std::i32; use std::sync::{Arc, Mutex}; use style::attr::{AttrValue, LengthOrPercentageOrAuto, parse_double, parse_unsigned_integer}; -use style::context::QuirksMode; -use style::media_queries::MediaQuery; -use style::parser::ParserContext; use style::str::is_ascii_digit; -use style::values::specified::{Length, ViewportPercentageLength}; -use style::values::specified::length::NoCalcLength; -use style_traits::ParsingMode; use task_source::TaskSource; enum ParseState { @@ -94,12 +87,6 @@ enum State { Broken, } -#[derive(Debug, PartialEq)] -pub struct Size { - pub query: Option, - pub length: Length, -} - #[derive(Clone, Copy, JSTraceable, MallocSizeOf)] enum ImageRequestPhase { Pending, @@ -758,63 +745,6 @@ impl LayoutHTMLImageElementHelpers for LayoutDom { } } -//https://html.spec.whatwg.org/multipage/#parse-a-sizes-attribute -pub fn parse_a_sizes_attribute(input: DOMString, width: Option) -> Vec { - let mut sizes = Vec::::new(); - for unparsed_size in input.split(',') { - let whitespace = unparsed_size.chars().rev().take_while(|c| char::is_whitespace(*c)).count(); - let trimmed: String = unparsed_size.chars().take(unparsed_size.chars().count() - whitespace).collect(); - - if trimmed.is_empty() { - continue; - } - let mut input = ParserInput::new(&trimmed); - let url = ServoUrl::parse("about:blank").unwrap(); - let context = ParserContext::new_for_cssom( - &url, - None, - ParsingMode::empty(), - QuirksMode::NoQuirks, - None, - ); - let mut parser = Parser::new(&mut input); - let length = parser.try(|i| Length::parse_non_negative(&context, i)); - match length { - Ok(len) => sizes.push(Size { - length: len, - query: None - }), - Err(_) => { - let mut media_query_parser = parser; - let media_query = media_query_parser.try(|i| MediaQuery::parse(&context, i)); - if let Ok(query) = media_query { - let length = Length::parse_non_negative(&context, &mut media_query_parser); - if let Ok(length) = length { - sizes.push(Size { - length: length, - query: Some(query) - }) - } - } - }, - } - } - if sizes.is_empty() { - let size = match width { - Some(w) => Size { - length: Length::from_px(w as f32), - query: None - }, - None => Size { - length: Length::NoCalc(NoCalcLength::ViewportPercentage(ViewportPercentageLength::Vw(100.))), - query: None - }, - }; - sizes.push(size); - } - sizes -} - impl HTMLImageElementMethods for HTMLImageElement { // https://html.spec.whatwg.org/multipage/#dom-img-alt make_getter!(Alt, "alt"); diff --git a/components/script/test.rs b/components/script/test.rs index 351c3447688..b907404da0b 100644 --- a/components/script/test.rs +++ b/components/script/test.rs @@ -15,10 +15,6 @@ pub mod area { pub use dom::htmlareaelement::{Area, Shape}; } -pub mod sizes { - pub use dom::htmlimageelement::{parse_a_sizes_attribute, Size}; -} - pub mod size_of { use dom::characterdata::CharacterData; use dom::element::Element; diff --git a/tests/unit/script/Cargo.toml b/tests/unit/script/Cargo.toml index f6743a7dba6..dc78113136f 100644 --- a/tests/unit/script/Cargo.toml +++ b/tests/unit/script/Cargo.toml @@ -13,4 +13,3 @@ euclid = "0.17" msg = {path = "../../../components/msg"} script = {path = "../../../components/script"} servo_url = {path = "../../../components/url"} -style = {path = "../../../components/style"} diff --git a/tests/unit/script/htmlimageelement.rs b/tests/unit/script/htmlimageelement.rs index 6d62f25a500..796e63c2e2b 100644 --- a/tests/unit/script/htmlimageelement.rs +++ b/tests/unit/script/htmlimageelement.rs @@ -2,104 +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 script::test::DOMString; -use script::test::sizes::{parse_a_sizes_attribute, Size}; use script::test::srcset::{Descriptor, ImageSource, parse_a_srcset_attribute}; -use style::media_queries::{MediaQuery, MediaQueryType}; -use style::media_queries::Expression; -use style::servo::media_queries::{ExpressionKind, Range}; -use style::values::specified::{Length, NoCalcLength, AbsoluteLength, ViewportPercentageLength}; - -pub fn test_length_for_no_default_provided(len: f32) -> Length { - let length = Length::NoCalc(NoCalcLength::ViewportPercentage(ViewportPercentageLength::Vw(len))); - return length; -} - -#[test] -fn no_default_provided() { - let mut a = vec![]; - let length = test_length_for_no_default_provided(100f32); - let size = Size { query: None, length: length }; - a.push(size); - assert_eq!(parse_a_sizes_attribute(DOMString::new(), None), a); -} - -pub fn test_length_for_default_provided(len: f32) -> Length { - let length = Length::NoCalc(NoCalcLength::Absolute(AbsoluteLength::Px(len))); - return length; -} - -#[test] -fn default_provided() { - let mut a = vec![]; - let length = test_length_for_default_provided(2f32); - let size = Size { query: None, length: length }; - a.push(size); - assert_eq!(parse_a_sizes_attribute(DOMString::new(), Some(2)), a); -} - -pub fn test_media_query(len: f32) -> MediaQuery { - let length = Length::NoCalc(NoCalcLength::Absolute(AbsoluteLength::Px(len))); - let expr = Expression(ExpressionKind::Width(Range::Max(length))); - let media_query = MediaQuery { - qualifier: None, - media_type: MediaQueryType::All, - expressions: vec![expr] - }; - media_query -} - -pub fn test_length(len: f32) -> Length { - let length = Length::NoCalc(NoCalcLength::Absolute(AbsoluteLength::Px(len))); - return length; -} - -#[test] -fn one_value() { - let mut a = vec![]; - let media_query = test_media_query(200f32); - let length = test_length(545f32); - let size = Size { query: Some(media_query), length: length }; - a.push(size); - assert_eq!(parse_a_sizes_attribute(DOMString::from("(max-width: 200px) 545px"), None), a); -} - -#[test] -fn more_then_one_value() { - let media_query = test_media_query(900f32); - let length = test_length(1000f32); - let size = Size { query: Some(media_query), length: length }; - let media_query1 = test_media_query(900f32); - let length1 = test_length(50f32); - let size1 = Size { query: Some(media_query1), length: length1 }; - let a = &[size, size1]; - assert_eq!(parse_a_sizes_attribute(DOMString::from("(max-width: 900px) 1000px, (max-width: 900px) 50px"), - None), a); -} - -#[test] -fn no_extra_whitespace() { - let mut a = vec![]; - let media_query = test_media_query(200f32); - let length = test_length(545f32); - let size = Size { query: Some(media_query), length: length }; - a.push(size); - assert_eq!(parse_a_sizes_attribute(DOMString::from("(max-width: 200px) 545px"), None), a); -} - -#[test] -fn extra_whitespace() { - let media_query = test_media_query(900f32); - let length = test_length(1000f32); - let size = Size { query: Some(media_query), length: length }; - let media_query1 = test_media_query(900f32); - let length1 = test_length(50f32); - let size1 = Size { query: Some(media_query1), length: length1 }; - let a = &[size, size1]; - assert_eq!(parse_a_sizes_attribute( - DOMString::from("(max-width: 900px) 1000px, (max-width: 900px) 50px"), - None), a); -} #[test] fn no_value() { diff --git a/tests/unit/script/lib.rs b/tests/unit/script/lib.rs index 6d724db5358..069417a5911 100644 --- a/tests/unit/script/lib.rs +++ b/tests/unit/script/lib.rs @@ -6,7 +6,6 @@ #[cfg(test)] extern crate msg; #[cfg(test)] extern crate script; #[cfg(test)] extern crate servo_url; -#[cfg(test)] extern crate style; #[cfg(test)] mod origin; #[cfg(all(test, target_pointer_width = "64"))] mod size_of;