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 8bdf38d4f847..dfd819bcedb6 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 1b01a86457a3..7f6b4cd9d433 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 d0463365fc24..a201ec0549e3 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 3d804d426059..07ac6441b6c5 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 f10b53fa7881..c3304b05af0a 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 771f4570b473..fd07595f4582 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 fd07595f4582..02fc864eb093 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 10fa84bd61a5..03e1e2de1087 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 8da14fc67e5b..8c0722265a79 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 336971f98df9..14dec24b2618 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 596c2f73951f..f0a0a47e4a96 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 | 235 +++++++++--------- .../style/media_queries/media_condition.rs | 109 ++++++++ components/style/media_queries/mod.rs | 2 + components/style/servo/media_queries.rs | 43 ++-- 4 files changed, 259 insertions(+), 130 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 02fc864eb093..a6322842f200 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; - { - 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; - } + Self::parse_in_parenthesis_block(context, input) + }) + } - 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; - } - } + /// 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 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 mut flags = 0; + + if context.chrome_rules_enabled() || context.stylesheet_origin == Origin::UserAgent + { + flags |= structs::nsMediaFeature_RequirementFlags_eUserAgentAndChromeOnly; + } - 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 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 }; - match result { - Ok((f, r)) => { - feature = f; - range = r; - }, - Err(()) => { - return Err(location.new_custom_error( - StyleParseErrorKind::MediaQueryExpectedFeatureName(ident.clone()), - )) - }, + 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(()), } + }; - if (feature.mReqFlags & !flags) != 0 { + match result { + Ok((f, r)) => { + feature = f; + range = r; + }, + Err(()) => { 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()), - )); - } + if (feature.mReqFlags & !flags) != 0 { + 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 - )); - } + if range.is_some() && + feature.mRangeType != nsMediaFeature_RangeType::eMinMaxAllowed + { + return Err(location.new_custom_error( + StyleParseErrorKind::MediaQueryExpectedFeatureName(ident.clone()), + )); + } + } - return Ok(Self::new(feature, None, None)); + 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 + )); } - 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)) + 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 + )); } - None => { - match operator { - Some(operator) => { - if !feature_allows_ranges { - return Err(input.new_custom_error( - StyleParseErrorKind::MediaQueryExpectedFeatureValue - )); - } - Some(RangeOrOperator::Operator(operator)) + 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 000000000000..cb3bd73ee200 --- /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 8c0722265a79..d27e33cc64c1 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 14dec24b2618..3a8235aa65c4 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 cb3bd73ee200..091d7049599f 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 091d7049599f..c2c29c129ca3 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 a5604b6dc19c..f8d15df7257d 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 03e1e2de1087..a2f4f9bea706 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 f0a0a47e4a96..d41fc283bcc6 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 | 42 +++++++------------ components/style_traits/lib.rs | 4 +- 4 files changed, 29 insertions(+), 57 deletions(-) diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index a6322842f200..9c5ef4d71a11 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 c2c29c129ca3..e56f02ff0712 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 a2f4f9bea706..089fc9412b24 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), - }) - } - - 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()) - )) - } + ) -> 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 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 b1eaf4e086fe..737450b5e059 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 3b9b6f201848..3b0dcca95252 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 b6a8879e106a..b27527d0132c 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 0ce4a45d175c..ffac86e974f7 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 c3591ff2727f..7c4812cb963c 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 | 123 +++++++++++++++++---------------- 1 file changed, 62 insertions(+), 61 deletions(-) diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index 3210f1085b00..ac5390451159 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; - }, - (_, _) => (), - } + // 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; - } + // 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 ac5390451159..399f21e6361b 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 95bb16229428..68d160ab5e08 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 8ce16e2ccf9c..c70a46c1c9d2 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 a0cb5fa0b1d8..85dc00bb9af9 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 41d87aa07a48..d2f9b691ad4e 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 92376a6cd61a..67718409311f 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 ee38f2ef2763..19adcdd15782 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 4fb50d8ad85f..b80b7572bbad 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 67a6c851dcea..9b2734727741 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 b27527d0132c..5ba3619611b4 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 b16c9918a241..dff9319cb30c 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 842dfbd72296..1425f0128546 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 a53f899851cb..75095bcebc23 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 aa73c77e46b3..03027359dd4a 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 18a0786494d8..e83d0b67544c 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 11a1283f1cc0..e07c9bd57cb7 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 a99cebfdba99..d7e4e27a3b74 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 d7e4e27a3b74..3b34047fc5ad 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 0960ca3d4e11..c57cf4febe6a 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 b02e21fbf8a8..56e62b6de4bc 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 57a98a89238b..7dfc9aa1bbc0 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 ffac86e974f7..47205f7378bb 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 56e62b6de4bc..04f57b73885d 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 0be3eb90e0cc..29ac822fa033 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 5ba3619611b4..d49eabefe6ed 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 47205f7378bb..ee0c134b269c 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 d49eabefe6ed..3e51c1a6c711 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 f83e0836c911..19a5fb99e8d8 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 19a5fb99e8d8..53d0baf9aba0 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 66da2625ffdd..779d56d65ee9 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 3a8235aa65c4..ba31cfcaf4b3 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 23778c73f931..77b0ac34fd19 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 2c46bb65063b..faa158451956 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 044dee3db727..000000000000 --- 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 c322fc2329f7..ec694c3ddc06 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 2c2a928e233b..e0a6db7fb2c6 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 226acf76dff5..8ec09cadbdd6 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 3887f141f521..9d0de5c3f41d 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 40dc8d99be7d..7f30c842d34a 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 49377cd10441..2841f3e9016c 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 4af1f6237c8b..f06ed32526ec 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 c57cf4febe6a..1652dee672fd 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 e56f02ff0712..4b80794af397 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 ec694c3ddc06..81d74f5088f1 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 61acecda71d5..b37199ca0dbc 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 351c3447688b..b907404da0b8 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 f6743a7dba6a..dc78113136f9 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 6d62f25a500b..796e63c2e2b1 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 6d724db5358f..069417a5911b 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;