sparklemotion / nokogiri

Nokogiri (鋸) makes it easy and painless to work with XML and HTML from Ruby.
https://nokogiri.org/
MIT License
6.15k stars 901 forks source link

CSS `foo~:nth-child(2)` gives incorrect XPath #707

Open SimonSapin opened 12 years ago

SimonSapin commented 12 years ago

Hi,

I’m the maintainer of cssselect, which does in Python pretty much the same as Nokogiri for CSS selectors: translate them to XPath. It looks like the SimonSapin/cssselect#12 bug also applies to Nokogiri. Namely, the XPath translation of :nth-child() and similar pseudo-classes is wrong when used after the + or ~ combinator. Here is a test case:

require 'nokogiri'
doc = Nokogiri::XML('<root><child1/><child2/><child3/></root>')
puts doc.css(':nth-child(2)').map { |e| e.name }
puts doc.css('child1 ~ :nth-child(2)').map { |e| e.name }

Expected output: child2 child2. Actual output child2 child3.

The problem is in the XPath translation of the later selector: //child1/following-sibling::*[position() = 2 and self::*] gives the element at position 2 when counting from child1, while we want the position among the parent’s children.

I am not sure it is even possible to correctly translate this selector to XPath: the = XPath operator on node-sets compares the text content of elements, not their identity.

The issue is similar for SimonSapin/cssselect#4 and Nokogiri’s #394.

AurelPaulovic commented 11 years ago

you should not use position() which depends on the context position

instead try

//child1/following-sibling::*[(count(preceding-sibling::*) +1)=2]

and similarly for Xn

//child1/following-sibling::*[(count(preceding-sibling::*)+1) mod X = 0]
SimonSapin commented 11 years ago

Thank you @AurelPaulovic , I think that should work.

Now for a selector h2 ~ div:nth-of-type(2) the XPath expression could be //h2/following-sibling::div[count(preceding-sibling::div)=1]. But for the more general case: h2 ~ *:nth-of-type(2), XPath is //h2/following-sibling::*[count(preceding-sibling::*[name(.)=name(…)])=1] is there some expression we could put instead of to refer to the outer scope? Or maybe a way to bind the outer scope to a variable?

AurelPaulovic commented 11 years ago

Sadly, there is no way how to do that in XPath 1.0. You can't assign any variables and there is no way how to get to the outer context.