Bug 1396057: Support calc() in media queries. r?xidorn draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 10 Nov 2017 16:49:25 +0100
changeset 696389 0907bf5a6ce74a82a159d20b7ba63a474ed97746
parent 696388 b6f7d52ecc3bd49506e41ab8826e1434a8197375
child 739845 6bb053f4056a7bf1990e0c006ac765e16de11659
push id88699
push userbmo:emilio@crisal.io
push dateFri, 10 Nov 2017 15:51:18 +0000
reviewersxidorn
bugs1396057
milestone58.0a1
Bug 1396057: Support calc() in media queries. r?xidorn MozReview-Commit-ID: 6YbeqInC5Mw
servo/components/style/gecko/media_queries.rs
--- a/servo/components/style/gecko/media_queries.rs
+++ b/servo/components/style/gecko/media_queries.rs
@@ -13,31 +13,31 @@ use euclid::Size2D;
 use gecko::values::{convert_nscolor_to_rgba, convert_rgba_to_nscolor};
 use gecko_bindings::bindings;
 use gecko_bindings::structs;
 use gecko_bindings::structs::{nsCSSKeyword, nsCSSProps_KTableEntry, nsCSSValue, nsCSSUnit};
 use gecko_bindings::structs::{nsMediaExpression_Range, nsMediaFeature};
 use gecko_bindings::structs::{nsMediaFeature_ValueType, nsMediaFeature_RangeType};
 use gecko_bindings::structs::{nsPresContext, RawGeckoPresContextOwned};
 use media_queries::MediaType;
-use parser::ParserContext;
+use parser::{Parse, ParserContext};
 use properties::ComputedValues;
 use servo_arc::Arc;
 use std::fmt::{self, Write};
 use std::sync::atomic::{AtomicBool, AtomicIsize, AtomicUsize, Ordering};
 use str::starts_with_ignore_ascii_case;
 use string_cache::Atom;
 use style_traits::{CSSPixel, DevicePixel};
 use style_traits::{ToCss, ParseError, StyleParseErrorKind};
 use style_traits::viewport::ViewportConstraints;
 use stylesheets::Origin;
 use values::{CSSFloat, CustomIdent, serialize_dimension};
 use values::computed::{self, ToComputedValue};
 use values::computed::font::FontSize;
-use values::specified::Length;
+use values::specified::{Integer, Length, Number};
 
 /// The `Device` in Gecko wraps a pres context, has a default values computed,
 /// and contains all the viewport rule state.
 pub struct Device {
     /// NB: The pres context lifetime is tied to the styleset, who owns the
     /// stylist, and thus the `Device`, so having a raw pres context pointer
     /// here is fine.
     pres_context: RawGeckoPresContextOwned,
@@ -311,16 +311,23 @@ impl ToCss for Resolution {
             Resolution::Dpi(v) => serialize_dimension(v, "dpi", dest),
             Resolution::Dppx(v) => serialize_dimension(v, "dppx", dest),
             Resolution::Dpcm(v) => serialize_dimension(v, "dpcm", dest),
         }
     }
 }
 
 /// A value found or expected in a media expression.
+///
+/// FIXME(emilio): How should calc() serialize in the Number / Integer /
+/// BoolInteger / IntRatio case, as computed or as specified value?
+///
+/// If the first, this would need to store the relevant values.
+///
+/// See: https://github.com/w3c/csswg-drafts/issues/1968
 #[derive(Clone, Debug, PartialEq)]
 pub enum MediaExpressionValue {
     /// A length.
     Length(Length),
     /// A (non-negative) integer.
     Integer(u32),
     /// A floating point value.
     Float(CSSFloat),
@@ -490,58 +497,39 @@ fn parse_feature_value<'i, 't>(
     feature: &nsMediaFeature,
     feature_value_type: nsMediaFeature_ValueType,
     context: &ParserContext,
     input: &mut Parser<'i, 't>,
 ) -> Result<MediaExpressionValue, ParseError<'i>> {
     let value = match feature_value_type {
         nsMediaFeature_ValueType::eLength => {
             let length = Length::parse_non_negative(context, input)?;
-            // FIXME(canaltinova): See bug 1396057. Gecko doesn't support calc
-            // inside media queries. This check is for temporarily remove it
-            // for parity with gecko. We should remove this check when we want
-            // to support it.
-            if let Length::Calc(_) = length {
-                return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
-            }
             MediaExpressionValue::Length(length)
         },
         nsMediaFeature_ValueType::eInteger => {
-            // FIXME(emilio): We should use `Integer::parse` to handle `calc`
-            // properly in integer expressions. Note that calc is still not
-            // supported in media queries per FIXME above.
-            let i = input.expect_integer()?;
-            if i < 0 {
-                return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
-            }
-            MediaExpressionValue::Integer(i as u32)
+            let integer = Integer::parse_non_negative(context, input)?;
+            MediaExpressionValue::Integer(integer.value() as u32)
         }
         nsMediaFeature_ValueType::eBoolInteger => {
-            let i = input.expect_integer()?;
-            if i < 0 || i > 1 {
+            let integer = Integer::parse_non_negative(context, input)?;
+            let value = integer.value();
+            if value > 1 {
                 return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
             }
-            MediaExpressionValue::BoolInteger(i == 1)
+            MediaExpressionValue::BoolInteger(value == 1)
         }
         nsMediaFeature_ValueType::eFloat => {
-            MediaExpressionValue::Float(input.expect_number()?)
+            let number = Number::parse(context, input)?;
+            MediaExpressionValue::Float(number.get())
         }
         nsMediaFeature_ValueType::eIntRatio => {
-            let a = input.expect_integer()?;
-            if a <= 0 {
-                return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
-            }
-
+            let a = Integer::parse_positive(context, input)?;
             input.expect_delim('/')?;
-
-            let b = input.expect_integer()?;
-            if b <= 0 {
-                return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
-            }
-            MediaExpressionValue::IntRatio(a as u32, b as u32)
+            let b = Integer::parse_positive(context, input)?;
+            MediaExpressionValue::IntRatio(a.value() as u32, b.value() as u32)
         }
         nsMediaFeature_ValueType::eResolution => {
             MediaExpressionValue::Resolution(Resolution::parse(input)?)
         }
         nsMediaFeature_ValueType::eEnumerated => {
             let location = input.current_source_location();
             let keyword = input.expect_ident()?;
             let keyword = unsafe {