Scenarios: 1. user runs firefox on windows and enters an http: url into the windows filepicker. The result is a temporary file containing the contents of the url (except when windows doesn't like the url structure) 2. user runs firefox on gnome and browses to an obex: url The result is nothing, because the file picker (which knows nothing about gnomevfs) fails to convert it to an nsILocalFile. And the nsIFileURL scheme would not be any more flexible 3. user wants to do something more interesting with the xul file picker The xul file picker itself is constrained by the contract. Proposal: change the contract so that for fileURLs, it returns an nsIURI (very flexible). This would mean that a filepicker (e.g. the xul filepicker, or the gnome file picker) could return a url (http:, ftp:, obex:, data:) to any consumer that asks for a url. If the consumer QIs to nsIFileURL, it either succeeds or fails (at present our tree doesn't seem to have *anyone* using this feature, and given that you can't get multiple nsIFileURLs whereas you can get multiple nsILocalFiles, I can't see why most consumers would consider asking for nsIFileURLs today). This is sort of step one in a 3 or 4 step process, the goal is to be able to upload from random protocols. Each of these changes should be relatively reasonable. I hope...

I'm using this patch (plus a couple of others) to upload this patch from an http url in linux :).

Comment on attachment 297590 [details] [diff] [review] change the interface IMHO you can't call it a file picker any more.

I spoke with neil and I think we came to an agreement that this would be OK. creating a new interface doesn't really help anyone, and this interface is basically flexible enough.

Comment on attachment 298480 [details] [diff] [review] use nsIFilePicker.appendFilters(nsIFilePicker.filterAllowURLs) >+ return NS_NewURI(aFileURL, mFilename); You can't make a URI out of a filename. >+ allowURLs = o.allowURLs; Turn on autocomplete from history perhaps? ;-) >- readonly attribute nsIFileURL fileURL; >+ readonly attribute nsIURI fileURL; (How?) do you know that nobody's expecting a fileURL? >- file->InitWithPath(mUnicodeFile); >+ *aFileURL = nsnull; >+ if (mFile.IsEmpty()) We don't use mFile here... >+ try { >+ if (this.mFileURL) >+ return this.mFileURL; How is this going to throw?

http://timeless.justdave.net/mxr-test/os2008/search?string=upload_dialog&find=microb-eal you can't see how this is implemented because it's hidden behind another trampoline, however it really uses the url flavor of the GTK APIs instead of the file flavor. note that I'm not changing that code, merely moving it. upload_dialog_cb (GtkMozEmbed *embed, const gchar *path, const gchar *filter, gchar **file_name_with_path, GMozillaEngine *self) ... g_signal_emit_by_name (G_OBJECT(self), G_WEBWIDGET_SIGNAL_UPLOAD_DIALOG, getenv("HOME"), "", file_name_with_path, &response, NULL);

Comment on attachment 301537 [details] [diff] [review] Fix windows instance, clarify embedding case >+ try { >+ var ios = Components.classes[NS_IOSERVICE_CONTRACTID].getService(Components.interfaces.nsIIOService); >+ retvals.fileURL = ios.newURI(textInput.value, '', null); >+ fileList = []; I don't think you should try to do this if allowURLs is false. >+ retvals.fileURL = null; (which means that you should do this earlier) >+ if (!allowURLs || !retvals.fileURL) { (which means that you won't have to check allowURLs here) >+ return NS_NewURI(aFileURL, mFile.get()); Comment #4 was supposed to apply in multiple places, also why the .get()? >+ if (!this.mFileURL && !this.mFilesEnumerator) >+ return null; >+ >+ var ioService = Components.classes["@mozilla.org/network/io-service;1"] > .getService(Components.interfaces.nsIIOService); >+ if (this.mFileURL) >+ return this.mFileURL; You're getting the ioService before you need to (might allow simplification). >+ var url = ioService.newFileURI(this.file); >+ return url; Doesn't need a temporary.

Comment on attachment 301865 [details] [diff] [review] Simplify JS case, redo others This was fine on Windows but unfortunately didn't compile for me on Linux. >+ retvals.fileURL = ios.newURI(textInput.value, '', null); null, null is allowed. >+ if (retvals.fileURL instanceof Components.interfaces.nsIFileURL) >+ fileList.push(retvals.fileURL.nsIFileURL.file); No need to QI twice.

Comment on attachment 301865 [details] [diff] [review] Simplify JS case, redo others a1.9+=damons

Comment on attachment 301865 [details] [diff] [review] Simplify JS case, redo others mozilla/embedding/browser/gtk/src/EmbedFilePicker.cpp 1.6 mozilla/embedding/browser/gtk/src/EmbedFilePicker.h 1.6 mozilla/toolkit/components/filepicker/content/filepicker.js 1.19 mozilla/toolkit/components/filepicker/content/filepicker.js 1.20 mozilla/widget/public/nsIFilePicker.idl 3.23 mozilla/widget/src/cocoa/nsFilePicker.h 1.12 mozilla/widget/src/cocoa/nsFilePicker.mm 1.21 mozilla/widget/src/beos/nsFilePicker.cpp 1.30 mozilla/widget/src/beos/nsFilePicker.h 1.13 mozilla/widget/src/gtk2/nsFilePicker.cpp 1.17 mozilla/widget/src/gtk2/nsFilePicker.cpp 1.18 mozilla/widget/src/gtk2/nsFilePicker.cpp 1.19 mozilla/widget/src/gtk2/nsFilePicker.h 1.6 mozilla/widget/src/os2/nsFilePicker.cpp 1.55 mozilla/widget/src/os2/nsFilePicker.h 1.16 mozilla/widget/src/photon/nsFilePicker.cpp 1.14 mozilla/widget/src/photon/nsFilePicker.h 1.8 mozilla/widget/src/windows/nsFilePicker.cpp 1.96 mozilla/widget/src/windows/nsFilePicker.h 1.23 mozilla/xpfe/components/filepicker/src/nsFilePicker.js.in 1.49