Bug 596511 (3/3 - <select> can be invalid if no option is selected or all of them have empty value. r=sicking a=blocking-betaN
authorMounir Lamouri <mounir.lamouri@gmail.com>
Wed, 17 Nov 2010 00:41:19 +0100
changeset 57620 433b919425d6cc473aa3dc3859bd11cda3d9f078
parent 57619 ccc6520fee81946a711e6770ea5e579f7fa68bde
child 57621 6f47bd55ca9e7eb51deeaf3744102549a3c6724b
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewerssicking, blocking-betaN
bugs596511
milestone2.0b8pre
Bug 596511 (3/3 - <select> can be invalid if no option is selected or all of them have empty value. r=sicking a=blocking-betaN
content/html/content/public/nsISelectElement.idl
content/html/content/src/nsHTMLOptGroupElement.cpp
content/html/content/src/nsHTMLSelectElement.cpp
content/html/content/src/nsHTMLSelectElement.h
content/html/content/test/test_bug596511.html
layout/reftests/css-invalid/select/reftest.list
layout/reftests/css-invalid/select/select-required-invalid.html
layout/reftests/css-invalid/select/select-required-multiple-invalid.html
layout/reftests/css-invalid/select/select-required-multiple-ref.html
layout/reftests/css-invalid/select/select-required-multiple-valid.html
layout/reftests/css-invalid/select/select-required-ref.html
layout/reftests/css-invalid/select/select-required-valid.html
layout/reftests/css-valid/select/reftest.list
layout/reftests/css-valid/select/select-required-invalid.html
layout/reftests/css-valid/select/select-required-multiple-invalid.html
layout/reftests/css-valid/select/select-required-multiple-ref.html
layout/reftests/css-valid/select/select-required-multiple-valid.html
layout/reftests/css-valid/select/select-required-ref.html
layout/reftests/css-valid/select/select-required-valid.html
--- a/content/html/content/public/nsISelectElement.idl
+++ b/content/html/content/public/nsISelectElement.idl
@@ -43,44 +43,46 @@ interface nsIDOMHTMLOptionElement;
 /** 
  * This interface is used to notify a SELECT when OPTION
  * elements are added and removed from its subtree.
  * Note that the nsIDOMHTMLSelectElement and nsIContent 
  * interfaces are the ones to use to access and enumerate
  * OPTIONs within a SELECT element.
  */
 
-[scriptable, uuid(35bd8ed5-5f34-4126-8c4f-38ba01681836)]
+[scriptable, uuid(aa73a61a-8ef2-402d-b86c-3a5c5f2a6027)]
 interface nsISelectElement : nsISupports
 {
 
   /**
    * To be called when stuff is added under a child of the select--but *before*
    * they are actually added.
    *
    * @param aOptions the content that was added (usually just an option, but
    *        could be an optgroup node with many child options)
    * @param aParent the parent the options were added to (could be an optgroup)
    * @param aContentIndex the index where the options are being added within the
    *        parent (if the parent is an optgroup, the index within the optgroup)
    */
   [noscript] void willAddOptions(in nsIContent aOptions,
                                  in nsIContent aParent,
-                                 in long aContentIndex);
+                                 in long aContentIndex,
+                                 in boolean aNotify);
 
   /**
    * To be called when stuff is removed under a child of the select--but
    * *before* they are actually removed.
    *
    * @param aParent the parent the option(s) are being removed from
    * @param aContentIndex the index of the option(s) within the parent (if the
    *        parent is an optgroup, the index within the optgroup)
    */
   [noscript] void willRemoveOptions(in nsIContent aParent,
-                                    in long aContentIndex);
+                                    in long aContentIndex,
+                                    in boolean aNotify);
 
   /**
    * Checks whether an option is disabled (even if it's part of an optgroup)
    *
    * @param aIndex the index of the option to check
    * @return whether the option is disabled
    */
   boolean isOptionDisabled(in long aIndex);
--- a/content/html/content/src/nsHTMLOptGroupElement.cpp
+++ b/content/html/content/src/nsHTMLOptGroupElement.cpp
@@ -170,28 +170,28 @@ nsHTMLOptGroupElement::GetSelect()
   return nsnull;
 }
 
 nsresult
 nsHTMLOptGroupElement::InsertChildAt(nsIContent* aKid,
                                      PRUint32 aIndex,
                                      PRBool aNotify)
 {
-  nsSafeOptionListMutation safeMutation(GetSelect(), this, aKid, aIndex);
+  nsSafeOptionListMutation safeMutation(GetSelect(), this, aKid, aIndex, aNotify);
   nsresult rv = nsGenericHTMLElement::InsertChildAt(aKid, aIndex, aNotify);
   if (NS_FAILED(rv)) {
     safeMutation.MutationFailed();
   }
   return rv;
 }
 
 nsresult
 nsHTMLOptGroupElement::RemoveChildAt(PRUint32 aIndex, PRBool aNotify, PRBool aMutationEvent)
 {
-  nsSafeOptionListMutation safeMutation(GetSelect(), this, nsnull, aIndex);
+  nsSafeOptionListMutation safeMutation(GetSelect(), this, nsnull, aIndex, aNotify);
   nsresult rv = nsGenericHTMLElement::RemoveChildAt(aIndex, aNotify, aMutationEvent);
   if (NS_FAILED(rv)) {
     safeMutation.MutationFailed();
   }
   return rv;
 }
 
 nsEventStates
