Bug 609417 - We shouldn't always call SetValueChanged when clearing the file list. r=smaug a=bsmedberg
authorMounir Lamouri <mounir.lamouri@gmail.com>
Wed, 24 Nov 2010 00:45:53 +0100
changeset 58140 023f675df2a0736749c35e0ac35f8feed46d76b5
parent 58139 fa6f069c82cbb703f4a7ce260e6a20b3541d5b71
child 58141 a26199cd24df75cd83fdce0439a77f722e0a7c0e
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewerssmaug, bsmedberg
bugs609417
milestone2.0b8pre
Bug 609417 - We shouldn't always call SetValueChanged when clearing the file list. r=smaug a=bsmedberg
content/html/content/src/nsHTMLInputElement.cpp
content/html/content/src/nsHTMLInputElement.h
layout/forms/nsFileControlFrame.cpp
--- a/content/html/content/src/nsHTMLInputElement.cpp
+++ b/content/html/content/src/nsHTMLInputElement.cpp
@@ -436,17 +436,17 @@ AsyncClickHandler::Run()
     }
   }
 
   // Set new selected files
   if (newFiles.Count()) {
     // The text control frame (if there is one) isn't going to send a change
     // event because it will think this is done by a script.
     // So, we can safely send one by ourself.
-    mInput->SetFiles(newFiles);
+    mInput->SetFiles(newFiles, true);
     nsContentUtils::DispatchTrustedEvent(mInput->GetOwnerDoc(),
                                          static_cast<nsIDOMHTMLInputElement*>(mInput.get()),
                                          NS_LITERAL_STRING("change"), PR_FALSE,
                                          PR_FALSE);
   }
 
   return NS_OK;
 }
@@ -1071,17 +1071,17 @@ nsHTMLInputElement::SetValue(const nsASt
         // setting the value of a "FILE" input widget requires the
         // UniversalFileRead privilege
         return NS_ERROR_DOM_SECURITY_ERR;
       }
       const PRUnichar *name = PromiseFlatString(aValue).get();
       return MozSetFileNameArray(&name, 1);
     }
     else {
-      ClearFiles();
+      ClearFiles(true);
     }
   }
   else {
     SetValueInternal(aValue, PR_FALSE, PR_TRUE);
   }
 
   return NS_OK;
 }
@@ -1165,17 +1165,17 @@ nsHTMLInputElement::MozSetFileNameArray(
       nsCOMPtr<nsIDOMFile> domFile = new nsDOMFile(file);
       files.AppendObject(domFile);
     } else {
       continue; // Not much we can do if the file doesn't exist
     }
 
   }
 
-  SetFiles(files);
+  SetFiles(files, true);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsHTMLInputElement::MozIsTextField(PRBool aExcludePassword, PRBool* aResult)
 {
   *aResult = IsSingleLineTextControl(aExcludePassword);
@@ -1325,34 +1325,38 @@ nsHTMLInputElement::GetDisplayFileName(n
     }
     else {
       aValue.Append(NS_LITERAL_STRING(", ") + str);
     }
   }
 }
 
 void
-nsHTMLInputElement::SetFiles(const nsCOMArray<nsIDOMFile>& aFiles)
+nsHTMLInputElement::SetFiles(const nsCOMArray<nsIDOMFile>& aFiles,
+                             bool aSetValueChanged)
 {
   mFiles.Clear();
   mFiles.AppendObjects(aFiles);
 
   // No need to flush here, if there's no frame at this point we
   // don't need to force creation of one just to tell it about this
   // new value.  We just want the display to update as needed.
   nsIFormControlFrame* formControlFrame = GetFormControlFrame(PR_FALSE);
   if (formControlFrame) {
     nsAutoString readableValue;
     GetDisplayFileName(readableValue);
     formControlFrame->SetFormProperty(nsGkAtoms::value, readableValue);
   }
 
   UpdateFileList();
 
-  SetValueChanged(PR_TRUE);
+  if (aSetValueChanged) {
+    SetValueChanged(PR_TRUE);
+  }
+
   UpdateAllValidityStates(PR_TRUE);
 }
 
 const nsCOMArray<nsIDOMFile>&
 nsHTMLInputElement::GetFiles()
 {
   return mFiles;
 }
