Namespace cache rework

[rdingwell]

I realize that this is a fairly large pull request but I believe that it address some issues on the Java side of Nokogiri. The main thrust of the work is a reworking of the NamespaceCache to cache namespaces based on the node from which they are declared. The caching itself is still performed in a central object but it has been reworked to store the namespaces in a hierarchal manor. This allows for a namespace prefixes and default namespaces to be redeclared on elements as allowed for by the xml specification. This pull request also address issues #902 #940 and #943.

All of the tests pass though 2 of them needed to be skipped do to allowing namespace redefinition. The 2 tests that were skipped are in the test_builder.rb file and are specifically related to testing that you cannot redefine namespaces.

[leejarvis]

Looks good. Can you squash these changes into a single commit and remove the whitespace changes? It makes it easier to review, thanks for all the work!

[rdingwell]

Lee,

Thanks for taking a look at this. I squashed the commits and cleaned up the whitespace issues.

[knu]

@yokolet Could you review this PR?

[yokolet]

Sure! Give a day or so.

[yokolet]

@rdingwell thanks for working on this! Namespace refactoring has been in my TODO list for a long time.

Patch looks good, however, newly skips two tests in xml/test_builder.rb. This will be a regression for some users.
Can you fix those skipped tests?

Pure Java Nokogiri maintainers have paid a lot of efforts to make it work consistent to CRuby version. Introducing new inconsistency is a bad news for users.

[rdingwell]

@yokolet The issue with the skipped tests has to do with this patch allowing for namespace prefixes to be redefined in a document. If you look at #943 you'll see that even CRuby has inconsistencies with what it does when parsing a document with redefined namespace prefixes and what it allows a user to do programmatically. The reason that the 2 tests are skipped is because this patch fixes issue #943 for JRuby Nokogiri, the issue still exists in CRuby Nokogiri though. The tests themselves are specifically looking for not being able to redefine namespaces, which IMO should be allowed seeing how it is allowed by the XML specification.

I had started to look at what it would take to do the same for the CRuby implementation but it's been a while since I've done any real C coding and decided to back off on that.

I think this is really an issue where the CRuby Nokogiri implementation should be brought up to date and aligned with what this patch fixes. I understand needing to keep things consistent but that should really be a 2 way street. I don't see a reason why the JRuby implementation cant take the lead on fixing some things ahead of the CRuby impl and have the CRuby impl come up to where they JRuby version is.

[yokolet]

@rdingwell I know your frustration. I'm the one who have fought over the same frustration for a long time.

However, it is important for pure Java Nokogiri.
People often complains about "JRuby version doesn't work the same as CRuby version." But, I've never heard the complaint, "CRuby version doesn't work the same as JRuby version." So, don't blame CRuby version's implementation.

I'll have a look to find a way to make those feature work using your namespace fix. So, please be patient for a while.

[ryanstout]

Sorry to ask, but any updates on this issue? Thanks.

[rdingwell]

I have not heard anything for a couple of months. What I wished would happen is that this would get incorporated and then have the discrepancies reported as bugs in the CRuby implementation so they could be addressed there by someone who is familiar with that code and more proficient in C then I am. While one of the issues that this pull request addresses is related to the Java implementation the other 2 are issues are related to both the C and Java implementations and what is allowed in XML spec as far as handling namespaces goes. This fixes those issues for the Java side and they should also be addressed on the C side of things. I think to some degree the Java side of nokogiri is still a second class citizen as issues do not appear to be able to be addressed in the Java side before they are in the C side but the other way around is expected. In some ways I understand it as nokogiri started out as a C impl and then the java version came later but why isnt anyone working on the C side of things to address the namespace issues that are reported.

[flavorjones]

I appreciate the work here, but I'm not comfortable introducing different behavior into JRuby that's not there for other versions of Ruby.

I'll dig into what's needed to make this work in the CRuby version.

[yokolet]

Thanks for the rework. However, the logic in the pull request has a flaw and breaks more tests.
Instead of merging the pull request, I rewrote namespace cache and fixed bugs, issue#934 and issue#940. Current master should not have this problem.

I'm going to close this pull request.

[rdingwell]

THe problem with your approach is that it does not allow for prefixes to be redefined at different levels in the tree. This is something that is allowed in the XML spec and is another thing that I had implemented. I understand the need to try and keep things in sync but I think my previous comments pretty much outline my feelings on this subject. Those tests that you refer to as being broken were not broken but simply additional functionality that I had been arguing should have been implemented in the CRuby impl as well. Either way I'm done with this.