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

Add support for canceling CSS transitions #26244

Merged
merged 1 commit into from Apr 22, 2020

Conversation

@mrobinson
Copy link
Member

mrobinson commented Apr 21, 2020

This change adds support for canceling CSS transitions when a property is
no longer transitionable. Support for canceling and replacing CSS
transitions when the end value changes is still pending. This change
also takes advantage of updating the constellation message to fix a bug
where transition events could be sent for closed pipelines.

Fixes #15079.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #15079.
  • There are tests for these changes OR
  • These changes do not require tests because ___
@mrobinson mrobinson requested review from jdm and emilio Apr 21, 2020
@highfive
Copy link

highfive commented Apr 21, 2020

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/webidls/EventHandler.webidl, components/script/dom/macros.rs, components/script/script_thread.rs
  • @KiChjang: components/script/dom/webidls/EventHandler.webidl, components/script/dom/macros.rs, components/script/script_thread.rs, components/script_traits/lib.rs
  • @emilio: components/style/context.rs, components/layout/animation.rs, components/style/animation.rs, components/style/matching.rs
@jdm
jdm approved these changes Apr 21, 2020
@jdm
Copy link
Member

jdm commented Apr 21, 2020

@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented Apr 21, 2020

Trying commit 110ed92 with merge cee29f4...

bors-servo added a commit that referenced this pull request Apr 21, 2020
Add support for canceling CSS transitions

This change adds support for canceling CSS transitions when a property is
no longer transitionable. Support for canceling and replacing CSS
transitions when the end value changes is still pending. This change
also takes advantage of updating the constellation message to fix a bug
where transition events could be sent for closed pipelines.

Fixes #15079.

<!-- 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 #15079.

<!-- 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
Copy link
Contributor

bors-servo commented Apr 21, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Apr 21, 2020

Since this exposes a new DOM event, it seems surprising that no tests demonstrate a change in behaviour. Maybe add a test that shows that these changes are working?

@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Apr 22, 2020

Opened new PR for upstreamable changes.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#23166.

@mrobinson mrobinson force-pushed the mrobinson:animation-cancel branch from 0107a44 to a02b13d Apr 22, 2020
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Apr 22, 2020

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#23166.

@mrobinson
Copy link
Member Author

mrobinson commented Apr 22, 2020

It seems like the web-platform-tests have a few tests which detect transitioncancel when an element becomes display: none or when it's removed from the DOM, but it doesn't seem like there are any testing when a property is no longer transitionable. I've done two things:

  • Added a test which verifies this case.
  • Modified my change to handle the element becoming display: none causing one more WPT test to pass.

The rest of the tests that test transitioncancel events seem to rely on other features that we don't implement yet (such as transitionrun events or DOM removal). Hopefully these will start passing as we implement more events.

I'm not sure what the procedure is when adding new WPT tests, but I'm happy to work through that.

(the @servo-wpt-sync bot is really cool!)

@mrobinson
Copy link
Member Author

mrobinson commented Apr 22, 2020

@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented Apr 22, 2020

Trying commit a02b13d with merge 1114a85...

bors-servo added a commit that referenced this pull request Apr 22, 2020
Add support for canceling CSS transitions

This change adds support for canceling CSS transitions when a property is
no longer transitionable. Support for canceling and replacing CSS
transitions when the end value changes is still pending. This change
also takes advantage of updating the constellation message to fix a bug
where transition events could be sent for closed pipelines.

Fixes #15079.

<!-- 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 #15079.

<!-- 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
Copy link
Contributor

bors-servo commented Apr 22, 2020

☀️ Test successful - status-taskcluster
State: approved= try=True

@jdm
Copy link
Member

jdm commented Apr 22, 2020

{"path": "css/css-transitions/transitioncancel-002.html", "message": "Test-file line has a `console.*(...)` call", "lineno": 29, "rule": "CONSOLE"}
{"path": "css/css-transitions/transitioncancel-002.html", "message": "setTimeout used", "lineno": 39, "rule": "SET TIMEOUT"}
{"path": "css/css-transitions/transitioncancel-002.html", "message": "setTimeout used", "lineno": 44, "rule": "SET TIMEOUT"}
Copy link
Member

jdm left a comment

looks good! don't forget to run mach update-manifest after changing the test file.

getComputedStyle(div).backgroundColor;

// Remove the transitioning property from transition-property asynchronously.
setTimeout(() => { div.style.transitionProperty = 'none'; }, 0);

This comment has been minimized.

Copy link
@jdm

jdm Apr 22, 2020

Member

use t.step_timeout instead.

await eventWatcher.wait_for('transitioncancel');

