Closed Bug 304835 Opened 16 years ago Closed 11 years ago

Attachments aren't draggable from compose windows.

Categories

(Thunderbird :: Message Compose Window, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.3a2

People

(Reporter: matthew, Assigned: squib)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file, 1 obsolete file)

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.
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.
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: unspecified → Trunk
QA Contact: message-compose
Assignee: mscott → nobody
Taking this. It should be pretty straightforward.
Assignee: nobody → squibblyflabbetydoo
Status: NEW → ASSIGNED
: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
perhaps mail/base/content/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!
Attachment #487059 - Flags: review?(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 on attachment 487059 [details] [diff] [review]
Move attachmentAreaDNDObserver to mailCore.js

minusing in favor of a patch that applies.
Attachment #487059 - Flags: review?(bienvenu) → review-
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.
Depends on: 195702
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?
Attachment #487059 - Flags: review- → review?(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.
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.
Attachment #487059 - Attachment is obsolete: true
Attachment #496565 - Flags: review?(bienvenu)
Attachment #487059 - Flags: review?(bienvenu)
after a full rebuild, drag drop did work fine. I'll try it with your latest patch as well.
Comment on attachment 496565 [details] [diff] [review]
Fix issue with file: URLs

thx for the patch...
Attachment #496565 - Flags: review?(bienvenu) → review+
Adding checkin-needed. Hopefully that's the correct thing to do now (I'm always a little unsure...)
Keywords: checkin-needed
(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.
Checked in: http://hg.mozilla.org/comm-central/rev/2df02366747d

Ludovic: we should probably put a test for this in litmus.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-litmus?(ludovic)
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a2
https://litmus.mozilla.org/show_test.cgi?id=15026
Flags: in-litmus?(ludovic) → in-litmus+
(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.
(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
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)
Upgrade to 9.0. This was broken briefly in 8.0.
Depends on: 702094
You need to log in before you can comment on or make changes to this bug.