Bug 1597273 - Handle logical shorthand animations with variable references correctly. r=hiro
When we physicalize the declarations for @keyframes, we end up having a physical
declaration with an unparsed value with `from_shorthand` being the logical
shorthand.
Account for this case properly when substituting custom properties, to avoid
panicking.
Differential Revision:
https://phabricator.services.mozilla.com/D53663
--- a/servo/components/style/properties/data.py
+++ b/servo/components/style/properties/data.py
@@ -252,30 +252,32 @@ class Longhand(object):
# See compute_damage for the various values this can take
self.servo_restyle_damage = servo_restyle_damage
@staticmethod
def type():
return "longhand"
- # For a given logical property return all the physical
- # property names corresponding to it.
- def all_physical_mapped_properties(self):
- assert self.logical
+ # For a given logical property return all the physical property names
+ # corresponding to it.
+ def all_physical_mapped_properties(self, data):
+ if not self.logical:
+ return []
+
candidates = [s for s in LOGICAL_SIDES + LOGICAL_SIZES + LOGICAL_CORNERS
if s in self.name] + [s for s in LOGICAL_AXES if self.name.endswith(s)]
assert(len(candidates) == 1)
logical_side = candidates[0]
physical = PHYSICAL_SIDES if logical_side in LOGICAL_SIDES \
else PHYSICAL_SIZES if logical_side in LOGICAL_SIZES \
else PHYSICAL_AXES if logical_side in LOGICAL_AXES \
else LOGICAL_CORNERS
- return [to_phys(self.name, logical_side, physical_side)
+ return [data.longhands_by_name[to_phys(self.name, logical_side, physical_side)]
for physical_side in physical]
def experimental(self, engine):
if engine == "gecko":
return bool(self.gecko_pref)
elif engine == "servo-2013":
return bool(self.servo_2013_pref)
elif engine == "servo-2020":
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -1648,23 +1648,35 @@ impl UnparsedValue {
// than a CSS-wide keyword, after variable substitution.
Err(input.new_custom_error(SelectorParseErrorKind::UnexpectedIdent("all".into())))
}
% for shorthand in data.shorthands_except_all():
Some(ShorthandId::${shorthand.camel_case}) => {
shorthands::${shorthand.ident}::parse_value(&context, input)
.map(|longhands| {
match longhand_id {
+ <% seen = set() %>
% for property in shorthand.sub_properties:
- LonghandId::${property.camel_case} => {
- PropertyDeclaration::${property.camel_case}(
- longhands.${property.ident}
- )
- }
+ // When animating logical properties, we end up
+ // physicalizing the value during the animation, but
+ // the value still comes from the logical shorthand.
+ //
+ // So we need to handle the physical properties too.
+ % for prop in [property] + property.all_physical_mapped_properties(data):
+ % if prop.camel_case not in seen:
+ LonghandId::${prop.camel_case} => {
+ PropertyDeclaration::${prop.camel_case}(
+ longhands.${property.ident}
+ )
+ }
+ <% seen.add(prop.camel_case) %>
+ % endif
% endfor
+ % endfor
+ <% del seen %>
_ => unreachable!()
}
})
}
% endfor
}
});
@@ -2173,24 +2185,22 @@ impl PropertyDeclaration {
% for prop in data.longhands:
PropertyDeclaration::${prop.camel_case}(..) => {},
% endfor
}
let mut ret = self.clone();
% for prop in data.longhands:
- % if prop.logical:
- % for physical_property in prop.all_physical_mapped_properties():
- % if data.longhands_by_name[physical_property].specified_type() != prop.specified_type():
+ % for physical_property in prop.all_physical_mapped_properties(data):
+ % if physical_property.specified_type() != prop.specified_type():
<% raise "Logical property %s should share specified value with physical property %s" % \
- (prop.name, physical_property) %>
+ (prop.name, physical_property.name) %>
% endif
% endfor
- % endif
% endfor
unsafe {
let longhand_id = *(&mut ret as *mut _ as *mut LonghandId);
debug_assert_eq!(
PropertyDeclarationId::Longhand(longhand_id),
ret.id()
--- a/testing/web-platform/tests/css/css-logical/animation-001.html
+++ b/testing/web-platform/tests/css/css-logical/animation-001.html
@@ -297,16 +297,28 @@ test(t => {
div.style.direction = 'rtl';
assert_equals(getComputedStyle(div).marginLeft, '0px');
assert_equals(getComputedStyle(div).marginRight, '50px');
}, 'Animations update when the direction is changed');
test(t => {
const div = addDiv(t);
const anim = div.animate(
+ {
+ insetBlock: ['var(--200px)', 'var(--300px)'],
+ },
+ 1000
+ );
+ anim.currentTime = 500;
+ assert_equals(getComputedStyle(div).insetBlockStart, '250px');
+}, 'Logical shorthand with variable references animates correctly');
+
+test(t => {
+ const div = addDiv(t);
+ const anim = div.animate(
{ writingMode: 'vertical-rl' },
{ duration: 1, easing: 'step-start' }
);
assert_equals(getComputedStyle(div).writingMode, 'horizontal-tb');
assert_equals(anim.effect.getKeyframes().length, 0);
}, 'writing-mode is not animatable');
test(t => {
--- a/testing/web-platform/tests/css/css-logical/animation-002.html
+++ b/testing/web-platform/tests/css/css-logical/animation-002.html
@@ -192,16 +192,29 @@ test(t => {
assert_equals(getComputedStyle(div).height, '50px');
div.style.setProperty('--writingMode', 'vertical-rl');
assert_equals(getComputedStyle(div).width, '50px');
assert_equals(getComputedStyle(div).height, '0px');
}, 'Animations update when the writing-mode is changed through a CSS variable');
test(t => {
+ addStyle(t, { ':root': '--200px: 200px; --300px: 300px' });
+ addStyle(t, {
+ '@keyframes anim': 'from { inset-block: var(--200px) } to { inset-block: var(--300px) }',
+ });
+ const div = addDiv(t, {
+ style:
+ 'animation: anim 10s -5s paused linear;'
+ + ' width: 0px; height: 0px;'
+ });
+ assert_equals(getComputedStyle(div).insetBlockStart, '250px');
+}, 'Logical shorthand with variable references animates correctly');
+
+test(t => {
addStyle(t, {
'@keyframes anim':
'from { margin-inline-start: 0px } to { margin-inline-start: 100px }',
});
const div = addDiv(t, { style: 'animation: anim 10s -5s paused linear' });
assert_equals(getComputedStyle(div).marginLeft, '50px');
assert_equals(getComputedStyle(div).marginRight, '0px');