Bug 870787 - Improve named getter for form, r=bz

content/base/public/nsContentUtils.h

@@ -510,17 +510,6 @@ public:
   static nsresult GuessCharset(const char *aData, uint32_t aDataLen,
                                nsACString &aCharset);
 
-  /**
-   * Determine whether aContent is in some way associated with aForm.  If the
-   * form is a container the only elements that are considered to be associated
-   * with a form are the elements that are contained within the form. If the
-   * form is a leaf element then all elements will be accepted into this list,
-   * since this can happen due to content fixup when a form spans table rows or
-   * table cells.
-   */
-  static bool BelongsInForm(nsIContent *aForm,
-                              nsIContent *aContent);
-
   static nsresult CheckQName(const nsAString& aQualifiedName,
                              bool aNamespaceAware = true,
                              const PRUnichar** aColon = nullptr);

content/base/src/nsContentList.cpp

@@ -140,26 +140,6 @@ nsSimpleContentList::WrapObject(JSContext *cx, JS::Handle<JSObject*> scope)
   return NodeListBinding::Wrap(cx, scope, this);
 }
 
-// nsFormContentList
-
-nsFormContentList::nsFormContentList(nsIContent *aForm,
-                                     nsBaseContentList& aContentList)
-  : nsSimpleContentList(aForm)
-{
-
-  // move elements that belong to mForm into this content list
-
-  uint32_t i, length = 0;
-  aContentList.GetLength(&length);
-
-  for (i = 0; i < length; i++) {
-    nsIContent *c = aContentList.Item(i);
-    if (c && nsContentUtils::BelongsInForm(aForm, c)) {
-      AppendElement(c);
-    }
-  }
-}
-
 // Hashtable for storing nsContentLists
 static PLDHashTable gContentListHashTable;
 

content/base/src/nsContentList.h

@@ -124,16 +124,6 @@ private:
   nsCOMPtr<nsINode> mRoot;
 };
 
-// This class is used only by form element code and this is a static
-// list of elements. NOTE! This list holds strong references to
-// the elements in the list.
-class nsFormContentList : public nsSimpleContentList
-{
-public:
-  nsFormContentList(nsIContent *aForm,
-                    nsBaseContentList& aContentList);
-};
-
 /**
  * Class that's used as the key to hash nsContentList implementations
  * for fast retrieval

content/base/src/nsContentUtils.cpp

@@ -2365,62 +2365,6 @@ nsContentUtils::NewURIWithDocumentCharset(nsIURI** aResult,
                    aBaseURI, sIOService);
 }
 
-// static
-bool
-nsContentUtils::BelongsInForm(nsIContent *aForm,
-                              nsIContent *aContent)
-{
-  NS_PRECONDITION(aForm, "Must have a form");
-  NS_PRECONDITION(aContent, "Must have a content node");
-
-  if (aForm == aContent) {
-    // A form does not belong inside itself, so we return false here
-
-    return false;
-  }
-
-  nsIContent* content = aContent->GetParent();
-
-  while (content) {
-    if (content == aForm) {
-      // aContent is contained within the form so we return true.
-
-      return true;
-    }
-
-    if (content->Tag() == nsGkAtoms::form &&
-        content->IsHTML()) {
-      // The child is contained within a form, but not the right form
-      // so we ignore it.
-
-      return false;
-    }
-
-    content = content->GetParent();
-  }
-
-  if (aForm->GetChildCount() > 0) {
-    // The form is a container but aContent wasn't inside the form,
-    // return false
-
-    return false;
-  }
-
-  // The form is a leaf and aContent wasn't inside any other form so
-  // we check whether the content comes after the form.  If it does,
-  // return true.  If it does not, then it couldn't have been inside
-  // the form in the HTML.
-  if (PositionIsBefore(aForm, aContent)) {
-    // We could be in this form!
-    // In the future, we may want to get document.forms, look at the
-    // form after aForm, and if aContent is after that form after
-    // aForm return false here....
-    return true;
-  }
-
-  return false;
-}
-
 // static
 nsresult
 nsContentUtils::CheckQName(const nsAString& aQualifiedName,

content/base/src/nsNodeUtils.cpp

@@ -25,6 +25,7 @@
 #endif
 #include "nsBindingManager.h"
 #include "nsGenericHTMLElement.h"
+#include "mozilla/dom/HTMLImageElement.h"
 #include "mozilla/dom/HTMLMediaElement.h"
 #include "nsWrapperCacheInlines.h"
 #include "nsObjectLoadingContent.h"
@@ -216,6 +217,11 @@ nsNodeUtils::LastRelease(nsINode* aNode)
       // notify, since we're being destroyed in any case.
       static_cast<nsGenericHTMLFormElement*>(aNode)->ClearForm(true);
     }
+
+    if (aNode->IsElement() && aNode->AsElement()->IsHTML(nsGkAtoms::img)) {
+      HTMLImageElement* imageElem = static_cast<HTMLImageElement*>(aNode);
+      imageElem->ClearForm(true);
+    }
   }
   aNode->UnsetFlags(NODE_HAS_PROPERTIES);
 

content/html/content/src/HTMLImageElement.cpp

@@ -25,6 +25,7 @@
 #include "nsContentPolicyUtils.h"
 #include "nsIDOMWindow.h"
 #include "nsFocusManager.h"
+#include "nsHTMLFormElement.h"
 
 #include "imgIContainer.h"
 #include "imgILoader.h"
@@ -67,6 +68,7 @@ namespace dom {
 
 HTMLImageElement::HTMLImageElement(already_AddRefed<nsINodeInfo> aNodeInfo)
   : nsGenericHTMLElement(aNodeInfo)
+  , mForm(nullptr)
 {
   // We start out broken
   AddStatesSilently(NS_EVENT_STATE_BROKEN);
@@ -84,7 +86,7 @@ NS_IMPL_RELEASE_INHERITED(HTMLImageElement, Element)
 
 
 // QueryInterface implementation for HTMLImageElement
-NS_INTERFACE_TABLE_HEAD(HTMLImageElement)
+NS_INTERFACE_TABLE_HEAD_CYCLE_COLLECTION_INHERITED(HTMLImageElement)
   NS_HTML_CONTENT_INTERFACES(nsGenericHTMLElement)
   NS_INTERFACE_TABLE_INHERITED4(HTMLImageElement,
                                 nsIDOMHTMLImageElement,
@@ -296,6 +298,46 @@ HTMLImageElement::GetAttributeMappingFunction() const
 }
 
 
+nsresult
+HTMLImageElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
+                                const nsAttrValueOrString* aValue,
+                                bool aNotify)
+{
+
+  if (aNameSpaceID == kNameSpaceID_None && mForm &&
+      (aName == nsGkAtoms::name || aName == nsGkAtoms::id)) {
+    // remove the image from the hashtable as needed
+    nsAutoString tmp;
+    GetAttr(kNameSpaceID_None, aName, tmp);
+
+    if (!tmp.IsEmpty()) {
+      mForm->RemoveImageElementFromTable(this, tmp);
+    }
+  }
+
+  return nsGenericHTMLElement::BeforeSetAttr(aNameSpaceID, aName,
+                                             aValue, aNotify);
+}
+
+nsresult
+HTMLImageElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
+                               const nsAttrValue* aValue, bool aNotify)
+{
+  if (aNameSpaceID == kNameSpaceID_None && mForm &&
+      (aName == nsGkAtoms::name || aName == nsGkAtoms::id) &&
+      aValue && !aValue->IsEmptyString()) {
+    // add the image to the hashtable as needed
+    NS_ABORT_IF_FALSE(aValue->Type() == nsAttrValue::eAtom,
+      "Expected atom value for name/id");
+    mForm->AddImageElementToTable(this,
+      nsDependentAtomString(aValue->GetAtomValue()));
+  }
+
+  return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName,
+                                            aValue, aNotify);
+}
+
+
 nsresult
 HTMLImageElement::PreHandleEvent(nsEventChainPreVisitor& aVisitor)
 {
@@ -414,6 +456,10 @@ HTMLImageElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent,
   nsImageLoadingContent::BindToTree(aDocument, aParent, aBindingParent,
                                     aCompileEventHandlers);
 
+  if (aParent) {
+    UpdateFormOwner();
+  }
+
   if (HasAttr(kNameSpaceID_None, nsGkAtoms::src)) {
     // FIXME: Bug 660963 it would be nice if we could just have
     // ClearBrokenState update our state and do it fast...
@@ -434,10 +480,45 @@ HTMLImageElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent,
 void
 HTMLImageElement::UnbindFromTree(bool aDeep, bool aNullParent)
 {
+  if (mForm) {
+    if (aNullParent || !FindAncestorForm(mForm)) {
+      ClearForm(true);
+    } else {
+      UnsetFlags(MAYBE_ORPHAN_FORM_ELEMENT);
+    }
+  }
+
   nsImageLoadingContent::UnbindFromTree(aDeep, aNullParent);
   nsGenericHTMLElement::UnbindFromTree(aDeep, aNullParent);
 }
 
+void
+HTMLImageElement::UpdateFormOwner()
+{
+  if (!mForm) {
+    mForm = FindAncestorForm();
+  }
+
+  if (mForm && !HasFlag(ADDED_TO_FORM)) {
+    // Now we need to add ourselves to the form
+    nsAutoString nameVal, idVal;
+    GetAttr(kNameSpaceID_None, nsGkAtoms::name, nameVal);
+    GetAttr(kNameSpaceID_None, nsGkAtoms::id, idVal);
+
+    SetFlags(ADDED_TO_FORM);
+
+    mForm->AddImageElement(this);
+
+    if (!nameVal.IsEmpty()) {
+      mForm->AddImageElementToTable(this, nameVal);
+    }
+
+    if (!idVal.IsEmpty()) {
+      mForm->AddImageElementToTable(this, idVal);
+    }
+  }
+}
+
 void
 HTMLImageElement::MaybeLoadImage()
 {
@@ -577,5 +658,53 @@ HTMLImageElement::WrapNode(JSContext* aCx, JS::Handle<JSObject*> aScope)
   return HTMLImageElementBinding::Wrap(aCx, aScope, this);
 }
 
+#ifdef DEBUG
+nsIDOMHTMLFormElement*
+HTMLImageElement::GetForm() const
+{
+  return mForm;
+}
+#endif
+
+void
+HTMLImageElement::SetForm(nsIDOMHTMLFormElement* aForm)
+{
+  NS_PRECONDITION(aForm, "Don't pass null here");
+  NS_ASSERTION(!mForm,
+               "We don't support switching from one non-null form to another.");
+
+  mForm = static_cast<nsHTMLFormElement*>(aForm);
+}
+
+void
+HTMLImageElement::ClearForm(bool aRemoveFromForm)
+{
+  NS_ASSERTION((mForm != nullptr) == HasFlag(ADDED_TO_FORM),
+               "Form control should have had flag set correctly");
+
+  if (!mForm) {
+    return;
+  }
+
+  if (aRemoveFromForm) {
+    nsAutoString nameVal, idVal;
+    GetAttr(kNameSpaceID_None, nsGkAtoms::name, nameVal);
+    GetAttr(kNameSpaceID_None, nsGkAtoms::id, idVal);
+
+    mForm->RemoveImageElement(this);
+
+    if (!nameVal.IsEmpty()) {
+      mForm->RemoveImageElementFromTable(this, nameVal);
+    }
+
+    if (!idVal.IsEmpty()) {
+      mForm->RemoveImageElementFromTable(this, idVal);
+    }
+  }
+
+  UnsetFlags(ADDED_TO_FORM);
+  mForm = nullptr;
+}
+
 } // namespace dom
 } // namespace mozilla

content/html/content/src/HTMLImageElement.h

@@ -174,12 +174,30 @@ public:
     SetHTMLAttr(nsGkAtoms::lowsrc, aLowsrc, aError);
   }
 
+#ifdef DEBUG
+  nsIDOMHTMLFormElement* GetForm() const;
+#endif
+  void SetForm(nsIDOMHTMLFormElement* aForm);
+  void ClearForm(bool aRemoveFromForm);
+
 protected:
   CSSIntPoint GetXY();
   virtual void GetItemValueText(nsAString& text) MOZ_OVERRIDE;
   virtual void SetItemValueText(const nsAString& text) MOZ_OVERRIDE;
   virtual JSObject* WrapNode(JSContext *aCx,
                              JS::Handle<JSObject*> aScope) MOZ_OVERRIDE;
+  void UpdateFormOwner();
+
+  virtual nsresult BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
+                                 const nsAttrValueOrString* aValue,
+                                 bool aNotify) MOZ_OVERRIDE;
+
+  virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
+                                const nsAttrValue* aValue, bool aNotify) MOZ_OVERRIDE;
+
+  // This is a weak reference that this element and the HTMLFormElement
+  // cooperate in maintaining.
+  nsHTMLFormElement* mForm;
 };
 
 } // namespace dom

content/html/content/src/nsGenericHTMLElement.h

@@ -1047,15 +1047,16 @@ class HTMLFieldSetElement;
 
 // Form element specific bits
 enum {
-  // If this flag is set on an nsGenericHTMLFormElement, that means that we have
+  // If this flag is set on an nsGenericHTMLFormElement or an HTMLImageElement,
-  // added ourselves to our mForm.  It's possible to have a non-null mForm, but
+  // that means that we have added ourselves to our mForm.  It's possible to
-  // not have this flag set.  That happens when the form is set via the content
+  // have a non-null mForm, but not have this flag set.  That happens when the
-  // sink.
+  // form is set via the content sink.
   ADDED_TO_FORM =                         FORM_ELEMENT_FLAG_BIT(0),
 
-  // If this flag is set on an nsGenericHTMLFormElement, that means that its form
+  // If this flag is set on an nsGenericHTMLFormElement or an HTMLImageElement,
-  // is in the process of being unbound from the tree, and this form element
+  // that means that its form is in the process of being unbound from the tree,
-  // hasn't re-found its form in nsGenericHTMLFormElement::UnbindFromTree yet.
+  // and this form element hasn't re-found its form in
+  // nsGenericHTMLFormElement::UnbindFromTree yet.
   MAYBE_ORPHAN_FORM_ELEMENT =             FORM_ELEMENT_FLAG_BIT(1)
 };
 

content/html/content/src/nsHTMLFormElement.cpp

@@ -54,6 +54,9 @@
 #include "mozilla/dom/BindingUtils.h"
 #include "nsSandboxFlags.h"
 
+// images
+#include "mozilla/dom/HTMLImageElement.h"
+
 using namespace mozilla::dom;
 
 static const int NS_FORM_CONTROL_LIST_HASHTABLE_SIZE = 16;
@@ -104,6 +107,8 @@ public:
 
   nsresult AddElementToTable(nsGenericHTMLFormElement* aChild,
                              const nsAString& aName);
+  nsresult AddImageElementToTable(HTMLImageElement* aChild,
+                                  const nsAString& aName);
   nsresult RemoveElementFromTable(nsGenericHTMLFormElement* aChild,
                                   const nsAString& aName);
   nsresult IndexOfControl(nsIFormControl* aControl,
@@ -241,6 +246,7 @@ nsHTMLFormElement::nsHTMLFormElement(already_AddRefed<nsINodeInfo> aNodeInfo)
     mInvalidElementsCount(0),
     mEverTriedInvalidSubmit(false)
 {
+  mImageNameLookupTable.Init(NS_FORM_CONTROL_LIST_HASHTABLE_SIZE);
 }
 
 nsHTMLFormElement::~nsHTMLFormElement()
@@ -248,6 +254,8 @@ nsHTMLFormElement::~nsHTMLFormElement()
   if (mControls) {
     mControls->DropFormReference();
   }
+
+  Clear();
 }
 
 nsresult
@@ -290,9 +298,15 @@ ElementTraverser(const nsAString& key, nsIDOMHTMLInputElement* element,
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(nsHTMLFormElement,
                                                   nsGenericHTMLElement)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mControls)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mImageNameLookupTable)
   tmp->mSelectedRadioButtons.EnumerateRead(ElementTraverser, &cb);
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
+NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(nsHTMLFormElement,
+                                                nsGenericHTMLElement)
+  tmp->Clear();
+NS_IMPL_CYCLE_COLLECTION_UNLINK_END
+
 NS_IMPL_ADDREF_INHERITED(nsHTMLFormElement, Element)
 NS_IMPL_RELEASE_INHERITED(nsHTMLFormElement, Element)
 
@@ -455,8 +469,9 @@ nsHTMLFormElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent,
   return rv;
 }
 
+template<typename T>
 static void
-MarkOrphans(const nsTArray<nsGenericHTMLFormElement*>& aArray)
+MarkOrphans(const nsTArray<T*>& aArray)
 {
   uint32_t length = aArray.Length();
   for (uint32_t i = 0; i < length; ++i) {
@@ -483,8 +498,7 @@ CollectOrphans(nsINode* aRemovalRoot,
     // Now if MAYBE_ORPHAN_FORM_ELEMENT is not set, that would mean that the
     // node is in fact a descendant of the form and hence should stay in the
     // form.  If it _is_ set, then we need to check whether the node is a
-    // descendant of aRemovalRoot.  If it is, we leave it in the form.  See
+    // descendant of aRemovalRoot.  If it is, we leave it in the form.
-    // also the code in nsGenericHTMLFormElement::FindForm.
 #ifdef DEBUG
     bool removed = false;
 #endif
@@ -511,6 +525,46 @@ CollectOrphans(nsINode* aRemovalRoot,
   }
 }
 
+static void
+CollectOrphans(nsINode* aRemovalRoot,
+               const nsTArray<HTMLImageElement*>& aArray
+#ifdef DEBUG
+               , nsIDOMHTMLFormElement* aThisForm
+#endif
+               )
+{
+  // Walk backwards so that if we remove elements we can just keep iterating
+  uint32_t length = aArray.Length();
+  for (uint32_t i = length; i > 0; --i) {
+    HTMLImageElement* node = aArray[i-1];
+
+    // Now if MAYBE_ORPHAN_FORM_ELEMENT is not set, that would mean that the
+    // node is in fact a descendant of the form and hence should stay in the
+    // form.  If it _is_ set, then we need to check whether the node is a
+    // descendant of aRemovalRoot.  If it is, we leave it in the form.
+#ifdef DEBUG
+    bool removed = false;
+#endif
+    if (node->HasFlag(MAYBE_ORPHAN_FORM_ELEMENT)) {
+      node->UnsetFlags(MAYBE_ORPHAN_FORM_ELEMENT);
+      if (!nsContentUtils::ContentIsDescendantOf(node, aRemovalRoot)) {
+        node->ClearForm(true);
+
+#ifdef DEBUG
+        removed = true;
+#endif
+      }
+    }
+
+#ifdef DEBUG
+    if (!removed) {
+      nsCOMPtr<nsIDOMHTMLFormElement> form = node->GetForm();
+      NS_ASSERTION(form == aThisForm, "How did that happen?");
+    }
+#endif /* DEBUG */
+  }
+}
+
 void
 nsHTMLFormElement::UnbindFromTree(bool aDeep, bool aNullParent)
 {
@@ -519,6 +573,7 @@ nsHTMLFormElement::UnbindFromTree(bool aDeep, bool aNullParent)
   // Mark all of our controls as maybe being orphans
   MarkOrphans(mControls->mElements);
   MarkOrphans(mControls->mNotInElements);
+  MarkOrphans(mImageElements);
 
   nsGenericHTMLElement::UnbindFromTree(aDeep, aNullParent);
 
@@ -540,6 +595,11 @@ nsHTMLFormElement::UnbindFromTree(bool aDeep, bool aNullParent)
   CollectOrphans(ancestor, mControls->mNotInElements
 #ifdef DEBUG
                  , this
+#endif
+                 );
+  CollectOrphans(ancestor, mImageElements
+#ifdef DEBUG
+                 , this
 #endif
                  );
 
@@ -1034,18 +1094,17 @@ nsHTMLFormElement::GetElementAt(int32_t aIndex) const
  *         0 otherwise
  */
 static inline int32_t
-CompareFormControlPosition(nsGenericHTMLFormElement *aControl1,
+CompareFormControlPosition(Element *aElement1, Element *aElement2,
-                           nsGenericHTMLFormElement *aControl2,
                            const nsIContent* aForm)
 {
-  NS_ASSERTION(aControl1 != aControl2, "Comparing a form control to itself");
+  NS_ASSERTION(aElement1 != aElement2, "Comparing a form control to itself");
 
   // If an element has a @form, we can assume it *might* be able to not have
   // a parent and still be in the form.
-  NS_ASSERTION((aControl1->HasAttr(kNameSpaceID_None, nsGkAtoms::form) ||
+  NS_ASSERTION((aElement1->HasAttr(kNameSpaceID_None, nsGkAtoms::form) ||
-                aControl1->GetParent()) &&
+                aElement1->GetParent()) &&
-               (aControl2->HasAttr(kNameSpaceID_None, nsGkAtoms::form) ||
+               (aElement2->HasAttr(kNameSpaceID_None, nsGkAtoms::form) ||
-                aControl2->GetParent()),
+                aElement2->GetParent()),
                "Form controls should always have parents");
 
   // If we pass aForm, we are assuming both controls are form descendants which
@@ -1054,12 +1113,12 @@ CompareFormControlPosition(nsGenericHTMLFormElement *aControl1,
   // TODO: remove the prevent asserts fix, see bug 598468.
 #ifdef DEBUG
   nsLayoutUtils::gPreventAssertInCompareTreePosition = true;
-  int32_t rVal = nsLayoutUtils::CompareTreePosition(aControl1, aControl2, aForm);
+  int32_t rVal = nsLayoutUtils::CompareTreePosition(aElement1, aElement2, aForm);
   nsLayoutUtils::gPreventAssertInCompareTreePosition = false;
 
   return rVal;
 #else // DEBUG
-  return nsLayoutUtils::CompareTreePosition(aControl1, aControl2, aForm);
+  return nsLayoutUtils::CompareTreePosition(aElement1, aElement2, aForm);
 #endif // DEBUG
 }
 
@@ -1106,6 +1165,57 @@ nsHTMLFormElement::PostPasswordEvent()
   event->PostDOMEvent();
 }
 
+// This function return true if the element, once appended, is the last one in
+// the array.
+template<typename ElementType>
+static bool
+AddElementToList(nsTArray<ElementType*>& aList, ElementType* aChild,
+                 nsHTMLFormElement* aForm)
+{
+  NS_ASSERTION(aList.IndexOf(aChild) == aList.NoIndex,
+               "aChild already in aList");
+
+  uint32_t count = aList.Length();
+  ElementType* element;
+  bool lastElement = false;
+
+  // Optimize most common case where we insert at the end.
+  int32_t position = -1;
+  if (count > 0) {
+    element = aList[count - 1];
+    position = CompareFormControlPosition(aChild, element, aForm);
+  }
+
+  // If this item comes after the last element, or the elements array is
+  // empty, we append to the end. Otherwise, we do a binary search to
+  // determine where the element should go.
+  if (position >= 0 || count == 0) {
+    // WEAK - don't addref
+    aList.AppendElement(aChild);
+    lastElement = true;
+  }
+  else {
+    int32_t low = 0, mid, high;
+    high = count - 1;
+
+    while (low <= high) {
+      mid = (low + high) / 2;
+
+      element = aList[mid];
+      position = CompareFormControlPosition(aChild, element, aForm);
+      if (position >= 0)
+        low = mid + 1;
+      else
+        high = mid - 1;
+    }
+
+    // WEAK - don't addref
+    aList.InsertElementAt(low, aChild);
+  }
+
+  return lastElement;
+}
+
 nsresult
 nsHTMLFormElement::AddElement(nsGenericHTMLFormElement* aChild,
                               bool aUpdateValidity, bool aNotify)
@@ -1122,46 +1232,7 @@ nsHTMLFormElement::AddElement(nsGenericHTMLFormElement* aChild,
   nsTArray<nsGenericHTMLFormElement*>& controlList = childInElements ?
       mControls->mElements : mControls->mNotInElements;
 
-  NS_ASSERTION(controlList.IndexOf(aChild) == controlList.NoIndex,
+  bool lastElement = AddElementToList(controlList, aChild, this);
-               "Form control already in form");
-
-  uint32_t count = controlList.Length();
-  nsGenericHTMLFormElement* element;
-  
-  // Optimize most common case where we insert at the end.
-  bool lastElement = false;
-  int32_t position = -1;
-  if (count > 0) {
-    element = controlList[count - 1];
-    position = CompareFormControlPosition(aChild, element, this);
-  }
-
-  // If this item comes after the last element, or the elements array is
-  // empty, we append to the end. Otherwise, we do a binary search to
-  // determine where the element should go.
-  if (position >= 0 || count == 0) {
-    // WEAK - don't addref
-    controlList.AppendElement(aChild);
-    lastElement = true;
-  }
-  else {
-    int32_t low = 0, mid, high;
-    high = count - 1;
-      
-    while (low <= high) {
-      mid = (low + high) / 2;
-        
-      element = controlList[mid];
-      position = CompareFormControlPosition(aChild, element, this);
-      if (position >= 0)
-        low = mid + 1;
-      else
-        high = mid - 1;
-    }
-      
-    // WEAK - don't addref
-    controlList.InsertElementAt(low, aChild);
-  }
 
 #ifdef DEBUG
   AssertDocumentOrder(controlList, this);
@@ -1359,6 +1430,55 @@ nsHTMLFormElement::HandleDefaultSubmitRemoval()
   }
 }
 
+static nsresult
+RemoveElementFromTableInternal(
+  nsInterfaceHashtable<nsStringHashKey,nsISupports>& aTable,
+  nsIContent* aChild, const nsAString& aName)
+{
+  nsCOMPtr<nsISupports> supports;
+
+  if (!aTable.Get(aName, getter_AddRefs(supports)))
+    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;
+  }
+
+  nsCOMPtr<nsIDOMNodeList> nodeList(do_QueryInterface(supports));
+  NS_ENSURE_TRUE(nodeList, NS_ERROR_FAILURE);
+
+  // Upcast, uggly, but it works!
+  nsBaseContentList *list = static_cast<nsBaseContentList*>(nodeList.get());
+
+  list->RemoveElement(aChild);
+
+  uint32_t length = 0;
+  list->GetLength(&length);
+
+  if (!length) {
+    // If the list is empty we remove if from our hash, this shouldn't
+    // happen tho
+    aTable.Remove(aName);
+  } else if (length == 1) {
+    // Only one element left, replace the list in the hash with the
+    // single element.
+    nsIContent* node = list->Item(0);
+    if (node) {
+      aTable.Put(aName, node);
+    }
+  }
+
+  return NS_OK;
+}
+
 nsresult
 nsHTMLFormElement::RemoveElementFromTable(nsGenericHTMLFormElement* aElement,
                                           const nsAString& aName)
@@ -1377,13 +1497,13 @@ nsHTMLFormElement::FindNamedItem(const nsAString& aName,
     return result.forget();
   }
 
-  nsCOMPtr<nsIHTMLDocument> htmlDoc = do_QueryInterface(GetCurrentDoc());
+  result = mImageNameLookupTable.GetWeak(aName);
-  if (!htmlDoc) {
+  if (result) {
     *aCache = nullptr;
-    return nullptr;
+    return result.forget();
   }
 
-  return htmlDoc->ResolveName(aName, this, aCache);
+  return nullptr;
 }
 
 already_AddRefed<nsISupports>
@@ -2141,6 +2261,16 @@ nsHTMLFormElement::IntrinsicState() const
   return state;
 }
 
