Bug 891952. Named getters for HTMLCollection should never return anything for "". r=smaug
☠☠ backed out by 919fcf9baa00 ☠ ☠
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 22 May 2014 00:23:51 -0400
changeset 184366 91dad1e50118138237aac4b031dc34ccf4b25020
parent 184365 e5b1eb74bb241c648cad9ddce9f56838dfbeabc6
child 184367 c0cf2e5d7c566adcc541f99812096baadc3c3961
push id43811
push userbzbarsky@mozilla.com
push dateThu, 22 May 2014 04:24:15 +0000
treeherdermozilla-inbound@cc4e2f5d0a2c [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',