Stylo - support image-orientation property #16334

chenpighead · 2017-04-10T07:34:53Z

First, we need to make Servo's image-orientation parser to be agreed with Gecko's.
Numbers without any AngleUnit, including unitless 0 angle, should be invalid
for image-orientation. However, rotate() and skew() for transform properties
accept unitless 0 angle. In order to make all these properties work properly,
I fixed Angle::parse() to match Gecko. For the existing users of Angle::parse(),
I create Angle::parse_with_unitless() and use it as an alternative for them.
Once w3c/csswg-drafts#1162 is resolved, we shall be
able to use an unified version of Angle::parse() then.

The parser of image-orientation is also fixed to report parsing errors on
empty string.

Then, with the newly added binding functions support in Gecko side, we shall
reuse the same methods from Gecko to pass the computed value from Servo to Gecko.

Gecko bug: Bug 1341758

./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 change is

highfive · 2017-04-10T07:34:59Z

Heads up! This PR modifies the following files:

@bholley: components/style/properties/gecko.mako.rs, components/style/properties/longhand/box.mako.rs, components/style/values/specified/mod.rs, components/style/properties/longhand/inherited_box.mako.rs
@emilio: components/style/properties/gecko.mako.rs, components/style/properties/longhand/box.mako.rs, components/style/values/specified/mod.rs, components/style/properties/longhand/inherited_box.mako.rs

highfive · 2017-04-10T07:35:00Z

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!

wafflespeanut · 2017-04-10T09:13:34Z

~~Unit tests need to be updated.~~ Oh, you just did 😄

chenpighead · 2017-04-10T09:15:02Z

Yeah, just noticed that and updated the PR.
I wonder why this only happens on Windows....

wafflespeanut · 2017-04-10T09:17:41Z

Oh, we've disabled some stuff in Travis since it has previously caused problems (#15076, #14723)

chenpighead · 2017-04-11T08:07:58Z

stylo try looks fine: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6608cc9bd1b117d58a980535560a3a8b7d5401f

r? @emilio

emilio · 2017-04-11T08:23:49Z

@bors-servo r+

Thanks!

bors-servo · 2017-04-11T08:23:50Z

📌 Commit e554254 has been approved by emilio

nox · 2017-04-11T08:25:10Z

components/style/properties/gecko.mako.rs

+        }
+    }
+
+    pub fn copy_image_orientation_from(&mut self, other: &Self) {


~~This would be better as Clone::clone_from, IMO.~~ Nevermind.

We'd need to change all the implementations in the style system though.

And this only copies a property, which isn't what that trait is meant to represent, right?

Err yes, forgot on which type this method is, with the mako stuff. :)

No idea what Clone::clone_from is...
Is this related to the clone_xxx_from()? I thought we need clone only for animatable properties, no?

bors-servo · 2017-04-11T08:49:16Z

⌛ Testing commit e554254 with merge a3bcbb7...


        Auto merge of #16334 - chenpighead:stylo-image-orientation-support, r…

…=emilio Stylo - support image-orientation property First, we need to make Servo's image-orientation parser to be agreed with Gecko's. Numbers without any AngleUnit, including unitless 0 angle, should be invalid for image-orientation. However, rotate() and skew() for transform properties accept unitless 0 angle. In order to make all these properties work properly, I fixed Angle::parse() to match Gecko. For the existing users of Angle::parse(), I create Angle::parse_with_unitless() and use it as an alternative for them. Once w3c/csswg-drafts#1162 is resolved, we shall be able to use an unified version of Angle::parse() then. The parser of image-orientation is also fixed to report parsing errors on empty string. Then, with the newly added binding functions support in Gecko side, we shall reuse the same methods from Gecko to pass the computed value from Servo to Gecko. Gecko bug: Bug 1341758  ---  - [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).  - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16334)

bors-servo · 2017-04-11T09:00:00Z

💔 Test failed - windows-msvc-dev

bholley · 2017-04-11T09:53:42Z

Needs a bindings.rs update. You can grab them as an artifact from your try push.

chenpighead · 2017-04-11T10:41:51Z

Ok, I grab the bindings update from try. Hopefully we could pass the tests now.
@bholley, thanks for the help. :)

bholley · 2017-04-11T10:48:57Z

@bors-servo r+

bors-servo · 2017-04-11T10:48:58Z

📌 Commit 9a1a9d8 has been approved by bholley

bors-servo · 2017-04-11T10:49:02Z

