From 5ba2e874b19c4de8be87644529747496519ac761 Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Wed, 21 Feb 2018 19:33:59 +0900 Subject: [PATCH 1/4] Factor out a new function to check display property is changed from 'none' to other --- components/style/matching.rs | 9 +-------- components/style/properties/gecko.mako.rs | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/components/style/matching.rs b/components/style/matching.rs index 89b60c9905a..82db5ded7ae 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -176,14 +176,7 @@ trait PrivateMatchMethods: TElement { return; } - let display_changed_from_none = old_values.map_or(false, |old| { - let old_display_style = old.get_box().clone_display(); - let new_display_style = new_values.get_box().clone_display(); - old_display_style == Display::None && - new_display_style != Display::None - }); - - if display_changed_from_none { + if new_values.is_display_property_changed_from_none(old_values) { // When display value is changed from none to other, we need to // traverse descendant elements in a subsequent normal // traversal (we can't traverse them in this animation-only restyle diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 11b8ac5210a..aea141323ce 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -144,6 +144,22 @@ impl ComputedValues { return our_type == CSSPseudoElementType_InheritingAnonBox || our_type == CSSPseudoElementType::NonInheritingAnonBox; } + + /// Returns true if the display property is changed from 'none' to others. + pub fn is_display_property_changed_from_none( + &self, + old_values: Option<<&ComputedValues> + ) -> bool { + use properties::longhands::display::computed_value::T as Display; + + old_values.map_or(false, |old| { + let old_display_style = old.get_box().clone_display(); + let new_display_style = self.get_box().clone_display(); + old_display_style == Display::None && + new_display_style != Display::None + }) + } + } impl Drop for ComputedValues { From 6bba9d4dab7143d77db5becad78f8f23f8bc201b Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Wed, 21 Feb 2018 19:34:48 +0900 Subject: [PATCH 2/4] Add a new sequential task flag for display propery changes from 'none' Unlike CSS animations/transitions, script animations keep alive on display:none elements, so once the display property was changed to others in normal styling, we need to do styling for the script animations in the second animation traversal. Otherwise, the styling for the script animations will be deferred to the next frame. --- components/style/context.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/components/style/context.rs b/components/style/context.rs index b173aad03eb..337afd65ceb 100644 --- a/components/style/context.rs +++ b/components/style/context.rs @@ -419,6 +419,11 @@ bitflags! { const EFFECT_PROPERTIES = structs::UpdateAnimationsTasks_EffectProperties; /// Update animation cacade results for animations running on the compositor. const CASCADE_RESULTS = structs::UpdateAnimationsTasks_CascadeResults; + /// Display property was changed from none. + /// Script animations keep alive on display:none elements, so we need to trigger + /// the second animation restyles for the script animations in the case where + /// the display property was changed from 'none' to others. + const DISPLAY_CHANGED_FROM_NONE = structs::UpdateAnimationsTasks_DisplayChangedFromNone; } } From 42b0ab77f42fe8a8122a73668b6ed6fdcbe7040b Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Wed, 21 Feb 2018 19:36:16 +0900 Subject: [PATCH 3/4] Post DISPLAY_CHANGED_FROM_NONE sequential task if the display property is changed from 'none' in the case the element has script animations --- components/style/matching.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/components/style/matching.rs b/components/style/matching.rs index 82db5ded7ae..594717cc52a 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -259,6 +259,9 @@ trait PrivateMatchMethods: TElement { if important_rules_changed { tasks.insert(UpdateAnimationsTasks::CASCADE_RESULTS); } + if new_values.is_display_property_changed_from_none(old_values.as_ref().map(|s| &**s)) { + tasks.insert(UpdateAnimationsTasks::DISPLAY_CHANGED_FROM_NONE); + } } if !tasks.is_empty() { From 2559cfd4042c21e67a05b516a4f7b5cd188f170a Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Thu, 22 Feb 2018 07:54:32 +0900 Subject: [PATCH 4/4] Update bindings. --- components/style/gecko/generated/structs.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/components/style/gecko/generated/structs.rs b/components/style/gecko/generated/structs.rs index 6187943584f..38e0d1f9d1b 100644 --- a/components/style/gecko/generated/structs.rs +++ b/components/style/gecko/generated/structs.rs @@ -5642,6 +5642,7 @@ pub mod root { pub const UpdateAnimationsTasks_CSSTransitions: root::mozilla::UpdateAnimationsTasks = 2; pub const UpdateAnimationsTasks_EffectProperties: root::mozilla::UpdateAnimationsTasks = 4; pub const UpdateAnimationsTasks_CascadeResults: root::mozilla::UpdateAnimationsTasks = 8; + pub const UpdateAnimationsTasks_DisplayChangedFromNone: root::mozilla::UpdateAnimationsTasks = 16; pub type UpdateAnimationsTasks = u8; pub const ParsingMode_Default: root::mozilla::ParsingMode = 0; pub const ParsingMode_AllowUnitlessLength: root::mozilla::ParsingMode = 1;