Fix up name resolution under forms to not find elements that are just

randomly in the document.  Bug 148782, r=caillon, sr=jst

content/base/public/nsContentUtils.h

@@ -263,6 +263,18 @@ public:
                                             const nsAString& aSpec,
                                             nsIDocument* aDocument,
                                             nsIURI* aBaseURI);
+
+  /**
+   * 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 PRBool BelongsInForm(nsIDOMHTMLFormElement *aForm,
+                              nsIContent *aContent);
+  
 private:
   static nsresult GetDocumentAndPrincipal(nsIDOMNode* aNode,
                                           nsIDocument** aDocument,

content/base/src/nsContentList.cpp

@@ -147,77 +147,6 @@ nsBaseContentList::Shutdown()
 
 // nsFormContentList
 
-// This helper function checks if aContent is in some way associated
-// with aForm, this check is only successful if the form is a
-// container (and a form is a container as long as the document is
-// wellformed). If the form is a container the only elements that are
-// considerd to be associated with a form are the elements that are
-// contained within the form. If the form is a leaf element then all
-// the elements will be accepted into this list.
-
-static PRBool BelongsInForm(nsIDOMHTMLFormElement *aForm,
-                            nsIContent *aContent)
-{
-  nsCOMPtr<nsIContent> form(do_QueryInterface(aForm));
-
-  if (!form) {
-    NS_WARNING("This should not happen, form is not an nsIContent!");
-
-    return PR_TRUE;
-  }
-
-  if (form.get() == aContent) {
-    // The list for aForm contains the form itself, forms should not
-    // be reachable by name in the form namespace, so we return false
-    // here.
-
-    return PR_FALSE;
-  }
-
-  nsCOMPtr<nsIContent> content;
-
-  aContent->GetParent(getter_AddRefs(content));
-
-  while (content) {
-    if (content == form) {
-      // aContent is contained within the form so we return true.
-
-      return PR_TRUE;
-    }
-
-    nsCOMPtr<nsIAtom> tag;
-
-    content->GetTag(getter_AddRefs(tag));
-
-    if (tag.get() == nsHTMLAtoms::form) {
-      // The child is contained within a form, but not the right form
-      // so we ignore it.
-
-      return PR_FALSE;
-    }
-
-    nsIContent *tmp = content;
-
-    tmp->GetParent(getter_AddRefs(content));
-  }
-
-  PRInt32 count = 0;
-
-  form->ChildCount(count);
-
-  if (!count) {
-    // The form is a leaf and aContent wasn't inside any other form so
-    // we return true
-
-    return PR_TRUE;
-  }
-
-  // The form is a container but aContent wasn't inside the form,
-  // return false
-
-  return PR_FALSE;
-}
-
 nsFormContentList::nsFormContentList(nsIDOMHTMLFormElement *aForm,
                                      nsBaseContentList& aContentList)
   : nsBaseContentList()
@@ -235,7 +164,7 @@ nsFormContentList::nsFormContentList(nsIDOMHTMLFormElement *aForm,
 
     nsCOMPtr<nsIContent> c(do_QueryInterface(item));
 
-    if (c && BelongsInForm(aForm, c)) {
+    if (c && nsContentUtils::BelongsInForm(aForm, c)) {
       AppendElement(c);
     }
   }

content/base/src/nsContentUtils.cpp

@@ -1541,7 +1541,8 @@ nsContentUtils::GenerateStateKey(nsIContent* aContent,
   return NS_OK;
 }
 
-/* static */ nsresult
+// static
+nsresult
 nsContentUtils::NewURIWithDocumentCharset(nsIURI** aResult,
                                           const nsAString& aSpec,
                                           nsIDocument* aDocument,
@@ -1555,6 +1556,88 @@ nsContentUtils::NewURIWithDocumentCharset(nsIURI** aResult,
                    aBaseURI, sIOService);
 }
 
