Closed Bug 338478 (CF_HDROP) Opened 18 years ago Closed 14 years ago

Support dragging and dropping files on windows


(Core :: Widget: Win32, enhancement)

Windows XP
Not set





(Reporter: u130342, Assigned: khuey)




(2 files, 8 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20060426 Firefox/
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20060426 Firefox/

I have a variable lf of type nsILocalFile and I want to get the CF_HDROP clipboard format (from javascript, latest FF1.5).

My understanding is that I need  to put a "application/x-moz-file" flavour in the clipboard, but setTransferData seems to fail.
Somehow there is a CF_HDROP on the clipboard, but seems to be zero data.

I tried many variants of pointers (nsIFile, nsILocalFile, nsISupports), also I used nsISupportsString encapsulating nsIFile.path, nsIURI.spec but nothing helps.

I'm able to put it there as text as "text/unicode" or "application/x-moz-url", but thats not what I want to realize.

Having a "Shell IDList Array" would also be nice.

Also this must support unicode filenames. This would invite a "application/x-moz-file-unicode"  (suggested in news).

Also copying multiple files should be supported ("application/x-moz-fileset-unicode").

Reproducible: Always

Steps to Reproduce:
Following snipped gives an idea whats not working ...

const CI=Components.interfaces;
 // lf = nsILocalFile
 // uri= nsIURI

var gClipboard = Components.classes[";1"].getService(CI.nsIClipboard);
  var trans = Components.classes[";1"].createInstance(CI.nsITransferable);

var kFileMime="application/x-moz-file";

// trans.setTransferData(kFileMime, lf.QueryInterface(CI.nsISupports), 4);

// trans.setTransferData(kFileMime, lf, 4); // 4=pointer size
// trans.setTransferData(kFileMime, lf, 1);
// trans.setTransferData(kFileMime, lf, lf.path.length);
// trans.setTransferData(kFileMime, lf, lf.path.length*2);
// trans.setTransferData(kFileMime, uri, 4);
// trans.setTransferData(kFileMime, uri, 1);
// trans.setTransferData(kFileMime, uri, uri.spec.length);
// trans.setTransferData(kFileMime, uri, uri.spec.length*2);

var sstr = Components.classes[";1"].createInstance(CI.nsISupportsString);
var text=uri.spec;;
// trans.setTransferData(kFileMime, sstr, text.length);
// trans.setTransferData(kFileMime, sstr, text.length*2);

Actual Results:  
nothing usable in clipboard.

Expected Results:  
paste in explorer should copy the file to the new target.
create link should create the link.
drag&drop from extension to another application should open the file.

There are two related posts:
netscape.public.mozilla.win32 "How to get a CF_HDROP from javascript" "Help about putting a file in clipboard"

nsIDragSession can handle multiple files by multiple nsITransferable. It could be possible to use a set of "application/x-moz-file-unicode" in nsITransferable, but then firefox needs to generate CF_HDROP, "Shell IDList Array" automatically.

It is possible to create "FileName" (plain ansi) and "FileNameW" (plain unicode) clipboard entries, but they're not useable for paste or drag&drop (and single filenames).
Component: Extension/Theme Manager → General
QA Contact: extension.manager → general
Assignee: nobody → win32
Severity: normal → enhancement
Component: General → Widget: Win32
Product: Firefox → Core
QA Contact: general → ian
Summary: It's impossible to copy a file link to clipboard for paste or. drag&drop → Support copying a file to clipboard for paste or drag and drop
Version: unspecified → Trunk
A program that can show the data structure of clipboard or drag and drop operation

Credits to original author Michael Dunn
I'll note that it appears the windows nsDataObj.cpp already has code to build the necessary CF_HDROP structure, but it will only do this for image files of the content type "application/x-moz-nativeimage"

Perhaps this function can be generalized
Assignee: win32 → nobody
QA Contact: ian → win32
I'm working on this intermittently, hope to have something done in a week or two.
Assignee: nobody → me
Ever confirmed: true
Brain dump to myself for when I pick this back up this weekend.

Gecko implements the IDataObject interface needed for OLE drag and drop through nsDataObj.  nsDataObj contains most of the code needed to do this.  Retrofitting the GetFile function tagged in comment 3 is fairly easy (and is already complete).  What is more difficult is handling drag and drop of multiple objects at once.

Gecko "handles" dragging and dropping multiple objects by wrapping the nsDataObj for each object in an nsDataObjCollection.  nsDataObjCollection implements IDataObject too, so the nsDataObjCollection instance is passed to the outside world and when IDataObject::GetData is called on nsDataObjCollection is passes that through to its children.  Very good idea, unfortunately, nsDataObjCollection was never fully implemented.  At least two major changes need to be made, the nsDataObj classes need to realize when they're chained and receiving a STGMEDIUM structure that has already been written to to avoid overwriting each other's data, and nsDataObjCollection needs to properly advertise the flavors it provides.

(Currently drag and drop flavors are set by nsClipboard::CreateNativeDataObject which never sees nsDataObjCollection.  As far as I can see the add flavor's method on nsDataObjCollection is never called.)

Note that all of the GetXXXXXX methods need to recognize when they are being called in a chain and play nicely.  That may be a project for another bug though (and I filed another one, don't have the number handy though).  Also need to investigate whether all of the D&D formats support multiple objects.  Thunderbird has the ability to drag and drop attachments because it uses the "application/x-moz-file-promise".  "application/x-moz-file-promise" maps to the native CFSTR_FILECONTENTS which is intended for virtual files, not for actual filesystem objects so we probably shouldn't go that route here.

Probably need to spend a decent amount of time reading all the MSDN docs too, some of this stuff is downright ancient (designed for Windows 3.1)
Target Milestone: --- → mozilla1.9.3a1
Not sure if it's related, but judging from file D&D should be supported in the recent Geckos.
(In reply to comment #6)
> Not sure if it's related, but judging from
> file D&D should be supported in the recent Geckos.

Gecko supports it internally, it's just the interfacing with Windows (and potentially other platforms, haven't looked at Linux or Mac) that's missing.
Attached patch WIP #1 (obsolete) — Splinter Review
This patch is alpha quality at best (there's a ridiculous amount of cleanup to do) but it works.  This also contains some patches to the download manager to let you drag and drop files out of it (to say the desktop).  It works with one or multiple files.

There's a lot of work remaining to be done.  All of the other GetXXX functions need to be updated to support being part of an nsDataObjContainer, and there's plenty of code cleanup to do.

I'll pick this up again next weekend.

And btw, Devon, thanks for that clipboard program.  It's incredibly useful when playing with this stuff.
Attached patch WIP #2 (obsolete) — Splinter Review
Attachment #411093 - Attachment is obsolete: true
Woops, forgot my comments on that:

This patch makes all of the existing data formats work correctly on a nsDataObj that is wrapped by an nsDataObjCollection.  As before, some download manager modifications are included so that this can be tested.  Not every native format accepts multiple objects, so for those that don't nsDataObjCollection just passes GetData along to the first object.

There is some hackery involved with IStreams because we need to know what index to respond to.  There is also a problem because this patch uses GlobalSize to figure out where to start writing, which seems to work ok on my system but the msdn docs say pretty clearly "the size of a memory block may be larger than the size requested when the memory was allocated."  It looks like there will need to be some more communication/synchronization between nsDataObj/*Collection or the concatenation of responses will need to be done in the nsDataObjCollection and not in the nsDataObj. (I'm actually leaning towards the latter).

Also updated the bug summary to make it clear that this has little to do with the clipboard.
Summary: Support copying a file to clipboard for paste or drag and drop → Support dragging and dropping multiple files on windows ( CF_HDROP, etc)
Sorry to edit the title again, I'm going to spin off the nsDataObjCollection changes to another bug.
Summary: Support dragging and dropping multiple files on windows ( CF_HDROP, etc) → Support dragging and dropping files on windows
Attached patch Patch (obsolete) — Splinter Review
For review.  I split out all of the nsDataObjCollection changes and all of the miscellaneous cleanup, so what's left should be pretty straightforward.
Attachment #411796 - Attachment is obsolete: true
This is not for review, but is useful for testing the drag and drop functionality.  It allows you to drag files out of the download manager.
Attached patch Patch V1.1 (obsolete) — Splinter Review
Fixed a couple whitespace nits.
Attachment #413011 - Attachment is obsolete: true
Attachment #413016 - Flags: review?(roc)
Attachment #413011 - Flags: review?(roc)
What can we do to get an automated test for this? It seems to me we might be able to test it with a unit test.
(In reply to comment #16)
> What can we do to get an automated test for this? It seems to me we might be
> able to test it with a unit test.

Maybe.  To test the native code we need to drag the file to another OLE drop target (otherwise Gecko will handle it internally).  It may be sufficient to drag it between different windows within one process.  MXR is being goofy right now, but I will investigate.

There aren't any existing DnD tests are there?  I didn't see any but I could be looking in the wrong place.
There are some, but I don't know if they test dragging to external apps.

Is the Explorer shell such a drop target? I wonder if we could open an Explorer window, simulate dragging the file into it, and test that Explorer did the right thing. We do have the ability to simulate native mouse events at a low level on Windows, see nsDOMWindowUtils::SendNativeMouseEvent.
(In reply to comment #18)
> Is the Explorer shell such a drop target? 

Yes, that's the primary use case for this.  I'll play around with it and see what I can come up with.
Attached patch Compiled test (obsolete) — Splinter Review
This obviously needs more work and is not ready for an r+, but I wanted to get some feedback on whether I'm going in the right direction here.  nsIDOMWindowUtils unfortunately doesn't have the ability to fake input to other program's windows so we'll have to use a compiled test.

What I've done is link against the compiled result of /widget/src/windows and its dependencies.  Then we can play fake nsDragService: make the transferables and pass them to nsClipboard which creates the data objects which we can test.

I think this is the best way to go about it.  Suggestions and comments welcome.  I'll expand this to cover other DnD scenarios too.
Attachment #413828 - Flags: review?(roc)
Blocks: 494989
Comment on attachment 413828 [details] [diff] [review]
Compiled test

Cool, I'll clean it up and expand.
Attachment #413828 - Attachment is obsolete: true
Attachment #413828 - Flags: review?(roc)
Attached patch Tests (obsolete) — Splinter Review
This tests this patch and also dragging single strings of data.  I've structured it so it'll be a lot easier to add tests of more types and tests of multiple objects which we'll need later in Bug 513464.
Attachment #414153 - Flags: review?(roc)
Blocks: 462172
Test does not build with --enable-libxul.  Clearing checkin-needed while I fix it.
Keywords: checkin-needed
--enable-libxul doesn't build a lot of the intermediate libraries I linked against.  The xpcom tests seem to #ifndef MOZ_ENABLE_LIBXUL their tests that rely on internal bits, so I may end up doing that, because it doesn't appear that I can link my way out of this.
--enable-libxul doesn't expose a lot of the symbols we need for this so just skip this test on an --enable-libxul build.  There are also a couple other cleanups here too.  This is a diff based off of what what already reviewed.
Attachment #416872 - Flags: review?(roc)
This includes the two patches already r+'d and the makefile changes I just uploaded.
Kyle, I think you need to r+ the patch and indicate it is forwarding r+=roc and replace those patches with this one.
see bug 450861 comment 47.
Comment on attachment 416873 [details] [diff] [review]
Cumulative patch for checkin

Thanks Phil.  In case it's not clear, this patch is a combination of three individual diffs that have all been r+'d by roc.
Attachment #416873 - Flags: review+
Closed: 14 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.