JRuby & Builder namespace improvements: Take 2

[mbklein]

Having screwed up the commit history on my previous pull request, I'm trying again.

I thought about splitting this into two separate pull requests, but everything's so intertwined. This change does three things:

1. Add tests for how I think Nokogiri is trying to handle namespaces and namespace inheritance – at least tests that are consistent with how most code I've come across expects namespaces to work.
2. Fix the node renaming issues that were causing Xerces to throw NAMESPACE_ERR exceptions in certain (sometimes inconsistent) circumstances.
3. Give Nokogiri::XML::Builder the ability to handle namespace prefixes that won't be formally declared until the first method_missing call (e.g., xml[:foo].bar(:'xmlns:foo' => 'baz')). The ArgumentError will still be raised if the new prefix isn't declared by the first element encountered.

The libxml C version of Nokogiri still doesn't pass the "expected namespace behavior" tests, but this at least establishes a common baseline for the two. EDIT: The C extension now behaves the same as the Java extension as far as namespaces are concerned.

[jcoyne]

This fixes #801 so I urge it be accepted into the official branch.

[mbklein]

I believe this also to be a more comprehensive fix to #814 than provided by 45e3a00.

[mbklein]

All namespace-related tests now pass on both CRuby and JRuby.

[mbklein]

This is the last of my commits for now. All tests now pass in the travis-ci environment under jruby (18mode and 19mode), MRI (1.8.7, 1.9.3, 2.0.0-rc1, and head), and the last release of ree. rbx-18mode segfaults, but does that anyway under master, so I can't see it affecting the viability of this pull request.

[anarchivist]

👍

Having proper namespace support is pretty essential for the library metadata community.

[barmintor]

+1 and thank you for the testing baseline

[jrochkind]

+1 Nokogiri has handled namespaces inconsistently for a long time, causing a lot of frustration for me and others. Having looked at code and discussed with mbklein, I think this solves a lot of the issues and makes the internals fundamentally rational and sane here.

If this gets applied, I would try to do some documentation updates to match, if needed. (I've had a couple docu patches accepted before around namespaces, as I tried to figure out what the heck the inconsistent behavior was)

[flavorjones]

I'll review tonight and put in the RC if all looks good.
On Feb 18, 2013 11:51 AM, "Jonathan Rochkind" [email protected]
wrote:

+1 Nokogiri has handled namespaces inconsistently for a long time, causing
a lot of frustration for me and others. Having looked at code and discussed
with mbklein, I think this solves a lot of the issues and makes the
internals fundamentally rational and sane here.

If this gets applied, I would try to do some documentation updates to
match, if needed. (I've had a couple docu patches accepted before around
namespaces, as I tried to figure out what the heck the inconsistent
behavior was)

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/846#issuecomment-13731143.

[jrochkind]

So, any hope?

[flavorjones]

This block doesn't appear to have test coverage.

[flavorjones]

Made two comments at https://github.com/sparklemotion/nokogiri/pull/846/files#L4R36 about lack of test coverage. I've also made a subsequent commit to change magic numbers to constants.

If you've got a response on the test coverage, let me know. In the meantime, I'm merging and this will be part of 1.5.7.rc2.

[mbklein]

Thanks! I'll take a look at the test coverage and add whatever's missing in the next few days.