Bug 1371518 - Make TransitionProperty treat all properties that are not transitionable as unsupported; r?hiro
Currently properties that are discretely animated cannot be transitioned. Now
that TransitionProperty should only be used for transitions, we can redefine it
to treat non-transitionable properties as unsupported. This should allow us to
simplify the code and make it more self-documenting (e.g. making
TransitionProperty actually relate to transitions).
MozReview-Commit-ID: 17NZjhyLq4B
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -1030,17 +1030,16 @@ impl<'le> TElement for GeckoElement<'le>
// update.
//
// https://drafts.csswg.org/css-transitions/#starting
fn needs_transitions_update(&self,
before_change_style: &ComputedValues,
after_change_style: &ComputedValues)
-> bool {
use gecko_bindings::structs::nsCSSPropertyID;
- use properties::{PropertyId, animated_properties};
use std::collections::HashSet;
debug_assert!(self.might_need_transitions_update(Some(before_change_style),
after_change_style),
"We should only call needs_transitions_update if \
might_need_transitions_update returns true");
let after_change_box_style = after_change_style.get_box();
@@ -1065,52 +1064,53 @@ impl<'le> TElement for GeckoElement<'le>
let property = after_change_box_style.transition_nscsspropertyid_at(i);
let combined_duration = after_change_box_style.transition_combined_duration_at(i);
// We don't need to update transition for none/custom properties.
if is_none_or_custom_property(property) {
continue;
}
+ let transition_property: TransitionProperty = property.into();
+
let mut property_check_helper = |property: &TransitionProperty| -> bool {
if self.needs_transitions_update_per_property(property,
combined_duration,
before_change_style,
after_change_style,
&existing_transitions) {
return true;
}
if let Some(set) = transitions_to_keep.as_mut() {
// The TransitionProperty here must be animatable, so cloning it is cheap
// because it is an integer-like enum.
set.insert(property.clone());
}
false
};
- if property == nsCSSPropertyID::eCSSPropertyExtra_all_properties {
- if TransitionProperty::any(property_check_helper) {
- return true;
- }
- } else {
- let is_shorthand = PropertyId::from_nscsspropertyid(property).ok().map_or(false, |p| {
- p.as_shorthand().is_ok()
- });
- if is_shorthand {
- let shorthand: TransitionProperty = property.into();
+
+ match transition_property {
+ TransitionProperty::All => {
+ if TransitionProperty::any(property_check_helper) {
+ return true;
+ }
+ },
+ TransitionProperty::Unsupported(_) => { },
+ ref shorthand if shorthand.is_shorthand() => {
if shorthand.longhands().iter().any(|p| property_check_helper(p)) {
return true;
}
- } else {
- if animated_properties::nscsspropertyid_is_animatable(property) &&
- property_check_helper(&property.into()) {
+ },
+ ref longhand => {
+ if property_check_helper(longhand) {
return true;
}
- }
- }
+ },
+ };
}
// Check if we have to cancel the running transition because this is not a matching
// transition-property value.
transitions_to_keep.map_or(false, |set| {
existing_transitions.keys().any(|property| !set.contains(property))
})
}
@@ -1123,21 +1123,16 @@ impl<'le> TElement for GeckoElement<'le>
existing_transitions: &HashMap<TransitionProperty,
Arc<AnimationValue>>)
-> bool {
use properties::animated_properties::AnimatedProperty;
// |property| should be an animatable longhand
let animatable_longhand = AnimatableLonghand::from_transition_property(property).unwrap();
- // We don't allow transitions on properties that are not interpolable.
- if animatable_longhand.is_discrete() {
- return false;
- }
-
if existing_transitions.contains_key(property) {
// If there is an existing transition, update only if the end value differs.
// If the end value has not changed, we should leave the currently running
// transition as-is since we don't want to interrupt its timing function.
let after_value =
Arc::new(AnimationValue::from_computed_values(&animatable_longhand,
after_change_style));
return existing_transitions.get(property).unwrap() != &after_value;
--- a/servo/components/style/properties/data.py
+++ b/servo/components/style/properties/data.py
@@ -189,22 +189,25 @@ class Longhand(object):
# This is done like this since just a plain bool argument seemed like
# really random.
if animation_value_type is None:
raise TypeError("animation_value_type should be specified for (" + name + ")")
self.animation_value_type = animation_value_type
self.animatable = animation_value_type != "none"
+ self.transitionable = animation_value_type != "none" \
+ and animation_value_type != "discrete"
self.is_animatable_with_computed_value = animation_value_type == "ComputedValue" \
or animation_value_type == "discrete"
if self.logical:
# Logical properties will be animatable (i.e. the animation type is
# discrete). For now, it is still non-animatable.
self.animatable = False
+ self.transitionable = False
self.animation_type = None
# NB: Animatable implies clone because a property animation requires a
# copy of the computed value.
#
# See components/style/helpers/animated_properties.mako.rs.
self.need_clone = need_clone or self.animatable
@@ -237,17 +240,26 @@ class Shorthand(object):
def get_animatable(self):
animatable = False
for sub in self.sub_properties:
if sub.animatable:
animatable = True
break
return animatable
+ def get_transitionable(self):
+ transitionable = False
+ for sub in self.sub_properties:
+ if sub.transitionable:
+ transitionable = True
+ break
+ return transitionable
+
animatable = property(get_animatable)
+ transitionable = property(get_transitionable)
class Method(object):
def __init__(self, name, return_type=None, arg_types=None, is_mut=False):
self.name = name
self.return_type = return_type
self.arg_types = arg_types or []
self.is_mut = is_mut
--- a/servo/components/style/properties/helpers/animated_properties.mako.rs
+++ b/servo/components/style/properties/helpers/animated_properties.mako.rs
@@ -90,23 +90,17 @@ impl AnimatableLonghand {
}
}
/// Converts from TransitionProperty. Returns None if the property is not an animatable
/// longhand.
pub fn from_transition_property(transition_property: &TransitionProperty) -> Option<Self> {
match *transition_property {
% for prop in data.longhands:
- <%
- # TODO: Later in this patch series, once we introduce the 'transitionable'
- # definition, we will need to make the below test:
- #
- # if prop.transitionable and prop.animatable:
- %>
- % if prop.animatable:
+ % if prop.transitionable and prop.animatable:
TransitionProperty::${prop.camel_case}
=> Some(AnimatableLonghand::${prop.camel_case}),
% endif
% endfor
_ => None
}
}
@@ -164,111 +158,111 @@ impl<'a> From<AnimatableLonghand> for Pr
AnimatableLonghand::${prop.camel_case}
=> PropertyDeclarationId::Longhand(LonghandId::${prop.camel_case}),
% endif
% endfor
}
}
}
-/// A given transition property, that is either `All`, an animatable longhand property,
-/// a shorthand with at least one animatable longhand component, or an unsupported property.
+/// A given transition property, that is either `All`, a transitionable longhand property,
+/// a shorthand with at least one transitionable longhand component, or an unsupported property.
// NB: This needs to be here because it needs all the longhands generated
// beforehand.
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
pub enum TransitionProperty {
- /// All, any animatable property changing should generate a transition.
+ /// All, any transitionable property changing should generate a transition.
All,
% for prop in data.longhands + data.shorthands_except_all():
- % if prop.animatable:
+ % if prop.transitionable:
/// ${prop.name}
${prop.camel_case},
% endif
% endfor
- /// Unrecognized property which could be any non-animatable, custom property, or
+ /// Unrecognized property which could be any non-transitionable, custom property, or
/// unknown property.
Unsupported(Atom)
}
no_viewport_percentage!(TransitionProperty);
impl ComputedValueAsSpecified for TransitionProperty {}
impl TransitionProperty {
/// Iterates over each longhand property.
pub fn each<F: FnMut(&TransitionProperty) -> ()>(mut cb: F) {
% for prop in data.longhands:
- % if prop.animatable:
+ % if prop.transitionable:
cb(&TransitionProperty::${prop.camel_case});
% endif
% endfor
}
- /// Iterates over every property that is not TransitionProperty::All, stopping and returning
- /// true when the provided callback returns true for the first time.
+ /// Iterates over every longhand property that is not TransitionProperty::All, stopping and
+ /// returning true when the provided callback returns true for the first time.
pub fn any<F: FnMut(&TransitionProperty) -> bool>(mut cb: F) -> bool {
% for prop in data.longhands:
- % if prop.animatable:
+ % if prop.transitionable:
if cb(&TransitionProperty::${prop.camel_case}) {
return true;
}
% endif
% endfor
false
}
/// Parse a transition-property value.
pub fn parse<'i, 't>(input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
let ident = try!(input.expect_ident());
(match_ignore_ascii_case! { &ident,
"all" => Ok(TransitionProperty::All),
% for prop in data.longhands + data.shorthands_except_all():
- % if prop.animatable:
+ % if prop.transitionable:
"${prop.name}" => Ok(TransitionProperty::${prop.camel_case}),
% endif
% endfor
"none" => Err(()),
_ => {
match CSSWideKeyword::from_ident(&ident) {
Some(_) => Err(()),
None => Ok(TransitionProperty::Unsupported((&*ident).into()))
}
}
}).map_err(|()| SelectorParseError::UnexpectedIdent(ident.into()).into())
}
- /// Return animatable longhands of this shorthand TransitionProperty, except for "all".
+ /// Return transitionable longhands of this shorthand TransitionProperty, except for "all".
pub fn longhands(&self) -> &'static [TransitionProperty] {
% for prop in data.shorthands_except_all():
- % if prop.animatable:
+ % if prop.transitionable:
static ${prop.ident.upper()}: &'static [TransitionProperty] = &[
% for sub in prop.sub_properties:
- % if sub.animatable:
+ % if sub.transitionable:
TransitionProperty::${sub.camel_case},
% endif
% endfor
];
% endif
% endfor
match *self {
% for prop in data.shorthands_except_all():
- % if prop.animatable:
+ % if prop.transitionable:
TransitionProperty::${prop.camel_case} => ${prop.ident.upper()},
% endif
% endfor
_ => panic!("Not allowed to call longhands() for this TransitionProperty")
}
}
/// Returns true if this TransitionProperty is a shorthand.
pub fn is_shorthand(&self) -> bool {
match *self {
% for prop in data.shorthands_except_all():
- % if prop.animatable:
+ % if prop.transitionable:
TransitionProperty::${prop.camel_case} => true,
% endif
% endfor
_ => false
}
}
}
@@ -287,17 +281,17 @@ pub fn nscsspropertyid_is_animatable(pro
impl ToCss for TransitionProperty {
fn to_css<W>(&self, dest: &mut W) -> fmt::Result
where W: fmt::Write,
{
match *self {
TransitionProperty::All => dest.write_str("all"),
% for prop in data.longhands + data.shorthands_except_all():
- % if prop.animatable:
+ % if prop.transitionable:
TransitionProperty::${prop.camel_case} => dest.write_str("${prop.name}"),
% endif
% endfor
#[cfg(feature = "gecko")]
TransitionProperty::Unsupported(ref atom) => serialize_identifier(&atom.to_string(),
dest),
#[cfg(feature = "servo")]
TransitionProperty::Unsupported(ref atom) => serialize_identifier(atom, dest),
@@ -307,17 +301,17 @@ impl ToCss for TransitionProperty {
/// Convert to nsCSSPropertyID.
#[cfg(feature = "gecko")]
#[allow(non_upper_case_globals)]
impl<'a> From< &'a TransitionProperty> for nsCSSPropertyID {
fn from(transition_property: &'a TransitionProperty) -> nsCSSPropertyID {
match *transition_property {
% for prop in data.longhands + data.shorthands_except_all():
- % if prop.animatable:
+ % if prop.transitionable:
TransitionProperty::${prop.camel_case}
=> ${helpers.to_nscsspropertyid(prop.ident)},
% endif
% endfor
TransitionProperty::All => nsCSSPropertyID::eCSSPropertyExtra_all_properties,
_ => panic!("Unconvertable Servo transition property: {:?}", transition_property),
}
}
@@ -325,17 +319,17 @@ impl<'a> From< &'a TransitionProperty> f
/// Convert nsCSSPropertyID to TransitionProperty
#[cfg(feature = "gecko")]
#[allow(non_upper_case_globals)]
impl From<nsCSSPropertyID> for TransitionProperty {
fn from(property: nsCSSPropertyID) -> TransitionProperty {
match property {
% for prop in data.longhands + data.shorthands_except_all():
- % if prop.animatable:
+ % if prop.transitionable:
${helpers.to_nscsspropertyid(prop.ident)}
=> TransitionProperty::${prop.camel_case},
% else:
${helpers.to_nscsspropertyid(prop.ident)}
=> TransitionProperty::Unsupported(Atom::from("${prop.ident}")),
% endif
% endfor
nsCSSPropertyID::eCSSPropertyExtra_all_properties => TransitionProperty::All,