diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4d723481f2..c6d7e39861 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -37,6 +37,7 @@ Nokogiri follows [Semantic Versioning](https://semver.org/), please see the [REA
* [CRuby] libgumbo (the HTML5 parser) treats reaching max-depth as EOF. This addresses a class of issues when the parser is interrupted in this way. [#3121] @stevecheckoway
* [CRuby] Update node GC lifecycle to avoid a potential memory leak with fragments in libxml 2.13.0 caused by changes in `xmlAddChild`. [#3156] @flavorjones
* [CRuby] libgumbo correctly prints nonstandard element names in error messages. [#3219] @stevecheckoway
+* [JRuby] Fixed some bugs in how `Node#attributes` handles attributes with namespaces. [#2677, #2679] @flavorjones
### Changed
diff --git a/ext/java/nokogiri/XmlNode.java b/ext/java/nokogiri/XmlNode.java
index 4ecc957954..70440b5351 100644
--- a/ext/java/nokogiri/XmlNode.java
+++ b/ext/java/nokogiri/XmlNode.java
@@ -643,12 +643,22 @@ public class XmlNode extends RubyObject
@JRubyMethod(name = {"attribute", "attr"})
public IRubyObject
- attribute(ThreadContext context, IRubyObject name)
+ attribute(ThreadContext context, IRubyObject rbName)
{
- NamedNodeMap attrs = this.node.getAttributes();
- Node attr = attrs.getNamedItem(rubyStringToString(name));
- if (attr == null) { return context.nil; }
- return getCachedNodeOrCreate(context.runtime, attr);
+ NamedNodeMap attributes = this.node.getAttributes();
+ String name = rubyStringToString(rbName);
+
+ for (int j = 0 ; j < attributes.getLength() ; j++) {
+ Node attribute = attributes.item(j);
+ String localName = attribute.getLocalName();
+ if (localName == null) {
+ continue;
+ }
+ if (localName.equals(name)) {
+ return getCachedNodeOrCreate(context.runtime, attribute);
+ }
+ }
+ return context.nil;
}
@JRubyMethod
@@ -1415,11 +1425,12 @@ public class XmlNode extends RubyObject
}
}
- if (uri != null) {
- element.setAttributeNS(uri, key, val);
- } else {
+ if (colonIndex > 0 && uri == null) {
element.setAttribute(key, val);
+ } else {
+ element.setAttributeNS(uri, key, val);
}
+
clearXpathContext(node);
}
diff --git a/test/xml/test_node_attributes.rb b/test/xml/test_node_attributes.rb
index a974273fea..3a3e465c0b 100644
--- a/test/xml/test_node_attributes.rb
+++ b/test/xml/test_node_attributes.rb
@@ -5,19 +5,71 @@
module Nokogiri
module XML
class TestNodeAttributes < Nokogiri::TestCase
- def test_attribute_with_ns
- doc = Nokogiri::XML(<<-eoxml)
-
-
-
- eoxml
-
- node = doc.at("node")
-
- assert_equal(
- "bar",
- node.attribute_with_ns("foo", "http://tenderlovemaking.com/").value,
- )
+ let(:simple_xml_with_namespaces) { <<~XML }
+
+
+
+
+ XML
+
+ describe "#attribute" do
+ it "returns an attribute that matches the local name" do
+ doc = Nokogiri.XML(simple_xml_with_namespaces)
+ node = doc.at_css("next")
+ refute_nil(attr = node.attribute("foo"))
+
+ # NOTE: that we don't make any claim over _which_ attribute should be returned.
+ # this situation is ambiguous and we make no guarantees.
+ assert_equal("foo", attr.name)
+ end
+
+ it "does not return an attribute that matches the full name" do
+ doc = Nokogiri.XML(simple_xml_with_namespaces)
+ node = doc.at_css("next")
+ assert_nil(node.attribute("tlm:foo"))
+ end
+ end
+
+ describe "#attribute_with_ns" do
+ it "returns the attribute that matches the name and namespace" do
+ doc = Nokogiri.XML(simple_xml_with_namespaces)
+ node = doc.at_css("node")
+
+ refute_nil(attr = node.attribute_with_ns("foo", "http://tenderlovemaking.com/"))
+ assert_equal("bar", attr.value)
+ end
+
+ it "returns the attribute that matches the name and nil namespace" do
+ doc = Nokogiri.XML(simple_xml_with_namespaces)
+ node = doc.at_css("node")
+
+ refute_nil(attr = node.attribute_with_ns("foo", nil))
+ assert_equal("baz", attr.value)
+ end
+
+ it "does not return a attribute that matches name but not namespace" do
+ doc = Nokogiri.XML(simple_xml_with_namespaces)
+ node = doc.at_css("node")
+
+ assert_nil(node.attribute_with_ns("foo", "http://nokogiri.org/"))
+ end
+
+ it "does not return a attribute that matches namespace but not name" do
+ doc = Nokogiri.XML(simple_xml_with_namespaces)
+ node = doc.at_css("node")
+
+ assert_nil(node.attribute_with_ns("not-present", "http://tenderlovemaking.com/"))
+ end
+ end
+
+ describe "#set_attribute" do
+ it "round trips" do
+ doc = Nokogiri.XML(simple_xml_with_namespaces)
+ node = doc.at_css("node")
+ node["xxx"] = "yyy"
+ refute_nil(node.attribute("xxx"))
+ assert_equal("yyy", node.attribute("xxx").value)
+ end
end
def test_prefixed_attributes
@@ -84,13 +136,8 @@ def test_append_child_element_with_prefixed_attributes
end
def test_namespace_key?
- doc = Nokogiri::XML(<<-eoxml)
-
-
-
- eoxml
-
- node = doc.at("node")
+ doc = Nokogiri.XML(simple_xml_with_namespaces)
+ node = doc.at_css("node")
assert(node.namespaced_key?("foo", "http://tenderlovemaking.com/"))
assert(node.namespaced_key?("foo", nil))