607114 - Present an easy way to create nsIDOMFiles in chrome

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Anant Narayanan [:anant]]

Would be nice for extensions to have access to an easy way of creating an nsIDOMFile object given a local path.

"let domfile = new File(path)" would be awesome, but even exposing nsDOMFile would do.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Adding a parameter to a standard constructor really isn't acceptable. This makes it impossible for the standard to define a constructor with an argument. If we want to do this, we should do it through a ChromeFile interface or some such.

Comment 3

[Anant Narayanan [:anant]]

Also, it would be nice to be able to create instances of nsIDOMFiles from C++ too. Let's not be "languageist" :)

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

You can already create them in C++, the constructors are right here:

http://mxr.mozilla.org/mozilla-central/source/content/base/public/nsDOMFile.h

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Ah, or do you mean using XPCOM? I suppose one way to get two birds with one stone would be to just expose a XPCOM contract-id and a init method. Though init methods are always tricky as we don't want it exposed to content, even by accident.

Comment 6

[Anant Narayanan [:anant]]

Yes, through XPCOM so that add-ons can use it. I know atleast two mozilla labs projects that would benefit from it, I think many other add-on developers would too.

We would have be to careful about not exposing it to content, yes.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Unfortunately there isn't really a way to make a function exposed only to chrome javascript. What we'd have to do is make the init function exposed to everyone, but if called by content JS make it throw.

Comment 8

[Anant Narayanan [:anant]]

Wouldn't something like

Cc["@mozilla.org/dom-file;1"].createInstance(Ci.nsIDOMFile)

be accessible to chrome javascript only?

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Yes, but then you need to initialize it to an actual file, right? I.e. you'll want to hand it a path or an nsIFile instance or some such I would imagine?

Comment 10

[Anant Narayanan [:anant]]

Yes. So something like:

let domFile = Components.Constructor("@mozilla.org/dom-file;1");
let tmp = new domFile(some_nsIFile);

I imagine this would also be inaccessible to content javascript unless I am missing something. Under the hood, there would an init function that will be called, but I don't see why that has to be exposed to non-chrome code?

Comment 11

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Kyle:
Do you want to write a new patch here to use an XPCOM ctor. I'm not actually sure that it is possible to write a XPCOM ctor that takes arguments like the proposal in comment 10.

I'd also be ok with adding a scriptable mozInit function which is only callable my chrome-script. 

Anant:
Would the current patch be ok with you if Kyle doesn't have time to write another patch.

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

It's not possible to have an XPCOM ctor as clean as this.  I don't think this blocks what Anant is working on at the moment anymore though, so perhaps we just let this lie for a bit and raise the question of what a File() constructor should do in the appropriate standards body.

Comment 13

[Anant Narayanan [:anant]]

Hmm, I'm afraid I'm not very familiar with XPCOM internals to know why such a constructor would not be possible. As Kyle mentioned, however, I am currently creating an nsIDOMFile via the nsIDOMHTMLInputElement interface, and so this bug is not blocking :)

In general though, it would be nice to have a clean way to create nsIDOMFile instances both from C++ and JS. I suspect many add-on authors would appreciate it.

Comment 14

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #13)
> Hmm, I'm afraid I'm not very familiar with XPCOM internals to know why such a
> constructor would not be possible. As Kyle mentioned, however, I am currently
> creating an nsIDOMFile via the nsIDOMHTMLInputElement interface, and so this
> bug is not blocking :)

You'd need an init method or similar.  XPConnect handles this magic for you from JS.  It's all possible, just a bit uglier.

> In general though, it would be nice to have a clean way to create nsIDOMFile
> instances both from C++ and JS. I suspect many add-on authors would appreciate
> it.

I agree, at least for JS, though I'm sympathetic to Ms2ger's concerns about not boxing in the FileAPI folks at the standard level.  I'm not sure it's that useful from C++ (unlike, say, XHR), because you can only do "DOMish" things with it and especially since we're trying to move people away from binary addons as much as possible.

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

The specs will not cover initializing a File using a path as they only cover things that are safe for the web platform.

So I think we should feel free to come up with whatever API we like the best.

Comment 16

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 485912 [details] [diff] [review]
Patch

reviewing this in case we want it (and to get it out of my queue)


>+nsDOMFile::NewFile(nsISupports* *aNewObject)
>+{
>+  nsRefPtr<nsDOMFile> file = new nsDOMFile(NULL);
>+  return CallQueryInterface(file, aNewObject);
>+}

s/NULL/nsnull/


