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

Add support for validation of all svg elements. #180

Merged
merged 8 commits into from
Mar 7, 2015
Merged

Add support for validation of all svg elements. #180

merged 8 commits into from
Mar 7, 2015

Conversation

mareksuscak
Copy link
Contributor

Html proofer fails to validate svg elements like <path> <defs> and others so I thought it would be nice to add support for it too.

@doktorbro
Copy link

With this change Proofer will ignore all SVG elements. Am I right?

@doktorbro
Copy link

Maybe it’s better to remove SVG elements in https://github.com/mareksuscak/html-proofer/blob/master/lib/html/proofer/check_runner.rb#L55?

@mareksuscak
Copy link
Contributor Author

@penibelst Yes you are perfectly right - sorry I have missed the part that removes tags completely. I'm going to revert my commit and re-implement the check in check_runner.rb on line 55.

@mareksuscak
Copy link
Contributor Author

@penibelst Ok seems like svg can't be queried using a CSS selector when it has a namespace defined. I'll try to use XPath.

@mareksuscak
Copy link
Contributor Author

@penibelst I have managed to fix my failing test somehow but seems like I have broken something else.

Could you please check my solution and help me to sort this out? I think I found out a bug in the code - nokogiri validates the document immediately after the file is loaded so when you want to unlink part of the document to prevent validation of that specific part you have to force re-validation by reloading the modified HTML.

@mareksuscak
Copy link
Contributor Author

Ok got it. nokogiri's to_s function returns formatted HTML so the documents get reformatted and thus cause a lot of tests to fail. Due to nokogiri's drawback that is mentioned in my previous comment I would suggest to revert back to my first solution because I don't see any other solution for that at moment. What's your opinion?

@doktorbro
Copy link

Maybe you try:

html.xpath('//code | //pre | //svg').each(&:unlink)

@mareksuscak
Copy link
Contributor Author

But like I said in my recent comments. The problem finally wasn't that SVG can not be selected using css selectors - it was selected & removed correctly but the document still contained errors because it gets validated immediately after it's been loaded and removal of any element doesn't change anything.

@doktorbro
Copy link

doktorbro commented Feb 23, 2015 via email

@mareksuscak
Copy link
Contributor Author

Fine. So what's your suggestion then? Revert to my very first solution?

@doktorbro
Copy link

Revert to my very first solution?

@mareksuscak Yes, please revert to your first draft. This solves your problem. Sorry for misleading.

I asked Nokogiri people to help with this issue. Maybe we can figure out a better solution.

@mareksuscak
Copy link
Contributor Author

Ok so what now will you accept the pull request or ask on the mailing list?

doktorbro pushed a commit that referenced this pull request Mar 7, 2015
Ignore all svg elements during validation
@doktorbro doktorbro merged commit 2f7f0bd into gjtorikian:master Mar 7, 2015
@doktorbro
Copy link

@mareksuscak Your fix will appear in the next version. Thank you.

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.

2 participants