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 id17175
push usermlamouri@mozilla.com
push dateWed, 24 Nov 2010 10:15:50 +0000
treeherdermozilla-central@7f5cd850578e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, bsmedberg
bugs609417
milestone2.0b8pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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;
 }