Namespaces are handled slightly wrong

[Sebmaster]

• It seems like tags are always lower-cased in a path even when they're not in the HTML namespace, which should not happen jsfiddle
• If no namespace is specified, only html namespaces should be matched jsfiddle
• ... Something still doesn't match up

Just comment out the install line vs leaving it to show the difference vs browser implementations.

[Sebmaster]

Added a PR which fixes at least one of the issues. I haven't exactly debugged where the other one originates from yet.

[Dominator008]

@Sebmaster Cool thanks. Are you going to work on the second part?

[Sebmaster]

Yeah, I'll probably have some time tomorrow so I'll try to dig into it again.

[Dominator008]

@Sebmaster Great. Thanks for looking into this!

[Sebmaster]

Okay got to it right now. This should be it for the namespace related stuff.
There's another bug which seems to restrict parsed expressions too much, but I'll create another issue for that one.

[Dominator008]

@Sebmaster Thanks for the PR. Just want to you to know that I'm working on solving #20 first before merging this, so that we can make sure the tests are still passing.

[nedjs]

I noticed with this PR its still not returning valid results for empty or null namespaces (no prefix). Might I suggest letting the test in node.js check if the node matches instead of using getElementsByTagNameNS, this not only seems to work but it relegates the actual testing of node matching to the test objects abstraction (in this case NameTest).

So at node.js:268 make it read:

    var nodes = node.getElementsByTagName(test.getName());
    goog.array.forEach(nodes, function(node) {
      if (test.matches(node) && wgxpath.Node.attrMatches(node, attrName, attrValue)) {
        nodeset.add(node);
      }
    });

Additionally this change in nameTest.js:

  if (this.namespaceUri_ == wgxpath.NameTest.HTML_NAMESPACE_URI_) {
    this.name_ = this.name_.toLowerCase();
  }

Causes case sensitive node names to not match if there parsed under a document which has no namespace.

---

I changed node.js and removed that part in nameTest.js and rebuilt the project. I put both this PR's code, the current wgxpath 1.3.0 release and my changes up in a plunker in a "test" so you can see where its failing.

Heres a link to the plunker

To use the plunker just comment out one of the script includes at the top of index.html and uncomment the on you want to test. The plunker runs 6 tests on similar documents but with different cases and namespaces. I took the code from @ScottFreeCode issue and modified it a bit. I compares the native browser implementations xpath result to wgxpaths. If they resulted in different things then it shows them as red.

[Sebmaster]

I noticed with this PR its still not returning valid results for empty or null namespaces (no prefix).

How are you reproducing this? XML doc without xmlns set?

Causes case sensitive node names to not match if there parsed under a document which has no namespace.

name_ has always been lower-cased before. Do you mean you rely on tags without namespace to always be lowercased? That doesn't sound right, case insensitivity is a HTML thing afaik.

---

Oh, just saw the plunker, I'll take a look.

[nedjs]

Sorry I should have posted some sample code with my comments. The plunker is there but ill add some code here to narrow it down and help snapshot the relevant info for this conversation in case that plunker goes away.

How are you reproducing this? XML doc without xmlns set?

Exactly no xmlns, lets look at this code

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

var xml = new DOMParser().parseFromString('<Root><Menu ID="11"></Menu></Root>', "application/xml");

var expression = '/Root';
var wgResult = wgXPathWindow.document.evaluate(expression, xml, null, XPathResult.FIRST_ORDERED_NODE_TYPE, null).singleNodeValue;
var nativeResult = xml.evaluate(expression, xml, null, XPathResult.FIRST_ORDERED_NODE_TYPE, null).singleNodeValue;

console.log(wgResult); // null
console.log(nativeResult); // <Root> node

The above code evaluates the expression using wgxpath expression and the browsers native impl. As you can see the browsers implementation is happy to work without a namespace and no prefix.

name_ has always been lower-cased before. Do you mean you rely on tags without namespace to always be lowercased? That doesn't sound right, case insensitivity is a HTML thing afaik.

Yeah your exactly right, previously name_ has been lower case in that particular section of the code. However stripping the name of the case is kinda dangerous IMO, I think it would be better ignore case latter on somewhere in the code if that is still an issue. Some initial testing with case sensitivity on a window.document shows that it seems to work fine without stripping the case.

I think it would be good to setup some unit tests for some of these things. I would be happy to do it (and even tried for a little) but I am having one heck of a time trying to even run the tests =P. Ill take a look at the unit tests later tonight again.

[ScottFreeCode]

May be worth double-checking that the predefined xml: namespace is properly supported too? I tried adding it to the Root and Menu tag names in both the XML source and the test expressions in my test from the other issue (where I use createNSResolver), and got the same results where only descendant doesn't work right; but while fixes are being kicked around we'll want to make sure we don't break part of the existing functionality. (Or maybe it's more a matter of how createNSResolver works and won't be affected by the evaluate code being tweaked here anyway?)