⌛ Testing commit 9a1a9d8 with merge 9d8f120...

nox · 2017-04-11T11:18:42Z

These 2 commits should be squashed together.

nox · 2017-04-11T11:19:33Z

Actually no, but the commit that updates the bindings should be first in the PR.


        Stylo - update bindings.


        Stylo - support image-orientation property

First, we need to make Servo's image-orientation parser to be agreed with Gecko's. Numbers without any AngleUnit, including unitless 0 angle, should be invalid for image-orientation. However, rotate() and skew() for transform properties accept unitless 0 angle. In order to make all these properties work properly, I fixed Angle::parse() to match Gecko. For the existing users of Angle::parse(), I create Angle::parse_with_unitless() and use it as an alternative for them. Once w3c/csswg-drafts#1162 is resolved, we shall be able to use an unified version of Angle::parse() then. The parser of image-orientation is also fixed to report parsing errors on empty string. Then, with the newly added binding functions support in Gecko side, we shall reuse the same methods from Gecko to pass the computed value from Servo to Gecko. Gecko bug: Bug 1341758

chenpighead · 2017-04-11T11:24:15Z

Ok, update on the way.

nox · 2017-04-11T11:42:07Z

@bors-servo r=bholley

bors-servo · 2017-04-11T11:42:08Z

📌 Commit 5332d4b has been approved by bholley

bors-servo · 2017-04-11T11:42:12Z

⌛ Testing commit 5332d4b with merge fcce57f...


        Auto merge of #16334 - chenpighead:stylo-image-orientation-support, r…

…=bholley Stylo - support image-orientation property First, we need to make Servo's image-orientation parser to be agreed with Gecko's. Numbers without any AngleUnit, including unitless 0 angle, should be invalid for image-orientation. However, rotate() and skew() for transform properties accept unitless 0 angle. In order to make all these properties work properly, I fixed Angle::parse() to match Gecko. For the existing users of Angle::parse(), I create Angle::parse_with_unitless() and use it as an alternative for them. Once w3c/csswg-drafts#1162 is resolved, we shall be able to use an unified version of Angle::parse() then. The parser of image-orientation is also fixed to report parsing errors on empty string. Then, with the newly added binding functions support in Gecko side, we shall reuse the same methods from Gecko to pass the computed value from Servo to Gecko. Gecko bug: Bug 1341758  ---  - [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).  - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16334)

bors-servo · 2017-04-11T12:14:41Z

💔 Test failed - linux-rel-css

chenpighead · 2017-04-11T13:14:21Z

Looks like just a timeout thing....:/
@bors-servo retry

bors-servo · 2017-04-11T13:14:22Z

@chenpighead: 🔑 Insufficient privileges: and not in try users

nox · 2017-04-11T13:15:22Z

@chenpighead What is "a timeout thing"? We usually don't retry PR without filing an intermittent issue for the failing tests, if we consider them intermittent.

emilio · 2017-04-11T14:09:21Z

@bors-servo retry #16351

bors-servo · 2017-04-11T14:37:24Z

⌛ Testing commit 5332d4b with merge 2544c08...


        Auto merge of #16334 - chenpighead:stylo-image-orientation-support, r…

…=bholley Stylo - support image-orientation property First, we need to make Servo's image-orientation parser to be agreed with Gecko's. Numbers without any AngleUnit, including unitless 0 angle, should be invalid for image-orientation. However, rotate() and skew() for transform properties accept unitless 0 angle. In order to make all these properties work properly, I fixed Angle::parse() to match Gecko. For the existing users of Angle::parse(), I create Angle::parse_with_unitless() and use it as an alternative for them. Once w3c/csswg-drafts#1162 is resolved, we shall be able to use an unified version of Angle::parse() then. The parser of image-orientation is also fixed to report parsing errors on empty string. Then, with the newly added binding functions support in Gecko side, we shall reuse the same methods from Gecko to pass the computed value from Servo to Gecko. Gecko bug: Bug 1341758  ---  - [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).  - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16334)

bors-servo · 2017-04-11T15:12:06Z

☀️ 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-msvc-dev
Approved by: bholley
Pushing 2544c08 to master...

highfive assigned emilio Apr 10, 2017

highfive added the S-awaiting-review label Apr 10, 2017

chenpighead force-pushed the chenpighead:stylo-image-orientation-support branch from 354e3e5 to e554254 Apr 10, 2017

highfive added S-awaiting-merge and removed S-awaiting-review labels Apr 11, 2017

