servo: Merge #17792 - Fix supports rule parsing issues with <any-value> (from upsuper:supports-any-value); r=SimonSapin
authorXidorn Quan <me@upsuper.org>
Thu, 20 Jul 2017 13:08:58 -0700
changeset 418762 4e8a7ad99f9f2377aa2a226e3a94550d9edfb607
parent 418761 a216d7ca2e9a2e9863d1c6f31f1966c0aa7c9a33
child 418763 90bb95e8afc8b9ce2d1c3f2c9675d8d0b171e3d2
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersSimonSapin
bugs17792, 15482, 883987
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #17792 - Fix supports rule parsing issues with <any-value> (from upsuper:supports-any-value); r=SimonSapin This eventually fixes #15482, as well as several reftests in mozilla-central which were added for [bug 883987](https://bugzilla.mozilla.org/show_bug.cgi?id=883987). The new function should probably be moved into cssparser crate at some point. Source-Repo: https://github.com/servo/servo Source-Revision: e19fefcb474ea6593a684a1ca4ce616e61188ff0
servo/components/style/stylesheets/supports_rule.rs
servo/tests/unit/style/parsing/supports.rs
--- a/servo/components/style/stylesheets/supports_rule.rs
+++ b/servo/components/style/stylesheets/supports_rule.rs
@@ -123,31 +123,28 @@ impl SupportsCondition {
     /// https://drafts.csswg.org/css-conditional-3/#supports_condition_in_parens
     fn parse_in_parens<'i, 't>(input: &mut Parser<'i, 't>) -> Result<SupportsCondition, ParseError<'i>> {
         // Whitespace is normally taken care of in `Parser::next`,
         // but we want to not include it in `pos` for the SupportsCondition::FutureSyntax cases.
         while input.try(Parser::expect_whitespace).is_ok() {}
         let pos = input.position();
         match input.next()? {
             Token::ParenthesisBlock => {
-                input.parse_nested_block(|input| {
-                    // `input.try()` not needed here since the alternative uses `consume_all()`.
-                    parse_condition_or_declaration(input).or_else(|_| {
-                        consume_all(input);
-                        Ok(SupportsCondition::FutureSyntax(input.slice_from(pos).to_owned()))
-                    })
-                })
+                let nested = input.try(|input| {
+                    input.parse_nested_block(|i| parse_condition_or_declaration(i))
+                });
+                if nested.is_ok() {
+                    return nested;
+                }
             }
-            Token::Function(_) => {
-                let result: Result<_, ParseError> = input.parse_nested_block(|i| Ok(consume_all(i)));
-                result.unwrap();
-                Ok(SupportsCondition::FutureSyntax(input.slice_from(pos).to_owned()))
-            }
-            t => Err(CssParseError::Basic(BasicParseError::UnexpectedToken(t)))
+            Token::Function(_) => {}
+            t => return Err(CssParseError::Basic(BasicParseError::UnexpectedToken(t))),
         }
+        input.parse_nested_block(|i| consume_any_value(i))?;
+        Ok(SupportsCondition::FutureSyntax(input.slice_from(pos).to_owned()))
     }
 
     /// Evaluate a supports condition
     pub fn eval(&self, cx: &ParserContext) -> bool {
         match *self {
             SupportsCondition::Not(ref cond) => !cond.eval(cx),
             SupportsCondition::Parenthesized(ref cond) => cond.eval(cx),
             SupportsCondition::And(ref vec) => vec.iter().all(|c| c.eval(cx)),
@@ -230,35 +227,29 @@ impl ToCss for Declaration {
     {
         dest.write_str(&self.prop)?;
         dest.write_str(":")?;
         // no space, the `val` already contains any possible spaces
         dest.write_str(&self.val)
     }
 }
 
-/// Slurps up input till exhausted, return string from source position
-fn parse_anything(input: &mut Parser) -> String {
-    let pos = input.position();
-    consume_all(input);
-    input.slice_from(pos).to_owned()
-}
-
-/// Consume input till done
-fn consume_all(input: &mut Parser) {
-    while let Ok(_) = input.next() {}
+/// https://drafts.csswg.org/css-syntax-3/#typedef-any-value
+fn consume_any_value<'i, 't>(input: &mut Parser<'i, 't>) -> Result<(), ParseError<'i>> {
+    input.expect_no_error_token().map_err(|err| err.into())
 }
 
 impl Declaration {
     /// Parse a declaration
     pub fn parse<'i, 't>(input: &mut Parser<'i, 't>) -> Result<Declaration, ParseError<'i>> {
         let prop = input.expect_ident()?.into_owned();
         input.expect_colon()?;
-        let val = parse_anything(input);
-        Ok(Declaration { prop: prop, val: val })
+        let pos = input.position();
+        consume_any_value(input)?;
+        Ok(Declaration { prop: prop, val: input.slice_from(pos).to_owned() })
     }
 
     /// Determine if a declaration parses
     ///
     /// https://drafts.csswg.org/css-conditional-3/#support-definition
     pub fn eval(&self, cx: &ParserContext) -> bool {
         let id = if let Ok(id) = PropertyId::parse((&*self.prop).into()) {
             id
--- a/servo/tests/unit/style/parsing/supports.rs
+++ b/servo/tests/unit/style/parsing/supports.rs
@@ -6,9 +6,11 @@ use cssparser::{Parser, ParserInput};
 use style::stylesheets::supports_rule::SupportsCondition;
 use style_traits::ToCss;
 
 #[test]
 fn test_supports_condition() {
     assert_roundtrip!(SupportsCondition::parse, "(margin: 1px)");
     assert_roundtrip!(SupportsCondition::parse, "not (--be: to be)");
     assert_roundtrip!(SupportsCondition::parse, "(color: blue) and future-extension(4)");
+    assert_roundtrip!(SupportsCondition::parse, "future-\\1 extension(4)");
+    assert_roundtrip!(SupportsCondition::parse, "((test))");
 }