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

Implement `animation-fill-mode` #26519

Merged
merged 1 commit into from May 17, 2020

Conversation

@mrobinson
Copy link
Member

mrobinson commented May 14, 2020

Fixes #26460.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • There are tests for these changes
@mrobinson mrobinson requested a review from emilio May 14, 2020
@highfive
Copy link

highfive commented May 14, 2020

Heads up! This PR modifies the following files:

  • @emilio: components/style/animation.rs, components/style/matching.rs
@mrobinson
Copy link
Member Author

mrobinson commented May 14, 2020

Note: There is a set of small manual tests in web-platform-tests, but I've added an automated test using Servo's testing time warp functionality.

@jdm
jdm approved these changes May 15, 2020
@emilio
emilio approved these changes May 16, 2020
@mrobinson
Copy link
Member Author

mrobinson commented May 16, 2020

@bors-servo r=jdm,emilio

@bors-servo
Copy link
Contributor

bors-servo commented May 16, 2020

📌 Commit ea58eeb has been approved by jdm,emilio

@highfive highfive assigned jdm and unassigned SimonSapin May 16, 2020
@bors-servo
Copy link
Contributor

bors-servo commented May 16, 2020

Testing commit ea58eeb with merge 9ccf667...

bors-servo added a commit that referenced this pull request May 16, 2020
…,emilio

Implement `animation-fill-mode`

Fixes #26460.

<!-- 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] There are tests for these changes

<!-- 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 May 16, 2020

💔 Test failed - status-taskcluster

@CYBAI
Copy link
Collaborator

CYBAI commented May 17, 2020

Failed in Linux:

error[E0433]: failed to resolve: could not find `animation_fill_mode` in `longhands`
  --> components/style/animation.rs:16:35
   |
16 | use crate::properties::longhands::animation_fill_mode::computed_value::single_value::T as AnimationFillMode;
   |                                   ^^^^^^^^^^^^^^^^^^^ could not find `animation_fill_mode` in `longhands`
error[E0433]: failed to resolve: use of undeclared type or module `AnimationFillMode`
   --> components/style/animation.rs:385:31
    |
385 |             self.fill_mode != AnimationFillMode::Backwards &&
    |                               ^^^^^^^^^^^^^^^^^ use of undeclared type or module `AnimationFillMode`
error[E0433]: failed to resolve: use of undeclared type or module `AnimationFillMode`
   --> components/style/animation.rs:386:31
    |
386 |             self.fill_mode != AnimationFillMode::Both
    |                               ^^^^^^^^^^^^^^^^^ use of undeclared type or module `AnimationFillMode`
error[E0433]: failed to resolve: use of undeclared type or module `AnimationFillMode`
   --> components/style/animation.rs:392:31
    |
392 |             self.fill_mode != AnimationFillMode::Forwards &&
    |                               ^^^^^^^^^^^^^^^^^ use of undeclared type or module `AnimationFillMode`
error[E0433]: failed to resolve: use of undeclared type or module `AnimationFillMode`
   --> components/style/animation.rs:393:31
    |
393 |             self.fill_mode != AnimationFillMode::Both
    |                               ^^^^^^^^^^^^^^^^^ use of undeclared type or module `AnimationFillMode`
error[E0412]: cannot find type `AnimationFillMode` in this scope
   --> components/style/animation.rs:189:20
    |
189 |     pub fill_mode: AnimationFillMode,
    |                    ^^^^^^^^^^^^^^^^^ not found in this scope
warning: ignoring -C extra-filename flag due to -o flag
error: aborting due to 6 previous errors
Some errors have detailed explanations: E0412, E0433.
For more information about an error, try `rustc --explain E0412`.
error: could not compile `style`.

and some more tests pass now 🎉

  ▶ Unexpected subtest result in /css/css-variables/variable-animation-substitute-within-keyframe-fallback.html:
  └ PASS [expected FAIL] Verify color before animation
  ▶ Unexpected subtest result in /css/css-variables/variable-animation-substitute-within-keyframe-multiple.html:
  └ PASS [expected FAIL] Verify color before animation
  ▶ Unexpected subtest result in /css/css-variables/variable-animation-substitute-into-keyframe.html:
  └ PASS [expected FAIL] Verify color before animation
  ▶ Unexpected subtest result in /css/css-variables/variable-animation-substitute-within-keyframe.html:
  └ PASS [expected FAIL] Verify color before animation
  ▶ Unexpected subtest result in /css/cssom/getComputedStyle-animations-replaced-into-ib-split.html:
  └ PASS [expected FAIL] getComputedStyle() should return animation styles for nodes just inserted into the document, even if they're in an IB-split
@mrobinson mrobinson force-pushed the mrobinson:animation-animation-fill-mode branch from ea58eeb to cd12c38 May 17, 2020
@mrobinson
Copy link
Member Author

mrobinson commented May 17, 2020

@bors-servo r=jdm,emilio

@bors-servo
Copy link
Contributor

bors-servo commented May 17, 2020

📌 Commit cd12c38 has been approved by jdm,emilio

@bors-servo
Copy link
Contributor

bors-servo commented May 17, 2020

Testing commit cd12c38 with merge abde2a6...

bors-servo added a commit that referenced this pull request May 17, 2020
…,emilio

Implement `animation-fill-mode`

Fixes #26460.

<!-- 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] There are tests for these changes

<!-- 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 May 17, 2020

💔 Test failed - status-taskcluster

Fixes #26460.
@mrobinson mrobinson force-pushed the mrobinson:animation-animation-fill-mode branch from cd12c38 to 183f15d May 17, 2020
@mrobinson
Copy link
Member Author

mrobinson commented May 17, 2020

Okay. I've updated the change to include the ini file and have also rebased the layout_2020 test results for this directory. The tests will fail because support for animations in layout 2020 hasn't landed yet.

@jdm
Copy link
Member

jdm commented May 17, 2020

@bors-servo
Copy link
Contributor

bors-servo commented May 17, 2020

📌 Commit 183f15d has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented May 17, 2020

Testing commit 183f15d with merge 50bd5c3...

@bors-servo
Copy link
Contributor

bors-servo commented May 17, 2020

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing 50bd5c3 to master...

@bors-servo bors-servo merged commit 50bd5c3 into servo:master May 17, 2020
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
@mrobinson mrobinson deleted the mrobinson:animation-animation-fill-mode branch May 17, 2020
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.

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