Last Comment Bug 526996 - access nsIDOMFileInternal in xpcom extension
: access nsIDOMFileInternal in xpcom extension
Status: VERIFIED FIXED
[evang-wanted-3.6]
: dev-doc-complete, verified1.9.2
Product: Core
Classification: Components
Component: Drag and Drop (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.9.3a3
Assigned To: Jan Gerber
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-11-06 06:26 PST by Jan Gerber
Modified: 2010-06-15 11:33 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.2-fixed


Attachments
patch to return full path if IsCallerTrustedForCapability("UniversalFileRead") (434 bytes, patch)
2010-01-07 13:25 PST, Jan Gerber
no flags Details | Diff | Splinter Review
add readonly attribute DOMString mozFullPath (434 bytes, patch)
2010-01-07 18:54 PST, Jan Gerber
no flags Details | Diff | Splinter Review
add readonly attribute DOMString mozFullPath (944 bytes, patch)
2010-01-07 18:56 PST, Jan Gerber
jonas: review+
jonas: superreview+
Details | Diff | Splinter Review
add readonly attribute DOMString mozFullPath (969 bytes, patch)
2010-01-08 01:25 PST, Jan Gerber
jonas: review+
jonas: superreview+
Details | Diff | Splinter Review
Patch 1_9_2_BRANCH (2.78 KB, patch)
2010-02-22 04:20 PST, Paul Rouget [:paul]
no flags Details | Diff | Splinter Review
Patch 1_9_2_BRANCH with classInfo (3.55 KB, patch)
2010-02-22 04:49 PST, Paul Rouget [:paul]
jonas: review+
mbeltzner: approval1.9.2.2+
Details | Diff | Splinter Review
test (enhanced previlegies) (596 bytes, text/plain)
2010-03-22 16:31 PDT, Henrik Skupin (:whimboo) [away 09/30 - 10/06]
no flags Details

Description Jan Gerber 2009-11-06 06:26:24 PST
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2b2pre) Gecko/20091105 Namoroka/3.6b2pre
Build Identifier: 

i want to allow drag and drop of files in my extension.
to do this i have a function in my extension that gets passed nsIDOMFile
via an api exposed to the page(global constructor)

boolean drop(in nsIDOMFile file);

and try to get nsIDOMFileInternal
 var internal = file.QueryInterface(Components.interfaces.nsIDOMFileInternal);

this fails.

there should be a way to get the local path inside of an xpcom component.


Reproducible: Always
Comment 1 Christopher Blizzard (:blizzard) 2009-11-11 13:41:55 PST
I'll bet you can do this today somehow.  Adding some people who might know how.
Comment 2 Jonas Sicking (:sicking) No longer reading bugmail consistently 2009-11-11 14:44:19 PST
Yeah, I don't think there's currently a way to get to the inner file from script. It's always scary making internal interfaces accessible on public objects because of the way that XPConnect works. Anytime you QI, the functions are permanently added to the object and so exposed to web pages.

Generally you can't make use of the functions, but it's still scary to rely on. And it alters the object in ways that could potentially break pages.

At least that is how it used to be. In this new world of wrappers everywhere things might be different.


There's still a few things we could do though:

* Make .name return the full path if the caller has chrome privileges. This is
  how inputElement.value works.
* Add a function on windowutils that returns the nsIFile inside a nsIDOMFile.
* Maybe something else through the magic of wrappers.
Comment 3 Jan Gerber 2009-11-14 05:24:49 PST
returning the full path in case caller has chrome privileges in .name sounds like a good solution to me.
Comment 4 Blake Kaplan (:mrbkap) 2009-11-16 07:43:26 PST
Yeah, the simpler the solution here, the better, I think.

Also, confirming based on comment 2.
Comment 5 Jan Gerber 2009-12-09 07:03:00 PST
has anyone looked into exposing full path in name in the privileged case?
Comment 6 Jan Gerber 2010-01-07 13:24:59 PST
so i went ahead and changed nsIFile to return the full path,
however someting else changed and my extension no longer works,

