Bug 259332: Improve support for document.all such that we can deal with multiple elements with the same id and/or name. r/sr=jst

content/base/src/nsContentList.h

@@ -85,10 +85,10 @@ public:
   NS_DECL_NSIDOMNODELIST
   NS_DECL_CYCLE_COLLECTION_CLASS(nsBaseContentList)
 
-  virtual void AppendElement(nsIContent *aContent);
-  virtual void RemoveElement(nsIContent *aContent);
+  void AppendElement(nsIContent *aContent);
+  void RemoveElement(nsIContent *aContent);
   virtual PRInt32 IndexOf(nsIContent *aContent, PRBool aDoFlush);
-  virtual void Reset();
+  void Reset();
 
   static void Shutdown();
 

content/html/document/src/nsHTMLDocument.cpp

@@ -250,6 +250,7 @@ public:
 
   nsCOMPtr<nsIAtom> mKey;
   nsBaseContentList *mNameContentList;
+  nsRefPtr<nsContentList> mDocAllList;
 private:
   nsSmallVoidArray mIdContentList;
 };
@@ -354,6 +355,8 @@ IdAndNameMapEntryTraverse(PLDHashTable *table, PLDHashEntryHdr *hdr,
   if (entry->mNameContentList != NAME_NOT_VALID)
     cb->NoteXPCOMChild(entry->mNameContentList);
 
+  cb->NoteXPCOMChild(entry->mDocAllList);
+
   return PL_DHASH_NEXT;
 }
 
@@ -3787,6 +3790,82 @@ nsHTMLDocument::ChangeContentEditableCount(nsIContent *aElement,
   return NS_OK;
 }
 
+static PRBool
+DocAllResultMatch(nsIContent* aContent, PRInt32 aNamespaceID, nsIAtom* aAtom,
+                  void* aData)
+{
+  if (aContent->GetID() == aAtom) {
+    return PR_TRUE;
+  }
+
+  nsGenericHTMLElement* elm = nsGenericHTMLElement::FromContent(aContent);
+  if (!elm || aContent->GetNameSpaceID() != kNameSpaceID_None) {
+    return PR_FALSE;
+  }
+
+  nsIAtom* tag = elm->Tag();
+  if (tag != nsGkAtoms::img    &&
+      tag != nsGkAtoms::form   &&
+      tag != nsGkAtoms::applet &&
+      tag != nsGkAtoms::embed  &&
+      tag != nsGkAtoms::object &&
+      tag != nsGkAtoms::input) {
+    return PR_FALSE;
+  }
+
+  const nsAttrValue* val = elm->GetParsedAttr(nsGkAtoms::name);
+  return val && val->Type() == nsAttrValue::eAtom &&
+         val->GetAtomValue() == aAtom;
+}
+
+
+nsresult
+nsHTMLDocument::GetDocumentAllResult(const nsAString& aID, nsISupports** aResult)
+{
+  *aResult = nsnull;
+
+  PLDHashOperator op = IdTableIsLive() ? PL_DHASH_LOOKUP : PL_DHASH_ADD;
+
+  nsCOMPtr<nsIAtom> id = do_GetAtom(aID);
+  IdAndNameMapEntry *entry =
+    static_cast<IdAndNameMapEntry *>
+               (PL_DHashTableOperate(&mIdAndNameHashTable, id, op));
+  NS_ENSURE_TRUE(entry, NS_ERROR_OUT_OF_MEMORY);
+
+  // If we did a lookup and it failed, there are no items with this id
+  if (PL_DHASH_ENTRY_IS_FREE(entry)) {
+    NS_ASSERTION(IdTableIsLive(), "should have gotten a busy entry");
+
+    return NS_OK;
+  }
+
+  if (!mRootContent) {
+    return NS_OK;
+  }
+
+  if (!entry->mDocAllList) {
+    entry->mDocAllList = new nsContentList(mRootContent, DocAllResultMatch,
+                                           nsnull, nsnull, PR_TRUE, id);
+    NS_ENSURE_TRUE(entry->mDocAllList, NS_ERROR_OUT_OF_MEMORY);
+  }
+
+  // Check if there are more than 1 entries. Do this by getting the second one
+  // rather than the length since getting the length always requires walking
+  // the entire document.
+
+  nsIContent* cont = entry->mDocAllList->Item(1, PR_TRUE);
+  if (cont) {
+    NS_ADDREF(*aResult = static_cast<nsIDOMNodeList*>(entry->mDocAllList));
+
+    return NS_OK;
+  }
+
+  // There's only 0 or 1 items. Return the first one or null.
+  NS_IF_ADDREF(*aResult = entry->mDocAllList->Item(0, PR_TRUE));
+
+  return NS_OK;
+}
+
 static void
 NotifyEditableStateChange(nsINode *aNode, nsIDocument *aDocument,
                           PRBool aEditable)

content/html/document/src/nsHTMLDocument.h

