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

:nth-of-type(n) produces incorrect XPath #1187

Closed
yorickpeterse opened this issue Nov 2, 2014 · 9 comments
Closed

:nth-of-type(n) produces incorrect XPath #1187

yorickpeterse opened this issue Nov 2, 2014 · 9 comments

Comments

@yorickpeterse
Copy link

This code:

Nokogiri::CSS.xpath_for(':nth-of-type(n)')

Produces the following XPath:

//*[position() = n]

This is incorrect and will result in an empty node set being returned when used to query a document. Instead the XPath should be //*[position() >= 1] as nth-of-type(n) simply returns all nodes matching the type.

@knu
Copy link
Member

knu commented Nov 5, 2014

You are right. Looks like CSS3 allows just n but nokogiri's parser is not aware of that. I'll fix it later.

@yorickpeterse
Copy link
Author

If it helps, the XPath predicate that can be used for this is simply ((count(preceding-sibling::*) + 1) mod 1) = 0, though technically leaving a predicate out all together would produce the same results.

@knu
Copy link
Member

knu commented Nov 5, 2014

Nokogiri::CSS.xpath_for(':nth-of-type(n)') should now return ["//*[(position() mod 1) = 0]"].

@knu
Copy link
Member

knu commented Nov 5, 2014

Oh wait, x:nth-of-type(n) and x:nth(n) produce the same expression. The current implementation looks broken.

@yorickpeterse
Copy link
Author

Argh, my bad. I grabbed an XPath expression for nth-of-child, not nth-of-type. For the latter it can simply be (position() mod 1) =0.

@knu
Copy link
Member

knu commented Nov 5, 2014

Ah, yes, Nokogiri::CSS.xpath_for('x:nth-child(n)') produces ["//x[((count(preceding-sibling::*) + 1) mod 1) = 0]"] which seems correct.

@knu
Copy link
Member

knu commented Nov 5, 2014

Could you confirm the fix?

@yorickpeterse
Copy link
Author

Looks good to me, thanks!

@knu
Copy link
Member

knu commented Nov 5, 2014

Thanks for the catch!

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

2 participants