in the idl file, dropFile is specified like this:
  boolean dropFile(in nsIDOMFile file);

in javascript its called like this:
dropContainer.addEventListener("drop", function (event) {
  var dt = event.dataTransfer,
  files = dt.files;
  if(files.length >= 1) {
    var ext = new Myextension();
    alert(files[0].name);
    ext.dropFile(files[0]);
  }
  event.stopPropagation();
  event.preventDefault();
}, false);

this now thows an exception:

Error: uncaught exception: [Exception... "Cannot find interface information for parameter arg 0 [myextension.dropFile]"  nsresult: "0x80570006 (NS_ERROR_XPC_CANT_GET_PARAM_IFACE_INFO)"  location: "JS frame :: file:///test/drop.html :: anonymous :: line 34"  data: no]

at the time i created this bug, dropFile was called and an nsIDOMFile was passed to it.
Comment 7 Jan Gerber 2010-01-07 13:25:52 PST
Created attachment 420606 [details] [diff] [review]
patch to return full path if IsCallerTrustedForCapability("UniversalFileRead")
Comment 8 Jonas Sicking (:sicking) No longer reading bugmail consistently 2010-01-07 14:14:47 PST
Please file a separate bug on the problem in comment 6. It seems unrelated to this bug.

As for the patch, the more I think about it, the more I think that it'd be better to have a separate property that exposed the full path. This both has the advantage that the path-less filename is still easily accessible, and would keep the API more consistent.

Call the new property .mozFullPath or something like that. And make it return the empty string if the caller doesn't have UniversalFileAccess
Comment 9 Jan Gerber 2010-01-07 18:54:56 PST
Created attachment 420671 [details] [diff] [review]
add readonly attribute DOMString mozFullPath

add new attribute mozFullPath to nsIDOMFile:

readonly attribute DOMString mozFullPath;

returns full path if nsContentUtils::IsCallerTrustedForCapability("UniversalFileRead")
otherwise it returns an empy string.
Comment 10 Jan Gerber 2010-01-07 18:56:06 PST
Created attachment 420672 [details] [diff] [review]
add readonly attribute DOMString mozFullPath

attach the right patch.
Comment 11 Jonas Sicking (:sicking) No longer reading bugmail consistently 2010-01-07 19:56:10 PST
Comment on attachment 420672 [details] [diff] [review]
add readonly attribute DOMString mozFullPath

You need to call

aFileName.Truncate()

when returning the empty string. To ensure that if aFileName was non-empty when the function is called it's still the empty string when we return.

r/sr=me with that fixed.
Comment 12 Jan Gerber 2010-01-08 01:25:06 PST
Created attachment 420702 [details] [diff] [review]
add readonly attribute DOMString mozFullPath

add aFileName.Truncate(); to patch
Comment 13 Jonas Sicking (:sicking) No longer reading bugmail consistently 2010-01-08 08:31:18 PST
Comment on attachment 420702 [details] [diff] [review]
add readonly attribute DOMString mozFullPath

Crap, I forgot that you need to update the interface iid. But I can do that before checking in.

Thanks for the patch!
Comment 14 Jan Gerber 2010-01-13 19:19:07 PST
any idea which version of firefox this will be availalbe?
Comment 15 Jonas Sicking (:sicking) No longer reading bugmail consistently 2010-02-02 16:56:45 PST
j@oil21.org: What's your name, so that I can add your proper name to the checkin

