From 18526a329eabd9a1d5681f13fbfa9c5699dffc9b Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@mit.edu>
Date: Thu, 22 May 2014 00:23:51 -0400
Subject: [PATCH] Bug 1013316.  Implement GetSupportedNames on
 HTMLAllCollection.  r=smaug

This also fixes the GetSupportedNames on nsContentList HTMLCollections to follow the spec.
---
 content/base/src/nsContentList.cpp            | 17 ++--
 .../content/test/test_htmlcollection.html     |  4 +-
 .../html/document/src/HTMLAllCollection.cpp   | 82 +++++++++++++++----
 content/html/document/src/HTMLAllCollection.h |  4 +-
 content/html/document/test/mochitest.ini      |  2 +
 .../html/document/test/test_bug1013316.html   | 46 +++++++++++
 6 files changed, 128 insertions(+), 27 deletions(-)
 create mode 100644 content/html/document/test/test_bug1013316.html

diff --git a/content/base/src/nsContentList.cpp b/content/base/src/nsContentList.cpp
index 346ca9c9872..a48d2a7aa82 100644
--- a/content/base/src/nsContentList.cpp
+++ b/content/base/src/nsContentList.cpp
@@ -569,6 +569,15 @@ nsContentList::GetSupportedNames(unsigned aFlags, nsTArray<nsString>& aNames)
   nsAutoTArray<nsIAtom*, 8> atoms;
   for (uint32_t i = 0; i < mElements.Length(); ++i) {
     nsIContent *content = mElements.ElementAt(i);
+    if (content->HasID()) {
+      nsIAtom* id = content->GetID();
+      MOZ_ASSERT(id != nsGkAtoms::_empty,
+                 "Empty ids don't get atomized");
+      if (!atoms.Contains(id)) {
+        atoms.AppendElement(id);
+      }
+    }
+
     nsGenericHTMLElement* el = nsGenericHTMLElement::FromContent(content);
     if (el) {
       // XXXbz should we be checking for particular tags here?  How
@@ -585,14 +594,6 @@ nsContentList::GetSupportedNames(unsigned aFlags, nsTArray<nsString>& aNames)
         }
       }
     }
-    if (content->HasID()) {
-      nsIAtom* id = content->GetID();
-      MOZ_ASSERT(id != nsGkAtoms::_empty,
-                 "Empty ids don't get atomized");
-      if (!atoms.Contains(id)) {
-        atoms.AppendElement(id);
-      }
-    }
   }
 
   aNames.SetCapacity(atoms.Length());
diff --git a/content/html/content/test/test_htmlcollection.html b/content/html/content/test/test_htmlcollection.html
index 1d90ad2e107..e5dd5bfb0e8 100644
--- a/content/html/content/test/test_htmlcollection.html
+++ b/content/html/content/test/test_htmlcollection.html
@@ -47,8 +47,8 @@ is(names[2], "2", "Entry 3")
 is(names[3], "3", "Entry 4")
 is(names[4], "x", "Entry 5")
 is(names[5], "y", "Entry 6")
-is(names[6], "z", "Entry 7")
-is(names[7], "w", "Entry 8")
+is(names[6], "w", "Entry 7")
+is(names[7], "z", "Entry 8")
 is(names[8], "something", "Entry 9")
 </script>
 </pre>
diff --git a/content/html/document/src/HTMLAllCollection.cpp b/content/html/document/src/HTMLAllCollection.cpp
index ed75ace0847..dab2589c261 100644
--- a/content/html/document/src/HTMLAllCollection.cpp
+++ b/content/html/document/src/HTMLAllCollection.cpp
@@ -67,6 +67,28 @@ HTMLAllCollection::Collection()
   return mCollection;
 }
 
+static bool
+IsAllNamedElement(nsIContent* aContent)
+{
+  nsIAtom* tag = aContent->Tag();
+  return
+    tag == nsGkAtoms::a ||
+    tag == nsGkAtoms::applet ||
+    tag == nsGkAtoms::button ||
+    tag == nsGkAtoms::embed ||
+    tag == nsGkAtoms::form ||
+    tag == nsGkAtoms::iframe ||
+    tag == nsGkAtoms::img ||
+    tag == nsGkAtoms::input ||
+    tag == nsGkAtoms::map ||
+    tag == nsGkAtoms::meta ||
+    tag == nsGkAtoms::object ||
+    tag == nsGkAtoms::select ||
+    tag == nsGkAtoms::textarea ||
+    tag == nsGkAtoms::frame ||
+    tag == nsGkAtoms::frameset;
+}
+
 static bool
 DocAllResultMatch(nsIContent* aContent, int32_t aNamespaceID, nsIAtom* aAtom,
                   void* aData)
@@ -80,20 +102,7 @@ DocAllResultMatch(nsIContent* aContent, int32_t aNamespaceID, nsIAtom* aAtom,
     return false;
   }
 
