Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
style: Handle logical shorthand animations with variable references c…
…orrectly.

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
  • Loading branch information
emilio committed Nov 30, 2019
1 parent 9a43ad9 commit a7c50b5
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 15 deletions.
12 changes: 7 additions & 5 deletions components/style/properties/data.py
Expand Up @@ -257,10 +257,12 @@ def __init__(self, style_struct, name, spec=None, animation_value_type=None, key
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)
Expand All @@ -270,7 +272,7 @@ def all_physical_mapped_properties(self):
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):
Expand Down
30 changes: 20 additions & 10 deletions components/style/properties/properties.mako.rs
Expand Up @@ -1651,13 +1651,25 @@ impl UnparsedValue {
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!()
}
})
Expand Down Expand Up @@ -2176,14 +2188,12 @@ impl PropertyDeclaration {
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 {
Expand Down

0 comments on commit a7c50b5

Please sign in to comment.