Bug 891952. Named getters for HTMLCollection should never return anything for "". r=smaug
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 22 May 2014 00:23:51 -0400
changeset 184381 582bd1b5934c7be3ef3d19ced967b2a3bce79463
parent 184380 6c25b11dbef015ac0ee446ff3bf2aed4bef31676
child 184382 dfd0c53bdf28cd37f0d9a3a2a5dbc02eac93bfd9
child 184421 77c433deac22c632e246614dae1c5c5637293cd4
push id43820
push userMs2ger@gmail.com
push dateThu, 22 May 2014 07:44:59 +0000
treeherdermozilla-inbound@582bd1b5934c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs891952
milestone32.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 891952. Named getters for HTMLCollection should never return anything for "". r=smaug
content/base/src/nsContentList.cpp
content/base/test/mochitest.ini
content/base/test/test_bug891952.html
content/html/document/src/HTMLAllCollection.cpp
dom/imptests/failures/html/dom/collections/mochitest.ini
dom/imptests/failures/html/dom/collections/test_HTMLCollection-empty-name.html.json
dom/imptests/failures/html/html/dom/documents/dta/mochitest.ini
dom/imptests/failures/html/html/dom/documents/dta/test_document.images.html.json
dom/imptests/moz.build
--- a/content/base/src/nsContentList.cpp
+++ b/content/base/src/nsContentList.cpp
@@ -525,16 +525,20 @@ nsContentList::Item(uint32_t aIndex, boo
                "PopulateSelf left the list in a dirty (useless) state!");
 
   return mElements.SafeElementAt(aIndex);
 }
 
 Element*
 nsContentList::NamedItem(const nsAString& aName, bool aDoFlush)
 {
+  if (aName.IsEmpty()) {
+    return nullptr;
+  }
+
   BringSelfUpToDate(aDoFlush);
 
   uint32_t i, count = mElements.Length();
 
   // Typically IDs and names are atomized
   nsCOMPtr<nsIAtom> name = do_GetAtom(aName);
   NS_ENSURE_TRUE(name, nullptr);
 
@@ -569,23 +573,27 @@ nsContentList::GetSupportedNames(unsigne
     if (el) {
       // XXXbz should we be checking for particular tags here?  How
       // stable is this part of the spec?
       // 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) {
         nsIAtom* name = val->GetAtomValue();
+        MOZ_ASSERT(name != nsGkAtoms::_empty,
+                   "Empty names don't get atomized");
         if (!atoms.Contains(name)) {
           atoms.AppendElement(name);
         }
       }
     }
     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());
   for (uint32_t i = 0; i < atoms.Length(); ++i) {
--- a/content/base/test/mochitest.ini
+++ b/content/base/test/mochitest.ini
@@ -540,16 +540,17 @@ skip-if = (buildapp == 'b2g' && toolkit 
 skip-if = buildapp == 'b2g' || toolkit == 'android' || e10s #needs plugin support # b2g(needs plugin support) b2g-debug(debug-only failure) b2g-desktop(needs plugin support)
 [test_bug840098.html]
 [test_bug868999.html]
 [test_bug869000.html]
 [test_bug869002.html]
 [test_bug869006.html]
 [test_bug876282.html]
 [test_bug890580.html]
+[test_bug891952.html]
 [test_bug894874.html]
 [test_bug895239.html]
 [test_bug895974.html]
 [test_bug902847.html]
 [test_bug907892.html]
 [test_bug922681.html]
 [test_bug927196.html]
 [test_bug945152.html]
new file mode 100644
--- /dev/null
+++ b/content/base/test/test_bug891952.html
@@ -0,0 +1,61 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=891952
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 891952</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 891952 **/
+    SimpleTest.waitForExplicitFinish();
+    addLoadEvent(function() {
+      var all = document.all;
+      is(all["content"], $("content"), "Should find the content");
+      ok(!("" in all), "Should not have an empty string prop on document.all");
+      ise(all[""], undefined, "Should not get empty string on document.all");
+      ise(all.namedItem(""), null,
+          "namedItem for empty string should return null on document.all");
+
+      var divs = document.getElementsByTagName("div");
+      ok(!("" in divs), "Should not have an empty string prop on getElementsByTagName");
+      ise(divs[""], undefined, "Should not get empty string on getElementsByTagName");
+      ise(divs.namedItem(""), null,
+          "namedItem for empty string should return null on getElementsByTagName");
+      var forms = document.forms;
+      ok(!("" in forms), "Should not have an empty string prop on document.forms");
+      ise(forms[""], undefined, "Should not get empty string on document.forms");
+      ise(forms.namedItem(""), null,
+          "namedItem for empty string should return null on document.forms");
+
+      var form = $("form");
+      ok(!("" in form), "Should not have an empty string prop on form");
+      ise(form[""], undefined, "Should not get empty string on form");
+
+      var formEls = $("form").elements;
+      ok(!("" in formEls), "Should not have an empty string prop on form.elements");
+      ise(formEls[""], undefined, "Should not get empty string on form.elements");
+      ise(formEls.namedItem(""), null,
+          "namedItem for empty string should return null on form.elements");
+      SimpleTest.finish();
+    });
+
+  </script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=891952">Mozilla Bug 891952</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  <div id=""></div>
+  <div name=""></div>
+  <form id="form" name="">
+    <input name="">
+  </form>
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>
--- a/content/html/document/src/HTMLAllCollection.cpp
+++ b/content/html/document/src/HTMLAllCollection.cpp
@@ -121,16 +121,22 @@ HTMLAllCollection::GetDocumentAllList(co
   return docAllList;
 }
 
 void
 HTMLAllCollection::NamedGetter(const nsAString& aID,
                                bool& aFound,
                                Nullable<OwningNodeOrHTMLCollection>& aResult)
 {
+  if (aID.IsEmpty()) {
+    aFound = false;
+    aResult.SetNull();
+    return;
+  }
+
   nsContentList* docAllList = GetDocumentAllList(aID);
   if (!docAllList) {
     aFound = false;
     aResult.SetNull();
     return;
   }
 
   // Check if there are more than 1 entries. Do this by getting the second one
deleted file mode 100644
--- a/dom/imptests/failures/html/dom/collections/mochitest.ini
+++ /dev/null
@@ -1,6 +0,0 @@
-# THIS FILE IS AUTOGENERATED BY parseFailures.py - DO NOT EDIT
-[DEFAULT]
-support-files =
-
-
-[test_HTMLCollection-empty-name.html.json]
deleted file mode 100644
--- a/dom/imptests/failures/html/dom/collections/test_HTMLCollection-empty-name.html.json
+++ /dev/null
@@ -1,9 +0,0 @@
-{
-  "Empty string as a name for Document.getElementsByTagName": true,
-  "Empty string as a name for Element.getElementsByTagName": true,
-  "Empty string as a name for Document.getElementsByTagNameNS": true,
-  "Empty string as a name for Element.getElementsByTagNameNS": true,
-  "Empty string as a name for Document.getElementsByClassName": true,
-  "Empty string as a name for Element.getElementsByClassName": true,
-  "Empty string as a name for Element.children": true
-}
--- a/dom/imptests/failures/html/html/dom/documents/dta/mochitest.ini
+++ b/dom/imptests/failures/html/html/dom/documents/dta/mochitest.ini
@@ -1,14 +1,13 @@
 # THIS FILE IS AUTOGENERATED BY parseFailures.py - DO NOT EDIT
 [DEFAULT]
 support-files =
 
 
-[test_document.images.html.json]
 [test_document.title-03.html.json]
 [test_document.title-04.xhtml.json]
 [test_document.title-06.html.json]
 [test_document.title-07.html.json]
 [test_nameditem-02.html.json]
 [test_nameditem-03.html.json]
 [test_nameditem-04.html.json]
 [test_nameditem-05.html.json]
deleted file mode 100644
--- a/dom/imptests/failures/html/html/dom/documents/dta/test_document.images.html.json
+++ /dev/null
@@ -1,3 +0,0 @@
-{
-  "The empty string should not be in the collections": true
-}
--- a/dom/imptests/moz.build
+++ b/dom/imptests/moz.build
@@ -9,17 +9,16 @@ MOCHITEST_MANIFESTS += [
     'html/mochitest.ini',
     'mochitest.ini',
     'webapps/mochitest.ini',
 ]
 
 MOCHITEST_MANIFESTS += [
     'failures/editing/conformancetest/mochitest.ini',
     'failures/editing/selecttest/mochitest.ini',
-    'failures/html/dom/collections/mochitest.ini',
     'failures/html/dom/errors/mochitest.ini',
     'failures/html/dom/lists/mochitest.ini',
     'failures/html/dom/mochitest.ini',
     'failures/html/dom/nodes/mochitest.ini',
     'failures/html/html/browsers/the-window-object/mochitest.ini',
     'failures/html/html/browsers/the-window-object/named-access-on-the-window-object/mochitest.ini',
     'failures/html/html/dom/documents/dta/doc.gEBN/mochitest.ini',
     'failures/html/html/dom/documents/dta/mochitest.ini',