>+nsDOMFile::Initialize(nsISupports* aOwner,
>+                      JSContext* aCx,
>+                      JSObject* aObj,
>+                      PRUint32 aArgc,
>+                      jsval* aArgv)
>+{
>+  if (!nsContentUtils::IsCallerChrome()) {
>+    return NS_ERROR_DOM_SECURITY_ERR; // Real short trip
>+  }
>+
>+  // We expect to get a path to represent as a File object
>+  JSString* str = JS_ValueToString(aCx, aArgv[0]);
>+  NS_ENSURE_TRUE(str, NS_ERROR_XPC_BAD_CONVERT_JS);

You need to check that aArgv > 0 first.

>+  nsCOMPtr<nsILocalFile> localFile;
>+  nsresult rv = NS_NewLocalFile(nsDependentJSString(str),
>+                                PR_FALSE, getter_AddRefs(localFile));
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  nsCOMPtr<nsIFile> file = do_QueryInterface(localFile, &rv);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  mFile = file;
>+  return NS_OK;

Do we also want to check that the file exists?

r=me with those things fixed. Last one optional.

Comment 17

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Unless anybody here has objections, I'm going to land this for Firefox 5/Gecko 3/2.2/whatever.  I think extensions will find this useful.

Comment 18

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

This makes the File constructor only available in the chrome scope and fixes Jonas's other comments.

Comment 19

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 522032 [details] [diff] [review]
Patch

This is no good. The problem is if chrome code reaches in to a content window and does |new contentWindow.File(...)| then the constructor will remain on the object forever allowing content code to access it.

Instead you should check the principal of the window on which the constructor is being looked up. Probably by passing aWeakOwner to FindConstructorFunc.

Hmm.. actually, check with mrbkap, I think he knows of another way we have of exposing chrome specific properties on windows.

Comment 21

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Updated with mrbkap's magic.

Comment 22

[Alex Vincent [:WeirdAl]]

This could potentially conflict with jslib.mozdev.org, which has a File constructor as well for its File I/O library.

Comment 23

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

That's unrelated to this bug. We already added a File "constructor" object when we exposed the File interface in Firefox 3.5 (or maybe even Firefox 3.0).

This bug just makes that constructor callable. Previously the only thing you could do with it is access constants as well as the prototype for File instances.

Comment 24

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

So we realized that we can't hide the "File" object entirely, so we went back to the original solution of just throwing if the caller isn't chrome in the constructor.

http://hg.mozilla.org/mozilla-central/rev/07e66863b615

Comment 25

[Eric Shepherd [:sheppy]]

So looking at the comments and test code here, looks like this just makes it so you can pass a path string to the File constructor if (and only if) you're in chrome code?

Comment 26

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Correct.

Comment 27

[Eric Shepherd [:sheppy]]

Documented:

https://developer.mozilla.org/en/Extensions/Using_the_DOM_File_API_in_chrome_code

Also added cross-links and notes to:

https://developer.mozilla.org/en/Using_files_from_web_applications
https://developer.mozilla.org/en/DOM/File

And a mention to Firefox 4 developers.

Comment 28

[Nickolay_Ponomarev]

Sheppy, this is Firefox 6, not 5 as far as I can see.

Comment 29

[Eric Shepherd [:sheppy]]

Oops, I misread the check-in date; I'll fix it.

Comment 30

[Eric Shepherd [:sheppy]]

OK, version note on this page fixed:

https://developer.mozilla.org/en/Extensions/Using_the_DOM_File_API_in_chrome_code

And I've created Firefox 6 for developers to list this there:

https://developer.mozilla.org/en/Firefox_6_for_developers

Comment 31

[Jan Gerber]

so how can this be used from xpcom components now? Trying with Firefox nightly, i get this:

var file = new File(path);

Error: uncaught exception: [Exception... "'[JavaScript Error: "File is not defined" {file: "..." line: ..}]' when calling method: [nsIFactory::createInstance]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"

Comment 32

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #31)
> so how can this be used from xpcom components now? Trying with Firefox
> nightly, i get this:
> 
> var file = new File(path);
> 
> Error: uncaught exception: [Exception... "'[JavaScript Error: "File is not
> defined" {file: "..." line: ..}]' when calling method:
> [nsIFactory::createInstance]"  nsresult: "0x80570021
> (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"

You have to be inside a Window.  This isn't defined for components.

Comment 33

[Jan Gerber]

in that case marking bug #647653 a duplicate of this one was wrong, i need this in a component.