jaxen

preceding axis fails to report reverse document order

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.1
  • Fix Version/s: 1.1
  • Component/s: None
  • Labels:
    None
  • Number of attachments :
    1

Description

From: Anna Afonchenko <anna@ubaccess.com>
To: jaxen-interest@lists.codehaus.org

I am using Jaxen to evaluate XPath expressions in Java (JDOM).
I encountered problems when comparing results returned by jaxen with
results returned by other XPath implementation - I was using Saxon.
Here is my example:
<html>
<head>
</head>
<body>
<p>
<span>text1</span>
</p>
<div>text2</div>
</body>
</html>

I want to get the first preceding element of the div element.
I apply the following XPath - //div/preceding::*[1]
Saxon results in span element matched.
Jaxen results in p element matched.

What I want is the span element.

Activity

Hide
Christian Nentwich added a comment -

This looks similar to JAXEN-21, but it isn't.. JAXEN-21 is actually misreported.

Here is some code that reproduces this issue:

import java.io.StringReader;
import java.util.Iterator;
import java.util.List;

import org.jaxen.jdom.JDOMXPath;
import org.jdom.Document;
import org.jdom.input.SAXBuilder;
import org.jdom.output.XMLOutputter;

public class TestJ {

public static void main(String[] args) throws Exception { XMLOutputter outputter = new XMLOutputter(); SAXBuilder builder = new SAXBuilder(); Document doc = builder.build( new StringReader( new String("<html><head> </head> <body> <p> <span> text1 </span> </p> <div> text2 </div> </body> </html>"))); JDOMXPath query = new JDOMXPath("//div/preceding::*"); List set = query.selectNodes(doc); for (Iterator iter=set.iterator();iter.hasNext();) System.out.println(iter.next()); outputter.output(set, System.out); }
}

Show
Christian Nentwich added a comment - This looks similar to JAXEN-21, but it isn't.. JAXEN-21 is actually misreported. Here is some code that reproduces this issue: import java.io.StringReader; import java.util.Iterator; import java.util.List; import org.jaxen.jdom.JDOMXPath; import org.jdom.Document; import org.jdom.input.SAXBuilder; import org.jdom.output.XMLOutputter; public class TestJ { public static void main(String[] args) throws Exception { XMLOutputter outputter = new XMLOutputter(); SAXBuilder builder = new SAXBuilder(); Document doc = builder.build( new StringReader( new String("<html><head> </head> <body> <p> <span> text1 </span> </p> <div> text2 </div> </body> </html>"))); JDOMXPath query = new JDOMXPath("//div/preceding::*"); List set = query.selectNodes(doc); for (Iterator iter=set.iterator();iter.hasNext();) System.out.println(iter.next()); outputter.output(set, System.out); } }
Hide
Elliotte Rusty Harold added a comment -

I can reproduce this with a slightly simpler document:

<body><p><span></span></p><div></div></body>

In code (for XOM):
public void testPrecedingAxis() { Element body = new Element("body"); Element p = new Element("p"); body.appendChild(p); Element span = new Element("span"); p.appendChild(span); Element div = new Element("div"); body.appendChild(div); Nodes result = div.query("preceding::*[1]"); assertEquals(1, result.size()); assertEquals(span, result.get(0)); }

Show
Elliotte Rusty Harold added a comment - I can reproduce this with a slightly simpler document: <body><p><span></span></p><div></div></body> In code (for XOM): public void testPrecedingAxis() { Element body = new Element("body"); Element p = new Element("p"); body.appendChild(p); Element span = new Element("span"); p.appendChild(span); Element div = new Element("div"); body.appendChild(div); Nodes result = div.query("preceding::*[1]"); assertEquals(1, result.size()); assertEquals(span, result.get(0)); }
Hide
Elliotte Rusty Harold added a comment -

I'm still trying to work through this one, but I think the problem is in StackedIterator, and I think this affects some other bugs as well. Specifically I think stacking the iterators is producing a breadth-first search when we need to be using a depth-first search instead.

Show
Elliotte Rusty Harold added a comment - I'm still trying to work through this one, but I think the problem is in StackedIterator, and I think this affects some other bugs as well. Specifically I think stacking the iterators is producing a breadth-first search when we need to be using a depth-first search instead.
Hide
Brian Ewins added a comment -

I don't think its the stacked iterator, I think its the use of 'ChildAxisIterator' in that stack.

The correct logic for PrecedingAxis is roughly this:

public boolean hasNext() {
while (!descendantOrSelfReversed.hasNext()) {
while (!precedingSibling.hasNext()) {
if (!ancestorOrSelf.hasNext()) { return false; }
precedingSibling = ps(ancestorOrSelf.next());
}
descendantOrSelfReversed = dosr(precedingSibling.next());
}
return true;
}

