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

Take the prefix from createElementNS into account for HTML elements #3575

Merged
merged 1 commit into from Oct 7, 2014

Conversation

@gilles-leblanc
Copy link
Contributor

gilles-leblanc commented Oct 5, 2014

Fixes #3139

@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Oct 5, 2014

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

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.

@gilles-leblanc
Copy link
Contributor Author

gilles-leblanc commented Oct 5, 2014

There's a lot of files but for most of them (all the HTML*Element) the changes is almost identical and only consist of adding a new parameter to the constructor and passing it in.

bors-servo pushed a commit that referenced this pull request Oct 6, 2014
@jdm
Copy link
Member

jdm commented Oct 6, 2014

Needs a rebase, unfortunately.

@gilles-leblanc
Copy link
Contributor Author

gilles-leblanc commented Oct 6, 2014

@Ms2ger @jdm I have rebased and pushed.

bors-servo pushed a commit that referenced this pull request Oct 6, 2014
@gilles-leblanc
Copy link
Contributor Author

gilles-leblanc commented Oct 7, 2014

It's my bad, I see the failures are in WPT, I have always run ref and content.

@jdm
Copy link
Member

jdm commented Oct 7, 2014

Yep, your change fixed a bunch failing tests. My favourite kind of problem.

@gilles-leblanc
Copy link
Contributor Author

gilles-leblanc commented Oct 7, 2014

I removed the expected fails in tests/wpt/metadata/dom/nodes/case.html.ini and rebased. Took a while because I haven't been able to run a subset of the wpt tests and they take over 30 minutes to run.

@jdm
Copy link
Member

jdm commented Oct 7, 2014

Haven't been able to? Even with ./mach wpt-test --include=/dom/nodes/case.html, for example? In addition, you can use --processes=4 to run multiple tests in parallel.

@jdm
Copy link
Member

jdm commented Oct 7, 2014

It looks like you should be able to remove the ini file altogether since there are no expected failures.

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented on 3a5a66d Oct 7, 2014

saw approval from Ms2ger
at gilles-leblanc@3a5a66d

This comment has been minimized.

Copy link
Contributor

bors-servo replied Oct 7, 2014

merging gilles-leblanc/servo/issue-3139 = 3a5a66d into auto

This comment has been minimized.

Copy link
Contributor

bors-servo replied Oct 7, 2014

gilles-leblanc/servo/issue-3139 = 3a5a66d merged ok, testing candidate = a4b4147

This comment has been minimized.

Copy link
Contributor

bors-servo replied Oct 7, 2014

This comment has been minimized.

Copy link
Contributor

bors-servo replied Oct 7, 2014

fast-forwarding master to auto = a4b4147

bors-servo pushed a commit that referenced this pull request Oct 7, 2014
@bors-servo bors-servo closed this Oct 7, 2014
@bors-servo bors-servo merged commit 3a5a66d into servo:master Oct 7, 2014
1 check passed
1 check passed
default all tests passed
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.

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