+// static
+PRBool
+nsContentUtils::BelongsInForm(nsIDOMHTMLFormElement *aForm,
+                              nsIContent *aContent)
+{
+  NS_PRECONDITION(aForm, "Must have a form");
+  NS_PRECONDITION(aContent, "Must have a content node");
+
+  nsCOMPtr<nsIContent> form(do_QueryInterface(aForm));
+
+  if (!form) {
+    NS_ERROR("This should not happen, form is not an nsIContent!");
+
+    return PR_TRUE;
+  }
+
+  if (form == aContent) {
+    // A form does not belong inside itself, so we return false here
+
+    return PR_FALSE;
+  }
+
+  nsCOMPtr<nsIContent> content;
+
+  aContent->GetParent(getter_AddRefs(content));
+
+  while (content) {
+    if (content == form) {
+      // aContent is contained within the form so we return true.
+
+      return PR_TRUE;
+    }
+
+    nsCOMPtr<nsIAtom> tag;
+
+    content->GetTag(getter_AddRefs(tag));
+
+    if (tag == nsHTMLAtoms::form) {
+      // The child is contained within a form, but not the right form
+      // so we ignore it.
+
+      return PR_FALSE;
+    }
+
+    nsIContent *tmp = content;
+
+    tmp->GetParent(getter_AddRefs(content));
+  }
+
+  PRInt32 count = 0;
+
+  form->ChildCount(count);
+
+  if (count > 0) {
+    // The form is a container but aContent wasn't inside the form,
+    // return false
+
+    return PR_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.
+  nsCOMPtr<nsIDOM3Node> contentAsDOM3(do_QueryInterface(aContent));
+  PRUint16 comparisonFlags = 0;
+  nsresult rv = NS_OK;
+  if (contentAsDOM3) {
+    rv = contentAsDOM3->CompareDocumentPosition(aForm, &comparisonFlags);
+  }
+  if (NS_FAILED(rv) ||
+      comparisonFlags & nsIDOM3Node::DOCUMENT_POSITION_PRECEDING) {
+    // 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 PR_TRUE;
+  }
+  
+  return PR_FALSE;
+}
+
 void
 nsCxPusher::Push(nsISupports *aCurrentTarget)
 {

content/html/document/src/nsHTMLDocument.cpp

@@ -3752,29 +3752,20 @@ nsHTMLDocument::ResolveName(const nsAString& aName,
   PRUint32 length;
   list->GetLength(&length);
 
-  if (length) {
+  if (length > 0) {
     if (length == 1) {
-      // Onle one element in the list, return the list in stead of
+      // Only one element in the list, return the element instead of
       // returning the list
 
       nsCOMPtr<nsIDOMNode> node;
 
       list->Item(0, getter_AddRefs(node));
 
-      if (aForm && node) {
-        // document.forms["foo"].bar should not map to <form
-        // name="bar"> so we check here to see if we found a form and
-        // if we did we ignore what we found in the document. This
-        // doesn't deal with the case where more than one element in
-        // found in the document (i.e. there are two named items in
-        // the document that have the name we're looking for), that
-        // case is dealt with in nsFormContentList
-
-        nsCOMPtr<nsIDOMHTMLFormElement> f(do_QueryInterface(node));
-
-        if (f) {
-          node = nsnull;
-        }
+      nsCOMPtr<nsIContent> ourContent(do_QueryInterface(node));
+      if (aForm && ourContent &&
+          !nsContentUtils::BelongsInForm(aForm, ourContent)) {
+        // This is not the content you are looking for
+        node = nsnull;
       }
 
       *aResult = node;
@@ -3783,43 +3774,40 @@ nsHTMLDocument::ResolveName(const nsAString& aName,
       return NS_OK;
     }
 
-    if (length > 1) {
-      // The list contains more than one element, return the whole
-      // list, unless...
+    // The list contains more than one element, return the whole
+    // list, unless...
 
-      if (aForm) {
-        // ... we're called from a form, in that case we create a
-        // nsFormContentList which will filter out the elements in the
-        // list that don't belong to aForm
+    if (aForm) {
+      // ... we're called from a form, in that case we create a
+      // nsFormContentList which will filter out the elements in the
+      // list that don't belong to aForm
 
-        nsFormContentList *fc_list = new nsFormContentList(aForm, *list);
-        NS_ENSURE_TRUE(fc_list, NS_ERROR_OUT_OF_MEMORY);
+      nsFormContentList *fc_list = new nsFormContentList(aForm, *list);
+      NS_ENSURE_TRUE(fc_list, NS_ERROR_OUT_OF_MEMORY);
 
-        PRUint32 len;
-        fc_list->GetLength(&len);
+      PRUint32 len;
+      fc_list->GetLength(&len);
 
-        if (len < 2) {
-          // After t nsFormContentList is done filtering there's zero
-          // or one element in the list, return that element, or null
-          // if there's no element in the list.
+      if (len < 2) {
+        // 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.
 
-          nsCOMPtr<nsIDOMNode> node;
+        nsCOMPtr<nsIDOMNode> node;
 
-          fc_list->Item(0, getter_AddRefs(node));
+        fc_list->Item(0, getter_AddRefs(node));
 
-          *aResult = node;
-          NS_IF_ADDREF(*aResult);
+        NS_IF_ADDREF(*aResult = node);
 
-          delete fc_list;
+        delete fc_list;
 
-          return NS_OK;
-        }
-
-        list = fc_list;
+        return NS_OK;
       }
 
-      return CallQueryInterface(list, aResult);
+      list = fc_list;
     }
+
+    return CallQueryInterface(list, aResult);
   }
 
   // No named items were found, see if there's one registerd by id for
@@ -3837,8 +3825,7 @@ nsHTMLDocument::ResolveName(const nsAString& aName,
         tag == nsHTMLAtoms::img    ||
         tag == nsHTMLAtoms::object ||
         tag == nsHTMLAtoms::applet) {
-      *aResult = e;
-      NS_ADDREF(*aResult);
+      NS_ADDREF(*aResult = e);
     }
   }