public Object next() {
if (hasNext()) { return descendantOrSelfReversed.next(); }
// throw an exception otherwise
}

initializing this with empty iterators for descendantOrSelfReversed and precedingSibling. The current implementation looks to me like its not reversing descendantOrSelf, but is reversing the order in which the children are accessed on that axis.

Show
Brian Ewins added a comment - I don't think its the stacked iterator, I think its the use of 'ChildAxisIterator' in that stack. The correct logic for PrecedingAxis is roughly this: public boolean hasNext() { while (!descendantOrSelfReversed.hasNext()) { while (!precedingSibling.hasNext()) { if (!ancestorOrSelf.hasNext()) { return false; } precedingSibling = ps(ancestorOrSelf.next()); } descendantOrSelfReversed = dosr(precedingSibling.next()); } return true; } public Object next() { if (hasNext()) { return descendantOrSelfReversed.next(); } // throw an exception otherwise } initializing this with empty iterators for descendantOrSelfReversed and precedingSibling. The current implementation looks to me like its not reversing descendantOrSelf, but is reversing the order in which the children are accessed on that axis.
Hide
Brian Ewins added a comment -

after some headscratching I wrote this: its self contained so its simpler to see that it is correct. It passes the tests for everything but dom4j, because, as far as I can tell, that navigator reports parent(Document) as Document, not null. Leaving this for others to see in case I can't figure it out but I think its worth committing.

Show
Brian Ewins added a comment - after some headscratching I wrote this: its self contained so its simpler to see that it is correct. It passes the tests for everything but dom4j, because, as far as I can tell, that navigator reports parent(Document) as Document, not null. Leaving this for others to see in case I can't figure it out but I think its worth committing.
Hide
Brian Ewins added a comment -

Elliotte, I've updated the code for the preceding, ancestor, and descendant axes, but added the test you described above for this bug afterwards. It passes, but I'd prefer if you can confirm and close this bug by just updating your 'xml' directory first? Its the very first test that runs (see top of tests.xml), I'd expect you to get later test failures for Docment node stringvalues etc.

Show
Brian Ewins added a comment - Elliotte, I've updated the code for the preceding, ancestor, and descendant axes, but added the test you described above for this bug afterwards. It passes, but I'd prefer if you can confirm and close this bug by just updating your 'xml' directory first? Its the very first test that runs (see top of tests.xml), I'd expect you to get later test failures for Docment node stringvalues etc.
Hide
Elliotte Rusty Harold added a comment -

Yes, Brian's latest patches appear to fix this (among other issues).

Show
Elliotte Rusty Harold added a comment - Yes, Brian's latest patches appear to fix this (among other issues).
Hide
Brian Ewins added a comment -

Tonight I got this bug back again...with the DOM navigator. It seems the bug is intermittent and it may depend on something odd like object identities as hash keys. Commenting out the code overriding the 'preceding' axis in the DOM so it gets the new version appears to resolve the problem, but I'll need to walk the DOM code to be sure its wrong.

Show
Brian Ewins added a comment - Tonight I got this bug back again...with the DOM navigator. It seems the bug is intermittent and it may depend on something odd like object identities as hash keys. Commenting out the code overriding the 'preceding' axis in the DOM so it gets the new version appears to resolve the problem, but I'll need to walk the DOM code to be sure its wrong.
Hide
Brian Ewins added a comment -

yep... the DOM code was wrong. Not sure why this didn't show up yesterday. Fixed.

Show
Brian Ewins added a comment - yep... the DOM code was wrong. Not sure why this didn't show up yesterday. Fixed.
Hide
Elliotte Rusty Harold added a comment -

Hmm, this seems to have resurfaced in the DOM navigator. Will investigate further.

Show
Elliotte Rusty Harold added a comment - Hmm, this seems to have resurfaced in the DOM navigator. Will investigate further.
Hide
Elliotte Rusty Harold added a comment -

I think I've got this fixed. The problem was in dom.DocumentNavigator's custom PrecedingAxisIterator. For the moment I've disabled this so a correct implementation is inherited from the superclass instead. Whether it makes sense to fix that or leave it as is remains to be seen, so I'll leave this one for now in case someone wants to revisit that.

Show
Elliotte Rusty Harold added a comment - I think I've got this fixed. The problem was in dom.DocumentNavigator's custom PrecedingAxisIterator. For the moment I've disabled this so a correct implementation is inherited from the superclass instead. Whether it makes sense to fix that or leave it as is remains to be seen, so I'll leave this one for now in case someone wants to revisit that.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: