Bug 1422225: Rename Expression to MediaFeatureExpression. r?xidorn draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 15 Jun 2018 21:29:57 -0700
changeset 809283 632611f94be481742bfd821955de113b53635f4c
parent 809282 bcca03533116e0ee752e4d8a54263085aa599976
child 809284 47af58e28f4050ab6864ff8538067dfebfb78c0a
push id113617
push userbmo:emilio@crisal.io
push dateThu, 21 Jun 2018 17:44:14 +0000
reviewersxidorn
bugs1422225
milestone62.0a1
Bug 1422225: Rename Expression to MediaFeatureExpression. r?xidorn Which is more appropriate, given it represents a `<media-feature>` per spec, and expression is a bit overloaded :) MozReview-Commit-ID: Fed1nJhHxDu
servo/components/style/gecko/media_queries.rs
servo/components/style/media_queries/media_query.rs
servo/components/style/media_queries/mod.rs
servo/components/style/servo/media_queries.rs
servo/components/style/values/specified/source_size_list.rs
--- a/servo/components/style/gecko/media_queries.rs
+++ b/servo/components/style/gecko/media_queries.rs
@@ -266,26 +266,26 @@ impl ToCss for 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
-/// 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<MediaExpressionValue>,
     range_or_operator: Option<RangeOrOperator>,
 }
 
