stylo: Fill animation/transition properties that the length of the property is less than the length of animation-name

RESOLVED FIXED in Firefox 55

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
a year ago
6 months ago

People

(Reporter: hiro, Assigned: hiro)

Tracking

(Blocks: 1 bug)

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

a year ago
I did miss below part of the spec.

https://drafts.csswg.org/css-animations-1/#animation-name

 If one of the other properties doesn’t have enough comma-separated values to match the number of values of animation-name, the UA must calculate its used value by repeating the list of values until there are enough.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c9354ec277a30276c4de8e3bf8e4f863b4db646d
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 5

a year ago
mozreview-review
Comment on attachment 8846442 [details]
Bug 1346655 - Don't set transition properties to mAnimations array.

https://reviewboard.mozilla.org/r/119466/#review121346
Attachment #8846442 - Flags: review?(cam) → review+

Comment 6

a year ago
mozreview-review
Comment on attachment 8846443 [details]
Bug 1346655 - Cascade animation-name property ealier than other animation properties.

https://reviewboard.mozilla.org/r/119468/#review121348

::: commit-message-9bc2b:1
(Diff revision 1)
> +Bug 1346655 - Cascade animation-name property ealier than other animation properties. r?heycam

s/ealier/earlier/

::: commit-message-9bc2b:5
(Diff revision 1)
> +Bug 1346655 - Cascade animation-name property ealier than other animation properties. r?heycam
> +
> +As per the CSS Animations spec, in the case when multiple values for an
> +animation property are set, if the value length is less than the length
> +of animation name property, then shortage values are filled up. Because

s/animation name/animation-name/
Attachment #8846443 - Flags: review?(cam) → review+

Comment 7

a year ago
mozreview-review
Comment on attachment 8846444 [details]
Bug 1346655 - Fill shortage values for animation/transition properties.

https://reviewboard.mozilla.org/r/119470/#review121354

::: servo/components/style/properties/gecko.mako.rs:1317
(Diff revision 1)
>      pub fn set_${type}_${ident}(&mut self, v: longhands::${type}_${ident}::computed_value::T) {
>          debug_assert!(!v.0.is_empty());
>          unsafe { self.gecko.m${type.capitalize()}s.ensure_len(v.0.len()) };
>  
> +        let input_len = v.0.len();
>          self.gecko.m${type.capitalize()}${gecko_ffi_name}Count = v.0.len() as u32;

Replace |v.0.len()| with |input_len| here.

::: servo/components/style/properties/gecko.mako.rs:1338
(Diff revision 1)
>      pub fn set_${type}_timing_function(&mut self, v: longhands::${type}_timing_function::computed_value::T) {
>          debug_assert!(!v.0.is_empty());
>          unsafe { self.gecko.m${type.capitalize()}s.ensure_len(v.0.len()) };
>  
> +        let input_len = v.0.len();
>          self.gecko.m${type.capitalize()}TimingFunctionCount = v.0.len() as u32;

And here.

::: servo/components/style/properties/gecko.mako.rs:1390
(Diff revision 1)
>          debug_assert!(!v.0.is_empty());
>          unsafe { self.gecko.mAnimations.ensure_len(v.0.len()) };
>  
>          self.gecko.mAnimation${gecko_ffi_name}Count = v.0.len() as u32;
>  
> -        for (servo, gecko) in v.0.into_iter().zip(self.gecko.mAnimations.iter_mut()) {
> +        let input_len = v.0.len();

Move this earlier, and re-use input_len where you can.

::: servo/components/style/properties/gecko.mako.rs:1855
(Diff revision 1)
>          use properties::longhands::animation_iteration_count::single_value::SpecifiedValue as AnimationIterationCount;
>  
>          debug_assert!(!v.0.is_empty());
>          unsafe { self.gecko.mAnimations.ensure_len(v.0.len()) };
>  
> +        let input_len = v.0.len();

And here.
Attachment #8846444 - Flags: review?(cam) → review+

Comment 8

a year ago
mozreview-review
Comment on attachment 8846445 [details]
Bug 1346655 - Update mochitest expectation.

https://reviewboard.mozilla.org/r/119472/#review121356
Attachment #8846445 - Flags: review?(cam) → review+
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8846442 - Attachment is obsolete: true
(Assignee)

Updated

a year ago
Attachment #8846443 - Attachment is obsolete: true
(Assignee)

Updated

a year ago
Attachment #8846444 - Attachment is obsolete: true
(Assignee)

Comment 10

a year ago
Thank you Cameron. 

https://github.com/servo/servo/pull/15923

I will land the patch here after our servo's vsync service gets back to work.

Comment 11

a year ago
Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/87f593de310b
Update mochitest expectation. r=heycam
(Assignee)

Comment 12

a year ago
At this moment, autoland looks like orange field, but I did land the patch to reduce some of oranges.

Comment 13

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/87f593de310b
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Depends on: 1420928
You need to log in before you can comment on or make changes to this bug.