-  nsIAtom* tag = elm->Tag();
-  if (tag != nsGkAtoms::a &&
-      tag != nsGkAtoms::applet &&
-      tag != nsGkAtoms::button &&
-      tag != nsGkAtoms::embed &&
-      tag != nsGkAtoms::form &&
-      tag != nsGkAtoms::iframe &&
-      tag != nsGkAtoms::img &&
-      tag != nsGkAtoms::input &&
-      tag != nsGkAtoms::map &&
-      tag != nsGkAtoms::meta &&
-      tag != nsGkAtoms::object &&
-      tag != nsGkAtoms::select &&
-      tag != nsGkAtoms::textarea) {
+  if (!IsAllNamedElement(elm)) {
     return false;
   }
 
@@ -159,6 +168,51 @@ HTMLAllCollection::NamedGetter(const nsAString& aID,
   aResult.SetNull();
 }
 
+void
+HTMLAllCollection::GetSupportedNames(unsigned aFlags, nsTArray<nsString>& aNames)
+{
+  if (!(aFlags & JSITER_HIDDEN)) {
+    return;
+  }
+
+  // XXXbz this is very similar to nsContentList::GetSupportedNames,
+  // but has to check IsAllNamedElement for the name case.
+  nsAutoTArray<nsIAtom*, 8> atoms;
+  for (uint32_t i = 0; i < Length(); ++i) {
+    nsIContent *content = Item(i);
+    if (content->HasID()) {
+      nsIAtom* id = content->GetID();
+      MOZ_ASSERT(id != nsGkAtoms::_empty,
+                 "Empty ids don't get atomized");
+      if (!atoms.Contains(id)) {
+        atoms.AppendElement(id);
+      }
+    }
+
+    nsGenericHTMLElement* el = nsGenericHTMLElement::FromContent(content);
+    if (el) {
+      // Note: nsINode::HasName means the name is exposed on the document,
+      // which is false for options, so we don't check it here.
+      const nsAttrValue* val = el->GetParsedAttr(nsGkAtoms::name);
+      if (val && val->Type() == nsAttrValue::eAtom &&
+          IsAllNamedElement(content)) {
+        nsIAtom* name = val->GetAtomValue();
+        MOZ_ASSERT(name != nsGkAtoms::_empty,
+                   "Empty names don't get atomized");
+        if (!atoms.Contains(name)) {
+          atoms.AppendElement(name);
+        }
+      }
+    }
+  }
+
+  aNames.SetCapacity(atoms.Length());
+  for (uint32_t i = 0; i < atoms.Length(); ++i) {
+    aNames.AppendElement(nsDependentAtomString(atoms[i]));
+  }
+}
+
+
 JSObject*
 HTMLAllCollection::WrapObject(JSContext* aCx)
 {
diff --git a/content/html/document/src/HTMLAllCollection.h b/content/html/document/src/HTMLAllCollection.h
index c922700a573..e876aa419a4 100644
--- a/content/html/document/src/HTMLAllCollection.h
+++ b/content/html/document/src/HTMLAllCollection.h
@@ -64,9 +64,7 @@ public:
   void NamedGetter(const nsAString& aName,
                    bool& aFound,
                    Nullable<OwningNodeOrHTMLCollection>& aResult);
-  void GetSupportedNames(unsigned flags, nsTArray<nsString>& aNames)
-  {
-  }
+  void GetSupportedNames(unsigned aFlags, nsTArray<nsString>& aNames);
   bool NameIsEnumerable(const nsAString& aName)
   {
     return false;
diff --git a/content/html/document/test/mochitest.ini b/content/html/document/test/mochitest.ini
index c6a262a0c8d..ebcaae61372 100644
--- a/content/html/document/test/mochitest.ini
+++ b/content/html/document/test/mochitest.ini
@@ -78,3 +78,5 @@ skip-if = buildapp == 'b2g' || e10s
 [test_bug871161.html]
 skip-if = (buildapp == 'b2g' && toolkit != 'gonk') || e10s #Bug 931116, b2g desktop specific, initial triage
 support-files = file_bug871161-1.html file_bug871161-2.html
+
+[test_bug1013316.html]
diff --git a/content/html/document/test/test_bug1013316.html b/content/html/document/test/test_bug1013316.html
new file mode 100644
index 00000000000..bb7aa928fd4
--- /dev/null
+++ b/content/html/document/test/test_bug1013316.html
@@ -0,0 +1,46 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1013316
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1013316</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <script type="application/javascript">
+
+  /** Test for Bug 1013316 **/
+  SimpleTest.waitForExplicitFinish();
+   addLoadEvent(function() {
+      is(Object.keys(document.all).length, 15, "We have 15 indexed props");
+      var props = Object.getOwnPropertyNames(document.all);
+      is(props.length, 20, "Should have five names");
+      is(props[15], "display", "display first");
+      is(props[16], "content", "content second");
+      is(props[17], "bar", "bar third");
+      is(props[18], "foo", "foo fourth");
+      is(props[19], "test", "test fifth");
+
+      is(Object.keys(document.images).length, 2, "We have 2 indexed props");
+      props = Object.getOwnPropertyNames(document.images);
+      is(props.length, 5, "Should have 3 names");
+      is(props[2], "display", "display first on document.images");
+      is(props[3], "bar", "bar second on document.images");
+      is(props[4], "foo", "foo third on document.images");
+      SimpleTest.finish();
+  })
+  </script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1013316">Mozilla Bug 1013316</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  <img id="display">
+  <img name="foo" id="bar">
+  <div name="baz">
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>