Bug 1223716. Make HTMLCollection check for the element being HTML before checking for its name inside its named getter. r=bkelly

dom/base/nsContentList.cpp

@@ -527,8 +527,9 @@ nsContentList::NamedItem(const nsAString& aName, bool aDoFlush)
     nsIContent *content = mElements[i];
     // XXX Should this pass eIgnoreCase?
     if (content &&
-        (content->AttrValueIs(kNameSpaceID_None, nsGkAtoms::name,
+        ((content->IsHTMLElement() &&
-                              name, eCaseMatters) ||
+          content->AttrValueIs(kNameSpaceID_None, nsGkAtoms::name,
+                               name, eCaseMatters)) ||
          content->AttrValueIs(kNameSpaceID_None, nsGkAtoms::id,
                               name, eCaseMatters))) {
       return content->AsElement();

testing/web-platform/tests/dom/nodes/Document-Element-getElementsByTagName.js

@@ -50,19 +50,44 @@ function test_getElementsByTagName(context, element) {
   }, "Should be able to set expando shadowing a proto prop (namedItem)")
 
   test(function() {
-    var t = element.appendChild(document.createElement("pre"));
+    var t1 = element.appendChild(document.createElement("pre"));
-    t.id = "x";
+    t1.id = "x";
-    this.add_cleanup(function() {element.removeChild(t)});
+    var t2 = element.appendChild(document.createElement("pre"));
+    t2.setAttribute("name", "y");
+    var t3 = element.appendChild(document.createElementNS("", "pre"));
+    t3.setAttribute("id", "z");
+    var t4 = element.appendChild(document.createElementNS("", "pre"));
+    t4.setAttribute("name", "w");
+    this.add_cleanup(function() {
+      element.removeChild(t1)
+      element.removeChild(t2)
+      element.removeChild(t3)
+      element.removeChild(t4)
+    });
 
     var list = context.getElementsByTagName('pre');
     var pre = list[0];
     assert_equals(pre.id, "x");
-    assert_equals(list['x'], pre);
 
-    assert_true('x' in list, "'x' in list");
+    var exposedNames = { 'x': 0, 'y': 1, 'z': 2 };
-    assert_true(list.hasOwnProperty('x'), "list.hasOwnProperty('x')");
+    for (var exposedName in exposedNames) {
+      assert_equals(list[exposedName], list[exposedNames[exposedName]]);
+      assert_equals(list[exposedName], list.namedItem(exposedName));
+      assert_true(exposedName in list, "'" + exposedName + "' in list");
+      assert_true(list.hasOwnProperty(exposedName),
+                  "list.hasOwnProperty('" + exposedName + "')");
+    }
 
-    assert_array_equals(Object.getOwnPropertyNames(list).sort(), ["0", "x"]);
+    var unexposedNames = ["w"];
+    for (var unexposedName of unexposedNames) {
+      assert_false(unexposedName in list);
+      assert_false(list.hasOwnProperty(unexposedName));
+      assert_equals(list[unexposedName], undefined);
+      assert_equals(list.namedItem(unexposedName), null);
+    }
+
+    assert_array_equals(Object.getOwnPropertyNames(list).sort(),
+                        ["0", "1", "2", "3", "x", "y", "z"]);
 
     var desc = Object.getOwnPropertyDescriptor(list, '0');
     assert_equals(typeof desc, "object", "descriptor should be an object");

testing/web-platform/tests/dom/nodes/Element-children.html

@@ -3,14 +3,27 @@
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 <div id="log"></div>
-<div id="test"><img><img id=foo><img id=foo></div>
+<div id="test"><img><img id=foo><img id=foo><img name="bar"></div>
 <script>
+setup(function() {
+  // Add some non-HTML elements in there to test what happens with those.
+  var container = document.getElementById("test");
+  var child = document.createElementNS("", "img");
+  child.setAttribute("id", "baz");
+  container.appendChild(child);
+
+  child = document.createElementNS("", "img");
+  child.setAttribute("name", "qux");
+  container.appendChild(child);
+});
+
 test(function() {
   var container = document.getElementById("test");
   var result = container.children.item("foo");
   assert_true(result instanceof Element, "Expected an Element.");
   assert_false(result.hasAttribute("id"), "Expected the IDless Element.")
 })
+
 test(function() {
   var container = document.getElementById("test");
   var list = container.children;
@@ -20,8 +33,26 @@ test(function() {
       result.push(p);
     }
   }
-  assert_array_equals(result, ['0', '1', '2']);
+  assert_array_equals(result, ['0', '1', '2', '3', '4', '5']);
   result = Object.getOwnPropertyNames(list);
-  assert_array_equals(result, ['0', '1', '2', 'foo']);
+  assert_array_equals(result, ['0', '1', '2', '3', '4', '5', 'foo', 'bar', 'baz']);
+
+  // Mapping of exposed names to their indices in the list.
+  var exposedNames = { 'foo': 1, 'bar': 3, 'baz': 4 };
+  for (var exposedName in exposedNames) {
+    assert_true(exposedName in list);
+    assert_true(list.hasOwnProperty(exposedName));
+    assert_equals(list[exposedName], list.namedItem(exposedName));
+    assert_equals(list[exposedName], list.item(exposedNames[exposedName]));
+    assert_true(list[exposedName] instanceof Element);
+  }
+
+  var unexposedNames = ['qux'];
+  for (var unexposedName of unexposedNames) {
+    assert_false(unexposedName in list);
+    assert_false(list.hasOwnProperty(unexposedName));
+    assert_equals(list[unexposedName], undefined);
+    assert_equals(list.namedItem(unexposedName), null);
+  }
 });
 </script>