377621 - [Thunderbird] Drag and Drop attachments to desktop or folders doesn't work

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect, P2)

Description

[rabo69]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; de; rv:1.8.1.3) Gecko/20060601 Firefox/2.0.0.3 (Ubuntu-edgy)
Build Identifier: Thunderbird Version 2.0.0.0 (20070326) and also Version 1.5.0.10 (20070306) 

If you try to move a attachment via Drag and Drop outside Thunderbird to the desktop or a folder on the PC the drag and drop cursor symbols shows up correctly but no file will be created/moved to this place (This works fine under windows)

(The error shows under Gnome Desktop Ubuntu EdgyEft 6.10 and also under Feisty Fawn 7.04)

Reproducible: Always

Steps to Reproduce:
1. drag an email attachment
2 [review]. move the cursor to desktop (press shift or control or nothing - same thing)
3. release mouse button
Actual Results:  
no file will be created/moved to the drop place

Expected Results:  
the attachment should moved/copied to the drop place (in windows this works fine)

Comment 1

[A S Alam]

I can confirm this behavior on my machine  on Fedora with KDE.
it ask to create file, but actually has no content from attachment

Comment 2

[Carsten Book [:Tomcat]]

confirmed on fedora fc 6 with thunderbird 2 release build.

Comment 3

[A S Alam]

with trunk build thunderbird-3.0a1.en-US.linux-i686, bug can also be reproduced

Comment 4

[frank]

Similar problem with OS WinXP as well. If you drag the attachment to the desktop you have to keep the left mouse button pressed for some seconds so the attachment is "loaded" (you see a progress bar). Then you can release the button and the attachment is successfull copied. Otherwise (without waiting) an errormessage shows up ("file in use"). You can work with this unconvenience but it is unexpected, not state of the art and contradicts usability a lot (because it took me quite a while to find out).
If you can confirm this behaviour please extend this bug to OS WinXP as well.

Comment 5

[rabo69]

bug still exists in new TB 2.0.0.4

Comment 6

[rabo69]

bug still exists in TB 2.0.0.6

Comment 7

[Tobias Koenig]

Hej,

the problem is that thunderbird doesn't provide a text/uri-list flavour that
could be evaluated by the desktop environment to copy the attachment somewhere
else.

One idea would be to save the attachment to a temporary file and pass the url
of that file as a test/uri-list flavour in the drag operation.
To make that feature work for all platforms, a java script based implementation
would be preferred. So can somebody point me to the place in the mozilla sources
where the drag for attachments is started?
 mailnews/base/resources/content/msgHdrOverlay.js:attachmentAreaDNDObserver
sounds promising, however that method seems not to be called when I try to
drag an attachment from the message view.

BTW, when I drag the attachment to the desktop, the JS error console shows the following message:

Error: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsITransferable.getTransferData]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://messenger/content/msgHdrViewOverlay.js :: anonymous :: line 1627"  data: no]
Source File: chrome://messenger/content/msgHdrViewOverlay.js
Line: 1627

It seems like the 'application/moz-file-promise-url' flavour is missing...
Can somebody enlighten me what that flavour is for and why it could be missing?

Ciao,
Tobias

Comment 8

[Tobias Koenig]

Attachment: Allows drag&drop of attachments from message view to desktop

Hej,

this patch allows drag&drop of attachments to the desktop by saving the requested attachment to a temporary file and passing the URL of that file in the drag object. The patch has still two issues: 1) '/tmp' is hard coded... It seems nsIFileUtilities::getTempDirPath() could help here, but how do I instantiate such a component?
2) The name of the temporary file is the same as stored in the attachment, however as it's a temporary file, it should have a unique name (like returned by mktemp(3) under Unix). nsIFileUtilities::newTempFileName() looks promising, but could it be that there is no implementation for that method?

Comment 9

[frank]

See similar Win XP bug here:
https://bugzilla.mozilla.org/show_bug.cgi?id=384590

Bug only occurs, when attachment is "large" (> 2MB).

Comment 10

[John Vivirito]

Ubuntu has this bug in bug tracker the link for it is
https://bugs.launchpad.net/ubuntu/+source/thunderbird/+bug/151162

Comment 11

[Tobias Koenig]

Hej,

any news on that issue? Is the patch ok for review/superreview?

Ciao,
Tobias

Comment 12

[Tobias Koenig]

Attachment: Use the directory-service for temporary file now

Hej,

the new patch uses directory-service to retrieve the temporary directory instead of hardcoding "/tmp".

Ciao,
Tobias

Comment 13

[Boying Lu]

Two comments:
1. The patch need to be updated to apply the trunk
2. I still can't DnD my attachment to my desktop

Comment 14

[David Ascher (:davida)]

Comment on attachment 340309 [details] [diff] [review]
Use the directory-service for temporary file now

dmose isn't building on linux much these days, so switching review to magnus.

Comment 15

[Tobias Koenig]

Attachment: adapt to HEAD

Adapted to HEAD. Could somebody do a review soon, please?
I really don't want to update the patch here forever :(

Comment 16

[Magnus Melin [:mkmelin]]

I hope to get to this later this week, sorry about the delay.

Comment 17

[Magnus Melin [:mkmelin]]

Comment on attachment 365417 [details] [diff] [review]
adapt to HEAD

Mozilla is using hg now, so for future ref, please make patches against hg tip (cvs HEAD is getting old). 

The patch doesn't apply cleanly to hg tip, but I've unbitrotted and played with it a bit. Will attach that in a moment.

