Backing out fix for bug 324918 so that <select> actually works again.

content/html/content/src/nsHTMLSelectElement.cpp

@@ -117,45 +117,18 @@ public:
    * @param aOption the option to insert
    * @param aIndex the index to insert at
    */
-  PRBool InsertOptionAt(nsIDOMHTMLOptionElement* aOption, PRInt32 aIndex)
+  nsresult InsertElementAt(nsIDOMHTMLOptionElement* aOption, PRInt32 aIndex);
-  {
-    return mElements.InsertObjectAt(aOption, aIndex);
-  }
-
   /**
    * Remove an option
    * @param aIndex the index of the option to remove
    */
-  void RemoveOptionAt(PRInt32 aIndex)
+  nsresult RemoveElementAt(PRInt32 aIndex);
-  {
-    mElements.RemoveObjectAt(aIndex);
-  }
-
   /**
    * Get the option at the index
    * @param aIndex the index
    * @param aReturn the option returned [OUT]
    */
-  nsIDOMHTMLOptionElement *ItemAsOption(PRInt32 aIndex)
+  nsIDOMHTMLOptionElement *ItemAsOption(PRInt32 aIndex);
-  {
-    return mElements.SafeObjectAt(aIndex);
-  }
-
-  /**
-   * Clears out all options
-   */
-  void Clear()
-  {
-    mElements.Clear();
-  }
-
-  /**
-   * Append an option to end of array
-   */
-  PRBool AppendOption(nsIDOMHTMLOptionElement* aOption)
-  {
-    return mElements.AppendObject(aOption);
-  }
 
   /**
    * Drop the reference to the select.  Called during select destruction.
@@ -256,10 +229,12 @@ public:
 
   virtual void SetFocus(nsPresContext* aPresContext);
   virtual PRBool IsFocusable(PRInt32 *aTabIndex = nsnull);
-  virtual nsresult InsertChildAt(nsIContent* aKid, PRUint32 aIndex,
-                                 PRBool aNotify);
-  virtual nsresult RemoveChildAt(PRUint32 aIndex, PRBool aNotify);
 
+  // nsGenericElement
+  virtual nsresult WillAddOrRemoveChild(nsIContent* aKid,
+                                        PRUint32 aIndex,
+                                        PRBool aRemove);
+  
   // Overriden nsIFormControl methods
   NS_IMETHOD_(PRInt32) GetType() const { return NS_FORM_SELECT; }
   NS_IMETHOD Reset();
@@ -445,17 +420,8 @@ protected:
    */
   void DispatchContentReset();
 
-  /**
-   * Rebuilds the options array from scratch as a fallback in error cases.
-   */
-  void RebuildOptionsArray();
-
-#ifdef DEBUG
-  void VerifyOptionsArray();
-#endif
-
   /** The options[] array */
-  nsRefPtr<nsHTMLOptionCollection> mOptions;
+  nsHTMLOptionCollection* mOptions;
   /** false if the parser is in the middle of adding children. */
   PRBool    mIsDoneAddingChildren;
   /** The number of non-options as children of the select */
@@ -471,7 +437,7 @@ protected:
    * The temporary restore state in case we try to restore before parser is
    * done adding options
    */
-  nsRefPtr<nsSelectState> mRestoreState;
+  nsSelectState* mRestoreState;
 };
 
 
@@ -492,20 +458,22 @@ nsHTMLSelectElement::nsHTMLSelectElement(nsINodeInfo *aNodeInfo,
     mIsDoneAddingChildren(!aFromParser),
     mNonOptionChildren(0),
     mOptGroupCount(0),
-    mSelectedIndex(-1)
+    mSelectedIndex(-1),
+    mRestoreState(nsnull)
 {
-  // FIXME: Bug 328908, set mOptions in an Init function and get rid of null
-  // checks.
-
   // DoneAddingChildren() will be called later if it's from the parser,
   // otherwise it is
+
+  NS_IF_ADDREF(mOptions);
 }
 
 nsHTMLSelectElement::~nsHTMLSelectElement()
 {
   if (mOptions) {
     mOptions->DropReference();
+    NS_RELEASE(mOptions);
   }
+  NS_IF_RELEASE(mRestoreState);
 }
 
 // ISupports
@@ -538,57 +506,17 @@ nsHTMLSelectElement::GetForm(nsIDOMHTMLFormElement** aForm)
 }
 
 nsresult
