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

Make transform property animatable for stylo #15363

Merged
merged 3 commits into from Feb 4, 2017

Conversation

Projects
None yet
7 participants
@hiikezoe
Contributor

hiikezoe commented Feb 3, 2017

This is the servo side fix for https://bugzilla.mozilla.org/show_bug.cgi?id=1332657

Reviewed by @heycam and @Manishearth.
Thanks!


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #__ (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because this is for stylo.

This change is Reviewable

@highfive

This comment has been minimized.

Show comment
Hide comment
@highfive

highfive Feb 3, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/gecko_bindings/sugar/ns_css_value.rs, components/style/properties/longhand/box.mako.rs, components/style/properties/gecko.mako.rs, components/style/properties/helpers/animated_properties.mako.rs
  • @emilio: components/style/gecko_bindings/sugar/ns_css_value.rs, components/style/properties/longhand/box.mako.rs, components/style/properties/gecko.mako.rs, components/style/properties/helpers/animated_properties.mako.rs

highfive commented Feb 3, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/gecko_bindings/sugar/ns_css_value.rs, components/style/properties/longhand/box.mako.rs, components/style/properties/gecko.mako.rs, components/style/properties/helpers/animated_properties.mako.rs
  • @emilio: components/style/gecko_bindings/sugar/ns_css_value.rs, components/style/properties/longhand/box.mako.rs, components/style/properties/gecko.mako.rs, components/style/properties/helpers/animated_properties.mako.rs
@highfive

This comment has been minimized.

Show comment
Hide comment
@highfive

highfive Feb 3, 2017

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify style code, but no tests are modified. Please consider adding a test!

highfive commented Feb 3, 2017

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify style code, but no tests are modified. Please consider adding a test!
@heycam

This comment has been minimized.

Show comment
Hide comment
@heycam

heycam Feb 3, 2017

Member

@hiikezoe can you adjust the commit messages here, so that they don't seem too Gecko-specific? I think "Implement clone_transform." and "Make transform property animatable." would be sufficient (i.e., without the bug/part number and r= bits).

Member

heycam commented Feb 3, 2017

@hiikezoe can you adjust the commit messages here, so that they don't seem too Gecko-specific? I think "Implement clone_transform." and "Make transform property animatable." would be sufficient (i.e., without the bug/part number and r= bits).

@KiChjang

This comment has been minimized.

Show comment
Hide comment
@KiChjang
Member

KiChjang commented Feb 3, 2017

r? @heycam

@highfive highfive assigned heycam and unassigned KiChjang Feb 3, 2017

@emilio

This comment has been minimized.

Show comment
Hide comment
@emilio

emilio Feb 3, 2017

Member

@bors-servo r=heycam,Manishearth

Member

emilio commented Feb 3, 2017

@bors-servo r=heycam,Manishearth

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Feb 3, 2017

Contributor

📌 Commit 693fece has been approved by heycam,Manishearth

Contributor

bors-servo commented Feb 3, 2017

📌 Commit 693fece has been approved by heycam,Manishearth

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Feb 3, 2017

Contributor

⌛️ Testing commit 693fece with merge 8412c56...

Contributor

bors-servo commented Feb 3, 2017

⌛️ Testing commit 693fece with merge 8412c56...

bors-servo added a commit that referenced this pull request Feb 3, 2017

Auto merge of #15363 - hiikezoe:transform-animatable, r=heycam,Manish…
…earth

Make transform property animatable for stylo

<!-- Please describe your changes on the following line: -->
This is the servo side fix for https://bugzilla.mozilla.org/show_bug.cgi?id=1332657

Reviewed by @heycam and @Manishearth.
Thanks!

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because this is for stylo.

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

<!-- 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/15363)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Feb 3, 2017

Contributor

💔 Test failed - mac-dev-unit

Contributor

bors-servo commented Feb 3, 2017

💔 Test failed - mac-dev-unit

@Manishearth

This comment has been minimized.

Show comment
Hide comment
@Manishearth

Manishearth Feb 3, 2017

Member

You need to commit the generated bindings changes, too.

Member

Manishearth commented Feb 3, 2017

You need to commit the generated bindings changes, too.

hiikezoe added some commits Feb 3, 2017

Implement clone_transform.
The implementation of clone_transform is an adaptation of set_transform.

MozReview-Commit-ID: ESE1ha0x666
Make transform property animatable.
This just changes animatable to True, drops 'if product == "servo" block
, fixes indentations and moves 'use' declarations at the top of the file.

MozReview-Commit-ID: A96oxYXmknV
@hiikezoe

This comment has been minimized.

Show comment
Hide comment
@hiikezoe

hiikezoe Feb 3, 2017

Contributor

Oops, I did totally forget about it since auto bindgen struff works perfectly these days.

Contributor

hiikezoe commented Feb 3, 2017

Oops, I did totally forget about it since auto bindgen struff works perfectly these days.

@KiChjang

This comment has been minimized.

Show comment
Hide comment
@KiChjang

KiChjang Feb 4, 2017

Member

@bors-servo r=heycam,Manishearth

Member

KiChjang commented Feb 4, 2017

@bors-servo r=heycam,Manishearth

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Feb 4, 2017

Contributor

📌 Commit d588427 has been approved by heycam,Manishearth

Contributor

bors-servo commented Feb 4, 2017

📌 Commit d588427 has been approved by heycam,Manishearth

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Feb 4, 2017

Contributor

⌛️ Testing commit d588427 with merge cd2dbd7...

Contributor

bors-servo commented Feb 4, 2017

⌛️ Testing commit d588427 with merge cd2dbd7...

bors-servo added a commit that referenced this pull request Feb 4, 2017

Auto merge of #15363 - hiikezoe:transform-animatable, r=heycam,Manish…
…earth

Make transform property animatable for stylo

<!-- Please describe your changes on the following line: -->
This is the servo side fix for https://bugzilla.mozilla.org/show_bug.cgi?id=1332657

Reviewed by @heycam and @Manishearth.
Thanks!

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because this is for stylo.

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

<!-- 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/15363)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Feb 4, 2017

Contributor

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-gnu-dev, windows-msvc-dev
Approved by: heycam,Manishearth
Pushing cd2dbd7 to master...

Contributor

bors-servo commented Feb 4, 2017

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-gnu-dev, windows-msvc-dev
Approved by: heycam,Manishearth
Pushing cd2dbd7 to master...

@bors-servo bors-servo merged commit d588427 into servo:master Feb 4, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@hiikezoe hiikezoe deleted the hiikezoe:transform-animatable branch Feb 4, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment