Bug 1419695: Make the transition-property code make more sense. r?xidorn
We were working around the lack of alias support during parsing in
TransitionProperty by doing a Gecko lookup. That's a hack and is now gone.
MozReview-Commit-ID: EptUvJNTrZr
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -769,30 +769,16 @@ Gecko_AnimationGetBaseStyle(void* aBaseS
{
auto base =
static_cast<nsRefPtrHashtable<nsUint32HashKey, RawServoAnimationValue>*>
(aBaseStyles);
return base->GetWeak(aProperty);
}
void
-Gecko_StyleTransition_SetUnsupportedProperty(StyleTransition* aTransition,
- nsAtom* aAtom)
-{
- nsCSSPropertyID id =
- nsCSSProps::LookupProperty(nsDependentAtomString(aAtom),
- CSSEnabledState::eForAllContent);
- if (id == eCSSProperty_UNKNOWN || id == eCSSPropertyExtra_variable) {
- aTransition->SetUnknownProperty(id, aAtom);
- } else {
- aTransition->SetProperty(id);
- }
-}
-
-void
Gecko_FillAllImageLayers(nsStyleImageLayers* aLayers, uint32_t aMaxLen)
{
aLayers->FillAllLayers(aMaxLen);
}
bool
Gecko_IsDocumentBody(RawGeckoElementBorrowed aElement)
{
--- a/servo/components/style/animation.rs
+++ b/servo/components/style/animation.rs
@@ -298,17 +298,18 @@ impl PropertyAnimation {
) -> Vec<PropertyAnimation> {
let mut result = vec![];
let box_style = new_style.get_box();
let transition_property = box_style.transition_property_at(transition_index);
let timing_function = box_style.transition_timing_function_mod(transition_index);
let duration = box_style.transition_duration_mod(transition_index);
match transition_property {
- TransitionProperty::Unsupported(_) => result,
+ TransitionProperty::Custom(..) |
+ TransitionProperty::Unsupported(..) => result,
TransitionProperty::Shorthand(ref shorthand_id) => shorthand_id
.longhands()
.filter_map(|longhand| {
PropertyAnimation::from_longhand(
longhand,
timing_function,
duration,
old_style,
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -1625,16 +1625,17 @@ impl<'le> TElement for GeckoElement<'le>
combined_duration,
before_change_style,
after_change_style,
&existing_transitions,
)
};
match transition_property {
+ TransitionProperty::Custom(..) |
TransitionProperty::Unsupported(..) => {},
TransitionProperty::Shorthand(ref shorthand) => {
if shorthand.longhands().any(property_check_helper) {
return true;
}
},
TransitionProperty::Longhand(longhand_id) => {
if property_check_helper(longhand_id) {
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -23,17 +23,16 @@ use gecko_bindings::bindings::Gecko_Dest
use gecko_bindings::bindings::Gecko_CopyCounterStyle;
use gecko_bindings::bindings::Gecko_CopyCursorArrayFrom;
use gecko_bindings::bindings::Gecko_CopyFontFamilyFrom;
use gecko_bindings::bindings::Gecko_CopyImageValueFrom;
use gecko_bindings::bindings::Gecko_CopyListStyleImageFrom;
use gecko_bindings::bindings::Gecko_EnsureImageLayersLength;
use gecko_bindings::bindings::Gecko_SetCursorArrayLength;
use gecko_bindings::bindings::Gecko_SetCursorImageValue;
-use gecko_bindings::bindings::Gecko_StyleTransition_SetUnsupportedProperty;
use gecko_bindings::bindings::Gecko_NewCSSShadowArray;
use gecko_bindings::bindings::Gecko_nsStyleFont_SetLang;
use gecko_bindings::bindings::Gecko_nsStyleFont_CopyLangFrom;
use gecko_bindings::bindings::Gecko_SetListStyleImageNone;
use gecko_bindings::bindings::Gecko_SetListStyleImageImageValue;
use gecko_bindings::bindings::Gecko_SetNullImageValue;
use gecko_bindings::bindings::{Gecko_ResetFilters, Gecko_CopyFiltersFrom};
use gecko_bindings::bindings::RawGeckoPresContextBorrowed;
@@ -3262,27 +3261,39 @@ fn static_assert() {
+ self.gecko.mTransitions[index % self.gecko.mTransitionDelayCount as usize].mDelay
}
pub fn set_transition_property<I>(&mut self, v: I)
where I: IntoIterator<Item = longhands::transition_property::computed_value::single_value::T>,
I::IntoIter: ExactSizeIterator
{
use gecko_bindings::structs::nsCSSPropertyID::eCSSPropertyExtra_no_properties;
+ use gecko_bindings::structs::nsCSSPropertyID::eCSSPropertyExtra_variable;
+ use gecko_bindings::structs::nsCSSPropertyID::eCSSProperty_UNKNOWN;
let v = v.into_iter();
if v.len() != 0 {
self.gecko.mTransitions.ensure_len(v.len());
self.gecko.mTransitionPropertyCount = v.len() as u32;
for (servo, gecko) in v.zip(self.gecko.mTransitions.iter_mut()) {
+ if !gecko.mUnknownProperty.mRawPtr.is_null() {
+ unsafe { Atom::from_addrefed(gecko.mUnknownProperty.mRawPtr) };
+ gecko.mUnknownProperty.mRawPtr = ptr::null_mut();
+ }
+
match servo {
- TransitionProperty::Unsupported(ref ident) => unsafe {
- Gecko_StyleTransition_SetUnsupportedProperty(gecko, ident.0.as_ptr())
+ TransitionProperty::Unsupported(ident) => {
+ gecko.mProperty = eCSSProperty_UNKNOWN;
+ gecko.mUnknownProperty.mRawPtr = ident.0.into_addrefed();
},
+ TransitionProperty::Custom(name) => {
+ gecko.mProperty = eCSSPropertyExtra_variable;
+ gecko.mUnknownProperty.mRawPtr = name.into_addrefed();
+ }
_ => gecko.mProperty = servo.to_nscsspropertyid().unwrap(),
}
}
} else {
// In gecko |none| is represented by eCSSPropertyExtra_no_properties.
self.gecko.mTransitionPropertyCount = 1;
self.gecko.mTransitions[0].mProperty = eCSSPropertyExtra_no_properties;
}
@@ -3302,25 +3313,34 @@ fn static_assert() {
pub fn transition_property_at(&self, index: usize)
-> longhands::transition_property::computed_value::SingleComputedValue {
use gecko_bindings::structs::nsCSSPropertyID::eCSSPropertyExtra_no_properties;
use gecko_bindings::structs::nsCSSPropertyID::eCSSPropertyExtra_variable;
use gecko_bindings::structs::nsCSSPropertyID::eCSSProperty_UNKNOWN;
let property = self.gecko.mTransitions[index].mProperty;
- if property == eCSSProperty_UNKNOWN || property == eCSSPropertyExtra_variable {
+ if property == eCSSProperty_UNKNOWN {
let atom = self.gecko.mTransitions[index].mUnknownProperty.mRawPtr;
debug_assert!(!atom.is_null());
TransitionProperty::Unsupported(CustomIdent(unsafe{
Atom::from_raw(atom)
}))
+ } else if property == eCSSPropertyExtra_variable {
+ let atom = self.gecko.mTransitions[index].mUnknownProperty.mRawPtr;
+ debug_assert!(!atom.is_null());
+ TransitionProperty::Custom(unsafe{
+ Atom::from_raw(atom)
+ })
} else if property == eCSSPropertyExtra_no_properties {
- // Actually, we don't expect TransitionProperty::Unsupported also represents "none",
- // but if the caller wants to convert it, it is fine. Please use it carefully.
+ // Actually, we don't expect TransitionProperty::Unsupported also
+ // represents "none", but if the caller wants to convert it, it is
+ // fine. Please use it carefully.
+ //
+ // FIXME(emilio): This is a hack, is this reachable?
TransitionProperty::Unsupported(CustomIdent(atom!("none")))
} else {
property.into()
}
}
pub fn transition_nscsspropertyid_at(&self, index: usize) -> nsCSSPropertyID {
self.gecko.mTransitions[index].mProperty
@@ -3331,21 +3351,25 @@ fn static_assert() {
use gecko_bindings::structs::nsCSSPropertyID::eCSSProperty_UNKNOWN;
self.gecko.mTransitions.ensure_len(other.gecko.mTransitions.len());
let count = other.gecko.mTransitionPropertyCount;
self.gecko.mTransitionPropertyCount = count;
for (index, transition) in self.gecko.mTransitions.iter_mut().enumerate().take(count as usize) {
transition.mProperty = other.gecko.mTransitions[index].mProperty;
+ if !transition.mUnknownProperty.mRawPtr.is_null() {
+ unsafe { Atom::from_addrefed(transition.mUnknownProperty.mRawPtr) };
+ transition.mUnknownProperty.mRawPtr = ptr::null_mut();
+ }
if transition.mProperty == eCSSProperty_UNKNOWN ||
transition.mProperty == eCSSPropertyExtra_variable {
let atom = other.gecko.mTransitions[index].mUnknownProperty.mRawPtr;
debug_assert!(!atom.is_null());
- unsafe { Gecko_StyleTransition_SetUnsupportedProperty(transition, atom) };
+ transition.mUnknownProperty.mRawPtr = unsafe { Atom::from_raw(atom) }.into_addrefed();
}
}
}
pub fn reset_transition_property(&mut self, other: &Self) {
self.copy_transition_property_from(other)
}
--- a/servo/components/style/properties/helpers/animated_properties.mako.rs
+++ b/servo/components/style/properties/helpers/animated_properties.mako.rs
@@ -4,35 +4,38 @@
<%namespace name="helpers" file="/helpers.mako.rs" />
<%
from data import to_idl_name, SYSTEM_FONT_LONGHANDS
from itertools import groupby
%>
+use Atom;
use cssparser::Parser;
#[cfg(feature = "gecko")] use gecko_bindings::bindings::RawServoAnimationValueMap;
#[cfg(feature = "gecko")] use gecko_bindings::structs::RawGeckoGfxMatrix4x4;
#[cfg(feature = "gecko")] use gecko_bindings::structs::nsCSSPropertyID;
#[cfg(feature = "gecko")] use gecko_bindings::sugar::ownership::{HasFFI, HasSimpleFFI};
use itertools::{EitherOrBoth, Itertools};
use num_traits::Zero;
-use properties::{CSSWideKeyword, PropertyDeclaration};
+use parser::ParserContext;
+use properties::{CSSWideKeyword, PropertyDeclaration, PropertyDeclarationId};
use properties::longhands;
use properties::longhands::font_weight::computed_value::T as FontWeight;
use properties::longhands::visibility::computed_value::T as Visibility;
use properties::PropertyId;
use properties::{LonghandId, ShorthandId};
use servo_arc::Arc;
use smallvec::SmallVec;
use std::{cmp, ptr};
+use std::fmt::{self, Write};
use std::mem::{self, ManuallyDrop};
#[cfg(feature = "gecko")] use hash::FnvHashMap;
-use style_traits::{KeywordsCollectFn, ParseError, SpecifiedValueInfo};
+use style_traits::{KeywordsCollectFn, ParseError, SpecifiedValueInfo, ToCss, CssWriter};
use super::ComputedValues;
use values::{CSSFloat, CustomIdent};
use values::animated::{Animate, Procedure, ToAnimatedValue, ToAnimatedZero};
use values::animated::color::RGBA as AnimatedRGBA;
use values::animated::effects::Filter as AnimatedFilter;
use values::computed::{Angle, CalcLengthOrPercentage};
use values::computed::{ClipRect, Context};
use values::computed::{Length, LengthOrPercentage, LengthOrPercentageOrAuto};
@@ -69,79 +72,92 @@ pub fn nscsspropertyid_is_animatable(pro
_ => false
}
}
/// 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, Eq, Hash, MallocSizeOf, PartialEq, ToComputedValue, ToCss)]
+#[derive(Clone, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToComputedValue)]
pub enum TransitionProperty {
/// A shorthand.
Shorthand(ShorthandId),
/// A longhand transitionable property.
Longhand(LonghandId),
+ /// A custom property.
+ Custom(Atom),
/// Unrecognized property which could be any non-transitionable, custom property, or
/// unknown property.
Unsupported(CustomIdent),
}
+impl ToCss for TransitionProperty {
+ fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
+ where
+ W: Write,
+ {
+ use values::serialize_atom_identifier;
+ match *self {
+ TransitionProperty::Shorthand(ref s) => s.to_css(dest),
+ TransitionProperty::Longhand(ref l) => l.to_css(dest),
+ TransitionProperty::Custom(ref name) => {
+ dest.write_str("--")?;
+ serialize_atom_identifier(name, dest)
+ }
+ TransitionProperty::Unsupported(ref i) => i.to_css(dest),
+ }
+ }
+}
+
impl TransitionProperty {
/// Returns `all`.
#[inline]
pub fn all() -> Self {
TransitionProperty::Shorthand(ShorthandId::All)
}
/// Parse a transition-property value.
- pub fn parse<'i, 't>(input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
- // FIXME(https://github.com/rust-lang/rust/issues/33156): remove this
- // enum and use PropertyId when stable Rust allows destructors in
- // statics.
- //
- // FIXME: This should handle aliases too.
- pub enum StaticId {
- Longhand(LonghandId),
- Shorthand(ShorthandId),
- }
- ascii_case_insensitive_phf_map! {
- static_id -> StaticId = {
- % for prop in data.shorthands:
- "${prop.name}" => StaticId::Shorthand(ShorthandId::${prop.camel_case}),
- % endfor
- % for prop in data.longhands:
- "${prop.name}" => StaticId::Longhand(LonghandId::${prop.camel_case}),
- % endfor
- }
- }
-
+ pub fn parse<'i, 't>(
+ context: &ParserContext,
+ input: &mut Parser<'i, 't>,
+ ) -> Result<Self, ParseError<'i>> {
let location = input.current_source_location();
let ident = input.expect_ident()?;
- Ok(match static_id(&ident) {
- Some(&StaticId::Longhand(id)) => TransitionProperty::Longhand(id),
- Some(&StaticId::Shorthand(id)) => TransitionProperty::Shorthand(id),
- None => {
- TransitionProperty::Unsupported(
- CustomIdent::from_ident(location, ident, &["none"])?,
- )
- },
+ let id = match PropertyId::parse(&ident, context) {
+ Ok(id) => id,
+ Err(..) => return Ok(TransitionProperty::Unsupported(
+ CustomIdent::from_ident(location, ident, &["none"])?,
+ )),
+ };
+
+ Ok(match id.as_shorthand() {
+ Ok(s) => TransitionProperty::Shorthand(s),
+ Err(longhand_or_custom) => {
+ match longhand_or_custom {
+ PropertyDeclarationId::Longhand(id) => TransitionProperty::Longhand(id),
+ PropertyDeclarationId::Custom(custom) => {
+ TransitionProperty::Custom(custom.clone())
+ }
+ }
+ }
})
}
/// Convert TransitionProperty to nsCSSPropertyID.
#[cfg(feature = "gecko")]
pub fn to_nscsspropertyid(&self) -> Result<nsCSSPropertyID, ()> {
Ok(match *self {
TransitionProperty::Shorthand(ShorthandId::All) => {
nsCSSPropertyID::eCSSPropertyExtra_all_properties
}
TransitionProperty::Shorthand(ref id) => id.to_nscsspropertyid(),
TransitionProperty::Longhand(ref id) => id.to_nscsspropertyid(),
+ TransitionProperty::Custom(..) |
TransitionProperty::Unsupported(..) => return Err(()),
})
}
}
/// Convert nsCSSPropertyID to TransitionProperty
#[cfg(feature = "gecko")]
#[allow(non_upper_case_globals)]
--- a/servo/components/style/properties/longhand/box.mako.rs
+++ b/servo/components/style/properties/longhand/box.mako.rs
@@ -252,17 +252,16 @@
${helpers.predefined_type(
"transition-property",
"TransitionProperty",
"computed::TransitionProperty::all()",
initial_specified_value="specified::TransitionProperty::all()",
vector=True,
allow_empty="NotInitial",
need_index=True,
- needs_context=False,
animation_value_type="none",
extra_prefixes=transition_extra_prefixes,
spec="https://drafts.csswg.org/css-transitions/#propdef-transition-property",
)}
${helpers.predefined_type("transition-delay",
"Time",
"computed::Time::zero()",
--- a/servo/components/style/properties/shorthand/box.mako.rs
+++ b/servo/components/style/properties/shorthand/box.mako.rs
@@ -153,20 +153,22 @@ macro_rules! try_parse_one {
parsed += 1;
try_parse_one!(context, input, duration, transition_duration);
try_parse_one!(context, input, timing_function, transition_timing_function);
try_parse_one!(context, input, delay, transition_delay);
// Must check 'transition-property' after 'transition-timing-function' since
// 'transition-property' accepts any keyword.
if property.is_none() {
- if let Ok(value) = input.try(|i| transition_property::SingleSpecifiedValue::parse(i)) {
+ if let Ok(value) = input.try(|i| transition_property::SingleSpecifiedValue::parse(context, i)) {
property = Some(Some(value));
continue;
- } else if input.try(|i| i.expect_ident_matching("none")).is_ok() {
+ }
+
+ if input.try(|i| i.expect_ident_matching("none")).is_ok() {
// 'none' is not a valid value for <single-transition-property>,
// so it's not acceptable in the function above.
property = Some(None);
continue;
}
}
parsed -= 1;