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

Update RemoveIFrame to use pipeline id rather than subpage. #7921

Merged
merged 2 commits into from Oct 12, 2015

Conversation

@glennw
Copy link
Member

glennw commented Oct 8, 2015

Review on Reviewable

@glennw
Copy link
Member Author

glennw commented Oct 8, 2015

r? @jdm

@jdm
Copy link
Member

jdm commented Oct 8, 2015

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


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


components/script/dom/htmliframeelement.rs, line 438 [r2] (raw file):
We should clear the pipeline here too, right?


Comments from the review on Reviewable.io

@glennw
Copy link
Member Author

glennw commented Oct 8, 2015

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


components/script/dom/htmliframeelement.rs, line 438 [r2] (raw file):
It's not actually needed (yet) since none of the constellation logic has been updated to make use of the pipeline id here yet - but I've added it anyway, so it's not forgotten in follow up PRs.


Comments from the review on Reviewable.io

@jdm
Copy link
Member

jdm commented Oct 8, 2015

Squash this last commit with the previous, please.
-S-awaiting-review +S-needs-squash


Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks pending.


Comments from the review on Reviewable.io

@jdm jdm added S-needs-squash and removed S-awaiting-review labels Oct 8, 2015
@glennw glennw force-pushed the glennw:subpage-fixes-2 branch from f4c3238 to 422f936 Oct 8, 2015
@jdm jdm removed the S-needs-squash label Oct 8, 2015
@jdm
Copy link
Member

jdm commented Oct 8, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Oct 8, 2015

📌 Commit 422f936 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Oct 8, 2015

Testing commit 422f936 with merge 8ca9be5...

bors-servo pushed a commit that referenced this pull request Oct 8, 2015
Update RemoveIFrame to use pipeline id rather than subpage.



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

bors-servo commented Oct 8, 2015

💔 Test failed - linux-dev

@jdm
Copy link
Member

jdm commented Oct 9, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Oct 9, 2015

Testing commit 422f936 with merge f9bc2d6...

bors-servo pushed a commit that referenced this pull request Oct 9, 2015
Update RemoveIFrame to use pipeline id rather than subpage.



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

bors-servo commented Oct 9, 2015

💔 Test failed - gonk

@bors-servo
Copy link
Contributor

bors-servo commented Oct 12, 2015

Testing commit 8d312b0 with merge 7d088e9...

bors-servo pushed a commit that referenced this pull request Oct 12, 2015
Update RemoveIFrame to use pipeline id rather than subpage.



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

bors-servo commented Oct 12, 2015

💔 Test failed - linux-rel

@Ms2ger
Copy link
Contributor

Ms2ger commented Oct 12, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Oct 12, 2015

Testing commit 8d312b0 with merge 7772cf8...

bors-servo pushed a commit that referenced this pull request Oct 12, 2015
Update RemoveIFrame to use pipeline id rather than subpage.



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

bors-servo commented Oct 12, 2015

💔 Test failed - mac-rel-wpt

@Ms2ger
Copy link
Contributor

Ms2ger commented Oct 12, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Oct 12, 2015

Testing commit 8d312b0 with merge af8f2bc...

bors-servo pushed a commit that referenced this pull request Oct 12, 2015
Update RemoveIFrame to use pipeline id rather than subpage.



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

bors-servo commented Oct 12, 2015

💔 Test failed - mac-rel-wpt

@jdm
Copy link
Member

jdm commented Oct 12, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Oct 12, 2015

Testing commit 8d312b0 with merge 5ffeb3b...

bors-servo pushed a commit that referenced this pull request Oct 12, 2015
Update RemoveIFrame to use pipeline id rather than subpage.



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

bors-servo commented Oct 12, 2015

@bors-servo bors-servo merged commit 8d312b0 into servo:master Oct 12, 2015
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
homu Test successful
Details
@glennw glennw deleted the glennw:subpage-fixes-2 branch Dec 12, 2016
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

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