Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

wgxpath cannot evaluate upper case. #46

Open
pyraman opened this issue Jun 17, 2016 · 8 comments
Open

wgxpath cannot evaluate upper case. #46

pyraman opened this issue Jun 17, 2016 · 8 comments

Comments

@pyraman
Copy link

pyraman commented Jun 17, 2016

To reproduce:

for This issue, I'm evaluating feFood svg control by wgxpath. If xpath passed to evaluate function is case-sensitive, then we get no result.

  1. Open sample attached in IE 11.
  2. press button 'install WGXPath'
  3. press buttons: "evaluate feFlood - Uppercase" -> result: Found: 0 items.
  4. press buttons: "evaluate feflood - Lowercase" -> result: Found: 1 items.
  5. expected: at step 3 we get: Found: 1 items. at step 4 we get: Found: 0 items. (the same as native xpath supported by Chrome and Firefox.

Sample:
feFood_sample.zip

@nthau2409
Copy link

hello, I got the same issue with pyraman.

Does any one have solution for this?

Thanks.

@pyraman
Copy link
Author

pyraman commented Jun 20, 2016

hi Nedjs! can you confirm this is an issue? I've tried newest release but no luck.
thanks,

@nedjs
Copy link
Contributor

nedjs commented Jun 20, 2016

Yeah I saw the issue, my previous pull request actually missed a few areas where names were being converted to lower case. I closed my old pull request and made a new one. Looks fixed with the additional changes.

In a jasmine test these fail (plunker for you to see)

var wgXPathWindow = { document: {} }; // fake window object used to install wgxpath on
wgxpath.install(wgXPathWindow);

var xml = new DOMParser().parseFromString('<Root><Child/></Root>', "application/xml")

describe("Case sensitive node name functions.", function() {
  it("Case sensitive name() function call", function() {
    var expression = "//*[name()='Child']";
    var wgResult = wgXPathWindow.document.evaluate(expression, xml, null, XPathResult.FIRST_ORDERED_NODE_TYPE, null);
    var nativeResult = window.document.evaluate(expression, xml, null, XPathResult.FIRST_ORDERED_NODE_TYPE, null);
    expect(wgResult.singleNodeValue === nativeResult.singleNodeValue).toBe(true);
  });
  it("Case sensitive local-name() function call", function() {
    var expression = "//*[local-name()='Child']";
    var wgResult = wgXPathWindow.document.evaluate(expression, xml, null, XPathResult.FIRST_ORDERED_NODE_TYPE, null);
    var nativeResult = window.document.evaluate(expression, xml, null, XPathResult.FIRST_ORDERED_NODE_TYPE, null);
    expect(wgResult.singleNodeValue === nativeResult.singleNodeValue).toBe(true);
  });
});

@pyraman
Copy link
Author

pyraman commented Jun 20, 2016

I've applied changes in 3 files in https://github.com/google/wicked-good-xpath/pull/48/files and built it myself (closure compiler). But no luck.

files changed:
1 - functionCall.js
2 - nameTest.js
3 - node.js.

hi Nedjs! plz help recheck.

thanks.

Update: just used files in https://github.com/nedjs/wicked-good-xpath - no luck also.

@nedjs
Copy link
Contributor

nedjs commented Jun 20, 2016

@pyraman I did not upload any of the dist/ files in my pull request, nor are they in my fork. I just started the repo from scratch and put those changes in and verified it worked with both my jasmine test and your feFood sample.

I placed the build of it for you on the plunkr https://plnkr.co/edit/qXCr9J2GbIBtfcuqcyiU under the file wgxpath.install.pull48.js. Additionally you can change the index.html file to point to that javascript file and see the tests pass. Also i forgot to mention those tests will only run in a browser which supports xpath (Chromium, FF, Edge)

@pyraman
Copy link
Author

pyraman commented Jun 20, 2016

@nedjs my issue is posted for IE 11. But I also run my sample with your file: xpath.install.pull48.js on Edge. The problem remains unchanged. on Edge, results i got after clicking 2 buttons is different from FF + Chrome. not familiar with Plunker https://plnkr.co/edit/nj9uj4

I'm familiar with jsfiddle - here is the link: https://jsfiddle.net/Pyraman/u6d9pzqj/

@nthau2409
Copy link

Hi @nedjs ,

Can you help to update this?

I'm interested in this issue 👍

Thanks you

@nedjs
Copy link
Contributor

nedjs commented Jun 28, 2016

@nthau2409 sorry about that, worked through this last week but didnt get around to actually writing anything up on it due to other obligations.

The basic problem here was that the name() function was returning the value of nodeName which is a case intensive version of the nodes name. Some browsers return a full upper case version, some lower case and some the actual case. This of course caused some issues with your test. I updated my branch and actually uploaded this dist/ folder if you want to check it out without building it https://github.com/nedjs/wicked-good-xpath/tree/master/dist

I removed my PR and am hesitant to put another one up for the following reasons:

  • Previous build always converted things to lower case, changing this now might cause upgrading issues for existing users, this PR (and subsequent PR's related to this) really should be cleaned up and put in a new version (major maybe?)
  • More work needs to be done for early browser versions. Node.nodeName's case is unpredictable, previously ALL name tests and name related functions converted to lower case. If changes are made to name it case sensitive then all functionality will just straight up not work in lower IE versions. For instance this expression //*[name()='FooBar'] can never be evaluated in lower IE versions because the DOM doesnt support case sensitive node names. FooBar's node name in IE is foobar. Im not really sure what the solution to this is because XPath was made for XML not HTML. The HTML spec if goofy and weird especially in early implementations of it (looking at you IE).

It should be noted that some IE DOM implementations do support case sensitive nodeName, I havent done a full range of experimentation out of lack of time and motive but it seems to be detectable VIA
document.implementation.hasFeature(feature, version); with a combination of ("XML", "1.0"), ("XML", "2.0"), ("HTML", "1.0") or ("HTML", "2.0")

In conclusion
Its diffucult to fix this, great care must be given to any implementation due to IE's quirks regarding nodeName. Any fix must also include tests targeted at browsers which dont support localName and prefix. Either way its a bit much for me given I still havent figured out how the tests work nor have the time ATM to work on this, but maybe someone else is up for a challenge.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants