Bug 1899318 - Fix at-property-animation to account for spec ambiguity. r=firefox-style-system-reviewers,zrhoffman

See https://github.com/w3c/csswg-drafts/issues/10371 for the spec issue.

Differential Revision: https://phabricator.services.mozilla.com/D211871
This commit is contained in:
Emilio Cobos Álvarez 2024-05-29 10:25:03 +00:00
parent 928494dd0c
commit 710739a2b0
9 changed files with 52 additions and 57 deletions

View file

@ -458,8 +458,8 @@ void nsComputedDOMStyle::GetPropertyValue(
MOZ_ASSERT(nsCSSProps::IsCustomPropertyName(aMaybeCustomPropertyName)); MOZ_ASSERT(nsCSSProps::IsCustomPropertyName(aMaybeCustomPropertyName));
const nsACString& name = const nsACString& name =
Substring(aMaybeCustomPropertyName, CSS_CUSTOM_NAME_PREFIX_LENGTH); Substring(aMaybeCustomPropertyName, CSS_CUSTOM_NAME_PREFIX_LENGTH);
Servo_GetCustomPropertyValue( Servo_GetCustomPropertyValue(mComputedStyle, &name,
mComputedStyle, mPresShell->StyleSet()->RawData(), &name, &aReturn); mPresShell->StyleSet()->RawData(), &aReturn);
return; return;
} }

View file

@ -302,7 +302,8 @@ impl ComputedCustomProperties {
} }
} }
fn get( /// Returns the relevant custom property value given a registration.
pub fn get(
&self, &self,
registration: &PropertyRegistrationData, registration: &PropertyRegistrationData,
name: &Name, name: &Name,

View file

@ -165,7 +165,7 @@ impl Parse for Descriptor {
} }
/// <https://drafts.css-houdini.org/css-properties-values-api-1/#multipliers> /// <https://drafts.css-houdini.org/css-properties-values-api-1/#multipliers>
#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue)] #[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue)]
pub enum Multiplier { pub enum Multiplier {
/// Indicates a space-separated list. /// Indicates a space-separated list.
Space, Space,

View file

@ -90,7 +90,7 @@ impl<L, N, P, LP, C, Image, U, Integer, A, T, R, Transform>
} }
/// A generic enum used for both specified value components and computed value components. /// A generic enum used for both specified value components and computed value components.
#[derive(Animate, Clone, ToCss, ToComputedValue, Debug, MallocSizeOf, PartialEq)] #[derive(Animate, Clone, ToCss, ToComputedValue, ToResolvedValue, Debug, MallocSizeOf, PartialEq)]
#[animation(no_bound(Image, Url))] #[animation(no_bound(Image, Url))]
pub enum GenericValueComponent< pub enum GenericValueComponent<
Length, Length,
@ -145,7 +145,7 @@ pub enum GenericValueComponent<
} }
/// A list of component values, including the list's multiplier. /// A list of component values, including the list's multiplier.
#[derive(Clone, ToComputedValue, Debug, MallocSizeOf, PartialEq)] #[derive(Clone, ToComputedValue, ToResolvedValue, Debug, MallocSizeOf, PartialEq)]
pub struct ComponentList<Component> { pub struct ComponentList<Component> {
/// Multiplier /// Multiplier
pub multiplier: Multiplier, pub multiplier: Multiplier,
@ -198,7 +198,7 @@ impl<Component: ToCss> ToCss for ComponentList<Component> {
/// A struct for a single specified registered custom property value that includes its original URL /// A struct for a single specified registered custom property value that includes its original URL
// data so the value can be uncomputed later. // data so the value can be uncomputed later.
#[derive(Clone, Debug, MallocSizeOf, PartialEq, ToCss)] #[derive(Clone, Debug, MallocSizeOf, PartialEq, ToCss, ToComputedValue, ToResolvedValue)]
pub struct Value<Component> { pub struct Value<Component> {
/// The registered custom property value. /// The registered custom property value.
pub(crate) v: ValueInner<Component>, pub(crate) v: ValueInner<Component>,
@ -233,7 +233,7 @@ impl<Component> Value<Component> {
} }
/// A specified registered custom property value. /// A specified registered custom property value.
#[derive(Animate, ToComputedValue, ToCss, Clone, Debug, MallocSizeOf, PartialEq)] #[derive(Animate, ToComputedValue, ToResolvedValue, ToCss, Clone, Debug, MallocSizeOf, PartialEq)]
pub enum ValueInner<Component> { pub enum ValueInner<Component> {
/// A single specified component value whose syntax descriptor component did not have a /// A single specified component value whose syntax descriptor component did not have a
/// multiplier. /// multiplier.
@ -248,24 +248,6 @@ pub enum ValueInner<Component> {
/// Specified custom property value. /// Specified custom property value.
pub type SpecifiedValue = Value<SpecifiedValueComponent>; pub type SpecifiedValue = Value<SpecifiedValueComponent>;
impl ToComputedValue for SpecifiedValue {
type ComputedValue = ComputedValue;
fn to_computed_value(&self, context: &computed::Context) -> Self::ComputedValue {
Self::ComputedValue {
v: self.v.to_computed_value(context),
url_data: self.url_data.clone(),
}
}
fn from_computed_value(computed: &Self::ComputedValue) -> Self {
Self {
v: ToComputedValue::from_computed_value(&computed.v),
url_data: computed.url_data.clone(),
}
}
}
/// Computed custom property value. /// Computed custom property value.
pub type ComputedValue = Value<ComputedValueComponent>; pub type ComputedValue = Value<ComputedValueComponent>;

View file

@ -726,6 +726,7 @@ trivial_to_computed_value!(crate::values::AtomIdent);
trivial_to_computed_value!(crate::Namespace); trivial_to_computed_value!(crate::Namespace);
#[cfg(feature = "servo")] #[cfg(feature = "servo")]
trivial_to_computed_value!(crate::Prefix); trivial_to_computed_value!(crate::Prefix);
trivial_to_computed_value!(crate::stylesheets::UrlExtraData);
trivial_to_computed_value!(String); trivial_to_computed_value!(String);
trivial_to_computed_value!(Box<str>); trivial_to_computed_value!(Box<str>);
trivial_to_computed_value!(crate::OwnedStr); trivial_to_computed_value!(crate::OwnedStr);

View file

@ -87,6 +87,8 @@ trivial_to_resolved_value!(crate::color::AbsoluteColor);
trivial_to_resolved_value!(crate::values::generics::color::ColorMixFlags); trivial_to_resolved_value!(crate::values::generics::color::ColorMixFlags);
trivial_to_resolved_value!(crate::Atom); trivial_to_resolved_value!(crate::Atom);
trivial_to_resolved_value!(crate::values::AtomIdent); trivial_to_resolved_value!(crate::values::AtomIdent);
trivial_to_resolved_value!(crate::custom_properties::VariableValue);
trivial_to_resolved_value!(crate::stylesheets::UrlExtraData);
trivial_to_resolved_value!(app_units::Au); trivial_to_resolved_value!(app_units::Au);
trivial_to_resolved_value!(computed::url::ComputedUrl); trivial_to_resolved_value!(computed::url::ComputedUrl);
#[cfg(feature = "gecko")] #[cfg(feature = "gecko")]

View file

@ -155,6 +155,7 @@ use style::values::computed::{self, Context, ToComputedValue};
use style::values::distance::ComputeSquaredDistance; use style::values::distance::ComputeSquaredDistance;
use style::values::generics::color::ColorMixFlags; use style::values::generics::color::ColorMixFlags;
use style::values::generics::easing::BeforeFlag; use style::values::generics::easing::BeforeFlag;
use style::values::resolved;
use style::values::specified::gecko::IntersectionObserverRootMargin; use style::values::specified::gecko::IntersectionObserverRootMargin;
use style::values::specified::source_size_list::SourceSizeList; use style::values::specified::source_size_list::SourceSizeList;
use style::values::specified::{AbsoluteLength, NoCalcLength}; use style::values::specified::{AbsoluteLength, NoCalcLength};
@ -7603,7 +7604,7 @@ pub extern "C" fn Servo_StyleSet_HasDocumentStateDependency(
fn computed_or_resolved_value( fn computed_or_resolved_value(
style: &ComputedValues, style: &ComputedValues,
prop: nsCSSPropertyID, prop: nsCSSPropertyID,
context: Option<&style::values::resolved::Context>, context: Option<&resolved::Context>,
value: &mut nsACString, value: &mut nsACString,
) { ) {
if let Some(longhand) = LonghandId::from_nscsspropertyid(prop) { if let Some(longhand) = LonghandId::from_nscsspropertyid(prop) {
@ -7641,8 +7642,6 @@ pub unsafe extern "C" fn Servo_GetResolvedValue(
element: &RawGeckoElement, element: &RawGeckoElement,
value: &mut nsACString, value: &mut nsACString,
) { ) {
use style::values::resolved;
let data = raw_data.borrow(); let data = raw_data.borrow();
let device = data.stylist.device(); let device = data.stylist.device();
let context = resolved::Context { let context = resolved::Context {
@ -7658,26 +7657,23 @@ pub unsafe extern "C" fn Servo_GetResolvedValue(
#[no_mangle] #[no_mangle]
pub unsafe extern "C" fn Servo_GetCustomPropertyValue( pub unsafe extern "C" fn Servo_GetCustomPropertyValue(
computed_values: &ComputedValues, style: &ComputedValues,
raw_style_set: &PerDocumentStyleData,
name: &nsACString, name: &nsACString,
raw_data: &PerDocumentStyleData,
value: &mut nsACString, value: &mut nsACString,
) -> bool { ) -> bool {
let doc_data = raw_style_set.borrow(); let data = raw_data.borrow();
let name = Atom::from(name.as_str_unchecked()); let name = Atom::from(name.as_str_unchecked());
let custom_registration = doc_data.stylist.get_custom_property_registration(&name); let custom_registration = data.stylist.get_custom_property_registration(&name);
let computed_value = if custom_registration.inherits() { let computed_value = style.custom_properties.get(custom_registration, &name);
computed_values.custom_properties.inherited.get(&name) let computed_value = match computed_value {
} else { Some(v) => v,
computed_values.custom_properties.non_inherited.get(&name) None => return false,
}; };
// TODO(emilio): This might want to return resolved colors and so on for example, see
if let Some(v) = computed_value { // https://github.com/w3c/csswg-drafts/issues/10371.
v.to_css(&mut CssWriter::new(value)).unwrap(); computed_value.to_css(&mut CssWriter::new(value)).unwrap();
true true
} else {
false
}
} }
#[no_mangle] #[no_mangle]

View file

@ -1,12 +1,5 @@
[at-property-animation.html] [at-property-animation.html]
[JS-originated animation setting "currentColor" for a custom property on a keyframe is responsive to changing "color" on the parent.]
expected: FAIL
[CSS animation setting "inherit" for a custom property on a keyframe is responsive to changing that custom property on the parent.] [CSS animation setting "inherit" for a custom property on a keyframe is responsive to changing that custom property on the parent.]
expected: FAIL expected: FAIL
[CSS animation setting "currentColor" for a custom property on a keyframe is responsive to changing "color" on the parent.]
expected: FAIL
[JS-originated animation setting "inherit" for a custom property on a keyframe is responsive to changing that custom property on the parent.] [JS-originated animation setting "inherit" for a custom property on a keyframe is responsive to changing that custom property on the parent.]
expected: FAIL expected: FAIL

View file

@ -449,13 +449,24 @@ test_with_at_property({
animation: test 100s -50s linear paused; animation: test 100s -50s linear paused;
} }
`, (style) => { `, (style) => {
assert_equals(getComputedStyle(div).getPropertyValue(name), 'rgb(150, 150, 150)'); // See https://github.com/w3c/csswg-drafts/issues/10371 for the two
// possibilities.
assert_in_array(getComputedStyle(div).getPropertyValue(name), [
'color-mix(in srgb, currentcolor, rgb(200, 200, 200))',
'rgb(150, 150, 150)',
]);
outer.style.color = 'rgb(50, 50, 50)'; outer.style.color = 'rgb(50, 50, 50)';
assert_equals(getComputedStyle(div).getPropertyValue(name), 'rgb(125, 125, 125)'); assert_in_array(getComputedStyle(div).getPropertyValue(name), [
'color-mix(in srgb, currentcolor, rgb(200, 200, 200))',
'rgb(125, 125, 125)',
]);
outer.style.color = 'rgb(150, 150, 150)'; outer.style.color = 'rgb(150, 150, 150)';
assert_equals(getComputedStyle(div).getPropertyValue(name), 'rgb(175, 175, 175)'); assert_in_array(getComputedStyle(div).getPropertyValue(name), [
'color-mix(in srgb, currentcolor, rgb(200, 200, 200))',
'rgb(175, 175, 175)',
]);
outer.style.removeProperty("color"); outer.style.removeProperty("color");
}); });
@ -475,13 +486,22 @@ test_with_at_property({
animation.currentTime = 500; animation.currentTime = 500;
animation.pause(); animation.pause();
assert_equals(getComputedStyle(div).getPropertyValue(name), 'rgb(150, 150, 150)'); assert_in_array(getComputedStyle(div).getPropertyValue(name), [
'color-mix(in srgb, currentcolor, rgb(200, 200, 200))',
'rgb(150, 150, 150)',
]);
outer.style.color = 'rgb(50, 50, 50)'; outer.style.color = 'rgb(50, 50, 50)';
assert_equals(getComputedStyle(div).getPropertyValue(name), 'rgb(125, 125, 125)'); assert_in_array(getComputedStyle(div).getPropertyValue(name), [
'color-mix(in srgb, currentcolor, rgb(200, 200, 200))',
'rgb(125, 125, 125)',
]);
outer.style.color = 'rgb(150, 150, 150)'; outer.style.color = 'rgb(150, 150, 150)';
assert_equals(getComputedStyle(div).getPropertyValue(name), 'rgb(175, 175, 175)'); assert_in_array(getComputedStyle(div).getPropertyValue(name), [
'color-mix(in srgb, currentcolor, rgb(200, 200, 200))',
'rgb(175, 175, 175)',
]);
outer.style.removeProperty("color"); outer.style.removeProperty("color");
}); });