Bug 529819. Don't double-add a node to form.elements['foo']. r=smaug
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 19 Nov 2009 15:58:46 -0500
changeset 33073 71678ba20f932e9496cb10494c3bba2ff1d31486
parent 33072 9d93308bf08fed496e128c40cb175df2ec9484cd
child 33074 e0af8962ea2956e7faec08edec2495ab9757aea7
push id751
push userbzbarsky@mozilla.com
push dateThu, 19 Nov 2009 21:14:10 +0000
reviewerssmaug
bugs529819
milestone1.9.2b4pre
Bug 529819. Don't double-add a node to form.elements['foo']. r=smaug
content/html/content/src/nsHTMLFormElement.cpp
content/html/content/test/Makefile.in
content/html/content/test/test_bug529819.html
--- a/content/html/content/src/nsHTMLFormElement.cpp
+++ b/content/html/content/src/nsHTMLFormElement.cpp
@@ -2274,37 +2274,47 @@ nsFormControlList::AddElementToTable(nsI
       NS_ENSURE_TRUE(nodeList, NS_ERROR_FAILURE);
 
       // Upcast, uggly, but it works!
       nsBaseContentList *list = static_cast<nsBaseContentList *>
                                            ((nsIDOMNodeList *)nodeList.get());
 
       NS_ASSERTION(list->Length() > 1,
                    "List should have been converted back to a single element");
-      
+
+      // Fast-path appends; this check is ok even if the child is
+      // already in the list, since if it tests true the child would
+      // have come at the end of the list, and the PositionIsBefore
+      // will test false.
       if(nsContentUtils::PositionIsBefore(list->GetNodeAt(list->Length() - 1), newChild)) {
-          list->AppendElement(newChild);
-          return NS_OK;
+        list->AppendElement(newChild);
+        return NS_OK;
+      }
+
+      // If a control has a name equal to its id, it could be in the
+      // list already.
+      if (list->IndexOf(newChild) != -1) {
+        return NS_OK;
       }
       
       // first is the first possible insertion index, last is the last possible
       // insertion index
       PRUint32 first = 0;
       PRUint32 last = list->Length() - 1;
       PRUint32 mid;
       
-      //Stop when there is only one index in our range
+      // Stop when there is only one index in our range
       while (last != first) {
-          mid = (first + last) / 2;
+        mid = (first + last) / 2;
           
-          if (nsContentUtils::PositionIsBefore(newChild, list->GetNodeAt(mid)))
-            last = mid;
-          else
-            first = mid + 1;
-        }
+        if (nsContentUtils::PositionIsBefore(newChild, list->GetNodeAt(mid)))
+          last = mid;
+        else
+          first = mid + 1;
+      }
 
       list->InsertElementAt(newChild, first);
     }
   }
 
   return NS_OK;
 }
 
--- a/content/html/content/test/Makefile.in
+++ b/content/html/content/test/Makefile.in
@@ -132,12 +132,13 @@ include $(topsrcdir)/config/rules.mk
 		347174transformable.xml \
 		347174transform.xsl \
 		test_bug481335.xhtml \
 		test_bug514856.html \
 		bug514856_iframe.html \
 		test_bug519987.html \
 		test_bug523771.html \
 		form_submit_server.sjs \
+		test_bug529819.html \
 		$(NULL)
 
 libs:: $(_TEST_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/content/html/content/test/test_bug529819.html
@@ -0,0 +1,33 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=529819
+-->
+<head>
+  <title>Test for Bug 529819</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/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=529819">Mozilla Bug 529819</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+<form id="form">
+  <input name="foo" id="foo">
+  <input name="bar" type="radio">
+  <input name="bar" id="bar" type="radio">
+</form>
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 529819 **/
+is($("form").elements["foo"] instanceof HTMLInputElement, true, "Should have an element here");
+is($("form").elements["bar"] instanceof HTMLInputElement, false, "Should have a list here");
+is($("form").elements["bar"].length, 2, "Should have a list with two elements here");
+
+</script>
+</pre>
+</body>
+</html>