Improve implementation of DOMRect and implement DOMRectReadOnly #7415

tschneidereit · 2015-08-27T19:16:13Z

Passes most tests from test-css. The remaining ones should pass once we have w3c/csswg-test#834.

tschneidereit · 2015-08-27T19:25:01Z

Rookie mistake. Will fix.

bors-servo · 2015-08-27T21:50:40Z

☔ The latest upstream changes (presumably #7416) made this pull request unmergeable. Please resolve the merge conflicts.

nox · 2015-08-28T11:59:27Z

-S-awaiting-review +S-needs-code-changes

Use drafts.fxtf.org links, fix the nits, and it's all good.

Reviewed 11 of 11 files at r1.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

^{components/script/devtools.rs, line 106 [r1] (raw file):}
.r() is useless now that Root implements Deref.

^{components/script/dom/domrect.rs, line 5 [r1] (raw file):}
Nit: We generally don't import Wrap in IDL implementations, instead we import both the Binding module and the Methods trait inside.

^{components/script/dom/domrectreadonly.rs, line 94 [r1] (raw file):}
Implement helper methods on DOMRect itself, without an intermediate trait.

Comments from the review on Reviewable.io

nox · 2015-08-28T12:41:03Z

Reviewed 1 of 12 files at r2, 11 of 11 files at r3.
Review status: all files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.

^{components/script/dom/domrectreadonly.rs, line 41 [r3] (raw file):}
set_x

^{components/script/dom/domrectreadonly.rs, line 45 [r3] (raw file):}
set_y

^{components/script/dom/domrectreadonly.rs, line 49 [r3] (raw file):}
set_width

^{components/script/dom/domrectreadonly.rs, line 53 [r3] (raw file):}
set_height

Comments from the review on Reviewable.io

nox · 2015-08-28T12:46:56Z

Reviewed 2 of 2 files at r4.
Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks pending.

Comments from the review on Reviewable.io

nox · 2015-08-28T12:47:14Z

@bors-servo r+

bors-servo · 2015-08-28T12:47:15Z

📌 Commit d5049c9 has been approved by nox

bors-servo · 2015-08-28T12:47:16Z

⌛ Testing commit d5049c9 with merge 32bb538...


        Auto merge of #7415 - tschneidereit:dom-rect, r=nox

@Ms2ger

Improve implementation of DOMRect and implement DOMRectReadOnly Passes most tests from test-css. The remaining ones should pass once we have w3c/csswg-test#834. r? @Ms2ger  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7415)

bors-servo · 2015-08-28T13:16:02Z

💔 Test failed - mac1

jdm · 2015-08-28T13:22:36Z

/_mozilla/mozilla/getBoundingClientRect.html
--------------------------------------------
FAIL Untitled
FAIL Untitled 1
/html/rendering/replaced-elements/svg-embedded-sizing/svg-in-img-auto.html
--------------------------------------------------------------------------
PASS expected FAIL placeholder: 'img', svgViewBoxAttr: '0 0 100 200', 
PASS expected FAIL placeholder: 'img', containerHeightStyle: '400px', svgViewBoxAttr: '0 0 100 200', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', svgViewBoxAttr: '0 0 100 200', 
PASS expected FAIL placeholder: 'img', containerHeightStyle: '400px', placeholderWidthAttr: '50%', svgViewBoxAttr: '0 0 100 200', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '200', 
PASS expected FAIL placeholder: 'img', containerHeightStyle: '400px', placeholderWidthAttr: '50%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '200', 
PASS expected FAIL placeholder: 'img', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '25%', 
PASS expected FAIL placeholder: 'img', containerHeightStyle: '400px', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '25%', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '25%', 
PASS expected FAIL placeholder: 'img', containerHeightStyle: '400px', placeholderWidthAttr: '50%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '25%', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', svgViewBoxAttr: '0 0 100 200', svgHeightAttr: '200', 
PASS expected FAIL placeholder: 'img', containerHeightStyle: '400px', placeholderWidthAttr: '50%', svgViewBoxAttr: '0 0 100 200', svgHeightAttr: '200', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', svgWidthAttr: '200', svgHeightAttr: '200', 
PASS expected FAIL placeholder: 'img', containerHeightStyle: '400px', placeholderWidthAttr: '50%', svgWidthAttr: '200', svgHeightAttr: '200', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '200', svgHeightAttr: '200', 
PASS expected FAIL placeholder: 'img', containerHeightStyle: '400px', placeholderWidthAttr: '50%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '200', svgHeightAttr: '200', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '25%', svgHeightAttr: '200', 
PASS expected FAIL placeholder: 'img', containerHeightStyle: '400px', placeholderWidthAttr: '50%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '25%', svgHeightAttr: '200', 
PASS expected FAIL placeholder: 'img', svgViewBoxAttr: '0 0 100 200', svgHeightAttr: '25%', 
PASS expected FAIL placeholder: 'img', containerHeightStyle: '400px', svgViewBoxAttr: '0 0 100 200', svgHeightAttr: '25%', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', svgViewBoxAttr: '0 0 100 200', svgHeightAttr: '25%', 
PASS expected FAIL placeholder: 'img', containerHeightStyle: '400px', placeholderWidthAttr: '50%', svgViewBoxAttr: '0 0 100 200', svgHeightAttr: '25%', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '200', svgHeightAttr: '25%', 
PASS expected FAIL placeholder: 'img', containerHeightStyle: '400px', placeholderWidthAttr: '50%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '200', svgHeightAttr: '25%', 
PASS expected FAIL placeholder: 'img', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '25%', svgHeightAttr: '25%', 
PASS expected FAIL placeholder: 'img', containerHeightStyle: '400px', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '25%', svgHeightAttr: '25%', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '25%', svgHeightAttr: '25%', 
PASS expected FAIL placeholder: 'img', containerHeightStyle: '400px', placeholderWidthAttr: '50%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '25%', svgHeightAttr: '25%', 
/html/rendering/replaced-elements/svg-embedded-sizing/svg-in-img-percentage.html
--------------------------------------------------------------------------------
PASS expected FAIL placeholder: 'img', placeholderHeightAttr: '100%', svgViewBoxAttr: '0 0 100 200', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', placeholderHeightAttr: '100%', svgViewBoxAttr: '0 0 100 200', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', placeholderHeightAttr: '100%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '200', 
PASS expected FAIL placeholder: 'img', placeholderHeightAttr: '100%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '25%', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', placeholderHeightAttr: '100%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '25%', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', placeholderHeightAttr: '100%', svgViewBoxAttr: '0 0 100 200', svgHeightAttr: '200', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', placeholderHeightAttr: '100%', svgWidthAttr: '200', svgHeightAttr: '200', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', placeholderHeightAttr: '100%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '200', svgHeightAttr: '200', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', placeholderHeightAttr: '100%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '25%', svgHeightAttr: '200', 
PASS expected FAIL placeholder: 'img', placeholderHeightAttr: '100%', svgViewBoxAttr: '0 0 100 200', svgHeightAttr: '25%', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', placeholderHeightAttr: '100%', svgViewBoxAttr: '0 0 100 200', svgHeightAttr: '25%', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', placeholderHeightAttr: '100%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '200', svgHeightAttr: '25%', 
PASS expected FAIL placeholder: 'img', placeholderHeightAttr: '100%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '25%', svgHeightAttr: '25%', 
PASS expected FAIL placeholder: 'img', placeholderWidthAttr: '50%', placeholderHeightAttr: '100%', svgViewBoxAttr: '0 0 100 200', svgWidthAttr: '25%', svgHeightAttr: '25%', 
/html/rendering/replaced-elements/svg-inline-sizing/svg-inline.html
-------------------------------------------------------------------
PASS expected FAIL svgViewBoxAttr: '0 0 100 200', 
PASS expected FAIL containerHeightStyle: '400px', svgViewBoxAttr: '0 0 100 200',

nox · 2015-09-13T08:24:14Z

Reviewed 3 of 3 files at r6.
Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

Comments from the review on Reviewable.io

bors-servo · 2015-09-18T21:46:40Z

☔ The latest upstream changes (presumably #7680) made this pull request unmergeable. Please resolve the merge conflicts.

nox · 2015-10-04T13:53:34Z

@tschneidereit Ping?


        Whitelist drafts.fxtf.org in tidy.py's webidl spec link check


        Improve implementation of DOMRect and implement DOMRectReadOnly

Passes most tests from test-css.

tschneidereit · 2015-10-17T20:51:20Z

@nox, I finally rebased this, should be ready for review again.

nox · 2015-10-17T20:54:51Z

Still one issue and this is good to go.

-S-awaiting-answer -S-awaiting-answer -S-needs-rebase

Reviewed 6 of 6 files at r7.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks pending.

^{tests/wpt/metadata-css/geometry-1_dev/html/DOMRect-001.htm.ini, line 14 [r7] (raw file):}
We still don't have the changes from your csswg-test fix?

Comments from the review on Reviewable.io

tschneidereit · 2015-10-17T21:15:59Z

Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks pending.

^{components/script/dom/element.rs, line 1269 [r5] (raw file):}
As discussed on IRC, that'd make things either more tedious by requiring us to first unwrap the scalar values, then convert them to f64, and then create {Point,Size}2D<f64> from them. Or implement a From trait, but that'd need to go into app_units (or Euclid, but probably the former). Punting for now.

^{tests/wpt/metadata-css/geometry-1_dev/html/DOMRect-001.htm.ini, line 14 [r7] (raw file):}
We don't, no. The last import was from a day before my fix landed upstream.

Comments from the review on Reviewable.io

nox · 2015-10-17T21:17:26Z

@bors-servo r+

Thanks for your contribution!

-S-needs-code-changes

Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks pending.

Comments from the review on Reviewable.io

bors-servo · 2015-10-17T21:17:27Z

📌 Commit 4c1c05f has been approved by nox

bors-servo · 2015-10-17T21:17:29Z

⌛ Testing commit 4c1c05f with merge 810d28a...


        Auto merge of #7415 - tschneidereit:dom-rect, r=nox

@Ms2ger

Improve implementation of DOMRect and implement DOMRectReadOnly Passes most tests from test-css. The remaining ones should pass once we have w3c/csswg-test#834. r? @Ms2ger  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7415)

bors-servo · 2015-10-17T21:56:02Z

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt

highfive added the S-awaiting-review label Aug 27, 2015

highfive added S-needs-rebase and removed S-awaiting-merge labels Aug 27, 2015

tschneidereit force-pushed the tschneidereit:dom-rect branch 3 times, most recently from c6332c0 to 6f4a68e Aug 28, 2015

nox added S-needs-code-changes and removed S-needs-rebase S-awaiting-review labels Aug 28, 2015

tschneidereit force-pushed the tschneidereit:dom-rect branch from 6f4a68e to 39ceb67 Aug 28, 2015

highfive added S-awaiting-review and removed S-needs-code-changes labels Aug 28, 2015

tschneidereit force-pushed the tschneidereit:dom-rect branch from 39ceb67 to d5049c9 Aug 28, 2015

nox self-assigned this Aug 28, 2015

highfive added S-awaiting-merge and removed S-awaiting-review labels Aug 28, 2015

highfive added S-tests-failed and removed S-awaiting-merge labels Aug 28, 2015

tschneidereit force-pushed the tschneidereit:dom-rect branch from d5049c9 to c336426 Aug 28, 2015

nox added S-awaiting-answer S-needs-code-changes labels Sep 13, 2015

highfive added S-needs-rebase and removed S-awaiting-merge labels Sep 18, 2015

tschneidereit added 2 commits Aug 28, 2015

Whitelist drafts.fxtf.org in tidy.py's webidl spec link check

499a847

Improve implementation of DOMRect and implement DOMRectReadOnly

Loading status checks…

4c1c05f

Passes most tests from test-css.

tschneidereit force-pushed the tschneidereit:dom-rect branch from bcd28ac to 4c1c05f Oct 17, 2015

highfive added S-awaiting-review and removed S-needs-code-changes labels Oct 17, 2015

nox added S-needs-code-changes and removed S-awaiting-answer S-needs-rebase S-awaiting-review labels Oct 17, 2015

nox removed the S-needs-code-changes label Oct 17, 2015

highfive added the S-awaiting-merge label Oct 17, 2015

bors-servo merged commit 4c1c05f into servo:master Oct 17, 2015
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

bors-servo mentioned this pull request Oct 17, 2015

Introduce trait Castable #8041

Merged

tschneidereit mentioned this pull request Nov 13, 2015

Web assembly support #8448

Closed

Ms2ger mentioned this pull request Nov 13, 2015

Get Shumway working in Servo #6097

Closed

servo / servo

Improve implementation of DOMRect and implement DOMRectReadOnly #7415

Improve implementation of DOMRect and implement DOMRectReadOnly #7415

tschneidereit commented Aug 27, 2015

tschneidereit commented Aug 27, 2015

bors-servo commented Aug 27, 2015

nox commented Aug 28, 2015

nox commented Aug 28, 2015

nox commented Aug 28, 2015

nox commented Aug 28, 2015

bors-servo commented Aug 28, 2015

bors-servo commented Aug 28, 2015

bors-servo commented Aug 28, 2015

jdm commented Aug 28, 2015

nox commented Sep 13, 2015

bors-servo commented Sep 18, 2015

nox commented Oct 4, 2015

tschneidereit commented Oct 17, 2015

nox commented Oct 17, 2015

tschneidereit commented Oct 17, 2015

nox commented Oct 17, 2015

bors-servo commented Oct 17, 2015

bors-servo commented Oct 17, 2015

bors-servo commented Oct 17, 2015

servo / servo

Join GitHub today

Improve implementation of DOMRect and implement DOMRectReadOnly #7415

Improve implementation of DOMRect and implement DOMRectReadOnly #7415

Conversation

tschneidereit commented Aug 27, 2015

tschneidereit commented Aug 27, 2015

bors-servo commented Aug 27, 2015

nox commented Aug 28, 2015

nox commented Aug 28, 2015

nox commented Aug 28, 2015

nox commented Aug 28, 2015

bors-servo commented Aug 28, 2015

bors-servo commented Aug 28, 2015

bors-servo commented Aug 28, 2015

jdm commented Aug 28, 2015

nox commented Sep 13, 2015

bors-servo commented Sep 18, 2015

nox commented Oct 4, 2015

tschneidereit commented Oct 17, 2015

nox commented Oct 17, 2015

tschneidereit commented Oct 17, 2015

nox commented Oct 17, 2015

bors-servo commented Oct 17, 2015

bors-servo commented Oct 17, 2015

bors-servo commented Oct 17, 2015