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

wrong context handling #7

Closed
pavelblk opened this issue May 17, 2017 · 7 comments
Closed

wrong context handling #7

pavelblk opened this issue May 17, 2017 · 7 comments
Labels

Comments

@pavelblk
Copy link

pavelblk commented May 17, 2017

I found the wrong context handling.
Exmaple: //element1[not(@attr=//element2/@attr)]
"//element2" will be started not from document root
"//element2" processing as ".//element2"

Solution for current release: //element1[not(@attr=ancestor::*[not(parent::*)]//element2/@attr)]

@bseddon
Copy link

bseddon commented Jan 5, 2018

I agree with this. The issue is that the grammar definition does not provide information to the PathNodeExpr instance to let it know when the relative path expression should be evaluated from the root (see XPath.y lines 528 and 533) or when it should be evaluate using the location of the context item (line 541).

In addition to the effect in a query like like the one provided by @pavelblk the effect can be seen in this query if the context node is not already the document root:

/myRootNode

In the conformance test cases the context item is always the root node so the issue never arises. However if the context node is some other node, such as the document element, then there is a problem.

A way to resolve this is to modify the PathNodeExpr class to add a new property to indicate whether the expression should be evaluated from the root or not. Then further update this class so that when the tail is created in the Execute() method (line 141) it is passed either the root node or the current context item depending upon the value of the property.

XPath.y can then be updated to set the new property of PathNodeExpr in the cases of '/' RelativePathExpr or DOUBLE_SLASH RelativePathExpr.

I've implemented such a change in my version of the source and it appears to work. If and when I have chance to run through all the conformance tests I'll submit a pull request.

@StefH
Copy link
Owner

StefH commented Jan 6, 2018

That's very cool ! If you have the code working you can make a PR.

@pavelblk
Copy link
Author

pavelblk commented Jan 6, 2018

Thank you for describing. I will try make a PR, when I close my current project. I will get you know when I start that.

@vwegert
Copy link

vwegert commented Apr 5, 2023

I have run into this issue while working on #62 - it appears to be unsolved. Does any of you already have a fix...?

@arkiaconsulting
Copy link

@bseddon Hi ! any chance that you publish a PR ?

@cbridet
Copy link
Contributor

cbridet commented Mar 21, 2024

I've just pushed a pull request solving this problem.

@StefH StefH assigned StefH and unassigned StefH Mar 21, 2024
@StefH StefH added the bug label Mar 21, 2024
@StefH
Copy link
Owner

StefH commented Mar 22, 2024

Closed by #67

@StefH StefH closed this as completed Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants