Closed
Bug 607114
Opened 14 years ago
Closed 14 years ago
Present an easy way to create nsIDOMFiles in chrome
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla6
People
(Reporter: anant, Assigned: khuey)
References
Details
(Keywords: dev-doc-complete)
Attachments
(1 file, 2 obsolete files)
8.41 KB,
patch
|
sicking
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•14 years ago
|
Assignee: jonas → administration
Assignee | ||
Updated•14 years ago
|
Assignee: administration → nobody
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → khuey
Assignee | ||
Comment 1•14 years ago
|
||
Attachment #485912 -
Flags: review?(jonas)
Comment 2•14 years ago
|
||
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.
Reporter | ||
Comment 3•14 years ago
|
||
Also, it would be nice to be able to create instances of nsIDOMFiles from C++ too. Let's not be "languageist" :)
You can already create them in C++, the constructors are right here:
http://mxr.mozilla.org/mozilla-central/source/content/base/public/nsDOMFile.h
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.
Reporter | ||
Comment 6•14 years ago
|
||
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.
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.
Reporter | ||
Comment 8•14 years ago
|
||
Wouldn't something like
Cc["@mozilla.org/dom-file;1"].createInstance(Ci.nsIDOMFile)
be accessible to chrome javascript only?
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?
Reporter | ||
Comment 10•14 years ago
|
||
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?
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.
Assignee | ||
Comment 12•14 years ago
|
||
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.
Reporter | ||
Comment 13•14 years ago
|
||
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.
Assignee | ||
Comment 14•14 years ago
|
||
(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.
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 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.
Attachment #485912 -
Flags: review?(jonas) → review+
Assignee | ||
Comment 17•14 years ago
|
||
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.
Assignee | ||
Comment 18•14 years ago
|
||
This makes the File constructor only available in the chrome scope and fixes Jonas's other comments.
Attachment #485912 -
Attachment is obsolete: true
Attachment #522032 -
Flags: superreview?(jst)
Attachment #522032 -
Flags: review?(jonas)
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.
Attachment #522032 -
Flags: superreview?(jst)
Attachment #522032 -
Flags: review?(jonas)
Attachment #522032 -
Flags: review-
Updated•14 years ago
|
Whiteboard: not-ready-for-cedar
Assignee | ||
Comment 21•14 years ago
|
||
Updated with mrbkap's magic.
Attachment #522032 -
Attachment is obsolete: true
Attachment #525125 -
Flags: review?(jonas)
Assignee | ||
Updated•14 years ago
|
Whiteboard: not-ready-for-cedar
Attachment #525125 -
Flags: review?(jonas) → review+
Comment 22•14 years ago
|
||
This could potentially conflict with jslib.mozdev.org, which has a File constructor as well for its File I/O library.
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.
Assignee | ||
Comment 24•14 years ago
|
||
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
Status: NEW → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Keywords: dev-doc-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
Comment 25•14 years ago
|
||
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?
Assignee | ||
Comment 26•14 years ago
|
||
Correct.
Comment 27•14 years ago
|
||
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.
Keywords: dev-doc-needed → dev-doc-complete
Comment 28•14 years ago
|
||
Sheppy, this is Firefox 6, not 5 as far as I can see.
Comment 29•14 years ago
|
||
Oops, I misread the check-in date; I'll fix it.
Comment 30•14 years ago
|
||
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•14 years ago
|
||
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)"
Assignee | ||
Comment 32•14 years ago
|
||
(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•14 years ago
|
||
in that case marking bug #647653 a duplicate of this one was wrong, i need this in a component.
Updated•12 years ago
|
Component: DOM: Mozilla Extensions → DOM
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•