Bug 641720 - Make nsHTMLInputElement::GetRadioGroupContainer saner. r=sicking,bz

content/html/content/src/nsHTMLInputElement.cpp

@@ -71,6 +71,7 @@
 #include "nsIEditor.h"
 #include "nsGUIEvent.h"
 #include "nsIIOService.h"
+#include "nsDocument.h"
 
 #include "nsPresState.h"
 #include "nsLayoutErrors.h"
@@ -909,12 +910,13 @@ nsHTMLInputElement::AfterSetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
     }
 
     if (mType == NS_FORM_INPUT_RADIO && aName == nsGkAtoms::required) {
-      nsCOMPtr<nsIRadioGroupContainer> c = GetRadioGroupContainer();
+      nsIRadioGroupContainer* c = GetRadioGroupContainer();
       nsCOMPtr<nsIRadioGroupContainer_MOZILLA_2_0_BRANCH> container =
         do_QueryInterface(c);
-      nsAutoString name;
 
-      if (container && GetNameIfExists(name)) {
+      if (container) {
+        nsAutoString name;
+        GetAttr(kNameSpaceID_None, nsGkAtoms::name, name);
         container->RadioRequiredChanged(name, this);
       }
     }
@@ -1576,12 +1578,11 @@ nsHTMLInputElement::DoSetChecked(PRBool aChecked, PRBool aNotify,
     if (aChecked) {
       rv = RadioSetChecked(aNotify);
     } else {
-      nsCOMPtr<nsIRadioGroupContainer> container = GetRadioGroupContainer();
+      nsIRadioGroupContainer* container = GetRadioGroupContainer();
       if (container) {
         nsAutoString name;
-        if (GetNameIfExists(name)) {
+        GetAttr(kNameSpaceID_None, nsGkAtoms::name, name);
-          container->SetCurrentRadioButton(name, nsnull);
+        container->SetCurrentRadioButton(name, nsnull);
-        }
       }
       // SetCheckedInternal is going to ask all radios to update their
       // validity state. We have to be sure the radio group container knows
@@ -1618,9 +1619,10 @@ nsHTMLInputElement::RadioSetChecked(PRBool aNotify)
   //
   // Let the group know that we are now the One True Radio Button
   //
-  nsCOMPtr<nsIRadioGroupContainer> container = GetRadioGroupContainer();
+  nsIRadioGroupContainer* container = GetRadioGroupContainer();
-  nsAutoString name;
+  if (container) {
-  if (container && GetNameIfExists(name)) {
+    nsAutoString name;
+    GetAttr(kNameSpaceID_None, nsGkAtoms::name, name);
     rv = container->SetCurrentRadioButton(name, this);
   }
 
@@ -1634,35 +1636,38 @@ nsHTMLInputElement::RadioSetChecked(PRBool aNotify)
   return rv;
 }
 
-/* virtual */ already_AddRefed<nsIRadioGroupContainer>
+nsIRadioGroupContainer*
-nsHTMLInputElement::GetRadioGroupContainer()
+nsHTMLInputElement::GetRadioGroupContainer() const
 {
-  nsIRadioGroupContainer* retval = nsnull;
+  NS_ASSERTION(mType == NS_FORM_INPUT_RADIO,
-  if (mForm) {
+               "GetRadioGroupContainer should only be called when type='radio'");
-    CallQueryInterface(mForm, &retval);
+
-  } else {
+  nsAutoString name;
-    nsIDocument* currentDoc = GetCurrentDoc();
+  GetAttr(kNameSpaceID_None, nsGkAtoms::name, name);
-    if (currentDoc) {
+
-      CallQueryInterface(currentDoc, &retval);
+  if (name.IsEmpty()) {
-    }
+    return nsnull;
   }
-  return retval;
+
+  if (mForm) {
+    return mForm;
+  }
+
+  return static_cast<nsDocument*>(GetCurrentDoc());
 }
 
 already_AddRefed<nsIDOMHTMLInputElement>
 nsHTMLInputElement::GetSelectedRadioButton()
 {
   nsIDOMHTMLInputElement* selected;
-  nsCOMPtr<nsIRadioGroupContainer> container = GetRadioGroupContainer();
+  nsIRadioGroupContainer* container = GetRadioGroupContainer();
 
   if (!container) {
     return nsnull;
   }
 
   nsAutoString name;
-  if (!GetNameIfExists(name)) {
+  GetAttr(kNameSpaceID_None, nsGkAtoms::name, name);
-    return nsnull;
-  }
 
   container->GetCurrentRadioButton(name, &selected);
   return selected;
@@ -2279,29 +2284,28 @@ nsHTMLInputElement::PostHandleEvent(nsEventChainPostVisitor& aVisitor)
               case NS_VK_DOWN:
               case NS_VK_RIGHT:
               // Arrow key pressed, focus+select prev/next radio button
-              nsCOMPtr<nsIRadioGroupContainer> container = GetRadioGroupContainer();
+              nsIRadioGroupContainer* container = GetRadioGroupContainer();
               if (container) {
                 nsAutoString name;
-                if (GetNameIfExists(name)) {
+                GetAttr(kNameSpaceID_None, nsGkAtoms::name, name);
-                  nsCOMPtr<nsIDOMHTMLInputElement> selectedRadioButton;
+                nsCOMPtr<nsIDOMHTMLInputElement> selectedRadioButton;
-                  container->GetNextRadioButton(name, isMovingBack, this,
+                container->GetNextRadioButton(name, isMovingBack, this,
-                                                getter_AddRefs(selectedRadioButton));
+                                              getter_AddRefs(selectedRadioButton));
-                  nsCOMPtr<nsIContent> radioContent =
+                nsCOMPtr<nsIContent> radioContent =
-                    do_QueryInterface(selectedRadioButton);
+                  do_QueryInterface(selectedRadioButton);
-                  if (radioContent) {
+                if (radioContent) {
-                    rv = selectedRadioButton->Focus();
+                  rv = selectedRadioButton->Focus();
+                  if (NS_SUCCEEDED(rv)) {
+                    nsEventStatus status = nsEventStatus_eIgnore;
+                    nsMouseEvent event(NS_IS_TRUSTED_EVENT(aVisitor.mEvent),
+                                       NS_MOUSE_CLICK, nsnull,
+                                       nsMouseEvent::eReal);
+                    event.inputSource = nsIDOMNSMouseEvent::MOZ_SOURCE_KEYBOARD;
+                    rv = nsEventDispatcher::Dispatch(radioContent,
+                                                     aVisitor.mPresContext,
+                                                     &event, nsnull, &status);
                     if (NS_SUCCEEDED(rv)) {
-                      nsEventStatus status = nsEventStatus_eIgnore;
+                      aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
-                      nsMouseEvent event(NS_IS_TRUSTED_EVENT(aVisitor.mEvent),
-                                         NS_MOUSE_CLICK, nsnull,
-                                         nsMouseEvent::eReal);
-                      event.inputSource = nsIDOMNSMouseEvent::MOZ_SOURCE_KEYBOARD;
-                      rv = nsEventDispatcher::Dispatch(radioContent,
-                                                       aVisitor.mPresContext,
-                                                       &event, nsnull, &status);
-                      if (NS_SUCCEEDED(rv)) {
-                        aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
-                      }
                     }
                   }
                 }
@@ -3406,69 +3410,40 @@ nsHTMLInputElement::AddedToRadioGroup()
   nsCOMPtr<nsIRadioGroupContainer> container = GetRadioGroupContainer();
   if (container) {
     nsAutoString name;
-    if (GetNameIfExists(name)) {
+    GetAttr(kNameSpaceID_None, nsGkAtoms::name, name);
-      container->AddToRadioGroup(name, static_cast<nsIFormControl*>(this));
+    container->AddToRadioGroup(name, static_cast<nsIFormControl*>(this));
 
-      // We initialize the validity of the element to the validity of the group
+    // We initialize the validity of the element to the validity of the group
-      // because we assume UpdateValueMissingState() will be called after.
+    // because we assume UpdateValueMissingState() will be called after.
-      nsCOMPtr<nsIRadioGroupContainer_MOZILLA_2_0_BRANCH> container2 =
+    nsCOMPtr<nsIRadioGroupContainer_MOZILLA_2_0_BRANCH> container2 =
-        do_QueryInterface(container);
+      do_QueryInterface(container);
-      SetValidityState(VALIDITY_STATE_VALUE_MISSING,
+    SetValidityState(VALIDITY_STATE_VALUE_MISSING,
-                       container2->GetValueMissingState(name));
+                     container2->GetValueMissingState(name));
-    }
   }
 }
 
 void
 nsHTMLInputElement::WillRemoveFromRadioGroup()
 {
-  //
+  nsIRadioGroupContainer* container = GetRadioGroupContainer();
-  // If the input element is not in a form and
+  if (!container) {
-  // not in a document, we just need to return.
-  //
-  if (!mForm && !(IsInDoc() && GetParent())) {
     return;
   }
 
-  //
+  nsAutoString name;
+  GetAttr(kNameSpaceID_None, nsGkAtoms::name, name);
+
   // If this button was checked, we need to notify the group that there is no
   // longer a selected radio button
-  //
-  nsAutoString name;
-  PRBool gotName = PR_FALSE;
   if (GetChecked()) {
-    if (!gotName) {
+    container->SetCurrentRadioButton(name, nsnull);
-      if (!GetNameIfExists(name)) {
-        // If the name doesn't exist, nothing is going to happen anyway
-        return;
-      }
-      gotName = PR_TRUE;
-    }
-
-    nsCOMPtr<nsIRadioGroupContainer> container = GetRadioGroupContainer();
-    if (container) {
-      container->SetCurrentRadioButton(name, nsnull);
-    }
   }
 
-  //
+  // Remove this radio from its group in the container.
-  // Remove this radio from its group in the container
+  // We need to call UpdateValueMissingValidityStateForRadio before to make sure
-  //
+  // the group validity is updated (with this element being ignored).
-  nsCOMPtr<nsIRadioGroupContainer> container = GetRadioGroupContainer();
+  UpdateValueMissingValidityStateForRadio(true);
-  if (container) {
+  container->RemoveFromRadioGroup(name, static_cast<nsIFormControl*>(this));
-    if (!gotName) {
-      if (!GetNameIfExists(name)) {
-        // If the name doesn't exist, nothing is going to happen anyway
-        return;
-      }
-      gotName = PR_TRUE;
-    }
-
-    UpdateValueMissingValidityStateForRadio(true);
-
-    container->RemoveFromRadioGroup(name,
-                                    static_cast<nsIFormControl*>(this));
-  }
 }
 
 PRBool
@@ -3529,13 +3504,15 @@ nsHTMLInputElement::IsHTMLFocusable(PRBool aWithMouse, PRBool *aIsFocusable, PRI
 
   // Current radio button is not selected.
   // But make it tabbable if nothing in group is selected.
-  nsCOMPtr<nsIRadioGroupContainer> container = GetRadioGroupContainer();
+  nsIRadioGroupContainer* container = GetRadioGroupContainer();
-  nsAutoString name;
+  if (!container) {
-  if (!container || !GetNameIfExists(name)) {
     *aIsFocusable = defaultFocusable;
     return PR_FALSE;
   }
 
+  nsAutoString name;
+  GetAttr(kNameSpaceID_None, nsGkAtoms::name, name);
+
   nsCOMPtr<nsIDOMHTMLInputElement> currentRadio;
   container->GetCurrentRadioButton(name, getter_AddRefs(currentRadio));
   if (currentRadio) {
@@ -3548,19 +3525,15 @@ nsHTMLInputElement::IsHTMLFocusable(PRBool aWithMouse, PRBool *aIsFocusable, PRI
 nsresult
 nsHTMLInputElement::VisitGroup(nsIRadioVisitor* aVisitor, PRBool aFlushContent)
 {
-  nsresult rv = NS_OK;
+  nsIRadioGroupContainer* container = GetRadioGroupContainer();
-  nsCOMPtr<nsIRadioGroupContainer> container = GetRadioGroupContainer();
   if (container) {
     nsAutoString name;
-    if (GetNameIfExists(name)) {
+    GetAttr(kNameSpaceID_None, nsGkAtoms::name, name);
-      rv = container->WalkRadioGroup(name, aVisitor, aFlushContent);
+    return container->WalkRadioGroup(name, aVisitor, aFlushContent);
-    } else {
-      aVisitor->Visit(this);
-    }
-  } else {
-    aVisitor->Visit(this);
   }
-  return rv;
+
+  aVisitor->Visit(this);
+  return NS_OK;
 }
 
 nsHTMLInputElement::ValueModeType
@@ -3839,15 +3812,21 @@ nsHTMLInputElement::UpdateValueMissingValidityStateForRadio(bool aIgnoreSelf)
                               : HasAttr(kNameSpaceID_None, nsGkAtoms::required);
   bool valueMissing = false;
 
-  nsCOMPtr<nsIRadioGroupContainer> c = GetRadioGroupContainer();
+  nsIRadioGroupContainer* c = GetRadioGroupContainer();
   nsCOMPtr<nsIRadioGroupContainer_MOZILLA_2_0_BRANCH> container =
     do_QueryInterface(c);
+
+  if (!container) {
+    SetValidityState(VALIDITY_STATE_VALUE_MISSING, required && !selected);
+    return;
+  }
+
   nsAutoString name;
-  GetNameIfExists(name);
+  GetAttr(kNameSpaceID_None, nsGkAtoms::name, name);
 
   // If the current radio is required and not ignored, we can assume the entire
   // group is required.
-  if (!required && container && !name.IsEmpty()) {
+  if (!required) {
     required = (aIgnoreSelf && HasAttr(kNameSpaceID_None, nsGkAtoms::required))
                  ? container->GetRequiredRadioCount(name) - 1
                  : container->GetRequiredRadioCount(name);
@@ -3855,18 +3834,14 @@ nsHTMLInputElement::UpdateValueMissingValidityStateForRadio(bool aIgnoreSelf)
 
   valueMissing = required && !selected;
 
-  if (container && !name.IsEmpty()) {
+  if (container->GetValueMissingState(name) != valueMissing) {
-    if (container->GetValueMissingState(name) != valueMissing) {
+    container->SetValueMissingState(name, valueMissing);
-      container->SetValueMissingState(name, valueMissing);
 
-      SetValidityState(VALIDITY_STATE_VALUE_MISSING, valueMissing);
-
-      nsCOMPtr<nsIRadioVisitor> visitor =
-        new nsRadioSetValueMissingState(this, valueMissing, notify);
-      VisitGroup(visitor, notify);
-    }
-  } else {
     SetValidityState(VALIDITY_STATE_VALUE_MISSING, valueMissing);
+
+    nsCOMPtr<nsIRadioVisitor> visitor =
+      new nsRadioSetValueMissingState(this, valueMissing, notify);
+    VisitGroup(visitor, notify);
   }
 }
 

content/html/content/src/nsHTMLInputElement.h

@@ -225,11 +225,6 @@ public:
   }
   void AddedToRadioGroup();
   void WillRemoveFromRadioGroup();
-  /**
-   * Get the radio group container for this button (form or document)
-   * @return the radio group container (or null if no form or document)
-   */
-  virtual already_AddRefed<nsIRadioGroupContainer> GetRadioGroupContainer();
 
  /**
    * Helper function returning the currently selected button in the radio group.
@@ -567,6 +562,14 @@ protected:
     }
   }
 
+  /**
+   * Returns the radio group container if the element has one, null otherwise.
+   * The radio group container will be the form owner if there is one.
+   * The current document otherwise.
+   * @return the radio group container if the element has one, null otherwise.
+   */
+  nsIRadioGroupContainer* GetRadioGroupContainer() const;
+
   nsCOMPtr<nsIControllers> mControllers;
 
   /**