From f82930075127419481380997b69bcb895a872938 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Fri, 1 Jun 2018 17:11:00 +0200
Subject: [PATCH] style: Don't look at the rule type from value parsing.

This would cause properties to change the value semantics between, e.g.,
@keyframes and non-@keyframes, which would be observable.

It happens not to be observable since the animation-* and transition-*
properties are not allowed in @keyframes, nor have bits in `contain`, and none
of the two properties are allowed in @page. But I think it's the right thing to
do.

This still causes a quirk like a property value in chrome / user origins being
potentially different if the value is specified via CSS var functions. But I
think that is fine.

Bug: 1466136
Reviewed-by: hiro
MozReview-Commit-ID: GhoPt0I34oO
---
 .../style/properties/properties.mako.rs       | 43 ++++++++++++++++++-
 components/style/values/specified/box.rs      |  4 +-
 2 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index b7bc96973b3..38245b707f6 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -499,6 +499,11 @@ impl NonCustomPropertyId {
             _ => {}
         }
 
+        self.allowed_in_ignoring_rule_type(context)
+    }
+
+
+    fn allowed_in_ignoring_rule_type(self, context: &ParserContext) -> bool {
         // The semantics of these are kinda hard to reason about, what follows
         // is a description of the different combinations that can happen with
         // these three sets.
@@ -1423,11 +1428,17 @@ impl UnparsedValue {
             // As of this writing, only the base URL is used for property
             // values.
             //
-            // FIXME(emilio): These bits are slightly fishy.
+            // NOTE(emilio): we intentionally pase `None` as the rule type here.
+            // If something starts depending on it, it's probably a bug, since
+            // it'd change how values are parsed depending on whether we're in a
+            // @keyframes rule or not, for example... So think twice about
+            // whether you want to do this!
+            //
+            // FIXME(emilio): ParsingMode is slightly fishy...
             let context = ParserContext::new(
                 Origin::Author,
                 &self.url_data,
-                Some(CssRuleType::Style),
+                None,
                 ParsingMode::DEFAULT,
                 quirks_mode,
             );
@@ -1674,6 +1685,24 @@ impl PropertyId {
         Ok(id)
     }
 
+    /// Parses a property name, and returns an error if it's unknown or isn't
+    /// allowed in this context, ignoring the rule_type checks.
+    ///
+    /// This is useful for parsing stuff from CSS values, for example.
+    #[inline]
+    pub fn parse_ignoring_rule_type(
+        name: &str,
+        context: &ParserContext,
+    ) -> Result<Self, ()> {
+        let id = Self::parse_unchecked(name)?;
+
+        if !id.allowed_in_ignoring_rule_type(context) {
+            return Err(());
+        }
+
+        Ok(id)
+    }
+
     /// Returns a property id from Gecko's nsCSSPropertyID.
     #[cfg(feature = "gecko")]
     #[allow(non_upper_case_globals)]
@@ -1787,6 +1816,16 @@ impl PropertyId {
         id.allowed_in(context)
     }
 
+    #[inline]
+    fn allowed_in_ignoring_rule_type(&self, context: &ParserContext) -> bool {
+        let id = match self.non_custom_id() {
+            // Custom properties are allowed everywhere
+            None => return true,
+            Some(id) => id,
+        };
+        id.allowed_in_ignoring_rule_type(context)
+    }
+
     /// Whether the property supports the given CSS type.
     /// `ty` should a bitflags of constants in style_traits::CssType.
     pub fn supports_type(&self, ty: u8) -> bool {
diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs
index 4890f225e6d..020c8112ad1 100644
--- a/components/style/values/specified/box.rs
+++ b/components/style/values/specified/box.rs
@@ -465,7 +465,7 @@ fn change_bits_for_longhand(longhand: LonghandId) -> WillChangeBits {
 }
 
 fn change_bits_for_maybe_property(ident: &str, context: &ParserContext) -> WillChangeBits {
-    let id = match PropertyId::parse(ident, context) {
+    let id = match PropertyId::parse_ignoring_rule_type(ident, context) {
         Ok(id) => id,
         Err(..) => return WillChangeBits::empty(),
     };
@@ -777,7 +777,7 @@ impl Parse for TransitionProperty {
         let location = input.current_source_location();
         let ident = input.expect_ident()?;
 
-        let id = match PropertyId::parse(&ident, context) {
+        let id = match PropertyId::parse_ignoring_rule_type(&ident, context) {
             Ok(id) => id,
             Err(..) => return Ok(TransitionProperty::Unsupported(
                 CustomIdent::from_ident(location, ident, &["none"])?,