-nsHTMLSelectElement::InsertChildAt(nsIContent* aKid,
+nsHTMLSelectElement::WillAddOrRemoveChild(nsIContent* aKid,
-                                   PRUint32 aIndex,
+                                          PRUint32 aIndex,
-                                   PRBool aNotify)
+                                          PRBool aRemove)
 {
-  PRUint32 prevOptGroups = mOptGroupCount;
+  if (aRemove) {
-
+    WillRemoveOptions(this, aIndex);
-  nsresult rv = WillAddOptions(aKid, this, aIndex);
+  } else {
-  PRBool rebuild = NS_FAILED(rv);
+    WillAddOptions(aKid, this, aIndex);
-
-  rv = nsGenericHTMLFormElement::InsertChildAt(aKid, aIndex, aNotify);
-  if (rebuild || NS_FAILED(rv)) {
-    RebuildOptionsArray();
-    return rv;
   }
 
-  if (mOptGroupCount && !prevOptGroups) {
+  return nsGenericHTMLFormElement::WillAddOrRemoveChild(aKid, aIndex, aRemove);
-    // FIXME: Bug 328907, get rid of this event
-    DispatchDOMEvent(NS_LITERAL_STRING("selectHasGroups"));
-  }
-
-#ifdef DEBUG
-  VerifyOptionsArray();
-#endif
-
-  return NS_OK;
-}
-
-nsresult
-nsHTMLSelectElement::RemoveChildAt(PRUint32 aIndex, PRBool aNotify)
-{
-  PRUint32 prevOptGroups = mOptGroupCount;
-
-  nsresult rv = WillRemoveOptions(this, aIndex);
-  PRBool rebuild = NS_FAILED(rv);
-
-  rv = nsGenericHTMLFormElement::RemoveChildAt(aIndex, aNotify);
-  if (rebuild || NS_FAILED(rv)) {
-    RebuildOptionsArray();
-    return rv;
-  }
-
-  if (!mOptGroupCount && prevOptGroups) {
-    // FIXME: Bug 328907, get rid of this event
-    DispatchDOMEvent(NS_LITERAL_STRING("selectHasNoGroups"));
-  }
-
-#ifdef DEBUG
-  VerifyOptionsArray();
-#endif
-
-  return NS_OK;
 }
 
 
@@ -674,12 +602,16 @@ nsHTMLSelectElement::PrintOptions(nsIContent* aOptions, PRInt32 tabs)
   }
 
   // Recurse down into optgroups
