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

Restrict output of getElementsByName to HTML elements #6231

Merged
merged 2 commits into from Jun 3, 2015
Merged

Restrict output of getElementsByName to HTML elements #6231

merged 2 commits into from Jun 3, 2015

Conversation

@aopicier
Copy link
Contributor

aopicier commented May 31, 2015

Fixes #1745
Should I adjust the expected result of the corresponding wpt test cases?

html/dom/documents/dom-tree-accessors/document.getElementsByName/document.getElementsByName-namespace.html
html/dom/documents/dom-tree-accessors/document.getElementsByName/document.getElementsByName-namespace.xhtml

Review on Reviewable
@aopicier
Restrict output of getElementsByName to HTML elements
Loading status checks…
2779e0a 
https://html.spec.whatwg.org/#dom-document-getelementsbyname
@highfive
Copy link

highfive commented May 31, 2015

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @pcwalton (or someone else) soon.
@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented May 31, 2015

Critic review: https://critic.hoppipolla.co.uk/r/5139

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.
@Ms2ger Ms2ger self-assigned this Jun 1, 2015
@Ms2ger
Copy link
Contributor

Ms2ger commented Jun 1, 2015

Yes, you should remove tests/wpt/metadata/html/dom/documents/dom-tree-accessors/document.getElementsByName/document.getElementsByName-namespace.{html,xhtml}.ini; if you don't, our continuous integration will reject your PR :)
@aopicier
Update test expectations for getElementsByName
Loading status checks…
56a6ffd
@Ms2ger
Copy link
Contributor

Ms2ger commented Jun 3, 2015

@aopicier: thank you so much!

@bors-servo: r+
@bors-servo
Copy link
Contributor

bors-servo commented Jun 3, 2015

📌 Commit 56a6ffd has been approved by Ms2ger
bors-servo pushed a commit that referenced this pull request Jun 3, 2015
bors-servo
Auto merge of #6231 - AopicieR:get_elements_by_name, r=Ms2ger
Loading status checks…
bb95e07 
Fixes #1745 
Should I adjust the expected result of the corresponding wpt test cases?

html/dom/documents/dom-tree-accessors/document.getElementsByName/document.getElementsByName-namespace.html
html/dom/documents/dom-tree-accessors/document.getElementsByName/document.getElementsByName-namespace.xhtml

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

bors-servo commented Jun 3, 2015

⌛ Testing commit 56a6ffd with merge bb95e07...
@bors-servo
Copy link
Contributor

bors-servo commented Jun 3, 2015

☀️ Test successful - android, gonk, linux1, linux2, linux3, mac1, mac2
@bors-servo bors-servo merged commit 56a6ffd into servo:master Jun 3, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@aopicier aopicier deleted the aopicier:get_elements_by_name branch Jun 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Ms2ger Ms2ger

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

Successfully merging this pull request may close these issues.

Document.getElementsByName and friends should only return elements in the HTML namespace
5 participants
@aopicier @highfive @hoppipolla-critic-bot @Ms2ger @bors-servo
You can’t perform that action at this time.