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

Improve DOM interfaces' extended attributes #7601

Merged
merged 5 commits into from Sep 20, 2015

Conversation

@nox
Copy link
Member

nox commented Sep 11, 2015

Some should have been [SameObject] instead of [Constant]. The rest of the changes are additional [Constant] and [Pure] extended attributes on many operations.

Review on Reviewable

nox added 4 commits Sep 11, 2015
ParentNode.children is [SameObject], querySelectorAll() is [NewObject].
Attributes classList and attributes are [SameObject].
It should be [SameObject].
@Ms2ger
Copy link
Contributor

Ms2ger commented Sep 11, 2015

Reviewed 1 of 1 files at r1, 1 of 1 files at r2, 1 of 1 files at r3, 1 of 1 files at r4, 2 of 18 files at r5.
Review status: 2 of 18 files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from the review on Reviewable.io

@Ms2ger
Copy link
Contributor

Ms2ger commented Sep 18, 2015

Reviewed 16 of 18 files at r5.
Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.


components/script/dom/webidls/Event.webidl, line 15 [r5] (raw file):
Gotcha! initEvent can break this. Try if you can write a test.


components/script/dom/webidls/NodeIterator.webidl, line 23 [r5] (raw file):
"Attributes/methods flagged in this way promise that they will keep returning the same value as long as nothing that has [Affects=Everything] executes."

These two always return another node.


Comments from the review on Reviewable.io

1 similar comment
@Ms2ger
Copy link
Contributor

Ms2ger commented Sep 19, 2015

Reviewed 16 of 18 files at r5.
Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.


components/script/dom/webidls/Event.webidl, line 15 [r5] (raw file):
Gotcha! initEvent can break this. Try if you can write a test.


components/script/dom/webidls/NodeIterator.webidl, line 23 [r5] (raw file):
"Attributes/methods flagged in this way promise that they will keep returning the same value as long as nothing that has [Affects=Everything] executes."

These two always return another node.


Comments from the review on Reviewable.io

@nox nox force-pushed the nox:cleanup-dom-webidls branch from 46f3841 to 49219ba Sep 19, 2015
@nox
Copy link
Member Author

nox commented Sep 19, 2015

@Ms2ger Amended.


Review status: 16 of 18 files reviewed at latest revision, all discussions resolved, some commit checks pending.


Comments from the review on Reviewable.io

@Ms2ger
Copy link
Contributor

Ms2ger commented Sep 19, 2015

@bors-servo r+

-S-awaiting-review +S-awaiting-merge


Reviewed 2 of 2 files at r6.
Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from the review on Reviewable.io

@bors-servo
Copy link
Contributor

bors-servo commented Sep 19, 2015

📌 Commit 49219ba has been approved by Ms2ger

@bors-servo
Copy link
Contributor

bors-servo commented Sep 19, 2015

Testing commit 49219ba with merge b6ec98b...

bors-servo pushed a commit that referenced this pull request Sep 19, 2015
Improve DOM interfaces' extended attributes

Some should have been `[SameObject]` instead of `[Constant]`. The rest of the changes are additional `[Constant]` and `[Pure]` extended attributes on many operations.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7601)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Sep 19, 2015

💔 Test failed - mac-rel-wpt

@nox
Copy link
Member Author

nox commented Sep 19, 2015

Agreeing to the Xcode/iOS license requires admin privileges, please re-run as root via sudo.

@larsbergstrom?

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Sep 19, 2015

Gah! I won't be around a computer with ssh for another six hours. @metajack
@edunham?

On Sep 19, 2015, at 9:28 AM, Anthony Ramine notifications@github.com
wrote:

Agreeing to the Xcode/iOS license requires admin privileges, please
re-run as root via sudo.

@larsbergstrom https://github.com/larsbergstrom?


Reply to this email directly or view it on GitHub
#7601 (comment).

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Sep 19, 2015

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Sep 19, 2015

Previous build results for android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css are reusable. Rebuilding only mac-rel-wpt...

@bors-servo
Copy link
Contributor

bors-servo commented Sep 19, 2015

💔 Test failed - mac-rel-wpt

@nox
Copy link
Member Author

nox commented Sep 19, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Sep 19, 2015

Testing commit 49219ba with merge 61267cd...

bors-servo pushed a commit that referenced this pull request Sep 19, 2015
Improve DOM interfaces' extended attributes

Some should have been `[SameObject]` instead of `[Constant]`. The rest of the changes are additional `[Constant]` and `[Pure]` extended attributes on many operations.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7601)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Sep 20, 2015

@bors-servo bors-servo merged commit 49219ba into servo:master Sep 20, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@nox nox mentioned this pull request Sep 21, 2015
@nox nox deleted the nox:cleanup-dom-webidls branch Sep 23, 2015
@saschanaz saschanaz mentioned this pull request Oct 25, 2019
4 of 4 tasks complete
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.

None yet

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