@@ -2649,17 +2653,17 @@ nsHTMLInputElement::ParseAttribute(PRInt
         // That way the logic in SetValueInternal() will work right (that logic
         // makes assumptions about our frame based on mType, but we won't have
         // had time to recreate frames yet -- that happens later in the
         // SetAttr() process).
         if (newType == NS_FORM_INPUT_FILE || mType == NS_FORM_INPUT_FILE) {
           // This call isn't strictly needed any more since we'll never
           // confuse values and filenames. However it's there for backwards
           // compat.
-          ClearFiles();
+          ClearFiles(false);
         }
 
         HandleTypeChange(newType);
       }
 
       return success;
     }
     if (aAttribute == nsGkAtoms::width) {
@@ -2979,17 +2983,17 @@ nsHTMLInputElement::Reset()
   switch (GetValueMode()) {
     case VALUE_MODE_VALUE:
       return SetDefaultValueAsValue();
     case VALUE_MODE_DEFAULT_ON:
       PRBool resetVal;
       GetDefaultChecked(&resetVal);
       return DoSetChecked(resetVal, PR_TRUE, PR_FALSE);
     case VALUE_MODE_FILENAME:
-      ClearFiles();
+      ClearFiles(false);
       return NS_OK;
     case VALUE_MODE_DEFAULT:
     default:
       return NS_OK;
   }
 }
 
 NS_IMETHODIMP
@@ -3329,17 +3333,17 @@ nsHTMLInputElement::RestoreState(nsPresS
       case NS_FORM_INPUT_HIDDEN:
         {
           SetValueInternal(inputState->GetValue(), PR_FALSE, PR_TRUE);
           break;
         }
       case NS_FORM_INPUT_FILE:
         {
           const nsCOMArray<nsIDOMFile>& files = inputState->GetFiles();
-          SetFiles(files);
+          SetFiles(files, true);
           break;
         }
     }
   }
 
   if (aState->IsDisabledSet()) {
     SetDisabled(aState->GetDisabled());
   }
--- a/content/html/content/src/nsHTMLInputElement.h
+++ b/content/html/content/src/nsHTMLInputElement.h
@@ -211,17 +211,17 @@ public:
   NS_IMETHOD_(void) UpdatePlaceholderText(PRBool aNotify);
   NS_IMETHOD_(void) SetPlaceholderClass(PRBool aVisible, PRBool aNotify);
   NS_IMETHOD_(void) InitializeKeyboardEventListeners();
   NS_IMETHOD_(void) OnValueChanged(PRBool aNotify);
 
   // nsIFileControlElement
   void GetDisplayFileName(nsAString& aFileName) const;
   const nsCOMArray<nsIDOMFile>& GetFiles();
-  void SetFiles(const nsCOMArray<nsIDOMFile>& aFiles);
+  void SetFiles(const nsCOMArray<nsIDOMFile>& aFiles, bool aSetValueChanged);
 
   void SetCheckedChangedInternal(PRBool aCheckedChanged);
   PRBool GetCheckedChanged();
   void AddedToRadioGroup(PRBool aNotify = PR_TRUE);
   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)
@@ -362,19 +362,19 @@ protected:
   static PRBool IsPatternMatching(nsAString& aValue, nsAString& aPattern,
                                   nsIDocument* aDocument);
 
   // Helper method
   nsresult SetValueInternal(const nsAString& aValue,
                             PRBool aUserInput,
                             PRBool aSetValueChanged);
 
-  void ClearFiles() {
+  void ClearFiles(bool aSetValueChanged) {
     nsCOMArray<nsIDOMFile> files;
-    SetFiles(files);
+    SetFiles(files, aSetValueChanged);
   }
 
   nsresult SetIndeterminateInternal(PRBool aValue,
                                     PRBool aShouldInvalidate);
 
   nsresult GetSelectionRange(PRInt32* aSelectionStart, PRInt32* aSelectionEnd);
 
   /**
--- a/layout/forms/nsFileControlFrame.cpp
+++ b/layout/forms/nsFileControlFrame.cpp
@@ -469,17 +469,17 @@ nsFileControlFrame::CaptureMouseListener
   // uneditable text box with the file name inside.
   // Set new selected files
   if (newFiles.Count()) {
     // Tell mTextFrame that this update of the value is a user initiated
     // change. Otherwise it'll think that the value is being set by a script
     // and not fire onchange when it should.
     PRBool oldState = mFrame->mTextFrame->GetFireChangeEventState();
     mFrame->mTextFrame->SetFireChangeEventState(PR_TRUE);
-    inputElement->SetFiles(newFiles);
+    inputElement->SetFiles(newFiles, true);
 
     mFrame->mTextFrame->SetFireChangeEventState(oldState);
     // May need to fire an onchange here
     mFrame->mTextFrame->CheckFireOnChange();
   }
 
   return NS_OK;
 }