Bug 879319 - implement past names map in HTMLFormElement, r=bz

content/html/content/src/HTMLImageElement.cpp

@@ -311,7 +311,8 @@ HTMLImageElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
     GetAttr(kNameSpaceID_None, aName, tmp);
 
     if (!tmp.IsEmpty()) {
-      mForm->RemoveImageElementFromTable(this, tmp);
+      mForm->RemoveImageElementFromTable(this, tmp,
+                                         nsHTMLFormElement::AttributeUpdated);
     }
   }
 
@@ -694,11 +695,13 @@ HTMLImageElement::ClearForm(bool aRemoveFromForm)
     mForm->RemoveImageElement(this);
 
     if (!nameVal.IsEmpty()) {
-      mForm->RemoveImageElementFromTable(this, nameVal);
+      mForm->RemoveImageElementFromTable(this, nameVal,
+                                         nsHTMLFormElement::ElementRemoved);
     }
 
     if (!idVal.IsEmpty()) {
-      mForm->RemoveImageElementFromTable(this, idVal);
+      mForm->RemoveImageElementFromTable(this, idVal,
+                                         nsHTMLFormElement::ElementRemoved);
     }
   }
 

content/html/content/src/nsGenericHTMLElement.cpp

@@ -2179,11 +2179,13 @@ nsGenericHTMLFormElement::ClearForm(bool aRemoveFromForm)
     mForm->RemoveElement(this, true);
 
     if (!nameVal.IsEmpty()) {
-      mForm->RemoveElementFromTable(this, nameVal);
+      mForm->RemoveElementFromTable(this, nameVal,
+                                    nsHTMLFormElement::ElementRemoved);
     }
 
     if (!idVal.IsEmpty()) {
-      mForm->RemoveElementFromTable(this, idVal);
+      mForm->RemoveElementFromTable(this, idVal,
+                                    nsHTMLFormElement::ElementRemoved);
     }
   }
 
@@ -2314,7 +2316,8 @@ nsGenericHTMLFormElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
       GetAttr(kNameSpaceID_None, aName, tmp);
 
       if (!tmp.IsEmpty()) {
-        mForm->RemoveElementFromTable(this, tmp);
+        mForm->RemoveElementFromTable(this, tmp,
+                                      nsHTMLFormElement::AttributeUpdated);
       }
     }
 
@@ -2322,13 +2325,15 @@ nsGenericHTMLFormElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
       GetAttr(kNameSpaceID_None, nsGkAtoms::name, tmp);
 
       if (!tmp.IsEmpty()) {
-        mForm->RemoveElementFromTable(this, tmp);
+        mForm->RemoveElementFromTable(this, tmp,
+                                      nsHTMLFormElement::AttributeUpdated);
       }
 
       GetAttr(kNameSpaceID_None, nsGkAtoms::id, tmp);
 
       if (!tmp.IsEmpty()) {
-        mForm->RemoveElementFromTable(this, tmp);
+        mForm->RemoveElementFromTable(this, tmp,
+                                      nsHTMLFormElement::AttributeUpdated);
       }
 
       mForm->RemoveElement(this, false);

content/html/content/src/nsHTMLFormElement.cpp

@@ -247,6 +247,7 @@ nsHTMLFormElement::nsHTMLFormElement(already_AddRefed<nsINodeInfo> aNodeInfo)
     mEverTriedInvalidSubmit(false)
 {
   mImageNameLookupTable.Init(NS_FORM_CONTROL_LIST_HASHTABLE_SIZE);
+  mPastNameLookupTable.Init(NS_FORM_CONTROL_LIST_HASHTABLE_SIZE);
 }
 
 nsHTMLFormElement::~nsHTMLFormElement()
