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

Namespace issues #21

Open
dhouck opened this issue Mar 22, 2023 · 4 comments
Open

Namespace issues #21

dhouck opened this issue Mar 22, 2023 · 4 comments

Comments

@dhouck
Copy link

dhouck commented Mar 22, 2023

Currently, sxd_html puts all elements in no namespace, instead of the correct HTML, SVG, or MathML namespaces. There is code for handling namespaces, but itʼs commented out; Iʼm not sure why this is but itʼd be nice if it could be working and uncommented. The html5ever crate already puts all the elements in their proper namespace (according to the HTML5 spec, at least).

@dhouck
Copy link
Author

dhouck commented Mar 22, 2023

Okay, I see that turning that on (and adding elem.set_default_namespace_uri(qname.namespace_uri()); somewhere at least good enough for testing), it behaves how I expect but lots of the tests are broken because XPath handles default namespace in a weird way.

I donʼt know enough about sxd_xpath (or XPath in general) to know how to make the tests work with namespaces or whether that would put an undue burden on other users of this library; I expect it would, so this could at best be a parse option instead of default behavior.

@kitsuyui
Copy link
Owner

kitsuyui commented Apr 6, 2023

@dhouck Thank you for your reply and I'm sorry for the late.
Thank you for checking that elem.set_default_namespace_uri(qname.namespace_uri()); works.

If it is possible to introduce it without breaking compatibility, I would like to implement it.

@diogo464 Is there any known concern?
It was introduced in diogo464@5eb0e3a
I think that it is likely that some simple workaround was selected, but if you remember any reason to keep it, please let me know.
If not, I would like to accept @dhouck 's proposal and add unit tests to deal with it.

@diogo464
Copy link

diogo464 commented Apr 8, 2023

@kitsuyui I don't remember why that was commented out. It was likely due to some issue that didn't impact what I was using that crate for and that was the easiest workaround, so if you can get namespaces to work that would be great.

@kitsuyui
Copy link
Owner

kitsuyui commented Apr 8, 2023

@diogo464
Thank you for your response!
I will add some tests and modify the code to ensure that the namespaces are working properly.

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

No branches or pull requests

3 participants