Skip to content

/ servo

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Servo's transitions code stopped making sense after #15900. #20116

Closed
emilio opened this issue Feb 24, 2018 · 1 comment
Closed

Servo's transitions code stopped making sense after #15900. #20116

emilio opened this issue Feb 24, 2018 · 1 comment
Assignees
@mrobinson
Labels
A-content/css A-layout/animations

Comments

@emilio
Copy link
Member

@emilio emilio commented Feb 24, 2018 •
edited

In particular, what used to update the new style (as expected, since that's what you're animating), started updating the old style:

       if let Some(ref mut old) = *old_values {
           self.update_animations_for_cascade(shared_context, old,
                                              possibly_expired_animations);
       }

Which doesn't really make sense.
emilio added a commit to emilio/servo that referenced this issue Feb 24, 2018
@emilio
style: Reference servo#20116 from a few places.
Unverified
This user has not uploaded their public key yet.
GPG key ID: 056B727BB9C1027C Learn about signing commits
a6c6ef9
@jdm jdm added the A-content/css label Feb 25, 2018
nupurbaghel added a commit to paavininanda/servo that referenced this issue Mar 14, 2018
@emilio @nupurbaghel
style: Reference servo#20116 from a few places.
4c0c775
pandusonu2 added a commit to pandusonu2/servo that referenced this issue Mar 15, 2018
@emilio @pandusonu2
style: Reference servo#20116 from a few places.
f32eaad
@fabricedesre fabricedesre mentioned this issue Mar 22, 2018
Closed
@emilio emilio mentioned this issue May 6, 2018
Merged
bors-servo added a commit that referenced this issue May 7, 2018
@bors-servo
Auto merge of #20757 - emilio:animations, r=<try>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
458a54c 
style: Some animation cleanups and fixes.

The transitions code is still terribly broken, but I ran out of time fixing it. We have nothing that stops transitions, which is just plain wrong. Most of this code should probably be rewritten, since with the current setup is pretty hard to get it right. Anyway...

Fixes #20731.
Fixes #20116.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20757)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Oct 15, 2018
@bors-servo
Auto merge of #20757 - emilio:animations, r=<try>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
c1a3d7e 
style: Some animation cleanups and fixes.

The transitions code is still terribly broken, but I ran out of time fixing it. We have nothing that stops transitions, which is just plain wrong. Most of this code should probably be rewritten, since with the current setup is pretty hard to get it right. Anyway...

Fixes #20731.
Fixes #20116.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20757)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Oct 15, 2018
@bors-servo
Auto merge of #20757 - emilio:animations, r=<try>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
7bdb82b 
style: Some animation cleanups and fixes.

The transitions code is still terribly broken, but I ran out of time fixing it. We have nothing that stops transitions, which is just plain wrong. Most of this code should probably be rewritten, since with the current setup is pretty hard to get it right. Anyway...

Fixes #20731.
Fixes #20116.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20757)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Oct 15, 2018
@bors-servo
Auto merge of #20757 - emilio:animations, r=<try>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
6d1f3b2 
style: Some animation cleanups and fixes.

The transitions code is still terribly broken, but I ran out of time fixing it. We have nothing that stops transitions, which is just plain wrong. Most of this code should probably be rewritten, since with the current setup is pretty hard to get it right. Anyway...

Fixes #20731.
Fixes #20116.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20757)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Oct 15, 2018
@bors-servo
Auto merge of #20757 - emilio:animations, r=<try>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
8cf3be0 
style: Some animation cleanups and fixes.

The transitions code is still terribly broken, but I ran out of time fixing it. We have nothing that stops transitions, which is just plain wrong. Most of this code should probably be rewritten, since with the current setup is pretty hard to get it right. Anyway...

Fixes #20731.
Fixes #20116.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20757)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Oct 15, 2018
@bors-servo
Auto merge of #20757 - emilio:animations, r=jdm
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
3b03f6d 
style: Some animation cleanups and fixes.

The transitions code is still terribly broken, but I ran out of time fixing it. We have nothing that stops transitions, which is just plain wrong. Most of this code should probably be rewritten, since with the current setup is pretty hard to get it right. Anyway...

Fixes #20731.
Fixes #20116.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20757)
<!-- Reviewable:end -->
@atouchet
Copy link
Contributor

@atouchet atouchet commented Oct 21, 2018

d6092fa was reverted in #21971.
@atouchet atouchet reopened this Oct 21, 2018
@atouchet atouchet mentioned this issue Mar 3, 2020
Closed
@mrobinson mrobinson mentioned this issue Apr 27, 2020
Merged
4 of 5 tasks complete
@mrobinson mrobinson self-assigned this Apr 27, 2020
bors-servo added a commit that referenced this issue Apr 28, 2020
@bors-servo
Auto merge of #26331 - mrobinson:animation-update-on-restyle, r=jdm,e…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
1274980 
…milio

