v1.56 changes attribute fetching

[jdeerin1]

1.56 seems to be requiring namespaces for fetching attribute values, while 1.55 would fail if you tried to include the namespace. Here is a test that demonstrates what I am seeing.

it 'shows what is failing' do
      xml='<rdf:RDF xmlns:fedora-model="info:fedora/fedora-system:def/model#" xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#" 
            xmlns:fedora="info:fedora/fedora-system:def/relations-external#" xmlns:hydra="http://projecthydra.org/ns/relations#">
            <rdf:Description rdf:about="info:fedora/druid:ab123cd4567">
              <fedora-model:hasModel rdf:resource="info:fedora/testObject"/>
              <hydra:isGovernedBy rdf:resource="info:fedora/druid:fg890hi1234"/>
            </rdf:Description>
          </rdf:RDF>'
          xml=Nokogiri::XML(xml)
          xml.search('//rdf:RDF/rdf:Description/hydra:isGovernedBy','hydra' => 'http://projecthydra.org/ns/relations#', 'fedora' => 'info:fedora/fedora-system:def/relations-external#', 'rdf' => 'http://www.w3.org/1999/02/22-rdf-syntax-ns#'     ).each do |node|
            #this works in 1.56 but not 1.55
            node['rdf:resource'].should == 'info:fedora/druid:fg890hi1234'
            #this works in 1.55 but not 1.56
            node['resource'].should == 'info:fedora/druid:fg890hi1234'
          end
    end```

[jdeerin1]

I did some testing, a4bd937 seems to be the commit that changed this.

[jvshahid]

Can you provide the output of nokogiri -v please.

[jdeerin1]

Nokogiri (1.5.6)

--- 
ruby: 
  engine: mri
  description: ruby 1.8.7 (2012-06-29 patchlevel 370) [i686-darwin11.4.0]
  platform: i686-darwin11.4.0
  version: 1.8.7
warnings: []

libxml: 
  loaded: 2.7.3
  compiled: 2.7.3
  binding: extension
nokogiri: 1.5.6

[jvshahid]

Thanks for the output and for reporting the bug. This behavior was changed for MRI, for more information see #712.

I'll go ahead and close this issue since it's not a bug.

[jcoyne]

That's a pretty significant interface change for a patch level release. -1

[mbklein]

There's still some weirdness in the (perfectly legal and kinda common) case where an element has a prefixed attribute in the same namespace. For example:

> puts @doc.to_xml
<?xml version="1.0"?>
<fruit xmlns="ns:fruit" xmlns:veg="ns:veg">
  <orange type="navel"/>
  <veg:carrot length="6" veg:color="orange"/>
</fruit>
 => nil
> @doc.root.elements[0].attributes.keys
 => ["type"] 
> @doc.root.elements[0]['type']
 => "navel" 
> @doc.root.elements[1].attributes.keys
 => ["color", "length"] 
> @doc.root.elements[1]['color']
 => nil 
> @doc.root.elements[1]['length']
 => "6" 
> @doc.root.elements[1]['veg:color']
 => "orange" 
> @doc.root.elements[1]['veg:length']
 => nil
> @doc.root.elements[1].attribute_nodes[0].name
 => "length" 
> @doc.root.elements[1].attribute_nodes[1].name
 => "color" 

The fact that #attributes comes back with non-prefixed keys for attributes that can only be accessed with a prefix is a bit counterintuitive, as is the fact that "veg:carrot/@veg:color" is in the veg namespace while "veg:carrot/@Length" has a nil namespace, since non-prefixed attributes are assumed to be in the element's namespace.

[flavorjones]

@jcoyne Thanks for weighing in!

@mbklein I'm looking into it.

Sorry for the confusion here, everyone. See the discussion on #712 for our rationale. Yes, a change like this shouldn't normally be in a patch release, however we consider JRuby/CRuby incompatibilities as bugs, since there's a significant number of users who expect, correctly, to be able to write platform-agnostic Nokogiri code.

[flavorjones]

@mbklein Can you provide a test case that succeeds under 1.5.5 and fails under 1.5.6? Or at least a test case that asserts what you believe the functionality should be (or was, previously)?

I want to make sure I understand your comments above, and I'm not sure I do right now.

[flavorjones]

@mbklein or, is this test sufficiently describing the issue:

      def test_something_foo
        doc = Nokogiri::XML <<-EOXML
<?xml version="1.0"?>
<fruit xmlns="ns:fruit" xmlns:veg="ns:veg">
  <orange type="navel"/>
  <veg:carrot length="6" veg:color="orange"/>
</fruit>
EOXML

        ## 1.5.6 passes these
        assert_equal nil, doc.root.elements[1]['color']
        assert_equal "6", doc.root.elements[1]['length']
        assert_equal "orange", doc.root.elements[1]['veg:color']
        assert_equal nil, doc.root.elements[1]['veg:length']

        ## 1.5.5 passes these
        assert_equal "orange", doc.root.elements[1]['color']
        assert_equal "6", doc.root.elements[1]['length']
        assert_equal nil, doc.root.elements[1]['veg:color']
        assert_equal nil, doc.root.elements[1]['veg:length']
      end

[flavorjones]

@mbklein The behavior of the length attribute has not changed between 1.5.5 and 1.5.6. The behavior of the color attribute is as @jdeerin1 described above. So can you help me understand your comments?

[mbklein]

Sorry for the confusion – this isn't something that changed between 1.5.5 and 1.5.6, or a new inconsistency. It's part of my ongoing campaign to clean up namespace behavior in general, and it reared its head in this issue. I also had a minor misconception about default namespaces for unprefixed elements, so I'm abandoning that line of thought.

The primary issue I have is that #attributes should return a hash in which the keys can be used to access the values, especially in cases where there might be a name collision (which, after all, is one of the primary purposes of namespaces). There's currently no easy way to enumerate the actual qnames of the elements that can be used as accessors on the parent element's #[] method.

I just whipped up a contrived test case. Comments are based on current (1.5.6) behavior, which I now see as an incremental improvement on 1.5.5 behavior (all issues about minor vs. patch releases aside).

def test_colliding_attributes
  doc = Nokogiri::XML <<-EOXML
  <f:fruit xmlns:f="ns:fruit" xmlns:v="ns:veg">
    <f:orange f:is="true" v:is="false"/>
  </f:fruit>
  EOXML

  e = doc.root.elements[0]

  # 1) Stuff that works as expected
  assert_equal 2, e.attribute_nodes.length      # CRuby: Yep!  JRuby: Yep!
  assert(e['f:is'])                             # CRuby: Yep!  JRuby: Yep!
  assert(e['v:is'])                             # CRuby: Yep!  JRuby: Yep!

  # 2) Stuff that's inconsistent between platforms
  assert(e.attribute_with_ns('is', 'ns:fruit')) # CRuby: Yep!  JRuby: Nope!
  assert(e.attribute_with_ns('is', 'ns:veg'))   # CRuby: Yep!  JRuby: Nope!

  # 3) Stuff that's not broken per se, but probably isn't what the user expects/wants
  assert_equal 2, e.attributes.length           # CRuby: Nope! JRuby: Nope!
  assert_equal 2, e.attributes.keys.length      # CRuby: Nope! JRuby: Nope!
  assert_includes 'f:is', e.attributes.keys     # CRuby: Nope! JRuby: Nope!
  assert_includes 'v:is', e.attributes.keys     # CRuby: Nope! JRuby: Nope!
  assert(e[e.attributes.keys.first])            # CRuby: Nope! JRuby: Nope!
end

[mbklein]

I have a fix in mind that would satisfy the assertions in section 3, but it would still be a big change to current behavior. You'll probably see a pull request from me on it later today.

[flavorjones]

@mbklein - I'd love to see your fix for this, if you've still got it somewhere.

[mbklein]

Yeah, I've got it in a branch. I'll rebase it on trunk and see what kind of shape it's in.

[flavorjones]

In 21f18de back in 2021 I wrote quite a bit of documentation to help users understand that #attribute and #attributes just don't handle namespaces well, and to use #attribute_with_ns and #attribute_nodes for those use cases that need namespaces.