nox reviewed Apr 11, 2017

View changes

highfive added S-tests-failed and removed S-awaiting-merge labels Apr 11, 2017

highfive added S-awaiting-review and removed S-tests-failed labels Apr 11, 2017

highfive assigned bholley and unassigned emilio Apr 11, 2017

highfive added S-awaiting-merge and removed S-awaiting-review labels Apr 11, 2017

highfive added S-tests-failed and removed S-awaiting-merge labels Apr 11, 2017

chenpighead added 2 commits Apr 11, 2017

Stylo - update bindings.

b5a0032

chenpighead force-pushed the chenpighead:stylo-image-orientation-support branch from 9a1a9d8 to 5332d4b Apr 11, 2017

highfive added S-awaiting-review and removed S-tests-failed labels Apr 11, 2017

highfive added S-awaiting-merge and removed S-awaiting-review labels Apr 11, 2017

highfive added S-tests-failed and removed S-awaiting-merge labels Apr 11, 2017

highfive added S-awaiting-merge and removed S-tests-failed labels Apr 11, 2017

bors-servo merged commit 5332d4b into servo:master Apr 11, 2017
3 checks passed

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

highfive removed the S-awaiting-merge label Apr 11, 2017

servo / servo

Stylo - support image-orientation property #16334

Stylo - support image-orientation property #16334

chenpighead commented Apr 10, 2017 •

edited by larsbergstrom

highfive commented Apr 10, 2017

highfive commented Apr 10, 2017

wafflespeanut commented Apr 10, 2017 •

edited

chenpighead commented Apr 10, 2017

wafflespeanut commented Apr 10, 2017

chenpighead commented Apr 11, 2017

emilio commented Apr 11, 2017

bors-servo commented Apr 11, 2017

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors-servo commented Apr 11, 2017

bors-servo commented Apr 11, 2017

bholley commented Apr 11, 2017

chenpighead commented Apr 11, 2017

bholley commented Apr 11, 2017

bors-servo commented Apr 11, 2017

bors-servo commented Apr 11, 2017

nox commented Apr 11, 2017

nox commented Apr 11, 2017

chenpighead commented Apr 11, 2017

nox commented Apr 11, 2017

bors-servo commented Apr 11, 2017

bors-servo commented Apr 11, 2017

bors-servo commented Apr 11, 2017

chenpighead commented Apr 11, 2017

bors-servo commented Apr 11, 2017

nox commented Apr 11, 2017

emilio commented Apr 11, 2017

bors-servo commented Apr 11, 2017

bors-servo commented Apr 11, 2017

servo / servo

Join GitHub today

Stylo - support image-orientation property #16334

Stylo - support image-orientation property #16334

Conversation

chenpighead commented Apr 10, 2017 • edited by larsbergstrom

highfive commented Apr 10, 2017

highfive commented Apr 10, 2017

wafflespeanut commented Apr 10, 2017 • edited

chenpighead commented Apr 10, 2017

wafflespeanut commented Apr 10, 2017

chenpighead commented Apr 11, 2017

emilio commented Apr 11, 2017

bors-servo commented Apr 11, 2017

This comment has been minimized.

nox Apr 11, 2017 • edited

This comment has been minimized.

emilio Apr 11, 2017

This comment has been minimized.

emilio Apr 11, 2017

This comment has been minimized.

nox Apr 11, 2017

This comment has been minimized.

chenpighead Apr 11, 2017

bors-servo commented Apr 11, 2017

bors-servo commented Apr 11, 2017

bholley commented Apr 11, 2017

chenpighead commented Apr 11, 2017

bholley commented Apr 11, 2017

bors-servo commented Apr 11, 2017

bors-servo commented Apr 11, 2017

nox commented Apr 11, 2017

nox commented Apr 11, 2017

chenpighead commented Apr 11, 2017

nox commented Apr 11, 2017

bors-servo commented Apr 11, 2017

bors-servo commented Apr 11, 2017

bors-servo commented Apr 11, 2017

chenpighead commented Apr 11, 2017

bors-servo commented Apr 11, 2017

nox commented Apr 11, 2017

emilio commented Apr 11, 2017

bors-servo commented Apr 11, 2017

bors-servo commented Apr 11, 2017

chenpighead commented Apr 10, 2017 •

edited by larsbergstrom

wafflespeanut commented Apr 10, 2017 •

edited

nox Apr 11, 2017 •

edited