--- a/content/html/content/src/nsHTMLSelectElement.cpp
+++ b/content/html/content/src/nsHTMLSelectElement.cpp
@@ -81,17 +81,18 @@ NS_DEFINE_STATIC_IID_ACCESSOR(nsSelectSt
 //----------------------------------------------------------------------
 //
 // nsSafeOptionListMutation
 //
 
 nsSafeOptionListMutation::nsSafeOptionListMutation(nsIContent* aSelect,
                                                    nsIContent* aParent,
                                                    nsIContent* aKid,
-                                                   PRUint32 aIndex)
+                                                   PRUint32 aIndex,
+                                                   PRBool aNotify)
   : mSelect(do_QueryInterface(aSelect)), mTopLevelMutation(PR_FALSE),
     mNeedsRebuild(PR_FALSE)
 {
   nsHTMLSelectElement* select = static_cast<nsHTMLSelectElement*>(mSelect.get());
   if (select) {
     mTopLevelMutation = !select->mMutating;
     if (mTopLevelMutation) {
       select->mMutating = PR_TRUE;
@@ -99,19 +100,19 @@ nsSafeOptionListMutation::nsSafeOptionLi
       // This is very unfortunate, but to handle mutation events properly,
       // option list must be up-to-date before inserting or removing options.
       // Fortunately this is called only if mutation event listener
       // adds or removes options.
       select->RebuildOptionsArray();
     }
     nsresult rv;
     if (aKid) {
-      rv = mSelect->WillAddOptions(aKid, aParent, aIndex);
+      rv = mSelect->WillAddOptions(aKid, aParent, aIndex, aNotify);
     } else {
-      rv = mSelect->WillRemoveOptions(aParent, aIndex);
+      rv = mSelect->WillRemoveOptions(aParent, aIndex, aNotify);
     }
     mNeedsRebuild = NS_FAILED(rv);
   }
 }
 
 nsSafeOptionListMutation::~nsSafeOptionListMutation()
 {
   if (mSelect) {
@@ -221,43 +222,44 @@ nsHTMLSelectElement::GetForm(nsIDOMHTMLF
   return nsGenericHTMLFormElement::GetForm(aForm);
 }
 
 nsresult
 nsHTMLSelectElement::InsertChildAt(nsIContent* aKid,
                                    PRUint32 aIndex,
                                    PRBool aNotify)
 {
-  nsSafeOptionListMutation safeMutation(this, this, aKid, aIndex);
+  nsSafeOptionListMutation safeMutation(this, this, aKid, aIndex, aNotify);
   nsresult rv = nsGenericHTMLFormElement::InsertChildAt(aKid, aIndex, aNotify);
   if (NS_FAILED(rv)) {
     safeMutation.MutationFailed();
   }
   return rv;
 }
 
 nsresult
 nsHTMLSelectElement::RemoveChildAt(PRUint32 aIndex, PRBool aNotify, PRBool aMutationEvent)
 {
   NS_ASSERTION(aMutationEvent, "Someone tried to inhibit mutations on select child removal.");
-  nsSafeOptionListMutation safeMutation(this, this, nsnull, aIndex);
+  nsSafeOptionListMutation safeMutation(this, this, nsnull, aIndex, aNotify);
   nsresult rv = nsGenericHTMLFormElement::RemoveChildAt(aIndex, aNotify, aMutationEvent);
   if (NS_FAILED(rv)) {
     safeMutation.MutationFailed();
   }
   return rv;
 }
 
 
 // SelectElement methods
 
 nsresult
 nsHTMLSelectElement::InsertOptionsIntoList(nsIContent* aOptions,
                                            PRInt32 aListIndex,
-                                           PRInt32 aDepth)
+                                           PRInt32 aDepth,
+                                           PRBool aNotify)
 {
   PRInt32 insertIndex = aListIndex;
   nsresult rv = InsertOptionsIntoListRecurse(aOptions, &insertIndex, aDepth);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Deal with the selected list
   if (insertIndex - aListIndex) {
     // Fix the currently selected index
@@ -301,26 +303,27 @@ nsHTMLSelectElement::InsertOptionsIntoLi
           // This is sort of a hack ... we need to notify that the option was
           // set and change selectedIndex even though we didn't really change
           // its value.
           OnOptionSelected(selectFrame, i, PR_TRUE, PR_FALSE, PR_FALSE);
         }
       }
     }
 
-    CheckSelectSomething();
+    CheckSelectSomething(aNotify);
   }
 
   return NS_OK;
 }
 
 nsresult
 nsHTMLSelectElement::RemoveOptionsFromList(nsIContent* aOptions,
                                            PRInt32 aListIndex,
-                                           PRInt32 aDepth)
+                                           PRInt32 aDepth,
+                                           PRBool aNotify)
 {
   PRInt32 numRemoved = 0;
   nsresult rv = RemoveOptionsFromListRecurse(aOptions, aListIndex, &numRemoved,
                                              aDepth);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (numRemoved) {
     // Tell the widget we removed the options
@@ -342,17 +345,30 @@ nsHTMLSelectElement::RemoveOptionsFromLi
         // Shift the selected index if something in front of it was removed
         // aListIndex+numRemoved <= mSelectedIndex
         mSelectedIndex -= numRemoved;
       }
     }
 
     // Select something in case we removed the selected option on a
     // single select
