358657 - Drag attachment to Desktop fails

Status: VERIFIED FIXED

(Thunderbird :: General, defect)

Description

[Mike Cowperthwaite]

With trunk builds, at least as far back as 3a1-1004, seen with 3a1-1018, dragging an attachment to the Windows desktop generates a Windows error:
  An error occurred while moving the file.

No corresponding error in TB's Error Console.

Probably OT, but:
The sequencing is odd: immediately after this error appears, a 'Moving' status window shows up -- but the Error box (which is Always On Top) is modally blocking that window; if the error gets the focus, the Moving box moves to the top of the window stack.

This problem is not seen in TB 2b1-1026.

Comment 1

[Scott MacGregor]

caused by Bug 203307 perhaps?

Comment 2

[Mike Cowperthwaite]

(In reply to comment #1)
> caused by Bug 203307 perhaps?

Possibly.

I've also noticed that trying to drag an attachment into an application window is consistently giving a DoNotDrop cursor, also on the trunk.

Comment 3

[Yuri Kovalchuk]

Probably true? I can take a look at it if I'd knew where to look. Can someone point me to the right source file? I mean the code that deals with dragging attachments.

Comment 4

[Yuri Kovalchuk]

Probably true. I can take a look at it if I'd knew where to look. Can someone point me to the right source file? I mean the code that deals with dragging attachments.

Comment 5

[Yuri Kovalchuk]

Yes, it is caused by 203307. Actually, it does not work because nsIChannel::Open(nsIInputStream **_retval) is not implemented for nsMsgProtocol. I need a synchronous version, since it returns nsIInputStream, and windows shell handles it correctly then, I mean it can create a new thread for the extraction it needs to. UI would not hang anyway. So to fix this have to implement Open for nsMsgProtocol. Who do I ask for r/sr when it's done?

Comment 6

[Yuri Kovalchuk]

I could fix this with the following change to mailnews/base/util/nsMsgProtocol.cpp around line 560:

NS_IMETHODIMP nsMsgProtocol::Open(nsIInputStream **_retval)
{
  //NS_NOTREACHED("Open");
  //return NS_ERROR_NOT_IMPLEMENTED;
  return NS_ImplementChannelOpen(this, _retval);
}

I simply used stock sync nsIChannel::Open implementation, can anyone tell me if this is the right way to do it?
 It worked with this change, but the file name was always Inbox for the created file. Could be fixed but I need more time to figure out how dragging of attachments works.

Comment 7

[Mike Cowperthwaite]

(In reply to comment #6)
> It worked with this change, but the file name was always Inbox for the
> created file. 

That same symptom seen at bug 332137.

Comment 8

[Yuri Kovalchuk]

(In reply to comment #7)
> That same symptom seen at bug 332137.
The drag code gets the name from the url, however I've added a possibility to specify alternative name in the drag flavor. So I do not think this will be a problem, I mean this can be fixed)

Comment 9

[Yuri Kovalchuk]

Attachment: this should fix drag and drop of attachments

This will fix drag and drop, please review.

Comment 10

[David :Bienvenu]

Comment on attachment 245888 [details] [diff] [review]
this should fix drag and drop of attachments

you need to request review from someone - (bugzilla really shouldn't allow you to request review from no-one, or should assign review to the bug owner, at the very least...)

Comment 11

[David :Bienvenu]

is the message in a local folder, imap folder, or newsgroup?

Comment 12

[Mike Cowperthwaite]

Local folders is where I saw this, but I just reproduced in IMAP.

Comment 13

[Yuri Kovalchuk]

Right now the drag should be broken for all cases. This is because sync nsIChannel::Open method in nsMsgProtocol was not implemented. This is waht I'm using in the new drag code on windows. The patch should fix this, at least it did for local folders, I do not have an IMAP mailbox to test it. Can anybody suggest a place where those things are being given away for free? :)

Comment 14

[Magnus Melin [:mkmelin]]

http://www.fastmail.fm/ perhaps?

Comment 15

[David :Bienvenu]

I tried this patch - as you indicated, the new file name isn't correct, and that definitely needs to be fixed...

Comment 16

[David :Bienvenu]

Attachment: more complete fix

this fixes it for local, imap, and news. I had to encode the filename in the url, since that's the only chance we get with the drag drop code, as near as I can tell.

Comment 17

[David :Bienvenu]

Comment on attachment 245888 [details] [diff] [review]
this should fix drag and drop of attachments

thx for the initial work!

Comment 18

[Scott MacGregor]

Comment on attachment 246096 [details] [diff] [review]
more complete fix

Should we put it on the branch too even though Bug 358657 got minused for the branch? Or just do the trunk only.

Comment 19

[David :Bienvenu]

I don't think there's any reason to land this on the branch, as long as  Bug 203307 doesn't land on the branch. It's probably harmless, but I don't know of any upside either...

Comment 20

[David :Bienvenu]

fixed on trunk, thx again, Yuri, for the initial work.

Comment 21

[Mike Cowperthwaite]

V with TB 3a1-1128, Win2K.  This also seems to have fixed bug 332137.

Comment 22

[David :Bienvenu]

*** Bug 351274 has been marked as a duplicate of this bug. ***

Comment 23

[Tim]

Thanks, everyone.  The version I downloaded a few weeks ago seems to work OK.