Skip to content
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

requestAnimationFrame seems to break CSS transitions #12749

Closed
Gozala opened this issue Aug 5, 2016 · 2 comments
Closed

requestAnimationFrame seems to break CSS transitions #12749

Gozala opened this issue Aug 5, 2016 · 2 comments

Comments

@Gozala
Copy link
Contributor

@Gozala Gozala commented Aug 5, 2016

I have created a following test case that you can try as servo -w https://Gozala.jsbin.com/vujonov

When "Request animation frames" is checked animation css transition is not performed, instead just last frame is drawn when transition is complete. If it is unchecked transitions are performed as expected. Checkbox just toggles requestAnimationFrame loop that seems to cause a bug in CSS transitions.

P.S.: It does not seems to matter if we do anything on animation frames or not.

@Gozala
Copy link
Contributor Author

@Gozala Gozala commented Aug 5, 2016

/cc @emilio

@emilio
Copy link
Member

@emilio emilio commented Aug 5, 2016

The problem is that the constellation doesn't send TickAnimations messages to layout if there's a requestAnimationFrame loop. I don't know how easy/hard to fix this is yet.

emilio added a commit to emilio/servo that referenced this issue Aug 5, 2016
…nimation frames.

The script tick ends up only processing JS callbacks related to animation
frames, so CSS transitions/animations end up not working as expected.

This could have accidentally worked before servo#12563 because we over-restyled, but
now this is no longer the case.

Other possible way to do it is making a layout reflow with RAF handle CSS
animations/transitions too, but that may not work if the reflow ends up being
suppressed (that could very well be the case), and we'd need to handle a lot
more state in the document, so this solution (assuming it doesn't break try)
seems a bit less flacky.

Fixes servo#12749.
bors-servo added a commit that referenced this issue Aug 5, 2016
compositor: Send animation ticks to layout even if there are script animation frames.

<!-- 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 #12749 (github issue number if applicable).

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

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

The script tick ends up only processing JS callbacks related to animation
frames, so CSS transitions/animations end up not working as expected.

This could have accidentally worked before #12563 because we over-restyled, but
now this is no longer the case.

Other possible way to do it is making a layout reflow with RAF handle CSS
animations/transitions too, but that may not work if the reflow ends up being
suppressed (that could very well be the case), and we'd need to handle a lot
more state in the document, so this solution (assuming it doesn't break try)
seems a bit less flacky.

Missing a test, will add one soon. Fixes #12749.
@jdm jdm added the A-constellation label Aug 5, 2016
emilio added a commit to emilio/servo that referenced this issue Aug 5, 2016
emilio added a commit to emilio/servo that referenced this issue Aug 5, 2016
emilio added a commit to emilio/servo that referenced this issue Aug 5, 2016
bors-servo added a commit that referenced this issue Aug 8, 2016
compositor: Send animation ticks to layout even if there are script animation frames.

<!-- 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 #12749 (github issue number if applicable).

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

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

The script tick ends up only processing JS callbacks related to animation
frames, so CSS transitions/animations end up not working as expected.

This could have accidentally worked before #12563 because we over-restyled, but
now this is no longer the case.

Other possible way to do it is making a layout reflow with RAF handle CSS
animations/transitions too, but that may not work if the reflow ends up being
suppressed (that could very well be the case), and we'd need to handle a lot
more state in the document, so this solution (assuming it doesn't break try)
seems a bit less flacky.

Missing a test, will add one soon. Fixes #12749.

<!-- 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/12751)
<!-- Reviewable:end -->
@pyfisch pyfisch mentioned this issue Jan 4, 2018
4 of 13 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.