-    CheckSelectSomething();
+    if (!CheckSelectSomething(aNotify) && mSelectedIndex == -1) {
+      // Update the validity state in case of we've just removed the last
+      // option.
+      UpdateValueMissingValidityState();
+
+      if (aNotify) {
+        nsIDocument* doc = GetCurrentDoc();
+        if (doc) {
+          MOZ_AUTO_DOC_UPDATE(doc, UPDATE_CONTENT_STATE, PR_TRUE);
+          doc->ContentStatesChanged(this, nsnull, NS_EVENT_STATE_VALID |
+                                                  NS_EVENT_STATE_INVALID);
+        }
+      }
+    }
   }
 
   return NS_OK;
 }
 
 static PRBool IsOptGroup(nsIContent *aContent)
 {
   return (aContent->NodeInfo()->Equals(nsGkAtoms::optgroup) &&
@@ -450,17 +466,18 @@ nsHTMLSelectElement::RemoveOptionsFromLi
 // XXXldb Doing the processing before the content nodes have been added
 // to the document (as the name of this function seems to require, and
 // as the callers do), is highly unusual.  Passing around unparented
 // content to other parts of the app can make those things think the
 // options are the root content node.
 NS_IMETHODIMP
 nsHTMLSelectElement::WillAddOptions(nsIContent* aOptions,
                                     nsIContent* aParent,
-                                    PRInt32 aContentIndex)
+                                    PRInt32 aContentIndex,
+                                    PRBool aNotify)
 {
   PRInt32 level = GetContentDepth(aParent);
   if (level == -1) {
     return NS_ERROR_FAILURE;
   }
 
   // Get the index where the options will be inserted
   PRInt32 ind = -1;
@@ -485,22 +502,23 @@ nsHTMLSelectElement::WillAddOptions(nsIC
       if (currentKid) {
         ind = GetOptionIndexAt(currentKid);
       } else {
         ind = -1;
       }
     }
   }
 
-  return InsertOptionsIntoList(aOptions, ind, level);
+  return InsertOptionsIntoList(aOptions, ind, level, aNotify);
 }
 
 NS_IMETHODIMP
 nsHTMLSelectElement::WillRemoveOptions(nsIContent* aParent,
-                                       PRInt32 aContentIndex)
+                                       PRInt32 aContentIndex,
+                                       PRBool aNotify)
 {
   PRInt32 level = GetContentDepth(aParent);
   NS_ASSERTION(level >= 0, "getting notified by unexpected content");
   if (level == -1) {
     return NS_ERROR_FAILURE;
   }
 
   // Get the index where the options will be removed
@@ -511,17 +529,17 @@ nsHTMLSelectElement::WillRemoveOptions(n
       // If there are no artifacts, aContentIndex == ind
       ind = aContentIndex;
     } else {
       // If there are artifacts, we have to get the index of the option the
       // hard way
       ind = GetFirstOptionIndex(currentKid);
     }
     if (ind != -1) {
-      nsresult rv = RemoveOptionsFromList(currentKid, ind, level);
+      nsresult rv = RemoveOptionsFromList(currentKid, ind, level, aNotify);
       NS_ENSURE_SUCCESS(rv, rv);
     }
   }
 
   return NS_OK;
 }
 
 PRInt32
@@ -852,16 +870,26 @@ nsHTMLSelectElement::OnOptionSelected(ns
       optionElement->SetSelectedInternal(aSelected, aNotify);
     }
   }
 
   // Let the frame know too
   if (aSelectFrame) {
     aSelectFrame->OnOptionSelected(aIndex, aSelected);
   }
+
+  UpdateValueMissingValidityState();
+  if (aNotify) {
+    nsIDocument* doc = GetCurrentDoc();
+    if (doc) {
+      MOZ_AUTO_DOC_UPDATE(doc, UPDATE_CONTENT_STATE, PR_TRUE);
+      doc->ContentStatesChanged(this, nsnull, NS_EVENT_STATE_VALID |
+                                              NS_EVENT_STATE_INVALID);
+    }
+  }
 }
 
 void
 nsHTMLSelectElement::FindSelectedIndex(PRInt32 aStartIndex)
 {
   mSelectedIndex = -1;
   PRUint32 len;
   GetLength(&len);
@@ -1078,17 +1106,17 @@ nsHTMLSelectElement::SetOptionsSelectedB
           optionsDeselected = PR_TRUE;
         }
       }
     }
   }
 
   // Make sure something is selected unless we were set to -1 (none)
   if (optionsDeselected && aStartIndex != -1) {
-    optionsSelected = CheckSelectSomething() || optionsSelected;
+    optionsSelected = CheckSelectSomething(aNotify) || optionsSelected;
   }
 
   // Let the caller know whether anything was changed
   if (optionsSelected || optionsDeselected) {
     if (aChangedSomething)
       *aChangedSomething = PR_TRUE;
   }
 
