Bug 1238515 - nsIFilePicker methods should clearly say when they return directories and files, r=smaug
☠☠ backed out by d2a2c77016be ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 12 Jan 2016 10:23:13 +0000
changeset 314690 b5e8cd00d7ef774e2d12427691f99c87cff364c4
parent 314689 531383863a30e6fd1cebc09fec0eca6b3dfe7133
child 314691 74737f15af19c82abff7d68082fb8e74efea2081
push id5703
push userraliiev@mozilla.com
push dateMon, 07 Mar 2016 14:18:41 +0000
treeherdermozilla-beta@31e373ad5b5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1238515
milestone46.0a1
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 1238515 - nsIFilePicker methods should clearly say when they return directories and files, r=smaug
b2g/components/FilePicker.js
dom/html/HTMLInputElement.cpp
mobile/android/components/FilePicker.js
mobile/android/tests/browser/robocop/testFilePicker.js
testing/specialpowers/content/MockFilePicker.jsm
toolkit/components/filepicker/nsFilePicker.js
widget/nsBaseFilePicker.cpp
widget/nsBaseFilePicker.h
widget/nsFilePickerProxy.cpp
widget/nsFilePickerProxy.h
widget/nsIFilePicker.idl
--- a/b2g/components/FilePicker.js
+++ b/b2g/components/FilePicker.js
@@ -66,21 +66,22 @@ FilePicker.prototype = {
       throw Cr.NS_ERROR_NOT_IMPLEMENTED;
     }
   },
 
   /* readonly attribute nsILocalFile file - not implemented; */
   /* readonly attribute nsISimpleEnumerator files - not implemented; */
   /* readonly attribute nsIURI fileURL - not implemented; */
 