Restyle should reflect animations and transitions

When doing a restyle, we should apply animations and transitions to the
new style so that it is reflected in `getComputedStyle()` and the new
style information properly cascades. This is the first part of properly
ticking animations and transitions.

This causes a couple new animations tests failures (along with many new
passes), but we currently don't have support for properly handling
animations after they have completed, so this isn't totally unexpected.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #20116

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Apr 29, 2020
@bors-servo
Auto merge of #26331 - mrobinson:animation-update-on-restyle, r=jdm,e…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
ef9bce0 
…milio

Restyle should reflect animations and transitions

When doing a restyle, we should apply animations and transitions to the
new style so that it is reflected in `getComputedStyle()` and the new
style information properly cascades. This is the first part of properly
ticking animations and transitions.

This causes a couple new animations tests failures (along with many new
passes), but we currently don't have support for properly handling
animations after they have completed, so this isn't totally unexpected.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #20116

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Apr 29, 2020
@bors-servo
Auto merge of #26331 - mrobinson:animation-update-on-restyle, r=jdm,e…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
7a75b25 
…milio

Restyle should reflect animations and transitions

When doing a restyle, we should apply animations and transitions to the
new style so that it is reflected in `getComputedStyle()` and the new
style information properly cascades. This is the first part of properly
ticking animations and transitions.

This causes a couple new animations tests failures (along with many new
passes), but we currently don't have support for properly handling
animations after they have completed, so this isn't totally unexpected.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #20116

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Apr 29, 2020
@bors-servo
Auto merge of #26331 - mrobinson:animation-update-on-restyle, r=<try>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
d0d306d 
Restyle should reflect animations and transitions

When doing a restyle, we should apply animations and transitions to the
new style so that it is reflected in `getComputedStyle()` and the new
style information properly cascades. This is the first part of properly
ticking animations and transitions.

This causes a couple new animations tests failures (along with many new
passes), but we currently don't have support for properly handling
animations after they have completed, so this isn't totally unexpected.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #20116

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Apr 29, 2020
@bors-servo
Auto merge of #26331 - mrobinson:animation-update-on-restyle, r=jdm,e…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
0d0cae6 
…milio

Restyle should reflect animations and transitions

When doing a restyle, we should apply animations and transitions to the
new style so that it is reflected in `getComputedStyle()` and the new
style information properly cascades. This is the first part of properly
ticking animations and transitions.

This causes a couple new animations tests failures (along with many new
passes), but we currently don't have support for properly handling
animations after they have completed, so this isn't totally unexpected.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #20116

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Apr 29, 2020
@bors-servo
Auto merge of #26331 - mrobinson:animation-update-on-restyle, r=<try>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
24c77b6 
Restyle should reflect animations and transitions

When doing a restyle, we should apply animations and transitions to the
new style so that it is reflected in `getComputedStyle()` and the new
style information properly cascades. This is the first part of properly
ticking animations and transitions.

This causes a couple new animations tests failures (along with many new
passes), but we currently don't have support for properly handling
animations after they have completed, so this isn't totally unexpected.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #20116

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Apr 29, 2020
@bors-servo
Auto merge of #26331 - mrobinson:animation-update-on-restyle, r=<try>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
258654e 
Restyle should reflect animations and transitions

When doing a restyle, we should apply animations and transitions to the
new style so that it is reflected in `getComputedStyle()` and the new
style information properly cascades. This is the first part of properly
ticking animations and transitions.

This causes a couple new animations tests failures (along with many new
passes), but we currently don't have support for properly handling
animations after they have completed, so this isn't totally unexpected.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #20116

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Apr 29, 2020
@bors-servo
Auto merge of #26331 - mrobinson:animation-update-on-restyle, r=jdm,e…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
5925d42 
…milio

Restyle should reflect animations and transitions

When doing a restyle, we should apply animations and transitions to the
new style so that it is reflected in `getComputedStyle()` and the new
style information properly cascades. This is the first part of properly
ticking animations and transitions.

This causes a couple new animations tests failures (along with many new
passes), but we currently don't have support for properly handling
animations after they have completed, so this isn't totally unexpected.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #20116

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mrobinson mrobinson

Labels
A-content/css A-layout/animations
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

style: Some animation cleanups and fixes.
Restyle should reflect animations and transitions
4 participants
@mrobinson @jdm @emilio @atouchet
You can’t perform that action at this time.