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
Merged

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

merged 1 commit into from
Oct 7, 2014

Conversation

gilles-leblanc
Copy link
Contributor

Fixes #3139

@hoppipolla-critic-bot
Copy link

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

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

@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

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

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 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Take the prefix from createElementNS into account for HTML elements
4 participants