From c99d6adf01d7ccaf309a921e8da7b80cb0195f32 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Sat, 19 Sep 2020 14:28:51 +0000
Subject: [PATCH] style: Consider display: none elements as having current
 style for animation-only traversal.

This is mostly a band-aid, though it also serves sorta as an
optimization.

The issue here is basically bug 1393323. By re-cascading, right now we
can't come up with the right before-change style if CSSOM has mutated
the rules. We really need a better way to come up with the before-change
style, as the animation-only traversal is not really sustainable (nor
fast, for that matter...).

But this avoids crashing and prevents the regression easily, so let's do
that for now.

Differential Revision: https://phabricator.services.mozilla.com/D90700
---
 components/style/dom.rs | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/components/style/dom.rs b/components/style/dom.rs
index 60e1be054d6..351f54af6e3 100644
--- a/components/style/dom.rs
+++ b/components/style/dom.rs
@@ -584,14 +584,35 @@ pub trait TElement:
         traversal_flags: TraversalFlags,
     ) -> bool {
         if traversal_flags.for_animation_only() {
-            // In animation-only restyle we never touch snapshots and don't
-            // care about them. But we can't assert '!self.handled_snapshot()'
+            // In animation-only restyle we never touch snapshots and don't care
+            // about them. But we can't assert '!self.handled_snapshot()'
             // here since there are some cases that a second animation-only
             // restyle which is a result of normal restyle (e.g. setting
             // animation-name in normal restyle and creating a new CSS
             // animation in a SequentialTask) is processed after the normal
             // traversal in that we had elements that handled snapshot.
-            return data.has_styles() && !data.hint.has_animation_hint_or_recascade();
+            if !data.has_styles() {
+                return false;
+            }
+
+            if !data.hint.has_animation_hint_or_recascade() {
+                return true;
+            }
+
+            // FIXME: This should ideally always return false, but it is a hack
+            // to work around our weird animation-only traversal
+            // stuff: If we're display: none and the rules we could match could
+            // change, we consider our style up-to-date. This is because
+            // re-cascading with and old style doesn't guarantee returning the
+            // correct animation style (that's bug 1393323). So if our display
+            // changed, and it changed from display: none, we would incorrectly
+            // forget about it and wouldn't be able to correctly style our
+            // descendants later.
+            if data.styles.is_display_none() && data.hint.match_self() {
+                return true;
+            }
+
+            return false;
         }
 
         if self.has_snapshot() && !self.handled_snapshot() {