@@ -299,6 +300,7 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(nsHTMLFormElement,
                                                   nsGenericHTMLElement)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mControls)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mImageNameLookupTable)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPastNameLookupTable)
   tmp->mSelectedRadioButtons.EnumerateRead(ElementTraverser, &cb);
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
@@ -1440,15 +1442,15 @@ RemoveElementFromTableInternal(
   if (!aTable.Get(aName, getter_AddRefs(supports)))
     return NS_OK;
 
+  // Single element in the hash, just remove it if it's the one
+  // we're trying to remove...
+  if (supports == aChild) {
+    aTable.Remove(aName);
+    return NS_OK;
+  }
+
   nsCOMPtr<nsIContent> content(do_QueryInterface(supports));
-
   if (content) {
-    // Single element in the hash, just remove it if it's the one
-    // we're trying to remove...
-    if (content == aChild) {
-      aTable.Remove(aName);
-    }
-
     return NS_OK;
   }
 
@@ -1479,10 +1481,25 @@ RemoveElementFromTableInternal(
   return NS_OK;
 }
 
+static PLDHashOperator
+RemovePastNames(const nsAString& aName,
+                nsCOMPtr<nsISupports>& aData,
+                void* aClosure)
+{
+  return aClosure == aData ? PL_DHASH_REMOVE : PL_DHASH_NEXT;
+}
+
 nsresult
 nsHTMLFormElement::RemoveElementFromTable(nsGenericHTMLFormElement* aElement,
-                                          const nsAString& aName)
+                                          const nsAString& aName,
+                                          RemoveElementReason aRemoveReason)
 {
+  // If the element is being removed from the form, we have to remove it from
+  // the past names map.
+  if (aRemoveReason == ElementRemoved) {
+    mPastNameLookupTable.Enumerate(RemovePastNames, aElement);
+  }
+
   return mControls->RemoveElementFromTable(aElement, aName);
 }
 
@@ -1494,10 +1511,18 @@ nsHTMLFormElement::FindNamedItem(const nsAString& aName,
   if (result) {
     // FIXME Get the wrapper cache from DoResolveName.
     *aCache = nullptr;
+    AddToPastNamesMap(aName, result);
     return result.forget();
   }
 
   result = mImageNameLookupTable.GetWeak(aName);
+  if (result) {
+    *aCache = nullptr;
+    AddToPastNamesMap(aName, result);
+    return result.forget();
+  }
+
+  result = mPastNameLookupTable.GetWeak(aName);
   if (result) {
     *aCache = nullptr;
     return result.forget();
@@ -2269,6 +2294,7 @@ nsHTMLFormElement::Clear()
   }
   mImageElements.Clear();
   mImageNameLookupTable.Clear();
+  mPastNameLookupTable.Clear();
 }
 
 //----------------------------------------------------------------------
@@ -2705,8 +2731,28 @@ nsHTMLFormElement::RemoveImageElement(HTMLImageElement* aChild)
 
 nsresult
 nsHTMLFormElement::RemoveImageElementFromTable(HTMLImageElement* aElement,
-                                               const nsAString& aName)
+                                               const nsAString& aName,
+                                               RemoveElementReason aRemoveReason)
 {
+  // If the element is being removed from the form, we have to remove it from
+  // the past names map.
+  if (aRemoveReason == ElementRemoved) {
+    mPastNameLookupTable.Enumerate(RemovePastNames, aElement);
+  }
+
   return RemoveElementFromTableInternal(mImageNameLookupTable, aElement, aName);
 }
 
+void
+nsHTMLFormElement::AddToPastNamesMap(const nsAString& aName,
+                                     nsISupports* aChild)
+{
+  // If candidates contains exactly one node. Add a mapping from name to the
+  // node in candidates in the form element's past names map, replacing the
+  // previous entry with the same name, if any.
+  nsCOMPtr<nsIContent> node = do_QueryInterface(aChild);
+  if (node) {
+    mPastNameLookupTable.Put(aName, aChild);
+  }
+}
+

content/html/content/src/nsHTMLFormElement.h

@@ -149,10 +149,20 @@ public:
    *
    * @param aElement the element to remove
    * @param aName the name or id of the element to remove
+   * @param aRemoveReason describe why this element is removed. If the element
+   *        is removed because it's removed from the form, it will be removed
+   *        from the past names map too, otherwise it will stay in the past
+   *        names map.
    * @return NS_OK if the element was successfully removed.
    */