@@ -1256,43 +1284,54 @@ nsHTMLSelectElement::Item(PRUint32 aInde
 NS_IMETHODIMP
 nsHTMLSelectElement::NamedItem(const nsAString& aName,
                                nsIDOMNode** aReturn)
 {
   return mOptions->NamedItem(aName, aReturn);
 }
 
 PRBool
-nsHTMLSelectElement::CheckSelectSomething()
+nsHTMLSelectElement::CheckSelectSomething(PRBool aNotify)
 {
   if (mIsDoneAddingChildren) {
     if (mSelectedIndex < 0 && IsCombobox()) {
-      return SelectSomething();
+      return SelectSomething(aNotify);
     }
   }
   return PR_FALSE;
 }
 
 PRBool
-nsHTMLSelectElement::SelectSomething()
+nsHTMLSelectElement::SelectSomething(PRBool aNotify)
 {
   // If we're not done building the select, don't play with this yet.
   if (!mIsDoneAddingChildren) {
     return PR_FALSE;
   }
 
   PRUint32 count;
   GetLength(&count);
   for (PRUint32 i=0; i<count; i++) {
     PRBool disabled;
     nsresult rv = IsOptionDisabled(i, &disabled);
 
     if (NS_FAILED(rv) || !disabled) {
       rv = SetSelectedIndex(i);
       NS_ENSURE_SUCCESS(rv, PR_FALSE);
+
+      UpdateValueMissingValidityState();
+      if (aNotify) {
+        nsIDocument* doc = GetCurrentDoc();
+        if (doc) {
+          MOZ_AUTO_DOC_UPDATE(doc, UPDATE_CONTENT_STATE, PR_TRUE);
+          doc->ContentStatesChanged(this, nsnull, NS_EVENT_STATE_VALID |
+                                                  NS_EVENT_STATE_INVALID);
+        }
+      }
+
       return PR_TRUE;
     }
   }
 
   return PR_FALSE;
 }
 
 nsresult
@@ -1324,25 +1363,33 @@ nsHTMLSelectElement::BeforeSetAttr(PRInt
   return nsGenericHTMLFormElement::BeforeSetAttr(aNameSpaceID, aName,
                                                  aValue, aNotify);
 }
 
 nsresult
 nsHTMLSelectElement::AfterSetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
                                   const nsAString* aValue, PRBool aNotify)
 {
-  if (aName == nsGkAtoms::disabled && aNameSpaceID == kNameSpaceID_None) {
-    UpdateBarredFromConstraintValidation();
-    if (aNotify) {
-      nsIDocument* doc = GetCurrentDoc();
-      if (doc) {
-        MOZ_AUTO_DOC_UPDATE(doc, UPDATE_CONTENT_STATE, PR_TRUE);
-        doc->ContentStatesChanged(this, nsnull, NS_EVENT_STATE_VALID |
-                                                NS_EVENT_STATE_INVALID);
-      }
+  nsEventStates states;
+
+  if (aNameSpaceID == kNameSpaceID_None) {
+    if (aName == nsGkAtoms::disabled) {
+      UpdateBarredFromConstraintValidation();
+      states |= NS_EVENT_STATE_VALID | NS_EVENT_STATE_INVALID;
+    } else if (aName == nsGkAtoms::required) {
+      UpdateValueMissingValidityState();
+      states |= NS_EVENT_STATE_VALID | NS_EVENT_STATE_INVALID;
+    }
+  }
+
+  if (aNotify && !states.IsEmpty()) {
+    nsIDocument* doc = GetCurrentDoc();
+    if (doc) {
+      MOZ_AUTO_DOC_UPDATE(doc, UPDATE_CONTENT_STATE, PR_TRUE);
+      doc->ContentStatesChanged(this, nsnull, states);
     }
   }
 
   return nsGenericHTMLFormElement::AfterSetAttr(aNameSpaceID, aName,
                                                 aValue, aNotify);
 }
 
 nsresult
@@ -1365,17 +1412,17 @@ nsHTMLSelectElement::UnsetAttr(PRInt32 a
   nsresult rv = nsGenericHTMLFormElement::UnsetAttr(aNameSpaceID, aAttribute,
                                                     aNotify);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (aNotify && aNameSpaceID == kNameSpaceID_None &&
       aAttribute == nsGkAtoms::multiple) {
     // We might have become a combobox; make sure _something_ gets
     // selected in that case
-    CheckSelectSomething();
+    CheckSelectSomething(aNotify);
   }
 
   return rv;
 }
 
 PRBool
 nsHTMLSelectElement::IsDoneAddingChildren()
 {
@@ -1403,17 +1450,22 @@ nsHTMLSelectElement::DoneAddingChildren(
 
   // Restore state
   if (!mInhibitStateRestoration) {
     RestoreFormControlState(this, this);
   }
 
   // Now that we're done, select something (if it's a single select something
   // must be selected)
-  CheckSelectSomething();
+  if (!CheckSelectSomething(PR_FALSE)) {
+    // If an option has @selected set, it will be selected during parsing but
+    // with an empty value. We have to make sure the select element updates it's
+    // validity state to take this into account.
+    UpdateValueMissingValidityState();
+  }
 
   return NS_OK;
 }
 
 PRBool
 nsHTMLSelectElement::ParseAttribute(PRInt32 aNamespaceID,
                                     nsIAtom* aAttribute,
                                     const nsAString& aValue,
@@ -1634,17 +1686,17 @@ nsHTMLSelectElement::Reset()
       }
     }
   }
 
   //
   // If nothing was selected and it's not multiple, select something
   //
   if (numSelected == 0 && IsCombobox()) {
-    SelectSomething();
+    SelectSomething(PR_TRUE);
   }
 
   //
   // Let the frame know we were reset
   //
   // Don't flush, if there's no frame yet it won't care about us being
   // reset even if we forced it to be created now.
   //
@@ -1771,16 +1823,85 @@ AddOptionsRecurse(nsIContent* aRoot, nsH
 void
 nsHTMLSelectElement::RebuildOptionsArray()
 {
   mOptions->Clear();
   AddOptionsRecurse(this, mOptions);
   FindSelectedIndex(0);
 }
 
+bool
+nsHTMLSelectElement::IsValueMissing()
+{
+  if (!HasAttr(kNameSpaceID_None, nsGkAtoms::required)) {
+    return false;
+  }
+
+  PRUint32 length;
+  nsIDOMHTMLOptionElement* option = nsnull;
+  PRBool disabled;
+  PRBool selected;
+
+  mOptions->GetLength(&length);
+
+  for (PRUint32 i=0; i<length; ++i) {
+    option = mOptions->ItemAsOption(i);
+    NS_ENSURE_SUCCESS(option->GetSelected(&selected), false);
+
+    if (!selected) {
+      continue;
+    }
+
+    IsOptionDisabled(i, &disabled);
+    if (disabled) {
+      continue;
+    }
+
+    nsAutoString value;
+    NS_ENSURE_SUCCESS(option->GetValue(value), false);
+    if (!value.IsEmpty()) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
+void
+nsHTMLSelectElement::UpdateValueMissingValidityState()
+{
+  SetValidityState(VALIDITY_STATE_VALUE_MISSING, IsValueMissing());
+}
+
+nsresult
+nsHTMLSelectElement::GetValidationMessage(nsAString& aValidationMessage,
+                                            ValidityStateType aType)
+{
+  nsresult rv = NS_OK;
+
+  switch (aType)
+  {
+    case VALIDITY_STATE_VALUE_MISSING:
+      {
+        nsXPIDLString message;
+
+        rv = nsContentUtils::GetLocalizedString(nsContentUtils::eDOM_PROPERTIES,
+                                                "FormValidationSelectMissing",
+                                                message);
+
+        aValidationMessage = message;
+      }
+      break;
+    default:
+      rv = nsIConstraintValidation::GetValidationMessage(aValidationMessage, aType);
+  }
+
+  return rv;
+}
+
 #ifdef DEBUG
 
 static void
 VerifyOptionsRecurse(nsIContent* aRoot, PRInt32& aIndex,
                      nsHTMLOptionCollection* aArray)
 {
   nsIContent* child;
   for(PRUint32 i = 0; (child = aRoot->GetChildAt(i)); ++i) {
--- a/content/html/content/src/nsHTMLSelectElement.h
+++ b/content/html/content/src/nsHTMLSelectElement.h
@@ -210,17 +210,17 @@ public:
    * @param aSelect The select element which option list is being mutated.
    *                Can be null.
    * @param aParent The content object which is being mutated.
    * @param aKid    If not null, a new child element is being inserted to
    *                aParent. Otherwise a child element will be removed.
    * @param aIndex  The index of the content object in the parent.
    */
   nsSafeOptionListMutation(nsIContent* aSelect, nsIContent* aParent,
-                           nsIContent* aKid, PRUint32 aIndex);
+                           nsIContent* aKid, PRUint32 aIndex, PRBool aNotify);
   ~nsSafeOptionListMutation();
   void MutationFailed() { mNeedsRebuild = PR_TRUE; }
 private:
   static void* operator new(size_t) CPP_THROW_NEW { return 0; }
   static void operator delete(void*, size_t) {}
   /** The select element which option list is being mutated. */
   nsCOMPtr<nsISelectElement> mSelect;
   /** PR_TRUE if the current mutation is the first one in the stack. */
@@ -325,17 +325,18 @@ public:
   static nsHTMLSelectElement *FromSupports(nsISupports *aSupports)
   {
     return static_cast<nsHTMLSelectElement*>(static_cast<nsINode*>(aSupports));
   }
 
   virtual nsXPCClassInfo* GetClassInfo();
 
   // nsIConstraintValidation
-  void UpdateBarredFromConstraintValidation();
+  nsresult GetValidationMessage(nsAString& aValidationMessage,
+                                ValidityStateType aType);
 
 protected:
   friend class nsSafeOptionListMutation;
 
   // Helper Methods
   /**
    * Check whether the option specified by the index is selected
    * @param aIndex the index
@@ -347,23 +348,23 @@ protected:
    * and set mSelectedIndex to it.
    * @param aStartIndex the index to start with
    */
   void FindSelectedIndex(PRInt32 aStartIndex);
   /**
    * Select some option if possible (generally the first non-disabled option).
    * @return true if something was selected, false otherwise
    */
-  PRBool SelectSomething();
+  PRBool SelectSomething(PRBool aNotify);
   /**
    * Call SelectSomething(), but only if nothing is selected
    * @see SelectSomething()
    * @return true if something was selected, false otherwise
    */
-  PRBool CheckSelectSomething();
+  PRBool CheckSelectSomething(PRBool aNotify);
   /**
    * Called to trigger notifications of frames and fixing selected index
    *
    * @param aSelectFrame the frame for this content (could be null)
    * @param aIndex the index that was selected or deselected
    * @param aSelected whether the index was selected or deselected
    * @param aChangeOptionState if false, don't do anything to the
    *                           nsHTMLOptionElement at aIndex.  If true, change
@@ -385,26 +386,28 @@ protected:
   /**
    * Insert option(s) into the options[] array and perform notifications
    * @param aOptions the option or optgroup being added
    * @param aListIndex the index to start adding options into the list at
    * @param aDepth the depth of aOptions (1=direct child of select ...)
    */
   nsresult InsertOptionsIntoList(nsIContent* aOptions,
                                  PRInt32 aListIndex,
-                                 PRInt32 aDepth);
+                                 PRInt32 aDepth,
+                                 PRBool aNotify);
   /**
    * Remove option(s) from the options[] array
    * @param aOptions the option or optgroup being added
    * @param aListIndex the index to start removing options from the list at
    * @param aDepth the depth of aOptions (1=direct child of select ...)
    */
   nsresult RemoveOptionsFromList(nsIContent* aOptions,
                                  PRInt32 aListIndex,
-                                 PRInt32 aDepth);
+                                 PRInt32 aDepth,
+                                 PRBool aNotify);
   /**
    * Insert option(s) into the options[] array (called by InsertOptionsIntoList)
    * @param aOptions the option or optgroup being added
    * @param aInsertIndex the index to start adding options into the list at
    * @param aDepth the depth of aOptions (1=direct child of select ...)
    */
   nsresult InsertOptionsIntoListRecurse(nsIContent* aOptions,
                                         PRInt32* aInsertIndex,
@@ -415,16 +418,22 @@ protected:
    * @param aListIndex the index to start removing options from the list at
    * @param aNumRemoved the number removed so far [OUT]
    * @param aDepth the depth of aOptions (1=direct child of select ...)
    */
   nsresult RemoveOptionsFromListRecurse(nsIContent* aOptions,
                                         PRInt32 aRemoveIndex,
                                         PRInt32* aNumRemoved,
                                         PRInt32 aDepth);
+
+  // nsIConstraintValidation
+  void UpdateBarredFromConstraintValidation();
+  bool IsValueMissing();
+  void UpdateValueMissingValidityState();
+
   /**
    * Find out how deep this content is from the select (1=direct child)
    * @param aContent the content to check
    * @return the depth
    */
   PRInt32 GetContentDepth(nsIContent* aContent);
   /**
    * Get the index of the first option at, under or following the content in
--- a/content/html/content/test/test_bug596511.html
+++ b/content/html/content/test/test_bug596511.html
@@ -3,28 +3,78 @@
 <!--
 https://bugzilla.mozilla.org/show_bug.cgi?id=596511
 -->
 <head>
   <title>Test for Bug 596511</title>
   <script type="application/javascript" src="/MochiKit/packed.js"></script>
   <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <style>
+    select:valid { background-color: green; }
+    select:invalid { background-color: red; }
+  </style>
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=596511">Mozilla Bug 596511</a>
 <p id="display"></p>
 <div id="content" style="display: none">
-  
 </div>
 <pre id="test">
 <script type="application/javascript">
 
 /** Test for Bug 596511 **/
 
+function checkNotSufferingFromBeingMissing(element, aTodo)
+{
+  if (aTodo) {
+    ok = todo;
+    is = todo_is;
+  }
+
+  ok(!element.validity.valueMissing,
+    "Element should not suffer from value missing");
+  ok(element.validity.valid, "Element should be valid");
+  ok(element.checkValidity(), "Element should be valid");
+
+  is(element.validationMessage, "",
+    "Validation message should be the empty string");
+
+  is(window.getComputedStyle(element, null).getPropertyValue('background-color'),
+     "rgb(0, 128, 0)", ":valid pseudo-class should apply");
+
+  if (aTodo) {
+    ok = SimpleTest.ok;
+    is = SimpleTest.is;
+  }
+}
+
+function checkSufferingFromBeingMissing(element, aTodo)
+{
+  if (aTodo) {
+    ok = todo;
+    is = todo_is;
+  }
+
+  ok(element.validity.valueMissing, "Element should suffer from value missing");
+  ok(!element.validity.valid, "Element should not be valid");
+  ok(!element.checkValidity(), "Element should not be valid");
+
+  is(element.validationMessage, "Please select an item in the list.",
+     "Validation message is wrong");
+
+  is(window.getComputedStyle(element, null).getPropertyValue('background-color'),
+     "rgb(255, 0, 0)", ":invalid pseudo-class should apply");
+
+  if (aTodo) {
+    ok = SimpleTest.ok;
+    is = SimpleTest.is;
+  }
+}
+
 function checkRequiredAttribute(element)
 {
   ok('required' in element, "select should have a required attribute");
 
   ok(!element.required, "select required attribute should be disabled");
   is(element.getAttribute('required'), null,
     "select required attribute should be disabled");
 
@@ -42,30 +92,139 @@ function checkRequiredAttribute(element)
   element.removeAttribute('required');
   ok(!element.required, "select required attribute should be disabled");
   is(element.getAttribute('required'), null,
     "select required attribute should be disabled");
 }
 
 function checkRequiredAndOptionalSelectors(element)
 {
-  is(document.querySelector("select:optional"), element, "select should be optional");
-  is(document.querySelector("select:required"), null, "select shouldn't be required");
+  is(document.querySelector("select:optional"), element,
+     "select should be optional");
+  is(document.querySelector("select:required"), null,
+     "select shouldn't be required");
 
   element.required = true;
 
-  is(document.querySelector("select:optional"), null, "select shouldn't be optional");
-  is(document.querySelector("select:required"), element, "select should be required");
+  is(document.querySelector("select:optional"), null,
+     "select shouldn't be optional");
+  is(document.querySelector("select:required"), element,
+     "select should be required");
 
   element.required = false;
 }
 
+function checkInvalidWhenValueMissing(element)
+{
+  checkNotSufferingFromBeingMissing(select);
+
+  element.required = true;
+  checkSufferingFromBeingMissing(select);
+
+  /**
+   * Non-multiple and size=1.
+   */
+  select.appendChild(new Option());
+  checkSufferingFromBeingMissing(select);
+
+  // When removing the required attribute, element should not be invalid.
+  element.required = false;
+  checkNotSufferingFromBeingMissing(select);
+
+  element.required = true;
+  select.options[0].textContent = "foo";
+  // TODO: having that working would require us to add a mutation observer on
+  // the select element.
+  checkNotSufferingFromBeingMissing(select, true);
+
+  select.remove(0);
+  checkSufferingFromBeingMissing(select);
+
+  select.add(new Option("foo", "foo"), null);
+  checkNotSufferingFromBeingMissing(select);
+
+  select.add(new Option(), null);
+  checkNotSufferingFromBeingMissing(select);
+
+  select.options[1].selected = true;
+  checkSufferingFromBeingMissing(select);
+
+  select.selectedIndex = 0;
+  checkNotSufferingFromBeingMissing(select);
+
+  select.selectedIndex = 1;
+  checkSufferingFromBeingMissing(select);
+
+  select.remove(1);
+  checkNotSufferingFromBeingMissing(select);
+
+  select.options[0].disabled = true;
+  // TODO: having that working would require us to add a mutation observer on
+  // the select element.
+  checkSufferingFromBeingMissing(select, true);
+
+  select.options[0].disabled = false
+  select.remove(0);
+  checkSufferingFromBeingMissing(select);
+
+  var option = new Option("foo", "foo");
+  option.disabled = true;
+  select.add(option, null);
+  select.add(new Option("bar"), null);
+  option.selected = true;
+  checkSufferingFromBeingMissing(select);
+
+  select.remove(0);
+  select.remove(0);
+
+  /**
+   * Non-multiple and size > 1.
+   * Everything should be the same except moving the selection.
+   */
+  select.multiple = false;
+  select.size = 4;
+  checkSufferingFromBeingMissing(select);
+
+  select.add(new Option("", "", true), null);
+  checkSufferingFromBeingMissing(select);
+
+  select.add(new Option("foo", "foo"), null);
+  select.remove(0);
+  checkSufferingFromBeingMissing(select);
+
+  select.options[0].selected = true;
+  checkNotSufferingFromBeingMissing(select);
+
+  select.remove(0);
+
+  /**
+   * Multiple, any size.
+   * We can select more than one element and at least needs a value.
+   */
+  select.multiple = true;
+  select.size = 4;
+  checkSufferingFromBeingMissing(select);
+
+  select.add(new Option("", "", true), null);
+  checkSufferingFromBeingMissing(select);
+
+  select.add(new Option("", "", true), null);
+  checkSufferingFromBeingMissing(select);
+
+  select.add(new Option("foo"), null);
+  checkSufferingFromBeingMissing(select);
+
+  select.options[2].selected = true;
+  checkNotSufferingFromBeingMissing(select);
+}
+
 var select = document.createElement("select");
 var content = document.getElementById('content');
 content.appendChild(select);
 
 checkRequiredAttribute(select);
 checkRequiredAndOptionalSelectors(select);
+checkInvalidWhenValueMissing(select);
 
 </script>
 </pre>
 </body>
 </html>
--- a/layout/reftests/css-invalid/select/reftest.list
+++ b/layout/reftests/css-invalid/select/reftest.list
@@ -1,8 +1,12 @@
 == select-valid.html select-ref.html
 == select-invalid.html select-ref.html
 == select-disabled.html select-disabled-ref.html
 == select-dyn-disabled.html select-disabled-ref.html
 == select-dyn-not-disabled.html select-ref.html
+== select-required-invalid.html select-required-ref.html
+== select-required-valid.html select-required-ref.html
+== select-required-multiple-invalid.html select-required-multiple-ref.html
+== select-required-multiple-valid.html select-required-multiple-ref.html
 == select-disabled-fieldset-1.html select-fieldset-ref.html
 == select-disabled-fieldset-2.html select-fieldset-ref.html
 == select-fieldset-legend.html select-fieldset-legend-ref.html
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-invalid/select/select-required-invalid.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html>
+  <!-- Test: if select is required and has a select option which has an empty
+             string value, :invalid should apply. -->
+  <link rel='stylesheet' type='text/css' href='style.css'>
+  <body>
+    <select class='invalid' required>
+      <option selected value="">foo</option>
+    </select>
+  </body>
+</html></html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-invalid/select/select-required-multiple-invalid.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+  <!-- Test: if select is required and has all selected option value set to the
+             string string, :invalid should apply. -->
+  <link rel='stylesheet' type='text/css' href='style.css'>
+  <body>
+    <select class='invalid' required multiple>
+      <option selected></option>
+      <option selected value="">foo</option>
+    </select>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-invalid/select/select-required-multiple-ref.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+  <link rel='stylesheet' type='text/css' href='style.css'>
+  <body>
+    <select multiple style="background-color: green;">
+      <option selected></option>
+      <option selected value="">foo</option>
+    </select>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-invalid/select/select-required-multiple-valid.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+  <!-- Test: if select is required and has a selected option which has value
+             different from the empty string, :invalid should not apply. -->
+  <link rel='stylesheet' type='text/css' href='style.css'>
+  <body>
+    <select class='notinvalid' required multiple>
+      <option selected></option>
+      <option selected>foo</option>
+    </select>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-invalid/select/select-required-ref.html
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+  <link rel='stylesheet' type='text/css' href='style.css'>
+  <body>
+    <select style="background-color: green;">
+      <option selected value="">foo</option>
+    </selecT>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-invalid/select/select-required-valid.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html>
+  <!-- Test: if select is required and has a select option which has value
+             different from the empty string, :invalid should not apply. -->
+  <link rel='stylesheet' type='text/css' href='style.css'>
+  <body>
+    <select class='notinvalid' required>
+      <option selected>foo</option>
+    </select>
+  </body>
+</html>
--- a/layout/reftests/css-valid/select/reftest.list
+++ b/layout/reftests/css-valid/select/reftest.list
@@ -1,8 +1,12 @@
 == select-valid.html select-ref.html
 == select-invalid.html select-ref.html
 == select-disabled.html select-disabled-ref.html
 == select-dyn-disabled.html select-disabled-ref.html
 == select-dyn-not-disabled.html select-ref.html
+== select-required-invalid.html select-required-ref.html
+== select-required-valid.html select-required-ref.html
+== select-required-multiple-invalid.html select-required-multiple-ref.html
+== select-required-multiple-valid.html select-required-multiple-ref.html
 == select-disabled-fieldset-1.html select-fieldset-ref.html
 == select-disabled-fieldset-2.html select-fieldset-ref.html
 == select-fieldset-legend.html select-fieldset-legend-ref.html
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-valid/select/select-required-invalid.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html>
+  <!-- Test: if select is required and has a select option which has an empty
+             string value, :valid should not apply. -->
+  <link rel='stylesheet' type='text/css' href='style.css'>
+  <body>
+    <select class='notvalid' required>
+      <option selected value="">foo</option>
+    </select>
+  </body>
+</html></html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-valid/select/select-required-multiple-invalid.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+  <!-- Test: if select is required and has all selected option value set to the
+             string string, :valid should not apply. -->
+  <link rel='stylesheet' type='text/css' href='style.css'>
+  <body>
+    <select class='notvalid' required multiple>
+      <option selected></option>
+      <option selected value="">foo</option>
+    </select>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-valid/select/select-required-multiple-ref.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+  <link rel='stylesheet' type='text/css' href='style.css'>
+  <body>
+    <select multiple style="background-color: green;">
+      <option selected></option>
+      <option selected value="">foo</option>
+    </select>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-valid/select/select-required-multiple-valid.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+  <!-- Test: if select is required and has a selected option which has value
+             different from the empty string, :valid should apply. -->
+  <link rel='stylesheet' type='text/css' href='style.css'>
+  <body>
+    <select class='valid' required multiple>
+      <option selected></option>
+      <option selected>foo</option>
+    </select>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-valid/select/select-required-ref.html
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+  <link rel='stylesheet' type='text/css' href='style.css'>
+  <body>
+    <select style="background-color: green;">
+      <option selected value="">foo</option>
+    </selecT>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-valid/select/select-required-valid.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html>
+  <!-- Test: if select is required and has a select option which has value
+             different from the empty string, :valid should apply. -->
+  <link rel='stylesheet' type='text/css' href='style.css'>
+  <body>
+    <select class='valid' required>
+      <option selected>foo</option>
+    </select>
+  </body>
+</html>