// There should be no transitionend events received.
await new Promise(resolve => setTimeout(resolve, 300));

This comment has been minimized.

Copy link
@jdm

jdm Apr 22, 2020

Member

t.step_timeout

// Attach event listeners
const eventWatcher = new EventWatcher(t, div, ['transitioncancel']);
div.addEventListener('transitionend', t.step_func((event) => {
console.log(event);

This comment has been minimized.

Copy link
@jdm

jdm Apr 22, 2020

Member

remove this to satisfy the linter.

@mrobinson mrobinson force-pushed the mrobinson:animation-cancel branch from a02b13d to 88a1f6c Apr 22, 2020
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Apr 22, 2020

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#23166.

@jdm
Copy link
Member

jdm commented Apr 22, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Apr 22, 2020

📌 Commit 88a1f6c has been approved by jdm

bors-servo added a commit that referenced this pull request Apr 22, 2020
Add support for canceling CSS transitions

This change adds support for canceling CSS transitions when a property is
no longer transitionable. Support for canceling and replacing CSS
transitions when the end value changes is still pending. This change
also takes advantage of updating the constellation message to fix a bug
where transition events could be sent for closed pipelines.

Fixes #15079.

<!-- 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 #15079.

<!-- 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
Copy link
Contributor

bors-servo commented Apr 22, 2020

Testing commit 453b252 with merge 61d5b47...

@bors-servo
Copy link
Contributor

bors-servo commented Apr 22, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Apr 22, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Apr 22, 2020

Testing commit 453b252 with merge ccdf2d7...

bors-servo added a commit that referenced this pull request Apr 22, 2020
Add support for canceling CSS transitions

This change adds support for canceling CSS transitions when a property is
no longer transitionable. Support for canceling and replacing CSS
transitions when the end value changes is still pending. This change
also takes advantage of updating the constellation message to fix a bug
where transition events could be sent for closed pipelines.

Fixes #15079.

<!-- 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 #15079.

<!-- 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
Copy link
Contributor

bors-servo commented Apr 22, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Apr 22, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Apr 22, 2020

Testing commit 453b252 with merge 610991b...

@bors-servo
Copy link
Contributor

bors-servo commented Apr 22, 2020

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing 610991b to master...

@bors-servo bors-servo merged commit 610991b into servo:master Apr 22, 2020
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
@mrobinson mrobinson deleted the mrobinson:animation-cancel branch Apr 22, 2020
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 23, 2020
This change adds support for canceling CSS transitions when a property
is no longer transitionable or when an element becomes styled with
display:none. Support for canceling and replacing CSS transitions when
the end value changes is still pending. This change also takes advantage
of updating the constellation message to fix a bug where transition
events could be sent for closed pipelines.

Cherry-picked from servo/servo#26244
(though this is not part of the Gecko build).
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Apr 23, 2020
This change adds support for canceling CSS transitions when a property
is no longer transitionable or when an element becomes styled with
display:none. Support for canceling and replacing CSS transitions when
the end value changes is still pending. This change also takes advantage
of updating the constellation message to fix a bug where transition
events could be sent for closed pipelines.

Cherry-picked from servo/servo#26244
(though this is not part of the Gecko build).
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Apr 25, 2020
This change adds support for canceling CSS transitions when a property
is no longer transitionable or when an element becomes styled with
display:none. Support for canceling and replacing CSS transitions when
the end value changes is still pending. This change also takes advantage
of updating the constellation message to fix a bug where transition
events could be sent for closed pipelines.

Cherry-picked from servo/servo#26244
(though this is not part of the Gecko build).

UltraBlame original commit: 612b48f120bac79925d232386e6fe12bb833b027
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Apr 25, 2020
This change adds support for canceling CSS transitions when a property
is no longer transitionable or when an element becomes styled with
display:none. Support for canceling and replacing CSS transitions when
the end value changes is still pending. This change also takes advantage
of updating the constellation message to fix a bug where transition
events could be sent for closed pipelines.

Cherry-picked from servo/servo#26244
(though this is not part of the Gecko build).

UltraBlame original commit: 612b48f120bac79925d232386e6fe12bb833b027
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Apr 25, 2020
This change adds support for canceling CSS transitions when a property
is no longer transitionable or when an element becomes styled with
display:none. Support for canceling and replacing CSS transitions when
the end value changes is still pending. This change also takes advantage
of updating the constellation message to fix a bug where transition
events could be sent for closed pipelines.

Cherry-picked from servo/servo#26244
(though this is not part of the Gecko build).

UltraBlame original commit: 612b48f120bac79925d232386e6fe12bb833b027
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

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