From a940999795671e20f41ab0df3f5347735e8138d6 Mon Sep 17 00:00:00 2001
From: Hiroyuki Ikezoe <hikezoe@mozilla.com>
Date: Wed, 20 Sep 2017 15:58:39 +0900
Subject: [PATCH] Filter out !important property in keyframes for stylo.

---
 .../style/properties/declaration_block.rs     | 39 +++++++++++++++++++
 .../style/stylesheets/keyframes_rule.rs       | 10 +++--
 ports/geckolib/glue.rs                        |  5 +--
 3 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs
index 15573cc6aefa..231972396bce 100644
--- a/components/style/properties/declaration_block.rs
+++ b/components/style/properties/declaration_block.rs
@@ -112,6 +112,40 @@ impl<'a> DoubleEndedIterator for DeclarationImportanceIterator<'a> {
     }
 }
 
+/// Iterator over `PropertyDeclaration` for Importance::Normal.
+pub struct NormalDeclarationIterator<'a>(DeclarationImportanceIterator<'a>);
+
+impl<'a> NormalDeclarationIterator<'a> {
+    /// Constructor
+    pub fn new(declarations: &'a [PropertyDeclaration], important: &'a SmallBitVec) -> Self {
+        NormalDeclarationIterator(
+            DeclarationImportanceIterator::new(declarations, important)
+        )
+    }
+}
+
+impl<'a> Iterator for NormalDeclarationIterator<'a> {
+    type Item = &'a PropertyDeclaration;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        loop {
+            let next = self.0.iter.next();
+            match next {
+                Some((decl, importance)) => {
+                    if !importance {
+                        return Some(decl);
+                    }
+                },
+                None => return None,
+            }
+        }
+    }
+
+    fn size_hint(&self) -> (usize, Option<usize>) {
+        self.0.iter.size_hint()
+    }
+}
+
 /// Iterator for AnimationValue to be generated from PropertyDeclarationBlock.
 pub struct AnimationValueIterator<'a, 'cx, 'cx_a:'cx> {
     iter: DeclarationImportanceIterator<'a>,
@@ -208,6 +242,11 @@ impl PropertyDeclarationBlock {
         DeclarationImportanceIterator::new(&self.declarations, &self.declarations_importance)
     }
 
+    /// Iterate over `PropertyDeclaration` for Importance::Normal
+    pub fn normal_declaration_iter(&self) -> NormalDeclarationIterator {
+        NormalDeclarationIterator::new(&self.declarations, &self.declarations_importance)
+    }
+
     /// Return an iterator of (AnimatableLonghand, AnimationValue).
     pub fn to_animation_value_iter<'a, 'cx, 'cx_a:'cx>(&'a self,
                                                        context: &'cx mut Context<'cx_a>,
diff --git a/components/style/stylesheets/keyframes_rule.rs b/components/style/stylesheets/keyframes_rule.rs
index 226b72bd560f..bd01a64529bc 100644
--- a/components/style/stylesheets/keyframes_rule.rs
+++ b/components/style/stylesheets/keyframes_rule.rs
@@ -375,9 +375,13 @@ fn get_animated_properties(keyframes: &[Arc<Locked<Keyframe>>], guard: &SharedRw
     for keyframe in keyframes {
         let keyframe = keyframe.read_with(&guard);
         let block = keyframe.block.read_with(guard);
-        for (declaration, importance) in block.declaration_importance_iter() {
-            assert!(!importance.important());
-
+        // CSS Animations spec clearly defines that properties with !important
+        // in keyframe rules are invalid and ignored, but it's still ambiguous
+        // whether we should drop the !important properties or retain the
+        // properties when they are set via CSSOM. So we assume there might
+        // be properties with !important in keyframe rules here.
+        // See the spec issue https://github.com/w3c/csswg-drafts/issues/1824
+        for declaration in block.normal_declaration_iter() {
             if let Some(property) = AnimatableLonghand::from_declaration(declaration) {
                 // Skip the 'display' property because although it is animatable from SMIL,
                 // it should not be animatable from CSS Animations or Web Animations.
diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs
index 92735be556cd..aa0cb73a2036 100644
--- a/ports/geckolib/glue.rs
+++ b/ports/geckolib/glue.rs
@@ -3596,10 +3596,9 @@ pub extern "C" fn Servo_StyleSet_GetKeyframesForName(raw_data: RawServoStyleSetB
             },
             KeyframesStepValue::Declarations { ref block } => {
                 let guard = block.read_with(&guard);
-                // Filter out non-animatable properties.
+                // Filter out non-animatable properties and properties with !important.
                 let animatable =
-                    guard.declarations()
-                         .iter()
+                    guard.normal_declaration_iter()
                          .filter(|declaration| declaration.is_animatable());
 
                 for declaration in animatable {