Bug 598236 - nsHTMLInputElement should not ask nsFileControlFrame to know the filters for the file picker. r=smaug a=sicking

content/html/content/src/nsHTMLInputElement.cpp

@@ -100,8 +100,6 @@
 #include "nsILocalFile.h"
 #include "nsNetUtil.h"
 #include "nsDOMFile.h"
-#include "nsFileControlFrame.h"
-#include "nsTextControlFrame.h"
 #include "nsIFilePicker.h"
 #include "nsDirectoryServiceDefs.h"
 #include "nsIPrivateBrowsingService.h"
@@ -305,9 +303,6 @@ AsyncClickHandler::Run()
   if (!filePicker)
     return NS_ERROR_FAILURE;
 
-  nsFileControlFrame* frame =
-    static_cast<nsFileControlFrame*>(mInput->GetPrimaryFrame());
-
   PRBool multi;
   rv = mInput->GetMultiple(&multi);
   NS_ENSURE_SUCCESS(rv, rv);
@@ -317,19 +312,22 @@ AsyncClickHandler::Run()
                         (PRInt16)nsIFilePicker::modeOpen);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // We want to get the file filter from the accept attribute and we add the
-  // |filterAll| filter to be sure the user has a valid fallback.
-  PRUint32 filter = 0;
-  if (frame)
-    filter = frame->GetFileFilterFromAccept();
-  filePicker->AppendFilters(filter | nsIFilePicker::filterAll);
+  if (mInput->HasAttr(kNameSpaceID_None, nsGkAtoms::accept)) {
+    PRInt32 filters = mInput->GetFiltersFromAccept();
 
-  // If the accept attribute asks for a filter, it has to be the default one.
-  if (filter) {
-    // We have two filters: |filterAll| and another one. |filterAll| is
-    // always the first one (index=0) so we can assume the one we want to be
-    // the default is at index 1.
-    filePicker->SetFilterIndex(1);
+    if (filters) {
+      // We add |filterAll| to be sure the user always has a sane fallback.
+      filePicker->AppendFilters(filters | nsIFilePicker::filterAll);
+
+      // If the accept attribute asked for a filter, we need to make it default.
+      // |filterAll| will always use index=0 so we need to set index=1 as the
+      // current filter.
+      filePicker->SetFilterIndex(1);
+    } else {
+      filePicker->AppendFilters(nsIFilePicker::filterAll);
+    }
+  } else {
+    filePicker->AppendFilters(nsIFilePicker::filterAll);
   }
 
   // Set default directry and filename
@@ -4413,3 +4411,32 @@ nsHTMLInputElement::FieldSetDisabledChanged(PRInt32 aStates)
   nsGenericHTMLFormElement::FieldSetDisabledChanged(aStates);
 }
 
+PRInt32
+nsHTMLInputElement::GetFiltersFromAccept()
+{
+  NS_ASSERTION(HasAttr(kNameSpaceID_None, nsGkAtoms::accept),
+               "You should not call GetFileFiltersFromAccept if the element"
+               " has no accept attribute!");
+
+  PRInt32 filters = 0;
+  nsAutoString accept;
+  GetAttr(kNameSpaceID_None, nsGkAtoms::accept, accept);
+
+  nsCharSeparatedTokenizerTemplate<nsContentUtils::IsHTMLWhitespace>
+    tokenizer(accept, ',');
+
+  while (tokenizer.hasMoreTokens()) {
+    const nsDependentSubstring token = tokenizer.nextToken();
+
+    if (token.EqualsLiteral("image/*")) {
+      filters |= nsIFilePicker::filterImages;
+    } else if (token.EqualsLiteral("audio/*")) {
+      filters |= nsIFilePicker::filterAudio;
+    } else if (token.EqualsLiteral("video/*")) {
+      filters |= nsIFilePicker::filterVideo;
+    }
+  }
+
+  return filters;
+}
+

content/html/content/src/nsHTMLInputElement.h

@@ -279,6 +279,19 @@ public:
   nsresult GetValidationMessage(nsAString& aValidationMessage,
                                 ValidityStateType aType);
 
+  /**
+   * Returns the filter which should be used for the file picker according to
+   * the accept attribute value.
+   *
+   * See:
+   * http://dev.w3.org/html5/spec/forms.html#attr-input-accept
+   *
+   * @return Filters to use on the file picker with AppendFilters, 0 if none.
+   *
+   * @note You should not call this function if the element has no @accept.
+   */
+  PRInt32 GetFiltersFromAccept();
+
 protected:
   // Pull IsSingleLineTextControl into our scope, otherwise it'd be hidden
   // by the nsITextControlElement version.

layout/forms/nsFileControlFrame.cpp

@@ -719,28 +719,6 @@ nsFileControlFrame::ParseAcceptAttribute(AcceptAttrCallback aCallback,
          (*aCallback)(tokenizer.nextToken(), aClosure));
 }
 
-PRBool FileFilterCallback(const nsAString& aVal, void* aClosure)
-{
-  PRInt32* filter = (PRInt32*)aClosure;
-
-  if (aVal.EqualsLiteral("image/*")) {
-    *filter |= nsIFilePicker::filterImages;
-  } else if (aVal.EqualsLiteral("audio/*")) {
-    *filter |= nsIFilePicker::filterAudio;
-  } else if (aVal.EqualsLiteral("video/*")) {
-    *filter |= nsIFilePicker::filterVideo;
-  }
-
-  return PR_TRUE;
-}
-
-PRInt32
-nsFileControlFrame::GetFileFilterFromAccept() const
-{
-  PRInt32 filterVal = 0;
-  this->ParseAcceptAttribute(&FileFilterCallback, (void*)&filterVal);
-  return filterVal;
-}
 ////////////////////////////////////////////////////////////
 // Mouse listener implementation
 

layout/forms/nsFileControlFrame.h

@@ -99,18 +99,6 @@ public:
   virtual already_AddRefed<nsAccessible> CreateAccessible();
 #endif
 
-  /**
-   * This methods return the file filter mask requested by the HTML5 accept
-   * attribute. If the accept attribute isn't present or the value isn't valid,
-   * the returned value will be 0.
-   *
-   * See:
-   * http://dev.w3.org/html5/spec/forms.html#attr-input-accept
-   *
-   * @return the file picker filter mask or 0 if there is no filter.
-   */
-  PRInt32 GetFileFilterFromAccept() const;
-
   typedef PRBool (*AcceptAttrCallback)(const nsAString&, void*);
   void ParseAcceptAttribute(AcceptAttrCallback aCallback, void* aClosure) const;