+  enum RemoveElementReason {
+    AttributeUpdated,
+    ElementRemoved
+  };
   nsresult RemoveElementFromTable(nsGenericHTMLFormElement* aElement,
-                                  const nsAString& aName);
+                                  const nsAString& aName,
+                                  RemoveElementReason aRemoveReason);
+
   /**
    * Add an element to end of this form's list of elements
    *
@@ -195,7 +205,8 @@ public:
    * @return NS_OK if the element was successfully removed.
    */
   nsresult RemoveImageElementFromTable(mozilla::dom::HTMLImageElement* aElement,
-                                      const nsAString& aName);
+                                      const nsAString& aName,
+                                      RemoveElementReason aRemoveReason);
   /**
    * Add an image element to the end of this form's list of image elements
    *
@@ -412,6 +423,9 @@ protected:
   // Clear the mImageNameLookupTable and mImageElements.
   void Clear();
 
+  // Insert a element into the past names map.
+  void AddToPastNamesMap(const nsAString& aName, nsISupports* aChild);
+
 public:
   /**
    * Flush a possible pending submission. If there was a scripted submission
@@ -480,6 +494,11 @@ protected:
 
   nsInterfaceHashtable<nsStringHashKey,nsISupports> mImageNameLookupTable;
 
+  // A map from names to elements that were gotten by those names from this
+  // form in that past.  See "past names map" in the HTML5 specification.
+
+  nsInterfaceHashtable<nsStringHashKey,nsISupports> mPastNameLookupTable;
+
   /**
    * Number of invalid and candidate for constraint validation elements in the
    * form the last time UpdateValidity has been called.

content/html/content/test/Makefile.in

@@ -358,6 +358,7 @@ MOCHITEST_FILES = \
 		wakelock.ogv \
 		test_bug869040.html \
 		test_bug870787.html \
+		test_bug879319.html \
 		allowMedia.sjs \
 		$(NULL)
 

content/html/content/test/test_bug879319.html

@@ -0,0 +1,91 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=879319
+-->
+<head>
+  <title>Test for Bug 879319</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript" src="reflect.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=879319">Mozilla Bug 879319</a>
+
+<p id="msg"></p>
+
+<form id="form">
+  <img id="img0" name="bar0" />
+</form>
+<input id="input0" name="foo0" form="form" />
+<input id="input1" name="foo1" form="form" />
+<input id="input2" name="foo2" form="form" />
+
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 879319 **/
+
+var input0 = document.getElementById("input0");
+ok(input0, "input0 exists");
+
+var form = document.getElementById("form");
+ok(form, "form exists");
+is(form.foo0, input0, "Form.foo0 should exist");
+
+ok("foo0" in form.elements, "foo0 in form.elements");
+is(input0.form, form, "input0.form is form");
+
+input0.setAttribute("name", "tmp0");
+ok("tmp0" in form.elements, "tmp0 is in form.elements");
+ok(!("foo0" in form.elements), "foo0 is not in form.elements");
+is(form.tmp0, input0, "Form.tmp0 == input0");
+is(form.foo0, input0, "Form.foo0 is still here");
+
+input0.setAttribute("name", "tmp1");
+ok("tmp1" in form.elements, "tmp1 is in form.elements");
+ok(!("tmp0" in form.elements), "tmp0 is not in form.elements");
+ok(!("foo0" in form.elements), "foo0 is not in form.elements");
+is(form.tmp0, input0, "Form.tmp0 == input0");
+is(form.tmp1, input0, "Form.tmp1 == input0");
+is(form.foo0, input0, "Form.foo0 is still here");
+
+input0.setAttribute("form", "");
+ok(!("foo0" in form.elements), "foo0 is not in form.elements");
+todo_is(form.foo0, undefined, "Form.foo0 should not still be here");
+todo_is(form.tmp0, undefined, "Form.tmp0 should not still be here");
+todo_is(form.tmp1, undefined, "Form.tmp1 should not still be here");
+
+var input1 = document.getElementById("input1");
+ok(input1, "input1 exists");
+is(form.foo1, input1, "Form.foo1 should exist");
+
+ok("foo1" in form.elements, "foo1 in form.elements");
+is(input1.form, form, "input1.form is form");
+
+input1.setAttribute("name", "foo0");
+ok("foo0" in form.elements, "foo0 is in form.elements");
+is(form.foo0, input1, "Form.foo0 should be input1");
+is(form.foo1, input1, "Form.foo1 should be input1");
+
+var input2 = document.getElementById("input2");
+ok(input2, "input2 exists");
+is(form.foo2, input2, "Form.foo2 should exist");
+input2.parentNode.removeChild(input2);
+ok(!("foo2" in form.elements), "foo2 is not in form.elements");
+todo_is(form.foo2, undefined, "Form.foo2 should not longer be there");
+
+var img0 = document.getElementById("img0");
+ok(img0, "img0 exists");
+is(form.bar0, img0, "Form.bar0 should exist");
+
+img0.setAttribute("name", "old_bar0");
+is(form.bar0, img0, "Form.bar0 is still here");
+
+img0.parentNode.removeChild(img0);
+todo_is(form.bar0, undefined, "Form.bar0 should not be here");
+
+</script>
+</pre>
+</body>
+</html>