-  if (IsOptGroup(aOptions)) {
+  //
-    PRUint32 numChildren = aOptions->GetChildCount();
+  // I *would* put a restriction in here to only search under
+  // optgroups (and not, for example, <P></P>), but it really
+  // doesn't *hurt* to search under other stuff and it's more
+  // efficient in the normal only-optgroup-and-option case
+  // (one less QueryInterface).
+  PRUint32 numChildren = aOptions->GetChildCount();
 
-    for (PRUint32 i = 0; i < numChildren; ++i) {
+  for (PRUint32 i = 0; i < numChildren; ++i) {
-      PrintOptions(aOptions->GetChildAt(i), tabs + 1);
+    PrintOptions(aOptions->GetChildAt(i), tabs + 1);
-    }
   }
 
   return NS_OK;
@@ -748,7 +680,7 @@ nsHTMLSelectElement::InsertOptionsIntoListRecurse(nsIContent* aOptions,
 
   nsCOMPtr<nsIDOMHTMLOptionElement> optElement(do_QueryInterface(aOptions));
   if (optElement) {
-    nsresult rv = mOptions->InsertOptionAt(optElement, *aInsertIndex);
+    nsresult rv = mOptions->InsertElementAt(optElement, *aInsertIndex);
     NS_ENSURE_SUCCESS(rv, rv);
     (*aInsertIndex)++;
     return NS_OK;
@@ -760,16 +692,24 @@ nsHTMLSelectElement::InsertOptionsIntoListRecurse(nsIContent* aOptions,
     mNonOptionChildren++;
   }
 
-  // Recurse down into optgroups
   if (IsOptGroup(aOptions)) {
     mOptGroupCount++;
+    DispatchDOMEvent(NS_LITERAL_STRING("selectHasGroups"));
+  }
 
-    PRUint32 numChildren = aOptions->GetChildCount();
+  // Recurse down into optgroups
-    for (PRUint32 i = 0; i < numChildren; ++i) {
+  //
-      nsresult rv = InsertOptionsIntoListRecurse(aOptions->GetChildAt(i),
+  // I *would* put a restriction in here to only search under
-                                                 aInsertIndex, aDepth+1);
+  // optgroups (and not, for example, <P></P>), but it really
-      NS_ENSURE_SUCCESS(rv, rv);
+  // doesn't *hurt* to search under other stuff and it's more
-    }
+  // efficient in the normal only-optgroup-and-option case
+  // (one less QueryInterface).
+  PRUint32 numChildren = aOptions->GetChildCount();
+
+  for (PRUint32 i = 0; i < numChildren; ++i) {
+    nsresult rv = InsertOptionsIntoListRecurse(aOptions->GetChildAt(i),
+                                               aInsertIndex, aDepth+1);
+    NS_ENSURE_SUCCESS(rv, rv);
   }
 
   return NS_OK;
@@ -790,11 +730,8 @@ nsHTMLSelectElement::RemoveOptionsFromListRecurse(nsIContent* aOptions,
 
   nsCOMPtr<nsIDOMHTMLOptionElement> optElement(do_QueryInterface(aOptions));
   if (optElement) {
-    if (mOptions->ItemAsOption(aRemoveIndex) != optElement) {
+    nsresult rv = mOptions->RemoveElementAt(aRemoveIndex);
-      NS_ERROR("wrong option at index");
+    NS_ENSURE_SUCCESS(rv, rv);
-      return NS_ERROR_UNEXPECTED;
-    }
-    mOptions->RemoveOptionAt(aRemoveIndex);
     (*aNumRemoved)++;
     return NS_OK;
   }
@@ -804,20 +741,30 @@ nsHTMLSelectElement::RemoveOptionsFromListRecurse(nsIContent* aOptions,
     mNonOptionChildren--;
   }
 
-  // Recurse down deeper for options
+  if (mOptGroupCount) {
-  if (mOptGroupCount && IsOptGroup(aOptions)) {
+    if (IsOptGroup(aOptions)) {
-    mOptGroupCount--;
+      mOptGroupCount--;
-
+      DispatchDOMEvent(NS_LITERAL_STRING("selectHasNoGroups"));
-    PRUint32 numChildren = aOptions->GetChildCount();
-    for (PRUint32 i = 0; i < numChildren; ++i) {
-      nsresult rv = RemoveOptionsFromListRecurse(aOptions->GetChildAt(i),
-                                                 aRemoveIndex,
-                                                 aNumRemoved,
-                                                 aDepth + 1);
-      NS_ENSURE_SUCCESS(rv, rv);
     }
   }
 
+  // Recurse down deeper for options
+  //
+  // I *would* put a restriction in here to only search under
+  // optgroups (and not, for example, <P></P>), but it really
+  // doesn't *hurt* to search under other stuff and it's more
+  // efficient in the normal only-optgroup-and-option case
+  // (one less QueryInterface).
+  PRUint32 numChildren = aOptions->GetChildCount();
+
+  for (PRUint32 i = 0; i < numChildren; ++i) {
+    nsresult rv = RemoveOptionsFromListRecurse(aOptions->GetChildAt(i),
+                                               aRemoveIndex,
+                                               aNumRemoved,
+                                               aDepth + 1);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
   return NS_OK;
 }
 
@@ -871,8 +818,9 @@ NS_IMETHODIMP
 nsHTMLSelectElement::WillRemoveOptions(nsIContent* aParent,
                                        PRInt32 aContentIndex)
 {
+  nsresult rv = NS_OK;
+
   PRInt32 level = GetContentDepth(aParent);
-  NS_ASSERTION(level >= 0, "getting notified by unexpected content");
   if (level == -1) {
     return NS_ERROR_FAILURE;
   }
@@ -890,12 +838,11 @@ nsHTMLSelectElement::WillRemoveOptions(nsIContent* aParent,
       ind = GetFirstOptionIndex(currentKid);
     }
     if (ind != -1) {
-      nsresult rv = RemoveOptionsFromList(currentKid, ind, level);
+      rv = RemoveOptionsFromList(currentKid, ind, level);
-      NS_ENSURE_SUCCESS(rv, rv);
     }
   }
 
-  return NS_OK;
+  return rv;
 }
 
 PRInt32
@@ -1741,7 +1688,7 @@ nsHTMLSelectElement::DoneAddingChildren(PRBool aHaveNotified)
   // content, restore the rest of the options proper-like
   if (mRestoreState) {
     RestoreStateTo(mRestoreState);
-    mRestoreState = nsnull;
+    NS_RELEASE(mRestoreState);
   }
 
   // Notify the frame
@@ -1859,10 +1806,11 @@ nsHTMLSelectElement::HandleDOMEvent(nsPresContext* aPresContext,
 NS_IMETHODIMP
 nsHTMLSelectElement::SaveState()
 {
-  nsRefPtr<nsSelectState> state = new nsSelectState();
+  nsSelectState* state = new nsSelectState();
   if (!state) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
+  NS_ADDREF(state);
 
   PRUint32 len;
   GetLength(&len);
@@ -1888,6 +1836,8 @@ nsHTMLSelectElement::SaveState()
     NS_ASSERTION(NS_SUCCEEDED(rv), "selecteditems set failed!");
   }
 
+  NS_RELEASE(state);
+
   return rv;
 }
 
@@ -1927,6 +1877,7 @@ nsHTMLSelectElement::RestoreStateTo(nsSelectState* aNewSelected)
 {
   if (!mIsDoneAddingChildren) {
     mRestoreState = aNewSelected;
+    NS_ADDREF(mRestoreState);
     return;
   }
 
@@ -2099,58 +2050,6 @@ void nsHTMLSelectElement::DispatchContentReset() {
   }
 }
 
-static void
-AddOptionsRecurse(nsIContent* aRoot, nsHTMLOptionCollection* aArray)
-{
-  nsIContent* child;
-  for(PRUint32 i = 0; (child = aRoot->GetChildAt(i)); ++i) {
-    nsCOMPtr<nsIDOMHTMLOptionElement> opt = do_QueryInterface(child);
-    if (opt) {
-      // If we fail here, then at least we've tried our best
-      aArray->AppendOption(opt);
-    }
-    else if (IsOptGroup(child)) {
-      AddOptionsRecurse(child, aArray);
-    }
-  }
-}
-
-void
-nsHTMLSelectElement::RebuildOptionsArray()
-{
-  mOptions->Clear();
-  AddOptionsRecurse(this, mOptions);
-}
-
-#ifdef DEBUG
-
-static void
-VerifyOptionsRecurse(nsIContent* aRoot, PRInt32& aIndex,
-                     nsHTMLOptionCollection* aArray)
-{
-  nsIContent* child;
-  for(PRUint32 i = 0; (child = aRoot->GetChildAt(i)); ++i) {
-    nsCOMPtr<nsIDOMHTMLOptionElement> opt = do_QueryInterface(child);
-    if (opt) {
-      NS_ASSERTION(opt == aArray->ItemAsOption(aIndex++),
-                   "Options collection broken");
-    }
-    else if (IsOptGroup(child)) {
-      VerifyOptionsRecurse(child, aIndex, aArray);
-    }
-  }
-}
-
-void
-nsHTMLSelectElement::VerifyOptionsArray()
-{
-  PRInt32 aIndex = 0;
-  VerifyOptionsRecurse(this, aIndex, mOptions);
-}
-
-
-#endif
-
 //----------------------------------------------------------------------
 //
 // nsHTMLOptionCollection implementation
@@ -2317,6 +2216,12 @@ nsHTMLOptionCollection::Item(PRUint32 aIndex, nsIDOMNode** aReturn)
   return NS_OK;
 }
 
+nsIDOMHTMLOptionElement *
+nsHTMLOptionCollection::ItemAsOption(PRInt32 aIndex)
+{
+  return mElements.SafeObjectAt(aIndex);
+}
+
 NS_IMETHODIMP
 nsHTMLOptionCollection::NamedItem(const nsAString& aName,
                                   nsIDOMNode** aReturn)
@@ -2350,3 +2255,16 @@ nsHTMLOptionCollection::GetSelect(nsIDOMHTMLSelectElement **aReturn)
   NS_IF_ADDREF(*aReturn = mSelect);
   return NS_OK;
 }
+
+nsresult
+nsHTMLOptionCollection::InsertElementAt(nsIDOMHTMLOptionElement* aOption,
+                                        PRInt32 aIndex)
+{
+  return mElements.InsertObjectAt(aOption, aIndex);
+}
+
+nsresult
+nsHTMLOptionCollection::RemoveElementAt(PRInt32 aIndex)
+{
+  return mElements.RemoveObjectAt(aIndex);
+}