338478 - (CF_HDROP) Support dragging and dropping files on windows

Status: RESOLVED FIXED

(Core :: Widget: Win32, enhancement)

Description

[u130342]

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

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["@mozilla.org/widget/clipboard;1"].getService(CI.nsIClipboard);
  var trans = Components.classes["@mozilla.org/widget/transferable;1"].createInstance(CI.nsITransferable);

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

trans.addDataFlavor(kFileMime);
// 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["@mozilla.org/supports-string;1"].createInstance(CI.nsISupportsString);
var text=uri.spec;
sstr.data=text;
// 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"
mozilla.dev.extensions: "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).

Comment 1

[Devon Jensen]

I would like to be able to drag and drop files, so this would be nice to have!

This explains the CF_HDROP structure:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/shellcc/platform/shell/programmersguide/shell_basics/shell_basics_programming/transferring/clipboard.asp
Also see:
http://www.codeproject.com/shell/explorerdragdrop.asp

Comment 2

[Devon Jensen]

Attachment: Small program to ease testing

ClipSpy.exe
A program that can show the data structure of clipboard or drag and drop operation

Credits to original author Michael Dunn
http://www.codeproject.com/clipboard/clipspy.asp

Comment 3

[Devon Jensen]

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"

http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsDataObj.cpp#1357

Perhaps this function can be generalized

Comment 4

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

I'm working on this intermittently, hope to have something done in a week or two.

Comment 5

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

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)

Comment 6

[Nickolay_Ponomarev]

Not sure if it's related, but judging from https://developer.mozilla.org/en/DragDrop/Recommended_Drag_Types#Dragging_Files file D&D should be supported in the recent Geckos.

Comment 7

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

(In reply to comment #6)
> Not sure if it's related, but judging from
> https://developer.mozilla.org/en/DragDrop/Recommended_Drag_Types#Dragging_Files
> 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.

Comment 8

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

Attachment: WIP #1

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.

Comment 9

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

Bug 524020 is the bug I mentioned in comment 5.

Comment 10

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

Attachment: WIP #2

Comment 11

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

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.

Comment 12

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

Sorry to edit the title again, I'm going to spin off the nsDataObjCollection changes to another bug.

Comment 13

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

Attachment: Patch

For review.  I split out all of the nsDataObjCollection changes and all of the miscellaneous cleanup, so what's left should be pretty straightforward.

Comment 14

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

Attachment: Code for testing (not for review)

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.

Comment 15

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

Attachment: Patch V1.1

Fixed a couple whitespace nits.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 17

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

(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.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 19

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

(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.

Comment 21

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

Attachment: Compiled test

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.

Comment 22

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This looks good, thanks!

Comment 23

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

Comment on attachment 413828 [details] [diff] [review]
Compiled test

Cool, I'll clean it up and expand.

Comment 24

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

Attachment: Tests

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.

Comment 25

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

Test does not build with --enable-libxul.  Clearing checkin-needed while I fix it.

Comment 26

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

--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.

Comment 27

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

Attachment: Changes necessary to avoid breaking libxul builds

--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.

Comment 28

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

Attachment: Cumulative patch for checkin

This includes the two patches already r+'d and the makefile changes I just uploaded.

Comment 29

[Phil Lacy]

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 30

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

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.

Comment 31

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/mozilla-central/rev/e2f9cde1e66d

Comment 32

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

(In reply to comment #31)
> http://hg.mozilla.org/mozilla-central/rev/e2f9cde1e66d

Thanks Phil!