@@ -171,6 +171,8 @@ public:
   NS_IMETHOD Writeln(const nsAString & text);
   NS_IMETHOD GetElementsByName(const nsAString & elementName,
                                nsIDOMNodeList **_retval);
+  virtual nsresult GetDocumentAllResult(const nsAString& aID,
+                                        nsISupports** aResult);
 
   // nsIDOMNSHTMLDocument interface
   NS_DECL_NSIDOMNSHTMLDOCUMENT

content/html/document/src/nsIHTMLDocument.h

@@ -144,6 +144,14 @@ public:
    * Returns whether the document is editable.
    */
   virtual PRBool IsEditingOn() = 0;
+
+  /**
+   * Returns the result of document.all[aID] which can either be a node
+   * or a nodelist depending on if there are multiple nodes with the same
+   * id.
+   */
+  virtual nsresult GetDocumentAllResult(const nsAString& aID,
+                                        nsISupports** aResult) = 0;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsIHTMLDocument, NS_IHTMLDOCUMENT_IID)

content/html/document/test/Makefile.in

@@ -54,6 +54,7 @@ _TEST_FILES = 	test_bug1682.html \
 		test_bug359657.html \
 		test_bug380383.html \
 		test_bug386495.html \
+		test_bug259332.html \
 		$(NULL)
 
 libs:: $(_TEST_FILES)

content/html/document/test/test_bug259332.html

@@ -0,0 +1,65 @@
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=259332
+-->
+<head>
+  <title>Test for Bug 259332</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>        
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=259332">Mozilla Bug 259332</a>
+<p id="display"></p>
+<div id="content">
+  <div id="a">a
+    <div id="a">a</div>
+    <input name="a" value="a">
+    <div id="b">b</div>
+    <input name="b" value="b">
+    <div id="c">c</div>
+  </div>
+  <input name="write">
+  <input name="write">
+  <input id="write">
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+/** Test for Bug 259332 **/
+
+list = document.all.a;
+ok(list.length == 3, "initial a length");
+
+blist = document.all.b;
+ok(document.all.b.length == 2, "initial b length");
+document.getElementById('b').id = 'a';
+ok(document.all.b.nodeName == "INPUT", "just one b");
+
+ok(blist.length == 1, "just one b");
+ok(list.length == 4, "one more a");
+
+newDiv = document.createElement('div');
+newDiv.id = 'a';
+newDiv.innerHTML = 'a';
+list[0].appendChild(newDiv);
+ok(list.length == 5, "two more a");
+
+ok(document.all.c.textContent == 'c', "one c");
+document.all.c.id = 'a';
+ok(!document.all.c, "no c");
+ok(list.length == 6, "three more a");
+
+ok(document.all.write.length == 3, "name is write");
+
+newDiv = document.createElement('div');
+newDiv.id = 'd';
+newDiv.innerHTML = 'd';
+list[0].appendChild(newDiv);
+ok(document.all.d.textContent == 'd', "new d");
+
+
+</script>
+</pre>
+</body>
+</html>

dom/src/base/nsDOMClassInfo.cpp

@@ -7876,44 +7876,12 @@ nsHTMLDocumentSH::DocumentAllGetProperty(JSContext *cx, JSObject *obj,
 
       nsDependentJSString str(id);
 
-      nsCOMPtr<nsIDOMElement> element;
-      domdoc->GetElementById(str, getter_AddRefs(element));
+      rv = doc->GetDocumentAllResult(str, getter_AddRefs(result));
 
-      result = element;
+      if (NS_FAILED(rv)) {
+        nsDOMClassInfo::ThrowJSException(cx, rv);
 
-      if (!result) {
-        doc->ResolveName(str, nsnull, getter_AddRefs(result));
-      }
-
-      if (!result) {
-        nsCOMPtr<nsIDOMNodeList> nodeList;
-        rv = domdoc->GetElementsByName(str, getter_AddRefs(nodeList));
-
-        if (nodeList) {
-          // Get the second node in the list, if found, we know
-          // there's more than one node (this is cheaper than getting
-          // the length of the collection since that requires walking
-          // the whole DOM tree in all cases, all we care about is if
-          // there's more than one item in the collection, or if
-          // there's only one, or no items at all).
-
-          nsCOMPtr<nsIDOMNode> node;
-          rv |= nodeList->Item(1, getter_AddRefs(node));
-
-          if (node) {
-            result = nodeList;
-          } else {
-            rv |= nodeList->Item(0, getter_AddRefs(node));
-
-            result = node;
-          }
-        }
-
-        if (NS_FAILED(rv)) {
-          nsDOMClassInfo::ThrowJSException(cx, rv);
-
-          return JS_FALSE;
-        }
+        return JS_FALSE;
       }
     }
   } else if (JSVAL_TO_INT(id) >= 0) {
@@ -7940,6 +7908,8 @@ nsHTMLDocumentSH::DocumentAllGetProperty(JSContext *cx, JSObject *obj,
 
       return JS_FALSE;
     }
+  } else {
+    *vp = JSVAL_VOID;
   }
 
   return JS_TRUE;