-  get domfiles() {
+  get domFileOrDirectoryEnumerator() {
     return this.mFilesEnumerator;
   },
 
-  get domfile() {
+  // We don't support directory selection yet.
+  get domFileOrDirectory() {
     return this.mFilesEnumerator ? this.mFilesEnumerator.mFiles[0] : null;
   },
 
   get mode() {
     return this.mMode;
   },
 
   appendFilters: function(filterMask) {
--- a/dom/html/HTMLInputElement.cpp
+++ b/dom/html/HTMLInputElement.cpp
@@ -381,17 +381,18 @@ HTMLInputElement::nsFilePickerShownCallb
 
   int16_t mode;
   mFilePicker->GetMode(&mode);
 
   // Collect new selected filenames
   nsTArray<RefPtr<File>> newFiles;
   if (mode == static_cast<int16_t>(nsIFilePicker::modeOpenMultiple)) {
     nsCOMPtr<nsISimpleEnumerator> iter;
-    nsresult rv = mFilePicker->GetDomfiles(getter_AddRefs(iter));
+    nsresult rv =
+      mFilePicker->GetDomFileOrDirectoryEnumerator(getter_AddRefs(iter));
     NS_ENSURE_SUCCESS(rv, rv);
 
     if (!iter) {
       return NS_OK;
     }
 
     nsCOMPtr<nsISupports> tmp;
     bool hasMore = true;
@@ -404,17 +405,17 @@ HTMLInputElement::nsFilePickerShownCallb
       if (domBlob) {
         newFiles.AppendElement(static_cast<File*>(domBlob.get()));
       }
     }
   } else {
     MOZ_ASSERT(mode == static_cast<int16_t>(nsIFilePicker::modeOpen) ||
                mode == static_cast<int16_t>(nsIFilePicker::modeGetFolder));
     nsCOMPtr<nsISupports> tmp;
-    nsresult rv = mFilePicker->GetDomfile(getter_AddRefs(tmp));
+    nsresult rv = mFilePicker->GetDomFileOrDirectory(getter_AddRefs(tmp));
     NS_ENSURE_SUCCESS(rv, rv);
 
     nsCOMPtr<nsIDOMBlob> blob = do_QueryInterface(tmp);
     if (blob) {
       RefPtr<File> file = static_cast<Blob*>(blob.get())->ToFile();
       newFiles.AppendElement(file);
     }
   }
--- a/mobile/android/components/FilePicker.js
+++ b/mobile/android/components/FilePicker.js
@@ -135,32 +135,33 @@ FilePicker.prototype = {
   },
 
   get files() {
     return this.getEnumerator([this.file], function(file) {
       return file;
     });
   },
 
-  get domfile() {
+  // We don't support directory selection yet.
+  get domFileOrDirectory() {
     let f = this.file;
     if (!f) {
         return null;
     }
 
     let win = this._domWin;
     if (win) {
       let utils = win.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils);
       return utils.wrapDOMFile(f);
     }
 
     return new File(f);
   },
 
-  get domfiles() {
+  get domFileOrDirectoryEnumerator() {
     let win = this._domWin;
     return this.getEnumerator([this.file], function(file) {
       if (win) {
         let utils = win.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils);
         return utils.wrapDOMFile(file);
       }
 
       return new File(file);
--- a/mobile/android/tests/browser/robocop/testFilePicker.js
+++ b/mobile/android/tests/browser/robocop/testFilePicker.js
@@ -26,24 +26,25 @@ add_test(function filepicker_open() {
 
       let files = fp.files;
       while (files.hasMoreElements()) {
         let file = files.getNext().QueryInterface(Ci.nsIFile);
         do_print("File: " + file.path);
         is(file.path, "/mnt/sdcard/my-favorite-martian.png", "Retrieve the right martian file from array!");
       }
 
-      do_print("DOMFile: " + fp.domfile.mozFullPath);
-      is(fp.domfile.mozFullPath, "/mnt/sdcard/my-favorite-martian.png", "Retrieve the right martian domfile!");
+      let file = fp.domFileOrDirectory;
+      do_print("DOMFile: " + file.mozFullPath);
+      is(file.mozFullPath, "/mnt/sdcard/my-favorite-martian.png", "Retrieve the right martian DOM File!");
 
-      let domfiles = fp.domfiles;
-      while (domfiles.hasMoreElements()) {
-        let domfile = domfiles.getNext();
-        do_print("DOMFile: " + domfile.mozFullPath);
-        is(domfile.mozFullPath, "/mnt/sdcard/my-favorite-martian.png", "Retrieve the right martian file from domfile array!");
+      let enum = fp.domFileOrDirectoryEnumerator;
+      while (enum.hasMoreElements()) {
+        let file = enum.getNext();
+        do_print("DOMFile: " + file.mozFullPath);
+        is(file.mozFullPath, "/mnt/sdcard/my-favorite-martian.png", "Retrieve the right martian file from domFileOrDirectoryEnumerator array!");
       }
 
       do_test_finished();
 
       run_next_test();
     }
   };
 
--- a/testing/specialpowers/content/MockFilePicker.jsm
+++ b/testing/specialpowers/content/MockFilePicker.jsm
@@ -141,17 +141,19 @@ MockFilePickerInstance.prototype = {
     if (MockFilePicker.returnFiles.length >= 1 &&
         // window.File does not implement nsIFile
         MockFilePicker.isNsIFile(MockFilePicker.returnFiles[0])) {
       return MockFilePicker.returnFiles[0];
     }
 
     return null;
   },
-  get domfile()  {
+
+  // We don't support directories here.
+  get domFileOrDirectory()  {
     if (MockFilePicker.returnFiles.length >= 1) {
       // window.File does not implement nsIFile
       if (!MockFilePicker.isNsIFile(MockFilePicker.returnFiles[0])) {
         return MockFilePicker.returnFiles[0];
       }
 
       let utils = this.parent.QueryInterface(Ci.nsIInterfaceRequestor)
                              .getInterface(Ci.nsIDOMWindowUtils);
@@ -179,17 +181,17 @@ MockFilePickerInstance.prototype = {
         // window.File does not implement nsIFile
         if (!MockFilePicker.isNsIFile(MockFilePicker.returnFiles[this.index])) {
           return null;
         }
         return MockFilePicker.returnFiles[this.index++];
       }
     };
   },
-  get domfiles()  {
+  get domFileOrDirectoryEnumerator()  {
     let utils = this.parent.QueryInterface(Ci.nsIInterfaceRequestor)
                            .getInterface(Ci.nsIDOMWindowUtils);
     return {
       index: 0,
       QueryInterface: XPCOMUtils.generateQI([Ci.nsISimpleEnumerator]),
       hasMoreElements: function() {
         return this.index < MockFilePicker.returnFiles.length;
       },
--- a/toolkit/components/filepicker/nsFilePicker.js
+++ b/toolkit/components/filepicker/nsFilePicker.js
@@ -88,24 +88,24 @@ nsFilePicker.prototype = {
   },
 
   /* readonly attribute nsILocalFile file; */
   get file()  { return this.mFilesEnumerator.mFiles[0]; },
 
   /* readonly attribute nsISimpleEnumerator files; */
   get files()  { return this.mFilesEnumerator; },
 
-  /* readonly attribute DOM File domfile; */
-  get domfile()  {
-    let enumerator = this.domfiles;
+  /* we don't support directories, yet */
+  get domFileOrDirectory()  {
+    let enumerator = this.domFileOrDirectoryEnumerator;
     return enumerator ? enumerator.mFiles[0] : null;
   },
 
-  /* readonly attribute nsISimpleEnumerator domfiles; */
-  get domfiles()  {
+  /* readonly attribute nsISimpleEnumerator domFileOrDirectoryEnumerator; */
+  get domFileOrDirectoryEnumerator()  {
     if (!this.mFilesEnumerator) {
       return null;
     }
 
     if (!this.mDOMFilesEnumerator) {
       this.mDOMFilesEnumerator = {
         QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsISimpleEnumerator]),
 
--- a/widget/nsBaseFilePicker.cpp
+++ b/widget/nsBaseFilePicker.cpp
@@ -332,39 +332,39 @@ nsBaseFilePicker::SetAddToRecentDocs(boo
 NS_IMETHODIMP
 nsBaseFilePicker::GetMode(int16_t* aMode)
 {
   *aMode = mMode;
   return NS_OK;
 }
 
 NS_IMETHODIMP
-nsBaseFilePicker::GetDomfile(nsISupports** aDomfile)
+nsBaseFilePicker::GetDomFileOrDirectory(nsISupports** aValue)
 {
   nsCOMPtr<nsIFile> localFile;
   nsresult rv = GetFile(getter_AddRefs(localFile));
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (!localFile) {
-    *aDomfile = nullptr;
+    *aValue = nullptr;
     return NS_OK;
   }
 
   RefPtr<File> domFile = File::CreateFromFile(mParent, localFile);
   domFile->Impl()->SetIsDirectory(mMode == nsIFilePicker::modeGetFolder);
-  nsCOMPtr<nsIDOMBlob>(domFile).forget(aDomfile);
+  nsCOMPtr<nsIDOMBlob>(domFile).forget(aValue);
   return NS_OK;
 }
 
 NS_IMETHODIMP
-nsBaseFilePicker::GetDomfiles(nsISimpleEnumerator** aDomfiles)
+nsBaseFilePicker::GetDomFileOrDirectoryEnumerator(nsISimpleEnumerator** aValue)
 {
   nsCOMPtr<nsISimpleEnumerator> iter;
   nsresult rv = GetFiles(getter_AddRefs(iter));
   NS_ENSURE_SUCCESS(rv, rv);
 
   RefPtr<nsBaseFilePickerEnumerator> retIter =
     new nsBaseFilePickerEnumerator(mParent, iter, mMode);
 
-  retIter.forget(aDomfiles);
+  retIter.forget(aValue);
   return NS_OK;
 }
 
--- a/widget/nsBaseFilePicker.h
+++ b/widget/nsBaseFilePicker.h
@@ -32,18 +32,18 @@ public:
   NS_IMETHOD SetFilterIndex(int32_t aFilterIndex);
   NS_IMETHOD GetFiles(nsISimpleEnumerator **aFiles);
   NS_IMETHOD GetDisplayDirectory(nsIFile * *aDisplayDirectory);
   NS_IMETHOD SetDisplayDirectory(nsIFile * aDisplayDirectory);
   NS_IMETHOD GetAddToRecentDocs(bool *aFlag);
   NS_IMETHOD SetAddToRecentDocs(bool aFlag);
   NS_IMETHOD GetMode(int16_t *aMode);
 
-  NS_IMETHOD GetDomfile(nsISupports** aDomfile);
-  NS_IMETHOD GetDomfiles(nsISimpleEnumerator** aDomfiles);
+  NS_IMETHOD GetDomFileOrDirectory(nsISupports** aValue);
+  NS_IMETHOD GetDomFileOrDirectoryEnumerator(nsISimpleEnumerator** aValue);
 
 protected:
 
   virtual void InitNative(nsIWidget *aParent, const nsAString& aTitle) = 0;
 
   bool mAddToRecentDocs;
   nsCOMPtr<nsIFile> mDisplayDirectory;
 
--- a/widget/nsFilePickerProxy.cpp
+++ b/widget/nsFilePickerProxy.cpp
@@ -97,31 +97,31 @@ nsFilePickerProxy::SetFilterIndex(int32_
 {
   mSelectedType = aFilterIndex;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsFilePickerProxy::GetFile(nsIFile** aFile)
 {
-  MOZ_ASSERT(false, "GetFile is unimplemented; use GetDomfile");
+  MOZ_ASSERT(false, "GetFile is unimplemented; use GetDomFileOrDirectory");
   return NS_ERROR_FAILURE;
 }
 
 NS_IMETHODIMP
 nsFilePickerProxy::GetFileURL(nsIURI** aFileURL)
 {
-  MOZ_ASSERT(false, "GetFileURL is unimplemented; use GetDomfile");
+  MOZ_ASSERT(false, "GetFileURL is unimplemented; use GetDomFileOrDirectory");
   return NS_ERROR_FAILURE;
 }
 
 NS_IMETHODIMP
 nsFilePickerProxy::GetFiles(nsISimpleEnumerator** aFiles)
 {
-  MOZ_ASSERT(false, "GetFiles is unimplemented; use GetDomfiles");
+  MOZ_ASSERT(false, "GetFiles is unimplemented; use GetDomFileOrDirectoryEnumerator");
   return NS_ERROR_FAILURE;
 }
 
 NS_IMETHODIMP
 nsFilePickerProxy::Show(int16_t* aReturn)
 {
   MOZ_ASSERT(false, "Show is unimplemented; use Open");
   return NS_ERROR_NOT_IMPLEMENTED;
@@ -156,39 +156,39 @@ nsFilePickerProxy::Recv__delete__(const 
 
       if (!blobImpl->IsFile()) {
         return true;
       }
 
       RefPtr<File> file = File::Create(mParent, blobImpl);
       MOZ_ASSERT(file);
 
-      mDomfiles.AppendElement(file);
+      mFilesOrDirectories.AppendElement(file);
     }
   }
 
   if (mCallback) {
     mCallback->Done(aResult);
     mCallback = nullptr;
   }
 
   return true;
 }
 
 NS_IMETHODIMP
-nsFilePickerProxy::GetDomfile(nsISupports** aDomfile)
+nsFilePickerProxy::GetDomFileOrDirectory(nsISupports** aValue)
 {
-  *aDomfile = nullptr;
-  if (mDomfiles.IsEmpty()) {
+  *aValue = nullptr;
+  if (mFilesOrDirectories.IsEmpty()) {
     return NS_OK;
   }
 
-  MOZ_ASSERT(mDomfiles.Length() == 1);
-  nsCOMPtr<nsIDOMBlob> blob = mDomfiles[0].get();
-  blob.forget(aDomfile);
+  MOZ_ASSERT(mFilesOrDirectories.Length() == 1);
+  nsCOMPtr<nsIDOMBlob> blob = mFilesOrDirectories[0].get();
+  blob.forget(aValue);
   return NS_OK;
 }
 
 namespace {
 
 class SimpleEnumerator final : public nsISimpleEnumerator
 {
 public:
@@ -225,14 +225,14 @@ private:
   uint32_t mIndex;
 };
 
 NS_IMPL_ISUPPORTS(SimpleEnumerator, nsISimpleEnumerator)
 
 } // namespace
 
 NS_IMETHODIMP
-nsFilePickerProxy::GetDomfiles(nsISimpleEnumerator** aDomfiles)
+nsFilePickerProxy::GetDomFileOrDirectoryEnumerator(nsISimpleEnumerator** aDomfiles)
 {
-  RefPtr<SimpleEnumerator> enumerator = new SimpleEnumerator(mDomfiles);
+  RefPtr<SimpleEnumerator> enumerator = new SimpleEnumerator(mFilesOrDirectories);
   enumerator.forget(aDomfiles);
   return NS_OK;
 }
--- a/widget/nsFilePickerProxy.h
+++ b/widget/nsFilePickerProxy.h
@@ -46,31 +46,31 @@ public:
     NS_IMETHODIMP GetDefaultExtension(nsAString& aDefaultExtension) override;
     NS_IMETHODIMP SetDefaultExtension(const nsAString& aDefaultExtension) override;
     NS_IMETHODIMP GetFilterIndex(int32_t* aFilterIndex) override;
     NS_IMETHODIMP SetFilterIndex(int32_t aFilterIndex) override;
     NS_IMETHODIMP GetFile(nsIFile** aFile) override;
     NS_IMETHODIMP GetFileURL(nsIURI** aFileURL) override;
     NS_IMETHODIMP GetFiles(nsISimpleEnumerator** aFiles) override;
 
-    NS_IMETHODIMP GetDomfile(nsISupports** aFile) override;
-    NS_IMETHODIMP GetDomfiles(nsISimpleEnumerator** aFiles) override;
+    NS_IMETHODIMP GetDomFileOrDirectory(nsISupports** aValue) override;
+    NS_IMETHODIMP GetDomFileOrDirectoryEnumerator(nsISimpleEnumerator** aValue) override;
 
     NS_IMETHODIMP Show(int16_t* aReturn) override;
     NS_IMETHODIMP Open(nsIFilePickerShownCallback* aCallback) override;
 
     // PFilePickerChild
     virtual bool
     Recv__delete__(const MaybeInputFiles& aFiles, const int16_t& aResult) override;
 
 private:
     ~nsFilePickerProxy();
     void InitNative(nsIWidget*, const nsAString&) override;
 
-    nsTArray<RefPtr<mozilla::dom::File>> mDomfiles;
+    nsTArray<RefPtr<mozilla::dom::File>> mFilesOrDirectories;
     nsCOMPtr<nsIFilePickerShownCallback> mCallback;
 
     int16_t   mSelectedType;
     nsString  mFile;
     nsString  mDefault;
     nsString  mDefaultExtension;
 
     InfallibleTArray<nsString> mFilters;
--- a/widget/nsIFilePicker.idl
+++ b/widget/nsIFilePicker.idl
@@ -18,17 +18,17 @@ interface nsIFilePickerShownCallback : n
   * Callback which is called when a filepicker is shown and a result
   * is returned.
   *
   * @param aResult One of returnOK, returnCancel, or returnReplace
   */
   void done(in short aResult);
 };
 
-[scriptable, uuid(9840d564-42c8-4d78-9a4d-71002343c918)]
+[scriptable, uuid(2a74ba0d-ffdd-4dad-b78a-98635e525a38)]
 interface nsIFilePicker : nsISupports
 {
   const short modeOpen        = 0;              // Load a file or directory
   const short modeSave        = 1;              // Save a file or directory
   const short modeGetFolder   = 2;              // Select a folder/directory
   const short modeOpenMultiple= 3;              // Load multiple files
 
   const short returnOK        = 0;              // User hit Ok, process selection
@@ -137,29 +137,29 @@ interface nsIFilePicker : nsISupports
   * Get the enumerator for the selected files
   * only works in the modeOpenMultiple mode
   *
   * @return Returns the files currently selected
   */
   readonly attribute nsISimpleEnumerator files;
 
  /**
-  * Get the DOM File for the file.
+  * Get the DOM File or the DOM Directory
   *
-  * @return Returns the file currently selected as File
+  * @return Returns the file or directory currently selected DOM object.
   */
-  readonly attribute nsISupports domfile;
+  readonly attribute nsISupports domFileOrDirectory;
 
  /**
-  * Get the enumerator for the selected files
+  * Get the enumerator for the selected files or directories
   * only works in the modeOpenMultiple mode
   *
-  * @return Returns the files currently selected as Files
+  * @return Returns the files/directories currently selected as DOM object.
   */
-  readonly attribute nsISimpleEnumerator domfiles;
+  readonly attribute nsISimpleEnumerator domFileOrDirectoryEnumerator;
 
  /**
   * Controls whether the chosen file(s) should be added to the system's recent
   * documents list. This attribute will be ignored if the system has no "Recent
   * Docs" concept, or if the application is in private browsing mode (in which
   * case the file will not be added). Defaults to true.
   */
   attribute boolean addToRecentDocs;