Skip to content

/ servo

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

Prepend, rather than append, SVG title elements. #8296

Merged
merged 1 commit into from Nov 7, 2015
Merged

Prepend, rather than append, SVG title elements. #8296

merged 1 commit into from Nov 7, 2015
+4 −10

Conversation

@notriddle
Copy link
Contributor

notriddle commented Nov 2, 2015

Fixes #8164

Review on Reviewable
@jdm
Copy link
Member

jdm commented Nov 2, 2015

r? @nox
@nox
Copy link
Member

nox commented Nov 4, 2015

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

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion.

components/script/dom/document.rs, line 1698 [r1] (raw file):
Please insert before first child instead. Please also upcast elem directly before inserting it. This way you don't need to upcast it against when returning it.

Comments from the review on Reviewable.io
@notriddle
Copy link
Contributor Author

notriddle commented Nov 4, 2015

Fixed.
@nox
Copy link
Member

nox commented Nov 5, 2015

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

Reviewed 1 of 1 files at r1, 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

components/script/dom/document.rs, line 1746 [r2] (raw file):
Use RootedReference::r: .InsertBefore(elem_node, root_node.GetFirstChild().r()) should work. By the way we generally just name these parent and child, not root_node and elem_node.

Comments from the review on Reviewable.io
@notriddle
Copy link
Contributor Author

notriddle commented Nov 6, 2015

Fixed more.
@nox
Copy link
Member

nox commented Nov 6, 2015

Please squash and I'll r+ it.

-S-awaiting-review +S-needs-squash

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

Comments from the review on Reviewable.io
@nox nox added S-needs-squash and removed S-awaiting-review labels Nov 6, 2015
@notriddle
Prepend, rather than append, SVG title elements.
Loading status checks…
71c5d17 
Fixes #8164
@notriddle
Copy link
Contributor Author

notriddle commented Nov 6, 2015

Squashed.
@jdm jdm removed the S-needs-squash label Nov 6, 2015
@nox
Copy link
Member

nox commented Nov 7, 2015

@bors-servo r+

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

Comments from the review on Reviewable.io
@bors-servo
Copy link
Contributor

bors-servo commented Nov 7, 2015

📌 Commit 71c5d17 has been approved by nox
@bors-servo
Copy link
Contributor

bors-servo commented Nov 7, 2015

⌛ Testing commit 71c5d17 with merge dbf27e7...
bors-servo added a commit that referenced this pull request Nov 7, 2015
@bors-servo
Auto merge of #8296 - notriddle:svg_title_prepend, r=nox
dbf27e7 
Prepend, rather than append, SVG title elements.

Fixes #8164

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

bors-servo commented Nov 7, 2015

💔 Test failed - gonk
@eefriedman
Copy link
Contributor

eefriedman commented Nov 7, 2015

@bors-servo retry (infrastructure weirdness)
@bors-servo
Copy link
Contributor

bors-servo commented Nov 7, 2015

⌛ Testing commit 71c5d17 with merge 6837bf1...
bors-servo added a commit that referenced this pull request Nov 7, 2015
@bors-servo
Auto merge of #8296 - notriddle:svg_title_prepend, r=nox
6837bf1 
Prepend, rather than append, SVG title elements.

Fixes #8164

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

bors-servo commented Nov 7, 2015

💔 Test failed - mac-rel-wpt
@jdm
Copy link
Member

jdm commented Nov 7, 2015

@bors-servo: retry
@bors-servo
Copy link
Contributor

bors-servo commented Nov 7, 2015

⌛ Testing commit 71c5d17 with merge 3c794d0...
bors-servo added a commit that referenced this pull request Nov 7, 2015
@bors-servo
Auto merge of #8296 - notriddle:svg_title_prepend, r=nox
Loading status checks…
3c794d0 
Prepend, rather than append, SVG title elements.

Fixes #8164

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

bors-servo commented Nov 7, 2015

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt
@bors-servo bors-servo merged commit 71c5d17 into servo:master Nov 7, 2015
3 checks passed
3 checks passed
code-review/reviewable Review complete: all files reviewed, all discussions resolved
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@notriddle notriddle deleted the notriddle:svg_title_prepend branch Nov 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nox nox

Labels
S-awaiting-merge
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
@notriddle @jdm @nox @bors-servo @eefriedman @highfive
You can’t perform that action at this time.