(no sr needed for mail/ only changes, and I'm not a super-reviewer)

Comment 18

[Magnus Melin [:mkmelin]]

Attachment: proposed fix (unbitrotted)

Unbitrotted, and creating the file:// url in a safer way.

This patch works, but it's a bit ugly.

Windows doesn't need to do it this way, so I'm thinking the "proper" fix is probably somewhere in the gtk drag service

http://mxr.mozilla.org/comm-central/source/mozilla/widget/src/windows/nsDragService.cpp#559
http://mxr.mozilla.org/comm-central/source/mozilla/widget/src/gtk2/nsDragService.cpp

What do you think?

Comment 19

[Magnus Melin [:mkmelin]]

E.g. bug 267426 may be of interest, though it's windows only afaikt.

Comment 20

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Moving from General -> MWFE, I was torn between MWFE and Message Reader UI, not so sure, feel free to change though.

We'd need litmus tests to cover this - I'm not sure if MozMill can do DnD out to a non-XUL app (i.e. to Desktop).

Comment 21

[Ludovic Hirlimann [:Usul]]

(In reply to comment #20)
 
> We'd need litmus tests to cover this - I'm not sure if MozMill can do DnD out
> to a non-XUL app (i.e. to Desktop).

Good point. Do you feel about adding them ?

Comment 22

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #21)
> (In reply to comment #20)
> 
> > We'd need litmus tests to cover this - I'm not sure if MozMill can do DnD out
> > to a non-XUL app (i.e. to Desktop).
> 
> Good point. Do you feel about adding them ?

Not at the moment - this is a Linux-only bug AFAICT (Windows is covered somewhere else), and I touch Linux only barely. Mac doesn't seem to have this issue (but has issues with DnD of multiple-selected attachments, but that's another thing altogether).

Comment 23

[Tobias Koenig]

(In reply to comment #18)
> Unbitrotted, and creating the file:// url in a safer way.
> 
> This patch works, but it's a bit ugly.
Thanks for the update!

> Windows doesn't need to do it this way, so I'm thinking the "proper" fix is
> probably somewhere in the gtk drag service
> 
> http://mxr.mozilla.org/comm-central/source/mozilla/widget/src/windows/nsDragService.cpp#559
> http://mxr.mozilla.org/comm-central/source/mozilla/widget/src/gtk2/nsDragService.cpp
Hmm, but how can I create such a temporary file inside the drag service implementation?

I have only a mailbox:// URL to the attachment there... what component can
I use to extract the attachment from the file, pointed by the mailbox:// URL, and store it in a temporary file?

Or is there another way to retrieve the attachment data somehow? /me is still missing the overview how these things work together :(

Ciao,
Tobias

Comment 24

[Magnus Melin [:mkmelin]]

Sorry, I don't think i'll be able to help much, and it might not be easy at all... First step is probably to figure out what windows does, and if you can do something similar on *nix.

Comment 25

[Phil Lacy]

Windows accepts native com interface iStream for data and our side fills that in. Here's a reference to the object created for windows dnd API compliance.

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

I can't do unix but writing the temp file for O/S use should be similiar backend IMO

Comment 26

[Phil Lacy]

Attachment: fixes url -- needs testing

This is a test patch for unix build.  I don't have unix to test so please someone on cc test this.

The url passed as flavour data has 2 '&filename='.  This may be a problem with unix DND code and may be the problem here.

Comment 27

[Phil Lacy]

the filename is unescaped so it should not have spaces or non-alpha char, see
bug 485514

Comment 28

[Lim Yuen Hoe]

(In reply to comment #26)
> This is a test patch for unix build.  I don't have unix to test so please
> someone on cc test this.

Tested briefly on my build (which is a little old but I don't think attachment code has changed much). Doesn't work. + cursor appears, appears to start moving on drop, but complains it couldn't find the file to move. Nothing gets dropped.

Comment 29

[Phil Lacy]

> (In reply to comment #26)
> Tested briefly on my build (which is a little old but I don't think attachment
> code has changed much). Doesn't work. + cursor appears, appears to start moving
> on drop, but complains it couldn't find the file to move. Nothing gets dropped.

would you or anyone with a Linux debug build post a log of nsDragService
export NSPR_LOG_MODULES=nsDragService:5
export NSPR_LOG_FILE=bug377621.log

Comment 30

[Phil Lacy]

Attachment: fixes url test patch-revised

could someone try this fix of the URL flavor revision. The other patch was on seamonkey not TB so it would not have any effect.

Comment 31

[Phil Lacy]

in windows the only flavours required for file drop are:
-text/x-moz-url 
-application/x-moz-file-promise-url",
-application/x-moz-file-promise, with 'null' for data.
if the x-moz-file-promise is omitted then an internet-type shortcut is created.

Comment 32

[Phil Lacy]

Tobias
To get this to do backend like windows create a stream from the URI then save the stream as the temp file then send it to the desktop in GTK manner. There may be other code to create a file directly from the URI.

Creating the stream is mozilla common code and the windows-specific backend code does it here which :
http://mxr.mozilla.org/comm-central/source/mozilla/widget/src/windows/nsDataObj.cpp#122

That will get windows and GTK to work and hopefully Mac works too, if not we need to do the same there.
This gives us a clean frontend which we can use to DND messages to the desktop.

Comment 33

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

Maybe Michael can mek sense of this.

Comment 34

[Tobias Koenig]

Attachment: Drag&Drop support by widget/gtk2 code

This patch implements drag&drop from thunderbird to the Linux desktop (or file manager) by changing code in widgets/src/gtk2 only.
It makes use of the kFilePromise feature to trigger the mail component to save the attachment to a temporary file whenever the text/uri-list flavor is requested on a drag&drop operation. The url of the temporary file is passed as flavor data then...
Could somebody comment on the code, please? I guess the string conversion can be done easier...

Comment 35

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

Don't you need to use CreateUnique to ensure that the temp file name doesn't clobber something else?

Instead of indenting a lot, use early exits and a helper function if necessary.

You're assuming the URI is UTF-8, I'm not sure if that's right...

Comment 36

[Phil Lacy]

https://bugzilla.mozilla.org/attachment.cgi?id=374754&action=diff#a/widget/src/gtk2/nsDragService.cpp_sec4

I may not read this correct but does this section make the transfer rely on the text/x-moz-url?

As a request to roc, I would think since a file transfer, like doing this attachment, will never create a shortcut url then this should not depend on it. So we should set it up where only the application/x-moz-file-promise-xx types are needed.

Comment 37

[Phil Lacy]

In the front end, msgHdrViewOverlay.js, we are still mixing the old technology of d&d with the new one. Going from TB2 to TB3 windows no longer uses the nsFlavorDataProvider implementation. In fact you can substituete null for the data and completely remove the interface here http://mxr.mozilla.org/comm-central/source/mail/base/content/msgHdrViewOverlay.js#1928 and it still works.

Windows gets the url from x-moz-file-promise-url and creates a stream which is sent to windows.  GTK and other platforms should either create a stream or create the temp file as suits the platform and we should remove the nsFlavorDataProvider implementation in msgHdrViewOverlay.js.

If we don't then we will have the front end doing platform specific stuff without anyway to tell without digging into the backend differences. The least we should do is comment the .js file what is being done.

Windows b/e does all this with essentially the one line of code here.

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

...then windows native reads the stream.  Here in GTK we can write it to the temp file and pass it back in the uri-list.

Also if we stick with this front end way we need to do the same messy stuff in bug 227305.

Or
We can scrap windows method and go back to this implementation for windows. In that way we can drag a txt file to word, or a .cpp file to visual studio or a doc file to word. Which can't be done with the stream. But that's not a big deal, we can always add x-moz-file flavour to tell windows to create a temp file and stick with the stream.

Comment 39

[Kai]

I like what Phil mentioned about going back to one implementation. Does this have an ETA? I have moved to Thunderbird (from KMail) and have since moved to GNOME (from KDE) and find this frustrating, since d&d works perfectly fine on windows but not on *nix.

Comment 40

[Phil Lacy]

Attachment: this is dnd requirements for windows XP

This will drag and drop attachments from IMAP or local folders to windows XP operating system. As one may notice, it is minimal front end requirements. Mac and GTK should conform to these flavors, except the x-moz-url which may be eliminated in a future bug.
Lets set this to mac o/s as well for testing and get a backend fix for GTK.
I am assuming if this gets reviewed and cleared by roc then Tobias can do the GTK stuff.

Comment 41

[Tobias Koenig]

Attachment: Drag&Drop support by widget/gtk2 code with streams

The new version uses streams instead of kFilePromiseUrl. However there are some outstanding issues (temporary files are not removed, too many files generated) that needs fixing, so this patch is a working proof of concept ;)

Comment 42

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

I assume the patch in comment #40 is not supposed to be checked in until Mac and GTK drag-drop implementations support sending URL contents to streams.

But I'm not really sure if this is the right way to go. Neil Deakin is the best person for this, I think.

Comment 43

[Phil Lacy]

Another option that may not be cleaner but clearer. In this front end, write to a temp file and transfer it as a 'move' with x-moz-file. Then we may be able to drag attachments to text editors or pdf readers etc.
If x-moz-file transfers aren't working then those are something that probably should be fixed in 1.9.1 whereas the moz-file-promise ones need looking into.
I'm not sure what the need for the x-moz-file-promise-xxx flavors originated that separates them from a simple temp file transfer, but maybe for TB3 we should avoid them altogether until they are revamped for use the same way in all platforms.

Comment 44

[Tobias Koenig]

Attachment: Drag&Drop support by widget/gtk2 code with streams

This version of the patch should be functional complete. It creates only
one temporary file for each drag&drop operation and removes all temporary
files at the end, when thunderbird is closed.

Could some of the core developers do a review (coding style, string conversion etc.), please?

Comment 45

[Martijn Wargers (dead)]

Comment on attachment 379666 [details] [diff] [review]
Drag&Drop support by widget/gtk2 code with streams

You have to specifically ask review to someone to get a review. I don't know if Robert is the correct person to review this (probably not), but hopefully he can point to someone who is more suited to review the patch.

Comment 46

[Neil Deakin]

What is this patch trying to accomplish? If you're expecting the operating system to handle the data as a file, you should be using a file promise or file type. When using the url type, it is up to destination application to retrieve it, we shouldn't be creating it for them.

Comment 47

[Tobias Koenig]

(In reply to comment #46)
> What is this patch trying to accomplish? If you're expecting the operating
> system to handle the data as a file, you should be using a file promise or file
> type.
That doesn't work... und Linux the desktop/filemanager expects that the file is passed as text/uri-list flavour and the desktop/filemanager is responsible to copy
the file to the correct destination.

> When using the url type, it is up to destination application to retrieve
> it, we shouldn't be creating it for them.
How should the destination application access the internal data of thunderbird
if it gets only a mailbox:// url passed? ;)
Now, we have to use the defined standards here...

Ciao,
Tobias

Comment 48

[Phil Lacy]

Per comment #46, in the backend if it receives the filepromise flavours it can still convert the data to the text/uri-list and do its thing.

In windows the x-moz-url flavor creates a link file of 'internet shortcut' type. If this is correct other o/s should do the same.

If we use the x-moz-file type then the front ends need to do the work outputting to a temp file and passing data as nsIFile per spec. That would facilitate drag and drop to other apps which can receive temp files but not streamed data. The temp file can still be deleted and the other app notes it and gives its user the option to save it.

Ideally, we should use the best of both worlds. This is data not a file and should use filepromise but in backend the data should be offered as both and either streamed or a file made as requested by the target.

Comment 49

[Phil Lacy]

Comment on attachment 379666 [details] [diff] [review]
Drag&Drop support by widget/gtk2 code with streams

>+    nsCOMPtr<nsISupports> data;
>+    rv = aTransferable->GetTransferData(kURLMime,
>+                                        

to comply with comment #46 can we make this kFilePromiseURLMime

>+        nsPrimitiveHelpers::CreateDataFromPrimitive(kURLMime,
>+                                                    data,
>+                                                    

and here

the url should be in the data for x-moz-file-promise-url


>+        if (strcmp(mimeFlavor, gTextUriListType) == 0 && !mTempFileCreated) {
>+            nsCOMPtr<nsIFile> tmpDir;

a check for kFilePromiseURLMime needed

Comment 50

[Tobias Koenig]

Attachment: Drag&Drop support by widget/gtk2 code with streams

This version of the patch uses kFilePromiseURLMime again as suggested by Phil in #49.

Comment 51

[Phil Lacy]

Comment on attachment 386466 [details] [diff] [review]
Drag&Drop support by widget/gtk2 code with streams

I think roc is the reviewer name to set here.
Here's my notes from what I can compare to windows version.  I don't have linux to check further than that.

>+    mTempFileCreated = PR_FALSE;

don't quite follow its need, but that may be me.

>+nsresult GetAttachmentDetails(nsITransferable * aTransferable, nsIURI 

maybe use 'GetDownloadDetails' like windows or similiar generic name. This can be used for messages, files, or other data besides attachments.

>+                        // check if text/x-moz-url is supported.

did you mean application/x-moz-file-promise-url

>+        // The desktop or file manager expects for drags of attachments the
>+        // text/uri-list flavor set to a temporary file that contains the
>+        // attachment.
>+        // We open a stream on the mailbox:// url here and save the content
>+        // to file:///tmp/dnd_attachment/<attachmentname> and pass this url
>+        // as text/uri-list flavor.

again, attachments are only one use. 'promise-file' or 'promise-file data' for clarity, and mailbox:// could be other handler <protocol>://

>+            // build the file:///tmp/dnd_attachment URL
>+            tmpDir->Append(NS_LITERAL_STRING("dnd_attachment"));
>+            tmpDir->CreateUnique(nsIFile::DIRECTORY_TYPE, 0775);

don't know how much 'naming' needs to be added to file name for linux use or obfuscation but I assume CreateUnique would be enough. Again 'attachment' should be more generic if needed at all.


>+            mTempFileCreated = PR_TRUE;
>+            mTemporaryFiles.AppendObject(tempFile);

again mTempFileCreated is set but can't see its use. If it is necessary it may be better set after the outputstream succeeds.

repeating! I'm not a reviewer. my comments are only notes. Use at your or reviewer's discretion.

Comment 52

[Magnus Melin [:mkmelin]]

Tobias: you need to ask review from a specific person. (Set the review flag to [e-mail])

You may want to update the patch according to comment 51 first though.

Comment 53

[Phil Lacy]

(In reply to comment #51)
> 
> >+    mTempFileCreated = PR_FALSE;
> 
> don't quite follow its need, but that may be me.

to clear this point, Tobias indicated that GTK (like windows) tends to call the file creation functions more than once so this prevents multiple copies of the temp file.

Comment 54

[Tobias Koenig]

Attachment: New version that removes all references to attachments

Sorry for the late reply, exams at university...

The new patch addresses all issues (references to 'attachment', outdated comments etc.) Phil mentioned in previous comment.

Comment 55

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

Comment on attachment 390806 [details] [diff] [review]
New version that removes all references to attachments

I'll hand this off to our GTK2 superhero

Comment 56

[Karl Tomlinson (:karlt)]

Comment on attachment 378778 [details] [diff] [review]
this is dnd requirements for windows XP

I assume that comment 46 effectively made this obselete.

Comment 57

[Karl Tomlinson (:karlt)]

Comment on attachment 390806 [details] [diff] [review]
New version that removes all references to attachments

Thank you for this.  The general approach looks good, but some of the finer
details should be touched up.

It makes it much easier for reviewers to understand the changes and work out
which code is being modified when patches show function names and have additional lines of context.  This can be done automatically by adding a [diff] section to ~/.hgrc or .hg/hgrc: https://developer.mozilla.org/en/Mercurial_FAQ#Configuration

>+    // We can not delete the temporary files immediately after the
>+    // drag has finished, because the target application might have not
>+    // copied the temporary file yet. Instead we collect all temporary
>+    // files in mTemporaryFiles array and remove them here in the destructor.

The destination app should not indicate that it is finished until it has
copied the file.  This seems to be implied by both the XDND and Motif
protocols:

http://newplanetsoftware.com/xdnd/dragging_files.html
http://www.lesstif.org/InsideLessTif/node89.html

So it should be safe to delete the temporary files in response to the drag-end
(or drag-failed) signals (in SourceEndDragSession()).

We don't really want to keep accumulating these files when the application
runs for months and the files would not be cleaned up if the app doesn't get a
chance to shut down nicely.

Did you see any apps that were indicating that they had finished before they
had copied the file?

In GetSourceList():

>+                        // check if application/x-moz-file-promise url is supported.
>+                        // If so, advertise text/uri-list.
>+                        if (strcmp(flavorStr, kFilePromiseURLMime) == 0) {
>+                            GdkAtom urlAtom = gdk_atom_intern(gTextUriListType, FALSE);
>+                            GtkTargetEntry *urlTarget =
>+                             (GtkTargetEntry *)g_malloc(sizeof(GtkTargetEntry));
>+                            urlTarget->target = g_strdup(gTextUriListType);
>+                            urlTarget->flags = 0;
>+                            /* Bug 331198 */
>+                            urlTarget->info = NS_PTR_TO_UINT32(urlAtom);
>+                            targetArray.AppendElement(urlTarget);
>+                        }

It looks like this can be done even when numDragItems > 1.

Though I don't think we want two gTextUriListType targets when both kURLMime
and kFilePromiseURLMime are supported flavours (unless there's a reason why
that won't happen).  If both kFilePromiseURLMime and kURLMime flavours are
available, then which should be used in the text/uri-list data?  (We won't
know whether the destination wants to copy or link.)  Perhaps they are
unlikely to both occur together and just picking one will be fine.

I don't think handling multiple files would be too difficult, but even if you
prefer to only implement this for one file right now, make sure that
SourceDataGet() is consistent with GetSourceList().

In SourceDataGet():

>+                void    *tmpData = NULL;
>+                nsresult rv;
>+                nsCOMPtr<nsISupports> data;
>+                rv = item->GetTransferData(kFilePromiseURLMime,
>+                                           getter_AddRefs(data),
>+                                           &tmpDataLen);
>+                if (NS_SUCCEEDED(rv)) {
>+                    foundFilePromiseFlavor = PR_TRUE;
>+                    if (tmpData) {

tmpData is not used.

>+            if (foundFilePromiseFlavor == PR_FALSE) {
>+                PR_LOG(sDragLm, PR_LOG_DEBUG, ("Transferable is missing kFilePromiseURLMime flavor\n"));
>+                return;
>+            }
>+

I expect the transferable may often not have kFilePromiseURLMime even though
someone asks for target type mimeFlavor == gTextUriListType.  (If kURLMime is
a supported flavour, then gTextUriListType will be in the GtkTargetList.)  This
function should continue to return gTextUriListType data even if there is no
kFilePromiseURLMime flavour.

>+            rv = NS_NewFileURI(getter_AddRefs(uri), tmpDir);

The uri will need to contain the hostname.  Will NS_NewFileURI do this?

http://newplanetsoftware.com/xdnd/dragging_files.html

Feel free to use glib functions (g_filename_to_uri() perhaps) when dealing
with GTK as in this function (and others in this file), if that makes things
easier.

>+                    // remove and re-add the text/uri-list flavor to discard any previous
>+                    // data of that flavor
>+                    item->RemoveDataFlavor(gTextUriListType);
>+                    item->AddDataFlavor(gTextUriListType);
>+
>+                    // set the file:///tmp/dnd_file/<filename> URL for text/uri-list flavor
>+                    item->SetTransferData(gTextUriListType, genericDataWrapper, ucs2Str.Length() * 2);

The side-effect of modifying the transferable here doesn't feel right to me.
The transferable is an app-internal reference to the data, while the external
uri list, generated here for the GtkSelectionData for other apps, is likely to
be different.  (Internally the data can be referenced from the original URI,
rather than from the file intended for external use.)

There is lot of code specific to kFilePromiseURLMime added to SourceDataGet()
here.  This dominates the function (which is actually much more general) and
makes it harder to follow the flow.  Most of the new code should be split out
into one or more static functions or private methods.

Comment 58

[Karl Tomlinson (:karlt)]

Comment on attachment 390806 [details] [diff] [review]
New version that removes all references to attachments

>+            tmpDir->CreateUnique(nsIFile::DIRECTORY_TYPE, 0775);

Any reason for g+w here?  Is this relying on umask.

>+            rv = NS_NewLocalFileOutputStream(getter_AddRefs(outputStream), tmpDir);

And does this just use umask?

I guess the destination app will copy permissions from the file.

Often mail is considered private and apps seem to use go-r even if umask is more permissive.
We don't know that this data is mail here, but I think it would be better to err on the side of being private for this data of unknown nature:  0700 for the dir, and 0600 for the file.

Comment 59

[Tobias Koenig]

(In reply to comment #57)

Hej Karl,

> It makes it much easier for reviewers to understand the changes and work out
> which code is being modified when patches show function names and have
> additional lines of context.
Ok, fixed, thanks for the hint!

> >+    // We can not delete the temporary files immediately after the
> >+    // drag has finished, because the target application might have not
> >+    // copied the temporary file yet. Instead we collect all temporary
> >+    // files in mTemporaryFiles array and remove them here in the destructor.
> 
> The destination app should not indicate that it is finished until it has
> copied the file.  This seems to be implied by both the XDND and Motif
> protocols:
Right, unfortunately the KDE applications have a bug there (well, it is actually a bug/missing feature in Qt) and they return the XDNDFinished signal
too early before they have started copying the file. I talked with the
Qt developers already, however it won't be fixed in near future, therefor
that delayed deleting would be a helpfull workaround.
(See last entry on http://www.freedesktop.org/wiki/Specifications/XDNDRevision)

> We don't really want to keep accumulating these files when the application
> runs for months and the files would not be cleaned up if the app doesn't get a
> chance to shut down nicely.
Is it so common that a Thunderbird app runs for months?

> In GetSourceList():
> 
> >+                        // check if application/x-moz-file-promise url is supported.
> >+                        // If so, advertise text/uri-list.
> >+                        if (strcmp(flavorStr, kFilePromiseURLMime) == 0) {
> >+                            GdkAtom urlAtom = gdk_atom_intern(gTextUriListType, FALSE);
> >+                            GtkTargetEntry *urlTarget =
> >+                             (GtkTargetEntry *)g_malloc(sizeof(GtkTargetEntry));
> >+                            urlTarget->target = g_strdup(gTextUriListType);
> >+                            urlTarget->flags = 0;
> >+                            /* Bug 331198 */
> >+                            urlTarget->info = NS_PTR_TO_UINT32(urlAtom);
> >+                            targetArray.AppendElement(urlTarget);
> >+                        }
> 
> It looks like this can be done even when numDragItems > 1.
Ok, will check

> I don't think handling multiple files would be too difficult, but even if you
> prefer to only implement this for one file right now, make sure that
> SourceDataGet() is consistent with GetSourceList().
Ok, will have a look.

> In SourceDataGet():
> 
> >+                void    *tmpData = NULL;
> >+                nsresult rv;
> >+                nsCOMPtr<nsISupports> data;
> >+                rv = item->GetTransferData(kFilePromiseURLMime,
> >+                                           getter_AddRefs(data),
> >+                                           &tmpDataLen);
> >+                if (NS_SUCCEEDED(rv)) {
> >+                    foundFilePromiseFlavor = PR_TRUE;
> >+                    if (tmpData) {
> 
> tmpData is not used.
> 
> >+            if (foundFilePromiseFlavor == PR_FALSE) {
> >+                PR_LOG(sDragLm, PR_LOG_DEBUG, ("Transferable is missing kFilePromiseURLMime flavor\n"));
> >+                return;
> >+            }
> >+
> 
> I expect the transferable may often not have kFilePromiseURLMime even though
> someone asks for target type mimeFlavor == gTextUriListType.  (If kURLMime is
> a supported flavour, then gTextUriListType will be in the GtkTargetList.)  This
> function should continue to return gTextUriListType data even if there is no
> kFilePromiseURLMime flavour.
Ok, have to check.

> >+            rv = NS_NewFileURI(getter_AddRefs(uri), tmpDir);
> 
> The uri will need to contain the hostname.  Will NS_NewFileURI do this?
Good to know...

> http://newplanetsoftware.com/xdnd/dragging_files.html
> 
> Feel free to use glib functions (g_filename_to_uri() perhaps) when dealing
> with GTK as in this function (and others in this file), if that makes things
> easier.
> 
> >+                    // remove and re-add the text/uri-list flavor to discard any previous
> >+                    // data of that flavor
> >+                    item->RemoveDataFlavor(gTextUriListType);
> >+                    item->AddDataFlavor(gTextUriListType);
> >+
> >+                    // set the file:///tmp/dnd_file/<filename> URL for text/uri-list flavor
> >+                    item->SetTransferData(gTextUriListType, genericDataWrapper, ucs2Str.Length() * 2);
> 
> The side-effect of modifying the transferable here doesn't feel right to me.
> The transferable is an app-internal reference to the data, while the external
> uri list, generated here for the GtkSelectionData for other apps, is likely to
> be different.  (Internally the data can be referenced from the original URI,
> rather than from the file intended for external use.)
> 
> There is lot of code specific to kFilePromiseURLMime added to SourceDataGet()
> here.  This dominates the function (which is actually much more general) and
> makes it harder to follow the flow.  Most of the new code should be split out
> into one or more static functions or private methods.
Ok, then I'll try to separate it more and send an updated Patch

Comment 60

[Karl Tomlinson (:karlt)]

(In reply to comment #59)
> (In reply to comment #57)
> > The destination app should not indicate that it is finished until it has
> > copied the file.  This seems to be implied by both the XDND and Motif
> > protocols:
> Right, unfortunately the KDE applications have a bug there (well, it is
> actually a bug/missing feature in Qt) and they return the XDNDFinished signal
> too early before they have started copying the file. I talked with the
> Qt developers already, however it won't be fixed in near future, therefor
> that delayed deleting would be a helpfull workaround.
> (See last entry on http://www.freedesktop.org/wiki/Specifications/XDNDRevision)
> 
> > We don't really want to keep accumulating these files when the application
> > runs for months and the files would not be cleaned up if the app doesn't get a
> > chance to shut down nicely.
> Is it so common that a Thunderbird app runs for months?

I assume mail apps tend to get run continuously (rather than started and quit
repeatedly) so that users are notified of new mail, so I hope Thunderbird will
run for months.

I don't know but I wonder whether or not the app shuts down cleanly when the X
session is ended.

This could be used by any app of unknown function too.

I think deleting the file on a timer could be suitable workaround.  (It would
still need to be deleted on exit as well, assuming the timer is not guaranteed
to run before exit.)

Either a glib timer (g_timeout_add? - beware we need to support glib-2.12.x) or
nsITimer should be helpful.
http://mxr.mozilla.org/mozilla-central/source/xpcom/threads/nsITimer.idl

Once the destination has a file handle, it should be safe to unlink, but it
could take a little while for the destination to get a file handle if it
prompts the user for anything.  I guess a timeout of a few minutes should be
suitable.

Comment 61

[Karl Tomlinson (:karlt)]

Just a record of my investigation into XDS:

I looked at the possibility of using the Direct Save Protocol (http://www.newplanetsoftware.com/xds/) as an alternative (more similar to MS behavior) because a blog (http://rodney.id.au/dev/gnome/an-xds-example) said that it was "supported by ROX, Thunar, Konqueror, and Nautilus file managers".

However, I can't see how GTK apps could easily implement this protocol because
GDK doesn't support XdndActionDirectSave.  Apparently that action was added
(October 26, 2000) but I don't know why it's needed.  Further, even if this
action could be provided, it would be difficult for destinations not
supporting XDS to know how to fall back to text/uri-list given the
unrecognized action.

Comment 62

[Tobias Koenig]

(In reply to comment #57)
Hej Karl,

> In GetSourceList():
> 
> >+                        // check if application/x-moz-file-promise url is supported.
> >+                        // If so, advertise text/uri-list.
> >+                        if (strcmp(flavorStr, kFilePromiseURLMime) == 0) {
> >+                            GdkAtom urlAtom = gdk_atom_intern(gTextUriListType, FALSE);
> >+                            GtkTargetEntry *urlTarget =
> >+                             (GtkTargetEntry *)g_malloc(sizeof(GtkTargetEntry));
> >+                            urlTarget->target = g_strdup(gTextUriListType);
> >+                            urlTarget->flags = 0;
> >+                            /* Bug 331198 */
> >+                            urlTarget->info = NS_PTR_TO_UINT32(urlAtom);
> >+                            targetArray.AppendElement(urlTarget);
> >+                        }
> 
> It looks like this can be done even when numDragItems > 1.
> 
> Though I don't think we want two gTextUriListType targets when both kURLMime
> and kFilePromiseURLMime are supported flavours (unless there's a reason why
> that won't happen).  If both kFilePromiseURLMime and kURLMime flavours are
> available, then which should be used in the text/uri-list data?  (We won't
> know whether the destination wants to copy or link.)  Perhaps they are
> unlikely to both occur together and just picking one will be fine.
So what shall I do? Checking whether an kURLMime/gTextUriListType already exists and only create a new gTextUriListType entry if not?

> I don't think handling multiple files would be too difficult, but even if you
> prefer to only implement this for one file right now, make sure that
> SourceDataGet() is consistent with GetSourceList().
What exactly do you mean with consistent?

> >+                    // remove and re-add the text/uri-list flavor to discard any previous
> >+                    // data of that flavor
> >+                    item->RemoveDataFlavor(gTextUriListType);
> >+                    item->AddDataFlavor(gTextUriListType);
> >+
> >+                    // set the file:///tmp/dnd_file/<filename> URL for text/uri-list flavor
> >+                    item->SetTransferData(gTextUriListType, genericDataWrapper, ucs2Str.Length() * 2);
> 
> The side-effect of modifying the transferable here doesn't feel right to me.
> The transferable is an app-internal reference to the data, while the external
> uri list, generated here for the GtkSelectionData for other apps, is likely to
> be different.  (Internally the data can be referenced from the original URI,
> rather than from the file intended for external use.)
So how can I return the url of the temporary file instead of changing the transferable?!?

Ciao,
Tobias

Comment 63

[Karl Tomlinson (:karlt)]

(In reply to comment #62)
> (In reply to comment #57)

> > If both kFilePromiseURLMime and kURLMime flavours are
> > available, then which should be used in the text/uri-list data?  (We won't
> > know whether the destination wants to copy or link.)  Perhaps they are
> > unlikely to both occur together and just picking one will be fine.

> So what shall I do? Checking whether an kURLMime/gTextUriListType already
> exists and only create a new gTextUriListType entry if not?

In GetSourceList() when numDragItems > 1, gTextUriListType is the only target
that will be supported, so it's fine to break from the loop when the first is
added.

The real decision will be in SourceDataGet() (or CreateUriList) when choosing
whether to provide the uri from the kFilePromiseURLMime or kURLMime flavor.  I
don't know what the right answer is.  I'd be happy with even just a comment
indicating that it is not clear which is better.

> > make sure that SourceDataGet() is consistent with GetSourceList().
> What exactly do you mean with consistent?

It's probably enough to make sure that SourceDataGet does
gtk_selection_data_set for targets that are advertised in GetSourceList.  (I
can't remember but I was probably also thinking that data should not be set
for targets that were not advertised, but that principal doesn't appear to be
satisfied in current code anyway.)

> So how can I return the url of the temporary file instead of changing the
> transferable?!?

Return it using gtk_selection_data_set().  I expect CreateUriList() will
probably need some modification.

Comment 64

[Tobias Koenig]

Attachment: Patch that addresses the problems from previous review

The new patch has the following improvements
  - uses a timer to delete the list of temporary files
  - uses gtk_selection_data_set instead of changing the Transferable object
  - code that creates temporary file is located in separated method

Comment 65

[Karl Tomlinson (:karlt)]

Comment on attachment 403730 [details] [diff] [review]
Patch that addresses the problems from previous review

>diff --git a/widget/src/gtk2/nsDragService.cpp b/widget/src/gtk2/nsDragService.cpp
>--- a/widget/src/gtk2/nsDragService.cpp
>+++ b/widget/src/gtk2/nsDragService.cpp
>@@ -53,26 +53,31 @@
> #include "nsNetUtil.h"
> #include "prlog.h"
> #include "nsVoidArray.h"
> #include "nsPrimitiveHelpers.h"
> #include "prtime.h"
> #include "prthread.h"
> #include <gtk/gtkinvisible.h>
> #include <gdk/gdkx.h>
>+#include <unistd.h>
> #include "nsCRT.h"
> 
> #include "gfxASurface.h"
> #include "gfxXlibSurface.h"
> #include "gfxContext.h"
> #include "nsImageToPixbuf.h"
> #include "nsIPresShell.h"
> #include "nsPresContext.h"
> #include "nsIDocument.h"
> #include "nsISelection.h"
>+#include "nsDirectoryService.h"
>+#include "nsDirectoryServiceDefs.h"
>+#include "nsEscape.h"
>+#include "nsString.h"
> 
> // This sets how opaque the drag image is
> #define DRAG_IMAGE_ALPHA_LEVEL 0.5
> 
> // These values are copied from GtkDragResult (rather than using GtkDragResult
> // directly) so that this code can be compiled against versions of GTK+ that
> // do not have GtkDragResult.
> // GtkDragResult is available from GTK+ version 2.12.
>@@ -141,16 +146,19 @@ nsDragService::nsDragService()
>     PR_LOG(sDragLm, PR_LOG_DEBUG, ("nsDragService::nsDragService"));
>     mTargetWidget = 0;
>     mTargetDragContext = 0;
>     mTargetTime = 0;
>     mCanDrop = PR_FALSE;
>     mTargetDragDataReceived = PR_FALSE;
>     mTargetDragData = 0;
>     mTargetDragDataLen = 0;
>+    mTempFileCreated = PR_FALSE;
>+    mTimer = do_CreateInstance("@mozilla.org/timer;1");
>+
> }
> 
> nsDragService::~nsDragService()
> {
>     PR_LOG(sDragLm, PR_LOG_DEBUG, ("nsDragService::~nsDragService"));
> }
> 
> NS_IMPL_ISUPPORTS_INHERITED2(nsDragService, nsBaseDragService,
>@@ -173,16 +181,36 @@ nsDragService::Observe(nsISupports *aSub
>   } else {
>     NS_NOTREACHED("unexpected topic");
>     return NS_ERROR_UNEXPECTED;
>   }
> 
>   return NS_OK;
> }
> 
>+// nsITimer
>+
>+NS_IMETHODIMP
>+nsDragService::Notify(nsITimer *timer)
>+{
>+  // We can not delete the temporary files immediately after the
>+  // drag has finished, because the target application might have not
>+  // copied the temporary file yet. Instead we collect all temporary
>+  // files in mTemporaryFiles array and remove them here in the timer event.
>+  PRUint32 count = mTemporaryFiles.Count();
>+
>+  while (count > 0) {
>+    --count;
>+    mTemporaryFiles[count]->Remove(PR_TRUE);
>+  }
>+  mTemporaryFiles.Clear();
>+
>+  return NS_OK;
>+}
>+
> // nsIDragService
> 
> NS_IMETHODIMP
> nsDragService::InvokeDragSession(nsIDOMNode *aDOMNode,
>                                  nsISupportsArray * aArrayTransferables,
>                                  nsIScriptableRegion * aRegion,
>                                  PRUint32 aActionType)
> {
>@@ -321,16 +349,18 @@ nsDragService::SetAlphaPixmap(gfxASurfac
>     gdk_pixmap_unref(pixmap);
>     return PR_TRUE;
> }
> 
> NS_IMETHODIMP
> nsDragService::StartDragSession()
> {
>     PR_LOG(sDragLm, PR_LOG_DEBUG, ("nsDragService::StartDragSession"));
>+    mTempFileCreated = false;
>+
>     return nsBaseDragService::StartDragSession();
> }
>  
> NS_IMETHODIMP
> nsDragService::EndDragSession(PRBool aDoneDrag)
> {
>     PR_LOG(sDragLm, PR_LOG_DEBUG, ("nsDragService::EndDragSession %d",
>                                    aDoneDrag));
>@@ -439,16 +469,59 @@ nsDragService::GetNumDropItems(PRUint32 
>             *aNumItems = CountTextUriListItems(data, mTargetDragDataLen);
>         } else
>             *aNumItems = 1;
>     }
>     PR_LOG(sDragLm, PR_LOG_DEBUG, ("%d items", *aNumItems));
>     return NS_OK;
> }
> 
>+nsresult GetDownloadDetails(nsITransferable * aTransferable, nsIURI **aSourceURI, nsAString &aFilename)
>+{
>+  *aSourceURI = nsnull;
>+
>+  NS_ENSURE_TRUE(aTransferable, NS_ERROR_FAILURE);
>+
>+  // get the URI from the kFilePromiseURLMime flavor
>+  nsCOMPtr<nsISupports> urlPrimitive;
>+  PRUint32 dataSize = 0;
>+  aTransferable->GetTransferData(kFilePromiseURLMime, getter_AddRefs(urlPrimitive), &dataSize);
>+  nsCOMPtr<nsISupportsString> srcUrlPrimitive = do_QueryInterface(urlPrimitive);
>+  NS_ENSURE_TRUE(srcUrlPrimitive, NS_ERROR_FAILURE);
>+
>+  nsAutoString srcUri;
>+  srcUrlPrimitive->GetData(srcUri);
>+  if (srcUri.IsEmpty())
>+    return NS_ERROR_FAILURE;
>+  nsCOMPtr<nsIURI> sourceURI;
>+  NS_NewURI(getter_AddRefs(sourceURI), srcUri);
>+
>+  nsAutoString srcFileName;
>+  nsCOMPtr<nsISupports> fileNamePrimitive;
>+  aTransferable->GetTransferData(kFilePromiseDestFilename, getter_AddRefs(fileNamePrimitive), &dataSize);
>+  nsCOMPtr<nsISupportsString> srcFileNamePrimitive = do_QueryInterface(fileNamePrimitive);
>+  if (srcFileNamePrimitive) {
>+    srcFileNamePrimitive->GetData(srcFileName);
>+  } else {
>+    nsCOMPtr<nsIURL> sourceURL = do_QueryInterface(sourceURI);
>+    if (!sourceURL)
>+      return NS_ERROR_FAILURE;
>+
>+    nsCAutoString urlFileName;
>+    sourceURL->GetFileName(urlFileName);
>+    NS_UnescapeURL(urlFileName);
>+    CopyUTF8toUTF16(urlFileName, srcFileName);
>+  }
>+  if (srcFileName.IsEmpty())
>+    return NS_ERROR_FAILURE;
>+
>+  sourceURI.swap(*aSourceURI);
>+  aFilename = srcFileName;
>+  return NS_OK;
>+}
> 
> NS_IMETHODIMP
> nsDragService::GetData(nsITransferable * aTransferable,
>                        PRUint32 aItemIndex)
> {
>     PR_LOG(sDragLm, PR_LOG_DEBUG, ("nsDragService::GetData %d", aItemIndex));
> 
>     // make sure that we have a transferable
>@@ -1063,16 +1136,17 @@ nsDragService::GetSourceList(void)
>                             listTarget->target = g_strdup(gTextUriListType);
>                             listTarget->flags = 0;
>                             /* Bug 331198 */
>                             listTarget->info = NS_PTR_TO_UINT32(listAtom);
>                             PR_LOG(sDragLm, PR_LOG_DEBUG,
>                                    ("automatically adding target %s with \
>                                    id %ld\n", listTarget->target, listAtom));
>                             targetArray.AppendElement(listTarget);
>+                            break;
>                         }
>                     }
>                 } // foreach flavor in item
>             } // if valid flavor list
>         } // if item is a transferable
>     } else if (numDragItems == 1) {
>         nsCOMPtr<nsISupports> genericItem;
>         mSourceDataItems->GetElementAt(0, getter_AddRefs(genericItem));
>@@ -1138,16 +1212,28 @@ nsDragService::GetSourceList(void)
>                             urlTarget->flags = 0;
>                             /* Bug 331198 */
>                             urlTarget->info = NS_PTR_TO_UINT32(urlAtom);
>                             PR_LOG(sDragLm, PR_LOG_DEBUG,
>                                    ("automatically adding target %s with \
>                                    id %ld\n", urlTarget->target, urlAtom));
>                             targetArray.AppendElement(urlTarget);
>                         }
>+                        // check if application/x-moz-file-promise url is supported.
>+                        // If so, advertise text/uri-list.
>+                        if (strcmp(flavorStr, kFilePromiseURLMime) == 0) {
>+                            GdkAtom urlAtom = gdk_atom_intern(gTextUriListType, FALSE);
>+                            GtkTargetEntry *urlTarget =
>+                             (GtkTargetEntry *)g_malloc(sizeof(GtkTargetEntry));
>+                            urlTarget->target = g_strdup(gTextUriListType);
>+                            urlTarget->flags = 0;
>+                            /* Bug 331198 */
>+                            urlTarget->info = NS_PTR_TO_UINT32(urlAtom);
>+                            targetArray.AppendElement(urlTarget);
>+                        }
>                     }
>                 } // foreach flavor in item
>             } // if valid flavor list
>         } // if item is a transferable
>     } // if it is a single item drag
> 
>     // get all the elements that we created.
>     targetCount = targetArray.Count();
>@@ -1235,16 +1321,18 @@ nsDragService::SourceEndDragSession(GdkD
>         do_QueryInterface(mDataTransfer);
> 
>     if (dataTransfer) {
>         dataTransfer->SetDropEffectInt(dropEffect);
>     }
> 
>     // Inform the drag session that we're ending the drag.
>     EndDragSession(PR_TRUE);
>+
>+    mTempFileCreated = PR_FALSE;
> }
> 
> static void
> CreateUriList(nsISupportsArray *items, gchar **text, gint *length)
> {
>     PRUint32 i, count;
>     GString *uriList = g_string_new(NULL);
> 
>@@ -1301,16 +1389,128 @@ CreateUriList(nsISupportsArray *items, g
>             }
>         }
>     }
>     *text = uriList->str;
>     *length = uriList->len + 1;
>     g_string_free(uriList, FALSE); // don't free the data
> }
> 
>+PRBool
>+nsDragService::CreateTempFile(nsITransferable *aItem, GtkSelectionData *aSelectionData)
>+{
>+    nsCOMPtr<nsIFile> tmpDir;
>+    nsresult rv = NS_GetSpecialDirectory(NS_OS_TEMP_DIR, getter_AddRefs(tmpDir));
>+
>+    if (NS_FAILED(rv)) {
>+        PR_LOG(sDragLm, PR_LOG_DEBUG, ("Failed to get temp directory\n"));
>+        return rv;
>+    }
>+
>+    nsCOMPtr<nsIInputStream> inputStream;
>+    nsCOMPtr<nsIChannel> channel;
>+
>+    // extract the file name and source uri of the promise-file data
>+    nsAutoString wideFileName;
>+    nsCOMPtr<nsIURI> sourceURI;
>+    rv = GetDownloadDetails(aItem, getter_AddRefs(sourceURI), wideFileName);
>+    if (NS_FAILED(rv)) {
>+        PR_LOG(sDragLm, PR_LOG_DEBUG, ("Failed to extract file name and source uri from download url\n"));
>+        return rv;
>+    }
>+
>+    // create and open channel for source uri
>+    rv = NS_NewChannel(getter_AddRefs(channel), sourceURI);
>+    if (NS_FAILED(rv)) {
>+        PR_LOG(sDragLm, PR_LOG_DEBUG, ("Failed to create new channel for source uri\n"));
>+        return rv;
>+    }
>+
>+    rv = channel->Open(getter_AddRefs(inputStream));
>+    if (NS_FAILED(rv)) {
>+        PR_LOG(sDragLm, PR_LOG_DEBUG, ("Failed to open channel for source uri\n"));
>+        return rv;
>+    }
>+
>+    // build the file:///tmp/dnd_file URL
>+    tmpDir->Append(NS_LITERAL_STRING("dnd_file"));
>+    tmpDir->CreateUnique(nsIFile::DIRECTORY_TYPE, 0700);
>+
>+    // store a copy of that temporary directory so we can
>+    // clean them up when nsDragService is destructed
>+    nsCOMPtr<nsIFile> tempFile;
>+    tmpDir->Clone(getter_AddRefs(tempFile));
>+
>+    mTemporaryFiles.AppendObject(tempFile);
>+
>+    mTimer->Cancel();
>+    mTimer->InitWithCallback(static_cast<nsITimerCallback*>(this), 
>+                             NS_DND_TIMEOUT,
>+                             nsITimer::TYPE_ONE_SHOT);
>+
>+    // extend file:///tmp/dnd_file/<filename> URL
>+    tmpDir->Append(wideFileName);
>+
>+    nsCOMPtr<nsIOutputStream> outputStream;
>+    rv = NS_NewLocalFileOutputStream(getter_AddRefs(outputStream), tmpDir);
>+    if (NS_FAILED(rv)) {
>+        PR_LOG(sDragLm, PR_LOG_DEBUG, ("Failed to open output stream for temporary file\n"));
>+        return rv;
>+    }
>+    mTempFileCreated = PR_TRUE;
>+
>+    char buffer[512];
>+    PRUint32 readCount = 0;
>+    PRUint32 writeCount = 0;
>+    while (1) {
>+        rv = inputStream->Read(buffer, sizeof(buffer), &readCount);
>+        if (NS_FAILED(rv)) {
>+            PR_LOG(sDragLm, PR_LOG_DEBUG, ("Failed to read data from source uri\n"));
>+            return rv;
>+        }
>+
>+        if (readCount == 0)
>+            break;
>+
>+        rv = outputStream->Write(buffer, readCount, &writeCount);
>+        if (NS_FAILED(rv)) {
>+            PR_LOG(sDragLm, PR_LOG_DEBUG, ("Failed to write data to temporary file\n"));
>+            return rv;
>+        }
>+    }
>+
>+    outputStream->Close();
>+    inputStream->Close();
>+
>+    nsCOMPtr<nsIURI> uri;
>+    rv = NS_NewFileURI(getter_AddRefs(uri), tmpDir);
>+    if (NS_SUCCEEDED(rv)) {
>+        nsCOMPtr<nsIURL> fileURL(do_QueryInterface(uri));
>+        if (fileURL) {
>+            // according to XDG spec, text/uri-list urls need a hostname set
>+            char hostname[255];
>+            gethostname(hostname, sizeof(hostname));
>+
>+            nsCAutoString hostnameStr;
>+            hostnameStr.AssignASCII(hostname);
>+
>+            fileURL->SetHost(hostnameStr);
>+
>+            nsCAutoString urltext;
>+            rv = fileURL->GetSpec(urltext);
>+            if (NS_SUCCEEDED(rv)) {
>+                // store url of temporary file
>+                mTempFileUrl.AssignASCII(urltext.get());
>+                return NS_OK;
>+            }
>+        }
>+    }
>+
>+    return NS_ERROR_FAILURE;
>+}
> 
> void
> nsDragService::SourceDataGet(GtkWidget        *aWidget,
>                              GdkDragContext   *aContext,
>                              GtkSelectionData *aSelectionData,
>                              guint             aInfo,
>                              guint32           aTime)
> {
>@@ -1357,16 +1557,63 @@ nsDragService::SourceDataGet(GtkWidget  
>         // plain text.
>         else if (strcmp(mimeFlavor, gTextUriListType) == 0) {
>             actualFlavor = gTextUriListType;
>             needToDoConversionToPlainText = PR_TRUE;
>         }
>         else
>             actualFlavor = mimeFlavor;
> 
>+        // The desktop or file manager expects for drags of promise-file data the
>+        // text/uri-list flavor set to a temporary file that contains the
>+        // promise-file data.
>+        // We open a stream on the <protocol>:// url here and save the content
>+        // to file:///tmp/dnd_file/<filename> and pass this url
>+        // as text/uri-list flavor.
>+        if (strcmp(mimeFlavor, gTextUriListType) == 0) {
>+            PRUint32 i, count;
>+            mSourceDataItems->Count(&count);
>+
>+            // check whether transferable contains FilePromiseUrl flavor...
>+            PRBool foundFilePromiseFlavor = PR_FALSE;
>+            for (i = 0; i < count; i++) {
>+                nsCOMPtr<nsISupports> genericItem;
>+                mSourceDataItems->GetElementAt(i, getter_AddRefs(genericItem));
>+                nsCOMPtr<nsITransferable> item;
>+                item = do_QueryInterface(genericItem);
>+
>+                PRUint32 tmpDataLen = 0;
>+                nsresult rv;
>+                nsCOMPtr<nsISupports> data;
>+                rv = item->GetTransferData(kFilePromiseURLMime,
>+                                           getter_AddRefs(data),
>+                                           &tmpDataLen);
>+                if (NS_SUCCEEDED(rv)) {
>+                    foundFilePromiseFlavor = PR_TRUE;
>+                    break;
>+                }
>+            }
>+
>+            // ... if so, create a temporary file and pass its url as text/uri-list flavor
>+            if (foundFilePromiseFlavor == PR_TRUE) {
>+                if (!mTempFileCreated) {
>+                    nsresult rv;
>+                    rv = CreateTempFile(item, aSelectionData);
>+                    if (NS_FAILED(rv))
>+                        return;
>+                }
>+
>+                NS_ConvertUTF16toUTF8 tempFileUrl(mTempFileUrl);
>+                gtk_selection_data_set(aSelectionData,
>+                                       aSelectionData->target,
>+                                       8, (guchar*)tempFileUrl.get(), tempFileUrl.Length());
>+                return;
>+            }
>+        }
>+
>         PRUint32 tmpDataLen = 0;
>         void    *tmpData = NULL;
>         nsresult rv;
>         nsCOMPtr<nsISupports> data;
>         rv = item->GetTransferData(actualFlavor,
>                                    getter_AddRefs(data),
>                                    &tmpDataLen);
>         if (NS_SUCCEEDED(rv)) {
>diff --git a/widget/src/gtk2/nsDragService.h b/widget/src/gtk2/nsDragService.h
>--- a/widget/src/gtk2/nsDragService.h
>+++ b/widget/src/gtk2/nsDragService.h
>@@ -37,36 +37,40 @@
>  * the terms of any one of the MPL, the GPL or the LGPL.
>  *
>  * ***** END LICENSE BLOCK ***** */
> 
> #ifndef nsDragService_h__
> #define nsDragService_h__
> 
> #include "nsBaseDragService.h"
>+#include "nsCOMArray.h"
> #include "nsIDragSessionGTK.h"
> #include "nsIObserver.h"
>+#include "nsITimer.h"
> #include <gtk/gtk.h>
> 
> 
> /**
>  * Native GTK DragService wrapper
>  */
> 
> class nsDragService : public nsBaseDragService,
>                       public nsIDragSessionGTK,
>+                      public nsITimerCallback,
>                       public nsIObserver
> {
> public:
>     nsDragService();
>     virtual ~nsDragService();
> 
>     NS_DECL_ISUPPORTS_INHERITED
> 
>     NS_DECL_NSIOBSERVER
>+    NS_DECL_NSITIMERCALLBACK
> 
>     // nsIDragService
>     NS_IMETHOD InvokeDragSession (nsIDOMNode *aDOMNode,
>                                   nsISupportsArray * anArrayTransferables,
>                                   nsIScriptableRegion * aRegion,
>                                   PRUint32 aActionType);
>     NS_IMETHOD StartDragSession();
>     NS_IMETHOD EndDragSession(PRBool aDoneDrag);
>@@ -146,12 +150,25 @@ private:
>     // attempts to create a semi-transparent drag image. Returns TRUE if
>     // successful, FALSE if not
>     PRBool SetAlphaPixmap(gfxASurface    *aPixbuf,
>                           GdkDragContext *aContext,
>                           PRInt32         aXOffset,
>                           PRInt32         aYOffset,
>                           const nsRect&   dragRect);
> 
>+    PRBool CreateTempFile(nsITransferable *aItem, GtkSelectionData *aSelectionData);
>+
>+    // stores whether a temporary file has been created for the
>+    // current drag session
>+    PRBool              mTempFileCreated;
>+
>+    nsString            mTempFileUrl;
>+
>+    // stores all temporary files
>+    nsCOMArray<nsIFile> mTemporaryFiles;
>+
>+    // timer to trigger deletion of temporary files
>+    nsCOMPtr<nsITimer>  mTimer;
> };
> 
> #endif // nsDragService_h__
>

Comment 66

[Karl Tomlinson (:karlt)]

Oops, sorry, clicked in the wrong place!

Comment 67

[Karl Tomlinson (:karlt)]

Comment on attachment 403730 [details] [diff] [review]
Patch that addresses the problems from previous review

Thanks, again!  Sorry for the delay on this.

>+  // We can not delete the temporary files immediately after the
>+  // drag has finished, because the target application might have not
>+  // copied the temporary file yet. Instead we collect all temporary
>+  // files in mTemporaryFiles array and remove them here in the timer event.

Can you be more specific about the Qt issue please, so that we will know when
it is fixed and this code can be removed?  A link to a bug report would be
ideal.

>+  PRUint32 count = mTemporaryFiles.Count();
>+
>+  while (count > 0) {
>+    --count;
>+    mTemporaryFiles[count]->Remove(PR_TRUE);
>+  }

A "for" loop would work well here.

>+    mTempFileCreated = false;

Use PR_FALSE for PRBools.

>+nsresult GetDownloadDetails(nsITransferable * aTransferable, nsIURI **aSourceURI, nsAString &aFilename)

This function can be static and file style is to put the function name on a
new line.

Can you move this to just above CreateTempFile(), where it is used, please?

>+  NS_ENSURE_TRUE(aTransferable, NS_ERROR_FAILURE);

NS_PRECONDITION is better here.  It is a debug-only check; it shouldn't be
necessary to check in production code.

>+  NS_ENSURE_TRUE(srcUrlPrimitive, NS_ERROR_FAILURE);

Consensus seems to be to deprecate NS_ENSURE_TRUE, and I agree:
http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/ccb87d7e7d9e688f

Instead, use something like:

  if (!srcUrlPrimitive)
    return NS_ERROR_FAILURE;

>+    mTimer->Cancel();
>+    mTimer->InitWithCallback(static_cast<nsITimerCallback*>(this), 
>+                             NS_DND_TIMEOUT,
>+                             nsITimer::TYPE_ONE_SHOT);

This looks like a sensible place to cancel the previous timer.

A better place to start the timer would be SourceEndDragSession().

Can you update the comment for NS_DND_TIMEOUT to explain the new use, please?

>+    outputStream->Close();

I think this is the place to check that buffers have been successfully flushed
to disk.  (No need to check inputStream->Close().)

>+            char hostname[255];
>+            gethostname(hostname, sizeof(hostname));

Need to ensure that there is a NUL terminator here.
(Just make the last byte NUL.)

>+            nsCAutoString hostnameStr;
>+            hostnameStr.AssignASCII(hostname);
>+
>+            fileURL->SetHost(hostnameStr);

The string copy can be skipped by using:

  fileURL->SetHost(nsDependentCString(hostname))

>+        if (strcmp(mimeFlavor, gTextUriListType) == 0) {
>+            PRUint32 i, count;
>+            mSourceDataItems->Count(&count);
>+
>+            // check whether transferable contains FilePromiseUrl flavor...
>+            PRBool foundFilePromiseFlavor = PR_FALSE;
>+            for (i = 0; i < count; i++) {

kFilePromiseURLMime was only advertised (in GetSourceList) as a uri-list when
numDragItems == 1 and only one temporary file will be created, so only
consider providing the temporary file when numDragItems == 1.

Can this block be merged with the previous "strcmp(mimeFlavor,
gTextUriListType) == 0" block?

>+                NS_ConvertUTF16toUTF8 tempFileUrl(mTempFileUrl);

By making mTempFileUri a nsCString, this conversion can be moved to
CreateTempFile() (and will only need to be done once).

>+    // stores whether a temporary file has been created for the
>+    // current drag session
>+    PRBool              mTempFileCreated;
>+
>+    nsString            mTempFileUrl;
>+
>+    // stores all temporary files
>+    nsCOMArray<nsIFile> mTemporaryFiles;
>+
>+    // timer to trigger deletion of temporary files
>+    nsCOMPtr<nsITimer>  mTimer;

It is normally best to declare member variables together so as to make it
clearer how best to pack them.  Unfortunately there are already two groups of
member variables in this class declaration, but try to keep the variables as
close as practical.

Can mTempFileUrl.IsEmpty() be used to make mTempFileCreated unnecessary?
Or maybe keeping mTempFileCreated would make it easier to know when to set the timer.

If keeping mTempFileCreated, the PRBool would pack better with the other PRBool members (and they should really all be PRPackedBool).

Resetting mTempFileUrl or mTempFileCreated is logically necessary in
the same places as mSourceDataItems is initialized.  The places you have initialized
mTempFileCreated are sufficient, but it would be clearer if temp file variable
initialization for each drag was moved to where mSourceDataItems is
initialized.

mTimer could have a more specific name such as mTempFileTimer or similar.

Comment 68

[Tobias Koenig]

Attachment: Patch that addresses the issues found on previous review

This patch should fix all mentioned issues. Only the 'check that all data are flushed' confused me... doesn't Close() flush the stream automatically before it closes?

Comment 70

[Karl Tomlinson (:karlt)]

Comment on attachment 411924 [details] [diff] [review]
Patch that addresses the issues found on previous review

(In reply to comment #68)
> This patch should fix all mentioned issues. Only the 'check that all data are
> flushed' confused me... doesn't Close() flush the stream automatically before
> it closes?

Yes, it tries to, but that may not succeed.  Check the return value of
outputStream->Close() to see whether it succeeded.

>+  // (http://bugreports.qt.nokia.com/browse/QTBUG-5441)

Thanks for filing that.  That looks like a sensible suggestion (from my
limited understanding of the Qt APIs).

>+    tmpDir->CreateUnique(nsIFile::DIRECTORY_TYPE, 0700);

I think there should be a check for success here before adding to
mTemporaryFiles to be deleted.

The directory is created here, but this function can still fail to set
mTempFileUri (after cancelling the timer) ...

>+    if (!mTempFileUrl.IsEmpty()) {
>+        // start time to remove temporary files
>+        mTempFileTimer->InitWithCallback(static_cast<nsITimerCallback*>(this), 
>+                                         NS_DND_TIMEOUT,
>+                                         nsITimer::TYPE_ONE_SHOT);

so I think this should check mTemporaryFiles.Count() instead of mTempFileUrl.

>+                // check whether transferable contains FilePromiseUrl flavor...
>+                PRBool foundFilePromiseFlavor = PR_FALSE;
>+                PRUint32 i;
>+                for (i = 0; i < numDragItems; i++) {

No need for a loop here as numDragItems == 1.

Comment 72

[Paolo Montrasio]

The bug is still there in Thunderbird 3.1.7 (Ubuntu 10.4)

Comment 73

[Lorenzo]

I can also confirm this in Thunderbird 3.1.7 on Ubuntu 10.4 (Gnome 
With a warning message:
Error while copying.
There was an error getting information about "[folder_name]>[number]".
An in the show more details:
The specified location is not supported

Lorenzo.

Comment 74

[Paolo Montrasio]

Bug still here, TB 3.1.7 + Ubuntu 10.10 + gnome 2.32.0 and almost 4 years since the creation of the bug (2007-04-16)

Comment 75

[Arne]

Just so no-one forgets about this bug, I'm happy to announce that Thunderbird 5.0 on Ubuntu 10.10 still has this bug.

;(

Comment 76

[Romano Giannetti]

Yep, confirmed here too. Quite unnerving, if I may say. It's a 4 years old bug on something that users will notice in the first 5 minutes of usage.

Comment 77

[Dan Lundmark]

Confirmed using TB 7.0a2 nightlies on OSX Lion, when I drag an attachment to desktop it creates an empty file named "undefined" instead of the expected attachment.

Comment 78

[aova]

Bug confirmed, TB 3.1.11 + Ubuntu 10.04 + gnome 2.32.0

Comment 79

[Paolo Montrasio]

Still there in Ubuntu 11.04, TB 3.1.12
I'm sure there is some good reason for the fix is taking so long to deliver after that spur of activity back in 2009. It would be appreciated if the developers could explain what is it. Maybe we can think all together about how to remove any hurdle preventing the resolution of this issue.

Comment 80

[Rain [:rain1] (inactive)]

Repeated comments are not going to help get the bug fixed any faster. Please stop commenting unless you're willing to take on the bug or have a patch.

Comment 81

[Paolo Montrasio]

Your comment reminds me of http://www.reddit.com/r/programming/comments/c2b4l/open_source_openofficeorg_we_have_a_problem/c0pqvz6

Comment 83

[Romano Giannetti]

The behaviour changed. Now when I drag and drop, on Ubuntu 11.10, i have a ".desktop" file created with a "link" to the mail object. The problem is that

1) clicking on the icon give the error Could not display "imap://romano%2Egiannetti%40...1.2&filename=13008%202011.pdf". 

2) ...and even if I manage to open it (I can, with some opening and cut and paste and based on the fact that the application understand the URI) what happens if I go offline? 

So I think it's worst than ever --- before you had an error, now it could seem that the operation succeeded.

Comment 84

[Paolo Montrasio]

Not sure it's worse but it seems not what we are used to.
All mail clients I know (and also Thunderbird) create a new file with a full copy of the attachment. That seems to create a link to the attachment. If wonder what happens if one edits the saved attachment (let's say it's a .doc on .odt) but I bet that it's just a bug, not the intended behavior.

Comment 85

[Paolo Montrasio]

I did a fresh install of Ubuntu 11.10 and installed some alternative desktop environments.
The behavior with the Unity and the GNOME desktops is what Romano describes.
With MATE (the Linux Mint desktop) I get the old error "The specified location is not supported".
That could mean that the error is in how the desktop environment handles the information passed to it by TB,

Comment 86

[Felix Möller]

I can confirm this with Thunderbird 14.0 on Ubuntu 12.04 and Unity with all updates applied.

Comment 88

[Paolo Montrasio]

Still in Thunderbird 24

Comment 89

[Paolo Montrasio]

Maybe we could use http://bribe.io/ to fix this when it will launch. I'm mostly kidding but I'm also thinking a little about it. However this bug is older than the first iPhone (April vs June 2007) so I wonder if there is anybody left in the dev team with the knowledge to fix it.

Comment 90

[Romano Giannetti]

@Paolo: sadly, it's probably true. I suppose we all got to grip with it (I just stopped trying it and right-click and save, you know, even in other mailer) but it is a very nasty (and, let me say, discreting) bug to have around for so much time.

Comment 91

[Romano Giannetti]

s/discreting/discrediting/ in the last message.

Comment 92

[ssameer]

I am using Thunderbird 24.5.0 on OpenSuse 13.1 with KDE 4.13.0. Trying to Drag and Drop an attachment to the Dolphin file manager only shows a menu for "link" (no copy). Clicking on "link" creates a .desktop file with a link to the attachment in the message. Clicking on the link just opens an empty location - the attachment does not open. It would be good if this behaviour is fixed.

Comment 93

[Romano Giannetti]

If I can chime in with a comment to the developers (if anyone is listening here): I am used to never use drag and drop of attachment on TB now, given the really broken behavior. 

Suggestion: ditch the thing completely. Remove drag and drop. For good.

It's so broken it is more a danger(1) than any other thing, and it is not so necessary in the end, you can live with "save as..." and company. I suppose that a bug like that not fixed in 7 years means it's too complex to fix; let's accept it and move on. Leaving an half-baked functionality is much worse than not having it.



(1) because sometime it *seems* to work; it creates an icon from which the content is not accesible at all, and can lead to data loss for unsuspecting users.

Comment 94

[Dani]

+1  Romano Giannetti 
Suggestion: ditch the thing completely. Remove drag and drop. For good.

Comment 95

[Milan Knížek]

Wow, after several years of not using TB I have recently migrated from Evolution Mail only to find out this stuff does not work yet. What a pity. (Arch Linux, MATE desktop.)

Comment 96

[Jérôme Bouat]

I encounter this bug with :
- Thunderbird 52.8.0
- Thunar 1.6.11 (the package depends on libgtk2.0-0 package)
- Debian 9.4 with a Linux kernel

Comment 97

[Dani]

The same:
Also I have found this bug with:
- Thunderbird 52.9.0
- Thunar 1.6.11 (the package depends on libgtk2.0-0 package)
- Debian 9.4
- Xfce 4.12

Comment 98

[Romano Giannetti]

Yes, still here; will not disappear alone. 

I still think that the worst problem of this bug is that is _seems_ to work, and can lead to data loss. It would be much better to forbid the drag and drop completely, if fixing it is too difficult.

Comment 99

[Romano Giannetti]

It seems that the firefox bug (which is listed as a duplicate, I do not think is 100% correct) #396370 has been marked as fixed. Could this be the case for thunderbird also? That would be great...

Comment 100

[Dani]

That would be great...
+1

Comment 101

[Romano Giannetti]

Checked right now with thunderbird 68.2.1 --- the bug is still here. The dropped attachment creates a file which consists of a broken link.

I suspect that this bug has been forgot; it seems really strange that, with all the nice things happened to thunderbird in the last couple of years, no one fixed this quite gross "feature..."

Comment 102

[Expert Alexa]

This issue is noticed on Windows 10 as well. Windows is updated. TB re-installed and still doesn't work.
Tested a new User Account and still the same.

Comment 103

[Romano Giannetti]

In version 78.7.1 the behavior is better: still not working, but it raises an error instead of copying an unusable link.
The error is also ok: "drag and drop not supported".

Definitely better!

Comment 104

[Martin Stránský [:stransky] (ni? me)]

I can reproduce it with TB91.

Comment 105

[Dani]

(In reply to Martin Stránský [:stransky] (ni? me) from comment #104)

I can reproduce it with TB91.

+1

Comment 106

[Martin Stránský [:stransky] (ni? me)]

Managed to update the patch to latest trunk so let's fix this one.

Comment 107

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 377621 [Linux] Support application/x-moz-file-promise mime type by text/uri-list, r?karlt

When application/x-moz-file-promise MIME content is adverised, save the file to /tmp/dnd_file-*/ directory and
offer it as text/uri-list MIME type.

Based on patches by Phil Lacy and Tobias Koenig.

Comment 108

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3b98ceafb59a
[Linux] Support application/x-moz-file-promise mime type by text/uri-list, r=karlt

Comment 109

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/3b98ceafb59a

Comment 110

[Shahar Or]

Thank you!

Comment 111

[Romano Giannetti]

I have Manjaro Linux with TB 102, and the problem is still here. When I do drag-and-drop to a Nemo folder I got the icon copied, and when going to Nautilus I still have "Drag and drop not supported", "an invalid drag type was used".

Comment 112

[Dani]

Ubuntu 20.04
Thunderbird 102.2.2
Thunar 4.16.10

Not work. Location specified is not compatible.

Comment 113

[Wayne Mery (:wsmwk)]

I have Manjaro Linux with TB 102, and the problem is still here.
...
Not work. Location specified is not compatible.

Bugs fixed long ago is not a great place to be reporting current issues. If it reproduces under Help > Troubleshooting Mode then please file a new bug report

Comment 114

[Martin Stránský [:stransky] (ni? me)]

Will look at it - still reproducible on TB 102.

Comment 115

[BugBot [:suhaib / :marco/ :calixte]]

In the process of migrating remaining bugs to the new severity system, the severity for this bug cannot be automatically determined. Please retriage this bug using the new severity system.

Comment 116

[Martin Stránský [:stransky] (ni? me)]

It's because we added text/uri-list but we still support _NETSCAPE_URL so an application can select _NETSCAPE_URL and we provide an url with "mailbox:///" prefix. We should filter out such internal URL.

Comment 117

[Martin Stránský [:stransky] (ni? me)]

If we can detect a drop target is a different application (not thunderbird itself) we may alter URL from mailbox:/// to file:/// and copy the file to /tmp.

Comment 118

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 377621 [Linux] Log correctly MIME type conversions r?emilio

Comment 119

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 377621 [Linux] Don't advertise _NETSCAPE_URL MIME type for internal data r?emilio

_NETSCAPE_URL hints target application it should store URL link instead of copy the pointed data.
Don't use that for internal URL (mailbox:/// and similar).

Depends on D161441

Comment 120

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cc7972f31488
[Linux] Log correctly MIME type conversions r=emilio
https://hg.mozilla.org/integration/autoland/rev/0cea06c45ea2
[Linux] Don't advertise _NETSCAPE_URL MIME type for internal data r=emilio

Comment 121

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/cc7972f31488
https://hg.mozilla.org/mozilla-central/rev/0cea06c45ea2

Comment 122

[Martin Stránský [:stransky] (ni? me)]

The D&D is still a bit broken but let's finish it at Bug 1800972. It also adds support for multiple file transfer.