-impl ToCss for Expression {
+impl ToCss for MediaFeatureExpression {
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
     where
         W: fmt::Write,
     {
         dest.write_str("(")?;
 
         if (self.feature.mReqFlags & structs::nsMediaFeature_RequirementFlags_eHasWebkitPrefix) != 0
         {
@@ -315,18 +315,18 @@ impl ToCss for Expression {
         if let Some(ref val) = self.value {
             val.to_css(dest, self)?;
         }
 
         dest.write_str(")")
     }
 }
 
-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
     }
 }
 
 /// A value found or expected in a media expression.
 ///
 /// FIXME(emilio): How should calc() serialize in the Number / Integer /
@@ -353,17 +353,20 @@ pub enum MediaExpressionValue {
     /// An enumerated value, defined by the variant keyword table in the
     /// feature's `mData` member.
     Enumerated(i16),
     /// An identifier.
     Ident(Atom),
 }
 
 impl MediaExpressionValue {
-    fn from_css_value(for_expr: &Expression, css_value: &nsCSSValue) -> Option<Self> {
+    fn from_css_value(
+        for_expr: &MediaFeatureExpression,
+        css_value: &nsCSSValue,
+    ) -> Option<Self> {
         // NB: If there's a null value, that means that we don't support the
         // feature.
         if css_value.mUnit == nsCSSUnit::eCSSUnit_Null {
             return None;
         }
 
         match for_expr.feature.mValueType {
             nsMediaFeature_ValueType::eLength => {
@@ -411,17 +414,17 @@ impl MediaExpressionValue {
                 debug_assert!(first >= 0 && second >= 0);
                 Some(MediaExpressionValue::IntRatio(first as u32, second as u32))
             },
         }
     }
 }
 
 impl MediaExpressionValue {
-    fn to_css<W>(&self, dest: &mut CssWriter<W>, for_expr: &Expression) -> fmt::Result
+    fn to_css<W>(&self, dest: &mut CssWriter<W>, for_expr: &MediaFeatureExpression) -> fmt::Result
     where
         W: fmt::Write,
     {
         match *self {
             MediaExpressionValue::Length(ref l) => l.to_css(dest),
             MediaExpressionValue::Integer(v) => v.to_css(dest),
             MediaExpressionValue::Float(v) => v.to_css(dest),
             MediaExpressionValue::BoolInteger(v) => dest.write_str(if v { "1" } else { "0" }),
@@ -589,17 +592,17 @@ fn consume_operation_or_colon(
             } else {
                 Operator::LessThan
             }
         }
         _ => return Err(()),
     }))
 }
 
-impl Expression {
+impl MediaFeatureExpression {
     /// Trivially construct a new expression.
     fn new(
         feature: &'static nsMediaFeature,
         value: Option<MediaExpressionValue>,
         range_or_operator: Option<RangeOrOperator>,
     ) -> Self {
         Self {
             feature,
@@ -720,21 +723,17 @@ impl Expression {
                     // 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(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(
@@ -759,17 +758,17 @@ impl Expression {
             };
 
             let value =
                 parse_feature_value(feature, feature.mValueType, context, input).map_err(|err| {
                     err.location
                         .new_custom_error(StyleParseErrorKind::MediaQueryExpectedFeatureValue)
                 })?;
 
-            Ok(Expression::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.
     pub fn matches(&self, device: &Device, quirks_mode: QuirksMode) -> bool {
         let mut css_value = nsCSSValue::null();
         unsafe {
             (self.feature.mGetter.unwrap())(
--- a/servo/components/style/media_queries/media_query.rs
+++ b/servo/components/style/media_queries/media_query.rs
@@ -8,17 +8,17 @@
 
 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, StyleParseErrorKind, ToCss};
-use super::Expression;
+use super::MediaFeatureExpression;
 use values::CustomIdent;
 
 /// <https://drafts.csswg.org/mediaqueries/#mq-prefix>
 #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, Parse, PartialEq, ToCss)]
 pub enum Qualifier {
     /// Hide a media query from legacy UAs:
     /// <https://drafts.csswg.org/mediaqueries/#mq-only>
     Only,
@@ -60,18 +60,18 @@ impl MediaType {
 ///
 /// [mq]: https://drafts.csswg.org/mediaqueries/
 #[derive(Clone, Debug, MallocSizeOf, PartialEq)]
 pub struct MediaQuery {
     /// The qualifier for this query.
     pub qualifier: Option<Qualifier>,
     /// 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<Expression>,
+    /// The set of range expressions that this media query contains.
+    pub expressions: Vec<MediaFeatureExpression>,
 }
 
 impl ToCss for MediaQuery {
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
     where
         W: Write,
     {
         if let Some(qual) = self.qualifier {
@@ -138,17 +138,17 @@ impl MediaQuery {
             })?,
             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(Expression::parse(context, input)?);
+                expressions.push(MediaFeatureExpression::parse(context, input)?);
 
                 MediaQueryType::All
             },
         };
 
         // Parse any subsequent expressions
         loop {
             if input
@@ -156,17 +156,17 @@ impl MediaQuery {
                 .is_err()
             {
                 return Ok(MediaQuery {
                     qualifier,
                     media_type,
                     expressions,
                 });
             }
-            expressions.push(Expression::parse(context, input)?)
+            expressions.push(MediaFeatureExpression::parse(context, input)?)
         }
     }
 }
 
 /// <http://dev.w3.org/csswg/mediaqueries-3/#media0>
 #[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq)]
 pub enum MediaQueryType {
     /// A media type that matches every device.
--- a/servo/components/style/media_queries/mod.rs
+++ b/servo/components/style/media_queries/mod.rs
@@ -8,11 +8,11 @@
 
 mod media_list;
 mod media_query;
 
 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};
--- a/servo/components/style/servo/media_queries.rs
+++ b/servo/components/style/servo/media_queries.rs
@@ -165,19 +165,19 @@ pub enum ExpressionKind {
     Width(Range<specified::Length>),
 }
 
 /// A single expression a per:
 ///
 /// <http://dev.w3.org/csswg/mediaqueries-3/#media1>
 #[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.
     pub fn kind_for_testing(&self) -> &ExpressionKind {
         &self.0
     }
 
     /// Parse a media expression of the form:
@@ -191,27 +191,27 @@ impl Expression {
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
         input.expect_parenthesis_block()?;
         input.parse_nested_block(|input| {
             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)?))
                 },
                 "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.clone())))
+                _ => 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 {
         let viewport_size = device.au_viewport_size();
@@ -223,17 +223,17 @@ impl Expression {
                     Range::Max(ref width) => value <= *width,
                     Range::Eq(ref width) => value == *width,
                 }
             },
         }
     }
 }
 
-impl ToCss for Expression {
+impl ToCss for MediaFeatureExpression {
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
     where
         W: Write,
     {
         let (s, l) = match self.0 {
             ExpressionKind::Width(Range::Min(ref l)) => ("(min-width: ", l),
             ExpressionKind::Width(Range::Max(ref l)) => ("(max-width: ", l),
             ExpressionKind::Width(Range::Eq(ref l)) => ("(width: ", l),
@@ -241,18 +241,18 @@ impl ToCss for Expression {
         dest.write_str(s)?;
         l.to_css(dest)?;
         dest.write_char(')')
     }
 }
 
 /// 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<T> {
     /// At least the inner value.
     Min(T),
     /// At most the inner value.
     Max(T),
     /// Exactly the inner value.
--- a/servo/components/style/values/specified/source_size_list.rs
+++ b/servo/components/style/values/specified/source_size_list.rs
@@ -3,39 +3,39 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! https://html.spec.whatwg.org/multipage/#source-size-list
 
 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;
 use values::computed::{self, ToComputedValue};
 use values::specified::{Length, NoCalcLength, ViewportPercentageLength};
 
 /// A value for a `<source-size>`:
 ///
 /// https://html.spec.whatwg.org/multipage/#source-size
 pub struct SourceSize {
     // FIXME(emilio): This should be a `MediaCondition`, and support `and` and
     // `or`.
-    condition: MediaExpression,
+    condition: MediaFeatureExpression,
     value: Length,
 }
 
 impl Parse for SourceSize {
     fn parse<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
-        let condition = MediaExpression::parse(context, input)?;
+        let condition = MediaFeatureExpression::parse(context, input)?;
         let value = Length::parse_non_negative(context, input)?;
 
         Ok(Self { condition, value })
     }
 }
 
 /// A value for a `<source-size-list>`:
 ///