304835 - Attachments aren't draggable from compose windows.

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, enhancement)

Description

[Matthew Hodgson]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050505 Firefox/1.0+
Build Identifier: 

If you attach a file to a mail you're writing, you can't then drag the
attachment out of its pane to attach it to another mail, or out of the program.
 Whilst this seems like an unlikely scenario to want to do when you're writing
your own mail, it's a complete showstopper when an application has autocomposed
the mail + attachment, and you then want to drag the attachment to a different mail.

This bug is singlehandedly stopping our Accounts department from switching to
Thunderbird, as Sage autoattaches invoices to a new mail (somehow) - but then
the attachment isn't transferrable to the appropriate composition window.

Reproducible: Always

Steps to Reproduce:
1. Start writing mail.  Attach a file to it.
2. Start writing another mail.
3. Drag and drop the attachment to the new mail window's address widget

Actual Results:  
You're not allowed to start dragging the attachment.

Comment 1

[Mike Cowperthwaite]

Huh.  Until you described the scenario, I was thinking, "Why would you want to?"

The clunky workaround is: save the generated message as a draft; go the drafts 
folder, view the draft, and you can drag the attachment from there.

Comment 2

[Jim Porter (:squib)]

Taking this. It should be pretty straightforward.

Comment 3

[Jim Porter (:squib)]

:Bienvenu, do you know where I could put attachmentAreaDNDObserver[1] so that the message reader and compose would see it? Copying and pasting the relevant code makes this work, but that's obviously less than optimal.

[1] http://mxr.mozilla.org/comm-central/source/mail/base/content/msgHdrViewOverlay.js#2183

Comment 4

[David :Bienvenu]

perhaps mail/base/content/mailCore.js ?

Comment 5

[Jim Porter (:squib)]

Attachment: Move attachmentAreaDNDObserver to mailCore.js

This moves attachmentAreaDNDObserver to mailCore.js so that MsgComposeCommands.js can pick it up. There are a couple of minor changes to the DND observer code too, to account for 1) a slightly different attachment object (maybe these should be unified one day), and 2) to avoid adding &type=foo&filename=bar repeatedly to the URL.

I'm not sure if there's a good way to test this, since I don't think Mozmill lets you do drag-and-drop with chrome stuff.

:Bienvenu, I hope you don't mind reviewing this. Otherwise, feel free to forward it onto whoever you think would be best. Thanks!

Comment 6

[David :Bienvenu]

Comment on attachment 487059 [details] [diff] [review]
Move attachmentAreaDNDObserver to mailCore.js

this patch doesn't apply cleanly to the trunk - was it made against a slightly out of date tree, or is it not against the trunk?

Comment 7

[David :Bienvenu]

Comment on attachment 487059 [details] [diff] [review]
Move attachmentAreaDNDObserver to mailCore.js

minusing in favor of a patch that applies.

Comment 8

[Jim Porter (:squib)]

Oh yeah... I forgot that this patch is dependent on my patch in bug 195702. I guess I'll wait for that one to get checked in, since it's review-ready anyway.

Comment 9

[Jim Porter (:squib)]

Comment on attachment 487059 [details] [diff] [review]
Move attachmentAreaDNDObserver to mailCore.js

Bug 195702 is checked in now, so this should apply. Do you mind checking it out now, :Bienvenu?

Comment 10

[David :Bienvenu]

this patch applies now, but it didn't seem to allow me to drag out of the compose window attachment pane. I only rebuilt in mail, so I'll try a full rebuild now.

Comment 11

[Jim Porter (:squib)]

Attachment: Fix issue with file: URLs

I tested it on my system and drag and drop works fine, but I did notice one thing I forgot to handle. file: URLs were getting munged when being dragged-and-dropped (they added some bogus type/filename params), so this patch fixes that.

Comment 12

[David :Bienvenu]

after a full rebuild, drag drop did work fine. I'll try it with your latest patch as well.

Comment 13

[David :Bienvenu]

Comment on attachment 496565 [details] [diff] [review]
Fix issue with file: URLs

thx for the patch...

Comment 14

[Jim Porter (:squib)]

Adding checkin-needed. Hopefully that's the correct thing to do now (I'm always a little unsure...)

Comment 15

[Ludovic Hirlimann [:Usul]]

(In reply to comment #14)
> Adding checkin-needed. Hopefully that's the correct thing to do now (I'm always
> a little unsure...)

It is.

Comment 16

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/2df02366747d

Ludovic: we should probably put a test for this in litmus.

Comment 17

[Ludovic Hirlimann [:Usul]]

https://litmus.mozilla.org/show_test.cgi?id=15026

Comment 18

[Jim Porter (:squib)]

(In reply to comment #17)
> https://litmus.mozilla.org/show_test.cgi?id=15026

It might be worth testing that the message created in step (3) can be sent with the attachment, since that's something I missed originally, and it takes a little bit of extra handling internally to ensure that file: URLs don't get munged up.

Comment 19

[Ludovic Hirlimann [:Usul]]

(In reply to comment #18)
> (In reply to comment #17)
> > https://litmus.mozilla.org/show_test.cgi?id=15026
> 
> It might be worth testing that the message created in step (3) can be sent with
> the attachment, since that's something I missed originally, and it takes a
> little bit of extra handling internally to ensure that file: URLs don't get
> munged up.

Added another test for that 15028

Comment 20

[Geert Poels]

With Thunderbird 8.0, this doesn't work (anymore).
Compose a new mail, attach an image.
Compose a second mail.
Dragging the attached image from 1 to 2 doesn't work. (not even a drag is shown)

Comment 21

[Jim Porter (:squib)]

Upgrade to 9.0. This was broken briefly in 8.0.