:)
Comment 16 Jan Gerber 2010-02-02 21:01:38 PST
(In reply to comment #15)
> j@oil21.org: What's your name, so that I can add your proper name to the
> checkin
> 
> :)
that would be Jan Gerber
Comment 17 d 2010-02-03 01:15:36 PST
(In reply to comment #14)
> any idea which version of firefox this will be availalbe?

That would probably be the next Firefox version after 3.6.x, Firefox 3.7 or 4.0. It's hard to tell because the schedule changes. This could get in to 3.6.(1/2) I guess, though, if we ask specifically for it. What do say, Jonas?
Comment 18 Jan Gerber 2010-02-21 09:10:27 PST
blame sicking, says Axel.
Comment 19 Jonas Sicking (:sicking) No longer reading bugmail consistently 2010-02-22 02:51:31 PST
Would be great if someone could write up a patch for 1.9.2 as well. It needs to not change any existing interfaces, but instead add the property to a new nsIDOMFile_1_9_2_BRANCH interface. That way we can fix this in firefox 3.6 as well.
Comment 20 Paul Rouget [:paul] 2010-02-22 04:20:32 PST
Created attachment 428174 [details] [diff] [review]
Patch 1_9_2_BRANCH
Comment 21 Jonas Sicking (:sicking) No longer reading bugmail consistently 2010-02-22 04:23:47 PST
You need to also add the interface here:

http://mxr.mozilla.org/mozilla-central/source/dom/base/nsDOMClassInfo.cpp#3571

r=me with that fixed
Comment 22 Paul Rouget [:paul] 2010-02-22 04:49:05 PST
Created attachment 428178 [details] [diff] [review]
Patch 1_9_2_BRANCH with classInfo
Comment 23 Jonas Sicking (:sicking) No longer reading bugmail consistently 2010-02-22 05:49:12 PST
Comment on attachment 428178 [details] [diff] [review]
Patch 1_9_2_BRANCH with classInfo

This is needed for extensions to be able to use some of the new File APIs we added in Firefox 3.6. Specifically, this will allow extensions to go get the full path of a DOMFile in order to access it directly using normal gecko or OS file APIs.

It's been baking a while on trunk and should be very safe.
Comment 24 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2010-02-22 19:58:45 PST
Clearing checkin-needed as it looks like everything reviewed and approved has been checked in.  If I'm incorrect please set checkin-needed again and make it clearer what needs to be checked in.
Comment 25 Mike Beltzner [:beltzner, not reading bugmail] 2010-02-26 13:57:42 PST
Comment on attachment 428178 [details] [diff] [review]
Patch 1_9_2_BRANCH with classInfo

a1922=beltzner
Comment 26 Jonas Sicking (:sicking) No longer reading bugmail consistently 2010-03-08 12:06:26 PST
Checked in to 1.9.2.2

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/ad92ebff4d0c
Comment 27 Mike Beltzner [:beltzner, not reading bugmail] 2010-03-08 20:17:06 PST
For the record, this seems to have caused a 3% regression on Ts Dirty Profile tests; not sure it requires backing out, but thought you'd like to know.
Comment 28 Mike Beltzner [:beltzner, not reading bugmail] 2010-03-08 20:17:43 PST
Wait - it doesn't make any sense for this to have caused that regression. Ignore comment 27, please.
Comment 29 Reed Loden [:reed] (use needinfo?) 2010-03-22 02:22:15 PDT
http://hg.mozilla.org/mozilla-central/rev/6d13028da120
Comment 30 Henrik Skupin (:whimboo) [away 09/30 - 10/06] 2010-03-22 16:30:25 PDT
Verified on trunk and 1.9.2 with the testcase I will attach shortly.

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a4pre) Gecko/20100321 Minefield/3.7a4pre ID:20100321123434

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2.3pre) Gecko/20100321 Namoroka/3.6.3pre ID:20100321033650

It also needs an update on MDC.
Comment 31 Henrik Skupin (:whimboo) [away 09/30 - 10/06] 2010-03-22 16:31:04 PDT
Created attachment 434071 [details]
test (enhanced previlegies)
Comment 32 Jan Gerber 2010-03-22 17:31:50 PDT
also made an example page using the new feature with Firefogg at 
http://firefogg.org/examples/drop/
Comment 33 Eric Shepherd [:sheppy] 2010-06-15 11:33:18 PDT
Documentation updated: https://developer.mozilla.org/en/DOM/File

Note You need to log in before you can comment on or make changes to this bug.