+void
+nsHTMLFormElement::Clear()
+{
+  for (int32_t i = mImageElements.Length() - 1; i >= 0; i--) {
+    mImageElements[i]->ClearForm(false);
+  }
+  mImageElements.Clear();
+  mImageNameLookupTable.Clear();
+}
+
 //----------------------------------------------------------------------
 // nsFormControlList implementation, this could go away if there were
 // a lightweight collection implementation somewhere
@@ -2298,20 +2428,17 @@ nsFormControlList::NamedItemInternal(const nsAString& aName,
   return mNameLookupTable.GetWeak(aName);
 }
 
-nsresult
+static nsresult
-nsFormControlList::AddElementToTable(nsGenericHTMLFormElement* aChild,
+AddElementToTableInternal(
-                                     const nsAString& aName)
+  nsInterfaceHashtable<nsStringHashKey,nsISupports>& aTable,
+  nsIContent* aChild, const nsAString& aName, nsHTMLFormElement* aForm)
 {
-  if (!ShouldBeInElements(aChild)) {
-    return NS_OK;
-  }
-
   nsCOMPtr<nsISupports> supports;
-  mNameLookupTable.Get(aName, getter_AddRefs(supports));
+  aTable.Get(aName, getter_AddRefs(supports));
 
   if (!supports) {
-    // No entry found, add the form control
+    // No entry found, add the element
-    mNameLookupTable.Put(aName, NS_ISUPPORTS_CAST(nsIContent*, aChild));
+    aTable.Put(aName, aChild);
   } else {
     // Found something in the hash, check its type
     nsCOMPtr<nsIContent> content = do_QueryInterface(supports);
@@ -2327,7 +2454,7 @@ nsFormControlList::AddElementToTable(nsGenericHTMLFormElement* aChild,
 
       // Found an element, create a list, add the element to the list and put
       // the list in the hash
-      nsSimpleContentList *list = new nsSimpleContentList(mForm);
+      nsSimpleContentList *list = new nsSimpleContentList(aForm);
 
       // If an element has a @form, we can assume it *might* be able to not have
       // a parent and still be in the form.
@@ -2337,14 +2464,14 @@ nsFormControlList::AddElementToTable(nsGenericHTMLFormElement* aChild,
       // Determine the ordering between the new and old element.
       bool newFirst = nsContentUtils::PositionIsBefore(aChild, content);
 
-      list->AppendElement(newFirst ? aChild : content);
+      list->AppendElement(newFirst ? aChild : content.get());
-      list->AppendElement(newFirst ? content : aChild);
+      list->AppendElement(newFirst ? content.get() : aChild);
 
 
       nsCOMPtr<nsISupports> listSupports = do_QueryObject(list);
 
       // Replace the element with the list.
-      mNameLookupTable.Put(aName, listSupports);
+      aTable.Put(aName, listSupports);
     } else {
       // There's already a list in the hash, add the child to the list
       nsCOMPtr<nsIDOMNodeList> nodeList = do_QueryInterface(supports);
@@ -2395,6 +2522,17 @@ nsFormControlList::AddElementToTable(nsGenericHTMLFormElement* aChild,
   return NS_OK;
 }
 
+nsresult
+nsFormControlList::AddElementToTable(nsGenericHTMLFormElement* aChild,
+                                     const nsAString& aName)
+{
+  if (!ShouldBeInElements(aChild)) {
+    return NS_OK;
+  }
+
+  return AddElementToTableInternal(mNameLookupTable, aChild, aName, mForm);
+}
+
 nsresult
 nsFormControlList::IndexOfControl(nsIFormControl* aControl,
                                   int32_t* aIndex)
@@ -2416,48 +2554,7 @@ nsFormControlList::RemoveElementFromTable(nsGenericHTMLFormElement* aChild,
     return NS_OK;
   }
 
-  nsCOMPtr<nsISupports> supports;
+  return RemoveElementFromTableInternal(mNameLookupTable, aChild, aName);
-
-  if (!mNameLookupTable.Get(aName, getter_AddRefs(supports)))
-    return NS_OK;
-
-  nsCOMPtr<nsIFormControl> fctrl(do_QueryInterface(supports));
-
-  if (fctrl) {
-    // Single element in the hash, just remove it if it's the one
-    // we're trying to remove...
-    if (fctrl == aChild) {
-      mNameLookupTable.Remove(aName);
-    }
-
-    return NS_OK;
-  }
-
-  nsCOMPtr<nsIDOMNodeList> nodeList(do_QueryInterface(supports));
-  NS_ENSURE_TRUE(nodeList, NS_ERROR_FAILURE);
-
-  // Upcast, uggly, but it works!
-  nsBaseContentList *list = static_cast<nsBaseContentList*>(nodeList.get());
-
-  list->RemoveElement(aChild);
-
-  uint32_t length = 0;
-  list->GetLength(&length);
-
-  if (!length) {
-    // If the list is empty we remove if from our hash, this shouldn't
-    // happen tho
-    mNameLookupTable.Remove(aName);
-  } else if (length == 1) {
-    // Only one element left, replace the list in the hash with the
-    // single element.
-    nsIContent* node = list->Item(0);
-    if (node) {
-      mNameLookupTable.Put(aName, node);
-    }
-  }
-
-  return NS_OK;
 }
 
 nsresult
@@ -2581,3 +2678,35 @@ nsFormControlList::GetSupportedNames(nsTArray<nsString>& aNames)
   // this enumeration.
   mNameLookupTable.EnumerateRead(CollectNames, &aNames);
 }
+
+nsresult
+nsHTMLFormElement::AddImageElement(HTMLImageElement* aChild)
+{
+  AddElementToList(mImageElements, aChild, this);
+  return NS_OK;
+}
+
+nsresult
+nsHTMLFormElement::AddImageElementToTable(HTMLImageElement* aChild,
+                                          const nsAString& aName)
+{
+  return AddElementToTableInternal(mImageNameLookupTable, aChild, aName, this);
+}
+
+nsresult
+nsHTMLFormElement::RemoveImageElement(HTMLImageElement* aChild)
+{
+  uint32_t index = mImageElements.IndexOf(aChild);
+  NS_ENSURE_STATE(index != mImageElements.NoIndex);
+
+  mImageElements.RemoveElementAt(index);
+  return NS_OK;
+}
+
+nsresult
+nsHTMLFormElement::RemoveImageElementFromTable(HTMLImageElement* aElement,
+                                               const nsAString& aName)
+{
+  return RemoveElementFromTableInternal(mImageNameLookupTable, aElement, aName);
+}
+

content/html/content/src/nsHTMLFormElement.h

@@ -32,6 +32,12 @@ class nsFormControlList;
 class nsIMutableArray;
 class nsIURI;
 
+namespace mozilla {
+namespace dom {
+class HTMLImageElement;
+}
+}
+
 class nsHTMLFormElement : public nsGenericHTMLElement,
                           public nsIDOMHTMLFormElement,
                           public nsIWebProgressListener,
@@ -121,7 +127,7 @@ public:
 
   virtual nsresult Clone(nsINodeInfo *aNodeInfo, nsINode **aResult) const MOZ_OVERRIDE;
 
-  NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED_NO_UNLINK(nsHTMLFormElement,
+  NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(nsHTMLFormElement,
                                            nsGenericHTMLElement)
 
   /**
@@ -168,6 +174,46 @@ public:
    */
   nsresult AddElementToTable(nsGenericHTMLFormElement* aChild,
                              const nsAString& aName);
+
+  /**
+   * Remove an image element from this form's list of image elements
+   *
+   * @param aElement the image element to remove
+   * @return NS_OK if the element was successfully removed.
+   */
+  nsresult RemoveImageElement(mozilla::dom::HTMLImageElement* aElement);
+
+  /**
+   * Remove an image element from the lookup table maintained by the form.
+   * We can't fold this method into RemoveImageElement() because when
+   * RemoveImageElement() is called it doesn't know if the element is
+   * removed because the id attribute has changed, or because the
+   * name attribute has changed.
+   *
+   * @param aElement the image element to remove
+   * @param aName the name or id of the element to remove
+   * @return NS_OK if the element was successfully removed.
+   */
+  nsresult RemoveImageElementFromTable(mozilla::dom::HTMLImageElement* aElement,
+                                      const nsAString& aName);
+  /**
+   * Add an image element to the end of this form's list of image elements
+   *
+   * @param aElement the element to add
+   * @return NS_OK if the element was successfully added
+   */
+  nsresult AddImageElement(mozilla::dom::HTMLImageElement* aElement);
+
+  /**
+   * Add an image element to the lookup table maintained by the form.
+   *
+   * We can't fold this method into AddImageElement() because when
+   * AddImageElement() is called, the image attributes can change.
+   * The name or id attributes of the image are used as a key into the table.
+   */
+  nsresult AddImageElementToTable(mozilla::dom::HTMLImageElement* aChild,
+                                 const nsAString& aName);
+
    /**
     * Return whether there is one and only one input text control.
     *
@@ -363,6 +409,9 @@ protected:
    */
   bool CheckFormValidity(nsIMutableArray* aInvalidElements) const;
 
+  // Clear the mImageNameLookupTable and mImageElements.
+  void Clear();
+
 public:
   /**
    * Flush a possible pending submission. If there was a scripted submission
@@ -417,6 +466,20 @@ protected:
   /** The first submit element in mNotInElements -- WEAK */
   nsGenericHTMLFormElement* mFirstSubmitNotInElements;
 
+  // This array holds on to all HTMLImageElement(s).
+  // This is needed to properly clean up the bi-directional references
+  // (both weak and strong) between the form and its HTMLImageElements.
+
+  nsTArray<mozilla::dom::HTMLImageElement*> mImageElements;  // Holds WEAK references
+
+  // A map from an ID or NAME attribute to the HTMLImageElement(s), this
+  // hash holds strong references either to the named HTMLImageElement, or
+  // to a list of named HTMLImageElement(s), in the case where this hash
+  // holds on to a list of named HTMLImageElement(s) the list has weak
+  // references to the HTMLImageElement.
+
+  nsInterfaceHashtable<nsStringHashKey,nsISupports> mImageNameLookupTable;
+
   /**
    * 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

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

content/html/content/test/test_bug870787.html

@@ -0,0 +1,84 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=870787
+-->
+<head>
+  <title>Test for Bug 870787</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=870787">Mozilla Bug 870787</a>
+
+<p id="msg"></p>
+
+<form id="form0"></form>
+<img name="img0" id="img0id">
+
+<img name="img1" id="img1id" />
+<form id="form1">
+  <img name="img2" id="img2id" />
+</form>
+<img name="img3" id="img3id" />
+
+<table>
+  <form id="form2">
+  <tr><td>
+    <button name="input1" id="input1id" />
+    <input name="input2" id="input2id" />
+  </form>
+</table>
+
+<table>
+  <form id="form3">
+  <tr><td>
+    <img name="img4" id="img4id" />
+    <img name="img5" id="img5id" />
+  </form>
+</table>
+
+<form id="form4"><img id="img6"></form>
+
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 870787 **/
+
+var form0 = document.getElementById("form0");
+ok(form0, "Form0 exists");
+ok(!form0.img0, "Form0.img0 doesn't exist");
+ok(!form0.img0id, "Form0.img0id doesn't exist");
+
+var form1 = document.getElementById("form1");
+ok(form1, "Form1 exists");
+ok(!form1.img1, "Form1.img1 doesn't exist");
+ok(!form1.img1id, "Form1.img1id doesn't exist");
+is(form1.img2, document.getElementById("img2id"), "Form1.img2 exists");
+is(form1.img2id, document.getElementById("img2id"), "Form1.img2id exists");
+ok(!form1.img3, "Form1.img3 doesn't exist");
+ok(!form1.img3id, "Form1.img3id doesn't exist");
+
+var form2 = document.getElementById("form2");
+ok(form2, "Form2 exists");
+is(form2.input1, document.getElementById("input1id"), "Form2.input1 exists");
+is(form2.input1id, document.getElementById("input1id"), "Form2.input1id exists");
+is(form2.input2, document.getElementById("input2id"), "Form2.input2 exists");
+is(form2.input2id, document.getElementById("input2id"), "Form2.input2id exists");
+
+var form3 = document.getElementById("form3");
+ok(form3, "Form3 exists");
+is(form3.img4, document.getElementById("img4id"), "Form3.img4 doesn't exists");
+is(form3.img4id, document.getElementById("img4id"), "Form3.img4id doesn't exists");
+is(form3.img5, document.getElementById("img5id"), "Form3.img5 doesn't exists");
+is(form3.img5id, document.getElementById("img5id"), "Form3.img5id doesn't exists");
+
+var form4 = document.getElementById("form4");
+ok(form4, "Form4 exists");
+is(Object.getOwnPropertyNames(form4.elements).indexOf("img6"), -1, "Form4.elements should not contain img6");
+
+</script>
+</pre>
+</body>
+</html>

content/html/document/src/nsHTMLDocument.cpp

@@ -2295,39 +2295,6 @@ nsHTMLDocument::ResolveName(const nsAString& aName, nsWrapperCache **aCache)
   return nullptr;
 }
 
-already_AddRefed<nsISupports>
-nsHTMLDocument::ResolveName(const nsAString& aName,
-                            nsIContent *aForm,
-                            nsWrapperCache **aCache)
-{
-  nsISupports* result = ResolveName(aName, aCache);
-  if (!result) {
-    return nullptr;
-  }
-
-  nsCOMPtr<nsIContent> node = do_QueryInterface(result);
-  if (!node) {
-    // We create a nsFormContentList which will filter out the elements in the
-    // list that don't belong to aForm.
-    nsRefPtr<nsBaseContentList> list =
-      new nsFormContentList(aForm, *static_cast<nsBaseContentList*>(result));
-    if (list->Length() > 1) {
-      *aCache = list;
-      return list.forget();
-    }
-
-    // After the nsFormContentList is done filtering there's either nothing or
-    // one element in the list. Return that element, or null if there's no
-    // element in the list.
-    node = list->Item(0);
-  } else if (!nsContentUtils::BelongsInForm(aForm, node)) {
-    node = nullptr;
-  }
-
-  *aCache = node;
-  return node.forget();
-}
-
 JSObject*
 nsHTMLDocument::NamedGetter(JSContext* cx, const nsAString& aName, bool& aFound,
                             ErrorResult& rv)

content/html/document/src/nsHTMLDocument.h

@@ -115,9 +115,6 @@ public:
   JSObject* GetAll(JSContext* aCx, mozilla::ErrorResult& aRv);
 
   nsISupports* ResolveName(const nsAString& aName, nsWrapperCache **aCache);
-  virtual already_AddRefed<nsISupports> ResolveName(const nsAString& aName,
-                                                    nsIContent *aForm,
-                                                    nsWrapperCache **aCache) MOZ_OVERRIDE;
 
   virtual void AddedForm() MOZ_OVERRIDE;
   virtual void RemovedForm() MOZ_OVERRIDE;

content/html/document/src/nsIHTMLDocument.h

@@ -33,10 +33,6 @@ public:
    */
   virtual void SetCompatibilityMode(nsCompatibility aMode) = 0;
 
-  virtual already_AddRefed<nsISupports> ResolveName(const nsAString& aName,
-                                                    nsIContent *aForm,
-                                                    nsWrapperCache **aCache) = 0;
-
   /**
    * Called when form->BindToTree() is called so that document knows
    * immediately when a form is added