270292 - Unable to drag and drop multiple attachments to OS file folder (e.g. Desktop)

Status: RESOLVED FIXED

(MailNews Core :: Attachments, defect)

Description

[mitchell ackerman]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0
Build Identifier: Thunderbird version 0.9 (20041103)

If i select multiple attachments (attachments are .jar files) and drag and drop
on a folder, only one file is copied.  also, occassionaly thunderbird
erroneously reports file in use if replacing a file.

Reproducible: Always
Steps to Reproduce:
1. receive email with .jar attachments
2. select more than one attachment
3 [review]. drag and drop on windows folder, only one file is copied

Actual Results:  
one file is copied

Expected Results:  
copied all files

Comment 1

[Mike Cowperthwaite]

Doesn't matter if they're .jar files; I see the same behavior with multiple .jpg 
files, for instance.

Comment 2

[Mike Cowperthwaite]

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

Comment 3

[Eric J. Schwarzenbach]

The occasional file in use error is also reported here: bug 285296

Comment 4

[Andreas Morawietz]

I can confirm this bug in

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20051008
Thunderbird/1.4.1 ID:2005100806

Comment 5

[Mike Cowperthwaite]

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

Comment 6

[Mike Cowperthwaite]

This bug also happens in Seamonkey, moving to Core.

Comment 7

[Jean-Michel Reghem]

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

Comment 8

[Wayne Mery (:wsmwk)]

in bug 285293 comment 1 Jeongkyu Kim asked for assistance and provided an experimental patch attachment 176834 [details].  his comments ...

Currently, drag and drop of multiple selection is not handled in onDragStart().
The function creates and passes data structure only for event target, which is
attached file drag is started on. 
http://lxr.mozilla.org/seamonkey/source/mail/base/content/msgHdrViewOverlay.js#1514

At a glance, I thought this could be solved by creating TransferDataSet object
(I attached experimental implementation). Interestingly enough, this also works
only for single selection.

When drag and drop is being doen, TransferData object is converted into
nsDataObj, which implements native interface IDataObject. If there are more
than one objects, they are wrapped with nsDataObjCollection object, which also
implements IDataObject. However, implementation of nsDataObjCollection might be
not delicate enough to cover drag and drop of multiple files.

Comment 12

[Sven Giermann]

A lot of inactivity for 3 years now...

Does anybody want to do a review of attachment 176834 [details] last mentioned in Comment #8 (18 months ago)?

Comment 14

[Phil Lacy]

Bug 227305 has an assignment and multiple drags may be resolved there. Maybe we want to have this bug depend on that one.

Comment 15

[Sven Giermann]

Since bug 227305 at least modifies something in DnD and no one seems to care about this one, setting dependency as suggested in comment #14.

Comment 19

[Jim Porter (:squib)]

Taking this, since I've been doing stuff in this part of the code anyway.

Comment 20

[Jim Porter (:squib)]

Attachment: Allow drag-and-drop of multiple items

Easy peasy. I'm going to give this a day or two to think about and then probably flag it for review. Maybe I'll even try to get tests working, though I still think MozMill doesn't support drag-and-drop in chrome.

Comment 21

[Phil Lacy]

Comment on attachment 501528 [details] [diff] [review]
Allow drag-and-drop of multiple items

>+      for (let i = 0; i < selected.length; i++)
>+      {
>+        var attachment = selected[i].attachment;
>+        if (attachment.contentType == "text/x-moz-deleted")
>+          return;
> 

can we do a continue now instead of the return?

Comment 22

[Jim Porter (:squib)]

Probably. I was waffling on whether we should ignore the error or yell at the user.

Comment 23

[Phil Lacy]

I've never really deleted attachments so if there is not big x or red circle bar stamp etc. then I'd say there should be. Which would be good enough for the user.
I'm all for ignoring the error but I guess the UI guys would need to be alerted for a decision on that one.

Comment 24

[Phil Lacy]

ignoring the error and creating a new bug, that is.

Comment 25

[Jim Porter (:squib)]

Deleted attachments have this icon and say "Deleted: file.txt": http://mxr.mozilla.org/comm-central/source/mail/themes/gnomestripe/mail/icons/attachment-deleted.png

I'm not sure what happens when you do this with detached attachments (they point to an external file on your filesystem), but that's probably out of scope for this bug.

:clarkbw, what's your opinion on this behavior? If we try to drag-and-drop a deleted attachment, should we just ignore it, or should we alert the user?

Comment 26

[Magnus Melin [:mkmelin]]

I think if all attachments are deleted drag should be disabled, but if only some are it should the "error" should just be ignored. Seems pretty self explaining if someone does start to investigate, and there's really nothing to do to change the fact that the attachment doesn't exist anymore.

Comment 27

[Jim Porter (:squib)]

Attachment: Ignore deleted attachments when dragging

This ignores deleted attachments when dragging. If all the attachments you're trying to drag are deleted, it won't let you drag at all.

Comment 28

[Bryan Clark (DevTools PM) [:clarkbw]]

(In reply to comment #25)
> :clarkbw, what's your opinion on this behavior? If we try to drag-and-drop a
> deleted attachment, should we just ignore it, or should we alert the user?

Right, just agreeing here.  If the user were to just drag a single deleted file (or all deleted files) I would want to prevent that drag from happening or give them an error.  However in the case of multiple (mixed) files I think it's better to ignore it.

Comment 29

[Jim Porter (:squib)]

Comment on attachment 501777 [details] [diff] [review]
Ignore deleted attachments when dragging

Sounds good. This is probably ready for review again. :Bienvenu, I'm requesting from you again, since you reviewed my patch in bug 304835, and this is just a small addition to it. No tests unfortunately, since that depends on bug 	515776 being fixed (unless someone has some tricky way of working around that).

Comment 30

[Philippp]

(In reply to comment #28)

> Right, just agreeing here.  If the user were to just drag a single deleted file
> (or all deleted files) I would want to prevent that drag from happening or give
> them an error.  However in the case of multiple (mixed) files I think it's
> better to ignore it.

Maybe I am late, but I want to share my suggestions (no idea how easy to implement):
Option 1:
Deleted attachments should not be selectable. Thus, user cannot drag anything that is deleted. :-)
If user presses ctrl-a (for selecting items to drag), then only non-deleted attachments should get selected.
This would give the user feedback about what will happen in the drag, and it would avoid the error instead of trying to deal with it.
Not sure if this works for detached attachments (I guess these should still be selectable?) - so here comes option 2:
If user starts to drag, reduce selection to only those attachments that can be saved.
This would give discreet feedback about what is happening (and the user will probably figure out the reason why selection changed, by examining the excluded attachments).

Comment 31

[David :Bienvenu]

I tried this patch, and I get a no entry sign on Windows when I try to drag drop multiple attachments from a message, and I drag over a folder. Dragging a single message works. W/o this patch, dragging multiple attachments really only drags one attachment. The attachments were normal, not deleted or detached, etc.

Comment 32

[Jim Porter (:squib)]

(In reply to comment #31)
> I tried this patch, and I get a no entry sign on Windows when I try to drag
> drop multiple attachments from a message, and I drag over a folder. Dragging a
> single message works. W/o this patch, dragging multiple attachments really only
> drags one attachment. The attachments were normal, not deleted or detached,
> etc.

You mean an OS folder right, and not a message folder? Does this fail on other OSes? (I can't drag anything to the desktop on Linux, where I do dev, because of an unrelated issue that I haven't figured out yet.) Does it work when you drag multiple attachments to a composition window?

It's possible that there are backend issues with this, since I know that not everything is supported properly on all platforms.

Comment 33

[Jim Porter (:squib)]

Yeah, it looks like these functions only have support for dropping a single file: * http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsDataObj.cpp#1319
* http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsDataObj.cpp#1521

This will be interesting, since I don't actually want to set stuff up to compile on Windows, but the actual fix is probably straightforward. I guess we'll see...

Comment 34

[David :Bienvenu]

Yes, a Windows Explorer file system folder. I also hit this assertion about our data obj not being async:

nsDragService::StartInvokingDragSession(IDataObject * aDataObj,
                                        PRUint32 aActionType)
...

  // Offer to do an async drag
  if (SUCCEEDED(aDataObj->QueryInterface(IID_IAsyncOperation,
                                         getter_AddRefs(pAsyncOp)))) {
    pAsyncOp->SetAsyncMode(VARIANT_TRUE);
  } else {
    NS_NOTREACHED("When did our data object stop being async");
  }

Comment 35

[David :Bienvenu]

Comment on attachment 501777 [details] [diff] [review]
Ignore deleted attachments when dragging

I'm going to clear the request since I can't test the patch, but not minus, since it's conceivable the other fix will be in core code.

Comment 36

[Jim Porter (:squib)]

It doesn't look like the core issue has a bug filed, so I'll write one up tomorrow. The other day, I asked about a similar issue on mozilla.dev.platform[1], so maybe something will come of that.

:Bienvenu, if you have any idea of who might know about this stuff, that would probably be helpful, since it doesn't seem like this is a priority outside of Thunderbird.

[1] http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/df7c5fcc64f114c7

Comment 37

[David :Bienvenu]

(In reply to comment #36)

> 
> :Bienvenu, if you have any idea of who might know about this stuff, that would
> probably be helpful, since it doesn't seem like this is a priority outside of
> Thunderbird.
>
Looking at the hg log for nsDataObj.cpp, Kyle Huey [e-mail] seems to have done significant work in that area.

Comment 42

[Christian Degenkolb]

Hi

Are there any news regarding this bug?
I can confirm with thunderbird 8.0 on windows.

Comment 45

[Franco Corbelli]

Amazingly this bug is present in thunderbird 14 (windows 7 64 bit).
It seems very similar to this https://bugzilla.mozilla.org/show_bug.cgi?id=333511

Is it possible to "migrate" the patch?

Comment 46

[Jim Porter (:squib)]

(In reply to Franco Corbelli from comment #45)
> Amazingly this bug is present in thunderbird 14 (windows 7 64 bit).
> It seems very similar to this
> https://bugzilla.mozilla.org/show_bug.cgi?id=333511
> 
> Is it possible to "migrate" the patch?

Unfortunately, no. This is pretty hard to fix, since it requires delving into platform-specific C++ (and possibly Obj-C).

Anyway, since I'm not actively working on this, I'm going to unassign myself. If anyone would like to work on this, I'm happy to help provide guidance, but I don't have enough free time to work on it myself.

Comment 47

[Thomas D. (:thomas8)]

Kyle Huey, this bug is stuck because there's seems to be a problem in core's nsDataObj.cpp that the functions for drag'n'drop do not support dropping multiple files, per comment 33. Given that you seem to have done significant work in this area (per David Bienvenu's comment 37), can you advise or assist in any way?

(In reply to Jim Porter (:squib) from comment #33)
> Yeah, it looks like these functions only have support for dropping a single
> file: *
> http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsDataObj.
> cpp#1319
> *
> http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsDataObj.
> cpp#1521
> 
> This will be interesting, since I don't actually want to set stuff up to
> compile on Windows, but the actual fix is probably straightforward. I guess
> we'll see...

Comment 48

[Thomas D. (:thomas8)]

Jim, can you assist? vv

Source link update - nsDataObj.cpp has moved and is now found here:
http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsDataObj.cpp

(In reply to Jim Porter (:squib) from comment #33)
> Yeah, it looks like these functions only have support for dropping a single
> file:
http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsDataObj.cpp#1319
http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsDataObj.cpp#1521

I'm not sure if these are still pointing to the lines intended in comment 33.
Jim, can you find the new lines for those functions which you think need to be changed?

Comment 49

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

(In reply to Thomas D. from comment #47)
> Kyle Huey, this bug is stuck because there's seems to be a problem in core's
> nsDataObj.cpp that the functions for drag'n'drop do not support dropping
> multiple files, per comment 33. Given that you seem to have done significant
> work in this area (per David Bienvenu's comment 37), can you advise or
> assist in any way?

It's been a while, but if I remember correctly, each nsDataObj corresponds to a single "thing" that is being dragged and dropped.  If multiple things are being dragged and dropped there is an nsDataObjCollection class that wraps a series of nsDataObj, one for each separate item.

Comment 50

[Geoffroy Ménard]

I have this bug with Thunderbird 24.0 Windows32.
If it's fixed this year it won't pass the ten years cap :)

Comment 51

[Serge Hamulus]

I have this bug since i'm using Thunderbird.
Is this problem so hard to resolve?

Comment 52

[Jim Porter (:squib)]

(In reply to Serge Hamulus from comment #51)
> I have this bug since i'm using Thunderbird.
> Is this problem so hard to resolve?

Pretty hard, unless you happen to know a whole bunch about platform APIs (Win32, etc).

Comment 53

[Thomas D. (:thomas8)]

Jim, you seem to say in your comment 33 that you were facing a problem where "these functions only have support for dropping a single file". According to Kyle's information, instead of nsDataObj for single files we could instead use "nsDataObjCollection class that wraps a series of nsDataObj".
Does that information help to identify what's missing to complete your excellent groundwork?

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #49)
> (In reply to Thomas D. from comment #47)
> > Kyle Huey, this bug is stuck because there's seems to be a problem in core's
> > nsDataObj.cpp that the functions for drag'n'drop do not support dropping
> > multiple files, per comment 33. Given that you seem to have done significant
> > work in this area (per David Bienvenu's comment 37), can you advise or
> > assist in any way?
> 
> It's been a while, but if I remember correctly, each nsDataObj corresponds
> to a single "thing" that is being dragged and dropped.  If multiple things
> are being dragged and dropped there is an nsDataObjCollection class that
> wraps a series of nsDataObj, one for each separate item.

Comment 54

[Jim Porter (:squib)]

(In reply to Thomas D. (away till 23rd Oct) from comment #53)
> Jim, you seem to say in your comment 33 that you were facing a problem where
> "these functions only have support for dropping a single file". According to
> Kyle's information, instead of nsDataObj for single files we could instead
> use "nsDataObjCollection class that wraps a series of nsDataObj".
> Does that information help to identify what's missing to complete your
> excellent groundwork?

Not really, no. I don't think that's actually sufficient, since my (extremely vague) recollection is that the platform-specific code just plain doesn't handle this case. You can certainly drag and drop multiple files to your heart's content *within* Thunderbird. It just becomes an issue when you try to drop them outside of Thunderbird.

In any case, I've almost completely forgotten everything about this bug, and I doubt I'll have time/inclination to work on it at any point in the foreseeable future. Besides that, my groundwork was literally just adding a for loop to a block of code, so there's not much there. :)

Basically, to fix this bug, you need to find someone who 1) knows Gecko, 2) knows platform-specific APIs for this stuff, 3) has some free time, and 4) wants to spend that free time on something that only benefits Thunderbird. I don't think there are many people who fit that description. I'm certainly not one of them (mainly due to (2) and (3)).

Comment 55

[Serge Hamulus]

(In reply to Jim Porter (:squib) from comment #52)
> (In reply to Serge Hamulus from comment #51)
> > I have this bug since i'm using Thunderbird.
> > Is this problem so hard to resolve?
> 
> Pretty hard, unless you happen to know a whole bunch about platform APIs
> (Win32, etc).

But why Ms Outlook Express don't have this problem?
Why many other software have possibility to drag'n'drop multiple (such as Total Commander, ACDSee and many others)?
Are 9 years not enough to resolve problem?

Comment 56

[Philippp]

Seems to be a matter of funding... How about a kickstarter project to pay someone to do this?

(I also think that it does not make Thunderbird look very professional if such a feature (what appears to be quite "basic" to the user) still is not implemented.)

Comment 57

[[e-mail]]

Hello, thank You everybody for Your interest about this.
It seems like a trivial problem, but it is not - this caused the loss Thunderbird in a duel with Outlook. This is an example from real life: People who work with many graphic files just this function was considered essential for the work they do.

Comment 58

[Herbert Eppel]

Have just voted for this but haven't read/digested all the comments, to be honest.
I'm puzzled about the fact that is was first reported back in 2004 for Thunderbird version 0.9. I'm pretty sure that multiple attachment dragging used to work OK for a while (can't remember when, possibly in TB 3) but got broken again later. The current behaviour can be very irritating.

Comment 59

[Jonathan Meier]

Attachment: Part 1: Core fix (nsDataObjectCollection)

I came across this bug report by chance and as I have recently worked with this part of the Windows API I decided to have a look at it and it seems that I was able to fix the issues. I am going to attach 5 patches each fixing something related to DnD to the file system. I would be glad if someone could review them.

This first patch fixes the core of the drag and drop interaction with the Windows API. It does not yet allow multiple attachments to be dropped in an OS file folder but it has the nice side effect that multiple e-mails can be dropped in an OS file folder from within SeaMonkey.

Comment 60

[Jonathan Meier]

Attachment: Part 2: DnD of multiple attachments in Thunderbird on Windows and Mac OS X

This second patch enables DnD of multiple attachments in an OS file folder from within Thunderbird on Windows and Mac OS X.

Comment 61

[Jonathan Meier]

Attachment: Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X

This third patch enables DnD of multiple messages in an OS file folder from within Thunderbird on Windows and Mac OS X.

Comment 62

[Jonathan Meier]

Attachment: Part 4: Dnd of multiple attachments in SeaMonkey on Windows and Mac OS X

This fourth patch enables DnD of multiple attachments in an OS file folder from within SeaMonkey on Windows and Mac OS X.

Comment 63

[Jonathan Meier]

Attachment: Part 5: DnD of multiple messages in SeaMonkey on Mac OS X

This fifth patch enables DnD of multiple messages in an OS file folder from within SeaMonkey on Mac OS X.

Comment 64

[Jonathan Meier]

Attachment: Part 5: DnD of multiple messages in SeaMonkey on Mac OS X

Comment 65

[Jim Porter (:squib)]

needinfo'ing myself so I remember to take a look at these patches tomorrow-ish. It'd be nice to get this in for TB 31.

Comment 66

[Jim Porter (:squib)]

Comment on attachment 8410686 [details] [diff] [review]
Part 2: DnD of multiple attachments in Thunderbird on Windows and Mac OS X

Review of attachment 8410686 [details] [diff] [review]:
-----------------------------------------------------------------

In the future, make sure to set the "review" flag to "?" and then put a reviewer in the "Requestee" field. I'm an ok reviewer for the TB parts.

::: mail/base/content/msgHdrViewOverlay.js
@@ +2708,5 @@
>  
> +    if (target.localName == "attachmentitem") {
> +      let selection = target.parentNode.selectedItems;
> +      aAttachmentData.data = new TransferDataSet();
> +      for(let i in selection) {

You can do the following here:

  for (let item of selection) {
    let transferData = CreateAttachmentTransferData(item.attachment);
    if (transferData)
      aAttachmentData.data.push(transferData);
  }

Comment 67

[Jim Porter (:squib)]

Comment on attachment 8410689 [details] [diff] [review]
Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X

Review of attachment 8410689 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/base/content/msgMail3PaneWindow.js
@@ +1385,2 @@
>  
> +  for (let i in messageUris) {

As mentioned in the other review, you should be able to do "for (let uri of messageUris)" here.

@@ +1398,5 @@
> +    messages[i] = { uri: messageUris[i], fileName: validateFileName(uniqueFileName) };
> +  }
> +
> +  messages.sort(function(a, b)
> +    {

Nit: put this brace on the preceding line.

@@ +1399,5 @@
> +  }
> +
> +  messages.sort(function(a, b)
> +    {
> +      return a.fileName.toLocaleLowerCase().localeCompare(b.fileName.toLocaleLowerCase());

The order doesn't actually matter here, right? It looks like you're just using this to ensure that we don't give messages duplicate filenames. In that case, I'd make a data structure more like this:

{
  "filename foo": [ "msgURI #1", "msgURI #2" ],
  "filename bar": [ "msgURI #3" ],
}

@@ +1402,5 @@
> +    {
> +      return a.fileName.toLocaleLowerCase().localeCompare(b.fileName.toLocaleLowerCase());
> +    });
> +
> +  let msgUrls = {};

Nit: put this inside the for loop below.

@@ +1403,5 @@
> +      return a.fileName.toLocaleLowerCase().localeCompare(b.fileName.toLocaleLowerCase());
> +    });
> +
> +  let msgUrls = {};
> +  

Nit: remove this trailing whitespace.

@@ +1405,5 @@
> +
> +  let msgUrls = {};
> +  
> +  let count = 0;
> +  

Nit: remove this trailing whitespace.

@@ +1406,5 @@
> +  let msgUrls = {};
> +  
> +  let count = 0;
> +  
> +  for (let i = 0; i < messages.length; i++) {

You can express this as: "for (let msg of messages) {"

@@ +1410,5 @@
> +  for (let i = 0; i < messages.length; i++) {
> +    messenger.messageServiceFromURI(messages[i].uri)
> +             .GetUrlForUri(messages[i].uri, msgUrls, null);
> +    aEvent.dataTransfer.mozSetDataAt("text/x-moz-message", messages[i].uri, i);
> +    aEvent.dataTransfer.mozSetDataAt("text/x-moz-url",msgUrls.value.spec, i);

Nit: add a space after the first comma.

@@ +1440,5 @@
>    aEvent.dataTransfer.addElement(aEvent.originalTarget);
>  }
>  
> +function messageFlavorDataProvider()
> +{

Nit: put this brace on the previous line (likewise below).

@@ +1457,5 @@
> +  getFlavorData : function(aTransferable, aFlavor, aData, aDataLen)
> +  {
> +    if (aFlavor == "application/x-moz-file-promise")
> +    {
> +      var fileUriPrimitive = { };

Nit: no space between "{" and "}" (likewise below).

@@ +1468,5 @@
> +                             .getService(Components.interfaces.nsIIOService);
> +      var fileUri = ioService.newURI(fileUriStr.data, null, null);
> +      var fileUrl = fileUri.QueryInterface(Components.interfaces.nsIURL);
> +      var fileName = fileUrl.fileName;
> +      

Nit: remove this trailing whitespace.

@@ +1475,5 @@
> +                                    destDirPrimitive, dataSize);
> +      var destDirectory = destDirPrimitive.value.QueryInterface(Components.interfaces.nsILocalFile);
> +      var file = destDirectory.clone();
> +      file.append(fileName);
> +      

Nit: remove this trailing whitespace.

@@ +1479,5 @@
> +      
> +      var messageUriPrimitive = { };
> +      aTransferable.getTransferData("text/x-moz-message", messageUriPrimitive, dataSize);
> +      var messageUri = messageUriPrimitive.value.QueryInterface(Components.interfaces.nsISupportsString);
> +        

Nit: remove this trailing whitespace.

Comment 68

[Jonathan Meier]

Attachment: [checked in, comment 106, Aurora comment 112] Part 2: DnD of multiple attachments in Thunderbird on Windows and Mac OS X v2

This is a new version based on the review in Comment 66.

Thank you very much for your comments and reviews, Jim! Please excuse all those little coding style mistakes as this is my first time contributing to mozilla.

I corrected them in the following patches and also implemented your other improvement suggestions.

Regarding the other three patches which you did not review: Will you review them too or should I flag them for review by someone else?

Comment 69

[Jonathan Meier]

Attachment: Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v2

This is a new version based on the review in Comment 67.

Comment 70

[Jonathan Meier]

Attachment: Part 4: Dnd of multiple attachments in SeaMonkey on Windows and Mac OS X v2

Even though this was not reviewed yet, this is a new version fixing the same mistakes and implementing the same suggestions pointed out in Comment 66 and Comment 67 as the code is very similar.

Comment 71

[Jonathan Meier]

Attachment: Part 5: DnD of multiple messages in SeaMonkey on Mac OS X v2

Even though this was not reviewed yet, this is a new version fixing the same mistakes and implementing the same suggestions pointed out in Comment 66 and Comment 67 as the code is very similar.

Comment 72

[Jim Porter (:squib)]

(In reply to Jonathan Meier from comment #68)
> Regarding the other three patches which you did not review: Will you review
> them too or should I flag them for review by someone else?

I don't really have the authority to review Core or Seamonkey patches, but I'll try to find the right people to do the review for you.

Comment 73

[Thomas D. (:thomas8)]

Mark, per :squibs comment 65, this would be really nice to land for TB31, and patches look good so it can land very quickly after review.

Comment 74

[Thomas D. (:thomas8)]

Mark, per :squibs comment 65, this would be really nice to land for TB31.
Patches are ready and look good so it can land very quickly after review.

Also much-wanted by users (15 duplicates, 19 votes).

Comment 75

[Thomas D. (:thomas8)]

Can we make sure that these highly valuable patches for two very popular bugs do not bitrot?
Jim (or anyone who knows), could you request the right people for reviewing, especially the Core patch (attachment 8410680 [details] [diff] [review])? All patches are pretty short, we're not reinventing the wheel here... but it would be very cool to roll this into TB31!

(In reply to Jim Porter (:squib) from comment #72)
> (In reply to Jonathan Meier from comment #68)
> > Regarding the other three patches which you did not review: Will you review
> > them too or should I flag them for review by someone else?
> 
> I don't really have the authority to review Core or Seamonkey patches, but
> I'll try to find the right people to do the review for you.

Comment 76

[Thomas D. (:thomas8)]

Patch 3 (attachment 8414758 [details] [diff] [review]) will also fix bug 634720 to drag and drop multiple messages to OS folder! :)
Sorry for spam, unintendedly removed dependency.

Comment 77

[Jim Porter (:squib)]

I'll try to look at this sometime during the weekend, since I just finished up a big project and have a bit more free time.

Comment 78

[Thomas D. (:thomas8)]

Comment on attachment 8414758 [details] [diff] [review]
Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v2

Review of attachment 8414758 [details] [diff] [review]:
-----------------------------------------------------------------

I didn't review this, just a comment/question on something I noticed.

::: mail/base/content/msgMail3PaneWindow.js
@@ +1389,1 @@
>      let uniqueFileName;

Even before this patch, I think we don't ensure that this is really a unique fileName for multiple different messages with same subject?

@@ +1395,2 @@
>        uniqueFileName = (subject.length <= maxUncutNameLength) ?
>                         subject : subject.substr(0, maxCutNameLength) + longSubjectTruncator;

Note:

This creates different naming pattern compared to *saving* multiple messages via Ctrl+S or File > Save as..., for which we add more message header information into the filenames to make them more descriptive and unique, with output file names like this:

[Bug 270292] Unable to drag multiple attachments to OS file folder - 'Bugzilla@Mozilla' ([e-mail]) - 2014-05-25 0014.eml
[Bug 270292] Unable to drag multiple attachments to OS file folder - 'Bugzilla@Mozilla' ([e-mail]) - 2014-05-25 0020.eml

I don't see a fundamental difference between saving and drag and drop of multiple messages, so probably behaviour should be same.
But if it's too complicated, we can postpone that to another bug.

For saving / drag and drop of *single* files, there's less need to add such descriptive extra information; however, we might still want to use same naming pattern so that even sequential saving of single messages, or saving a bunch and then saving/dragging out the one msg which you forgot, should probably still create sets of .eml files with consistent naming pattern.

@@ +1419,5 @@
> +
> +      let fileName = msg.fileName;
> +
> +      if (msgCount > 0) {
> +        fileName = fileName + msgCount;

Can you provide some examples of filename output, and explain why this is used? Is this perhaps a workaround for the uniqueness problem seen above?

Subject of msg1: Identical foo
Subject of msg2: Identical foo
Subject of msg3: Short subject
Subject of msg4: Overlong....subject

Comment 79

[Jim Porter (:squib)]

Comment on attachment 8410680 [details] [diff] [review]
Part 1: Core fix (nsDataObjectCollection)

:jimm, this is a Win32 widget patch. Do you think you could take a look at this (or pass it on to someone else who could)? This would be pretty nice for Thunderbird. Thanks!

Comment 80

[Jim Porter (:squib)]

Comment on attachment 8414754 [details] [diff] [review]
[checked in, comment 106, Aurora comment 112] Part 2: DnD of multiple attachments in Thunderbird on Windows and Mac OS X v2

Review of attachment 8414754 [details] [diff] [review]:
-----------------------------------------------------------------

Ok, I think this looks right for the most part. Just one thing I noticed that needs fixing, and then I'll try to get a try build run for this so I can make sure it all works...

::: mail/base/content/msgHdrViewOverlay.js
@@ +2715,5 @@
>  
> +    if (target.localName == "attachmentitem") {
> +      let selection = target.parentNode.selectedItems;
> +      aAttachmentData.data = new TransferDataSet();
> +      for(let item of selection) {

Nit: add a space after the "for".

@@ +2720,5 @@
> +        let transferData = CreateAttachmentTransferData(item.attachment);
> +        if (transferData)
> +          aAttachmentData.data.push(transferData);
> +      }
> +    }

If we end up with no TransferDatas in the TransferDataSet, we should bail out here. I *think* you can do something like:

  aEvent.dataTransfer.dropEffect = 'none';

Comment 81

[Jonathan Meier]

(In reply to Thomas D. from comment #78)

> Even before this patch, I think we don't ensure that this is really a unique
> fileName for multiple different messages with same subject?

We actually did ensure unique file names within the dragged set before this patch. There was a method taking care of it by appending continuous numbers to the colliding messages. But there was a note that this method might be too slow for 10'000+ messages, which is why I improved the speed of it which then resulted in the code below:

> @@ +1419,5 @@
> > +
> > +      let fileName = msg.fileName;
> > +
> > +      if (msgCount > 0) {
> > +        fileName = fileName + msgCount;
> 
> Can you provide some examples of filename output, and explain why this is
> used? Is this perhaps a workaround for the uniqueness problem seen above?
> 
> Subject of msg1: Identical foo
> Subject of msg2: Identical foo
> Subject of msg3: Short subject
> Subject of msg4: Overlong....subject

If all four messages above would be dragged, the following is the result:
msg1: Identical foo.eml
msg2: Identical foo1.eml
msg3: Short subject.eml
msg4: Overlong....su.eml (assuming letter b is the 125th letter in the overlong subject. This limitation seems to come from the Windows API, however I just adopted this from the previous code.)

So it actually is a workaround for the uniqueness problem, but it was already there before this patch and therefore not my invention.

> I don't see a fundamental difference between saving and drag and drop of
> multiple messages, so probably behaviour should be same.
> But if it's too complicated, we can postpone that to another bug.

I agree with you and I think it wouldn't be that complicated.

> For saving / drag and drop of *single* files, there's less need to add such
> descriptive extra information; however, we might still want to use same
> naming pattern so that even sequential saving of single messages, or saving
> a bunch and then saving/dragging out the one msg which you forgot, should
> probably still create sets of .eml files with consistent naming pattern.

It is indeed a problem with the current implementation that if you drag msg1 and msg2 from above individually to the same destination directory, they both result in Identical foo.eml because they are not in the same dragging set. Using the same naming scheme as "Save As..." does, would drastically reduce the collision probability as it incorporates the receiving date and time of the message. The only case in which two different message result in the same file name would be if both of them had the same subject and arrived in the exact same minute.

Comment 82

[:aceman]

I put the comment there about the 10000 messages. It does not mean anybody wants to drag 10000 messages onto the desktop. But those paths are generated in each case even when dragging to another folder inside TB. So that use case is valid and we need to keep the speed sane.

Comment 83

[Jonathan Meier]

(In reply to Jim Porter (:squib) from comment #80)

> If we end up with no TransferDatas in the TransferDataSet, we should bail
> out here. I *think* you can do something like:
> 
>   aEvent.dataTransfer.dropEffect = 'none';

Is this really necessary? The DOM EventStateManager handles empty transfer sets correctly by not invoking a drag session at all in that case.

Comment 84

[Jim Mathies [:jimm]]

Comment on attachment 8410680 [details] [diff] [review]
Part 1: Core fix (nsDataObjectCollection)

Review of attachment 8410680 [details] [diff] [review]:
-----------------------------------------------------------------

minusing for now because some of the changes here are unclear, generally though looks to be headed in the right direction.

::: widget/windows/nsDataObjCollection.cpp
@@ +54,4 @@
>      return NOERROR;
>    }
>  
> +  if (IID_IAsyncOperation == riid) {

nit - comment here explaining what's going on.

::: widget/windows/nsDataObjCollection.h
@@ -123,5 @@
>  
>      ULONG m_cRef;              // the reference count
>  
> -    // nsDataObjCollection owns and ref counts CEnumFormatEtc
> -    CEnumFormatEtc   * m_enumFE;

Why was this removed? I still see references to it in the code post this patch.

@@ -129,4 @@
>      nsTArray<nsRefPtr<nsDataObj> > mDataObjects;
> -    
> -    BOOL mIsAsyncMode;
> -    BOOL mIsInOperation;

You removed these but you kept the asignment in the constructor, but moved the assignments down into the method? Curious what's going on here.

Comment 85

[Mark Banner (:standard8)]

Sorry for not responding earlier, unfortunately "has patches" and "want to get in" doesn't warrant release tracking. Release tracking is primarily for regressions/stability rather than for features.

Comment 86

[Jim Porter (:squib)]

Comment on attachment 8414758 [details] [diff] [review]
Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v2

Review of attachment 8414758 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay on this. This generally looks ok, but you should probably get someone who knows more about Windows/Mac builds to take a look at this too. Maybe Magnus?

Comment 87

[Jonathan Meier]

Attachment: [checked into M-C, comment 96] Part 1: Core fix (nsDataObjectCollection) v2

Sorry for my late reply. It seems like I have uploaded the wrong patch for Part 1: Core fix (nsDataObjectCollection). So here is (hopefully) the right patch.

I'll shortly explain the three essential steps taken to fix nsDataObjectCollection:

1. nsDataObjectCollection needs to provide the IAsyncOperation interface. This is required by the nsDragService in the method StartInvokingDragSession. Since nsDataObjectCollection inherits from nsDataObject which already implements IAsyncOperation we just need to return the this pointer in QueryInterface.

2. Because nsClipboard operates on nsDataObject and nsDataObjectCollection only through the nsDataObject interface the method AddDataFlavor in nsDataObject needs to be virtual since nsDataObjectCollection overrides this method.

3. In the method GetFileDescriptors of nsDataObjectCollection the FILEDESCRIPTOR and FILEGROUPDESCRIPTER structures were not correctly set up regarding the Windows API documentation (memory allocation and structure).

In addition there were many methods in nsDataObjectCollection copied from nsDataObject which I removed since they can be inherited directly.

Comment 88

[Jim Mathies [:jimm]]

Comment on attachment 8479412 [details] [diff] [review]
[checked into M-C, comment 96] Part 1: Core fix (nsDataObjectCollection) v2

Review of attachment 8479412 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/nsDataObjCollection.cpp
@@ +386,5 @@
>      if (dataObj->QueryGetData(&fEtc) != S_OK)
>        continue;
>      if (num == numwanted)
>        return dataObj->GetData(pFE, pSTM);
> +    num++;

eek, nice catch.

Comment 89

[Magnus Melin [:mkmelin]]

Comment on attachment 8414758 [details] [diff] [review]
Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v2

Review of attachment 8414758 [details] [diff] [review]:
-----------------------------------------------------------------

I'm on linux so I haven't tested this. Can you fix the minor issues with this patch and have someone else give the final review. Maybe bwinton?

::: mail/base/content/msgMail3PaneWindow.js
@@ +1399,5 @@
> +    let msgFileName = validateFileName(uniqueFileName);
> +    let msgFileNameLowerCase = msgFileName.toLocaleLowerCase();
> +
> +    if(!messages[msgFileNameLowerCase]){
> +      messages[msgFileNameLowerCase] = new Array();

= [];

@@ +1451,5 @@
> +
> +  getFlavorData : function(aTransferable, aFlavor, aData, aDataLen) {
> +    if (aFlavor == "application/x-moz-file-promise") {
> +      var fileUriPrimitive = {};
> +      var dataSize = {};

please use let instead of var for everyting you touch

@@ +1457,5 @@
> +                                    fileUriPrimitive, dataSize);
> +
> +      var fileUriStr = fileUriPrimitive.value.QueryInterface(Components.interfaces.nsISupportsString);
> +      var ioService = Components.classes["@mozilla.org/network/io-service;1"]
> +                             .getService(Components.interfaces.nsIIOService);

Services.io

Comment 90

[Jonathan Meier]

Attachment: Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v3

Fixed issues pointed out in Comment 89.

Comment 91

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 8414754 [details] [diff] [review]
[checked in, comment 106, Aurora comment 112] Part 2: DnD of multiple attachments in Thunderbird on Windows and Mac OS X v2

This seems fine to me, although I think I would be happier if we special-cased the code when there was only one selected item to be more like the original code, instead of always passing a TransferDataSet.

Comment 92

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 8490765 [details] [diff] [review]
Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v3

Review of attachment 8490765 [details] [diff] [review]:
-----------------------------------------------------------------

r-, not because I think it's a bad patch.  Indeed, I'm overall happy with it!  But instead because I'ld like to give it another once-over, and read the answers to my last question before we check it in.  :)

For my part, I promise to review the next version within 12 hours of you posting it.  None of this week-long waiting for a review.  (Also, sorry about that!)

Thanks,
Blake.

::: mail/base/content/msgMail3PaneWindow.js
@@ +1428,2 @@
>  
> +  let messages = {};

I think we want to use a Map here…
(https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map)

@@ +1454,5 @@
> +  let index = 0;
> +
> +  for (let msgName in messages) {
> +    let msgCount = 0;
> +    for (let msg of messages[msgName]) {

If you use:
for ([msgCount, msg] of messages[msgName].entries()) {
instead, then you don't need to manually increment msgCount.  :)

@@ +1456,5 @@
> +  for (let msgName in messages) {
> +    let msgCount = 0;
> +    for (let msg of messages[msgName]) {
> +      let msgUrls = {};
> +      messenger.messageServiceFromURI(msg.uri)

Instead of re-looking up the messageServiceFromURI here, how about we get the url in the "for (let msgUri of messageUris) {" loop above, and stuff its ".value.spec" property in the messages object as "msg.url"?

@@ +1467,5 @@
> +      if (msgCount > 0) {
> +        fileName = fileName + msgCount;
> +      }
> +
> +      fileName = fileName + ".eml";

Why not do this filename processing in the loop above, too?
We could add:
(messages[msgFileNameLowerCase].length || "") + ".eml"
to the end of the filename up there (I think).  And then we wouldn't need msgCount at all!  :)

@@ +1490,5 @@
> +  QueryInterface : function(iid) {
> +      if (iid.equals(Components.interfaces.nsIFlavorDataProvider) ||
> +          iid.equals(Components.interfaces.nsISupports))
> +        return this;
> +      throw Components.results.NS_NOINTERFACE;

I think this function should be replaced with:
QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsIFlavorDataProvider,
                       Components.interfaces.nsISupports]),
like in http://mxr.mozilla.org/mozilla-central/source/accessible/jsat/EventManager.jsm#537

@@ +1516,5 @@
> +      let messageUriPrimitive = {};
> +      aTransferable.getTransferData("text/x-moz-message", messageUriPrimitive, dataSize);
> +      let messageUri = messageUriPrimitive.value.QueryInterface(Components.interfaces.nsISupportsString);
> +
> +      messenger.saveAs(messageUri.data, true, null, decodeURIComponent(file.path), true);

What happens if this function fails?
(Like, if you try to drag files into a directory you don't have permissions to write to, or one of the filenames contains invalid characters, or you run out of space on the drive?)

Comment 93

[Jonathan Meier]

Attachment: Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v4

Thanks a lot for the thorough review!

I now use a map and only a single loop for composing the file names.

Regarding your last question:
Invalid characters get replaced on both Windows and Mac.
On Windows, dragging messages onto a folder that you don't have write permission or not enough space behaves exactly the same as if you would drag some files from another folder. It displays an error message from Windows Explorer indicating the issue.
On Mac, you can't even drop on a folder with insufficient permissions, however if there is not enough space available at the drop target, dropping just silently fails.

Comment 94

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 8516699 [details] [diff] [review]
Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v4

Review of attachment 8516699 [details] [diff] [review]:
-----------------------------------------------------------------

Okay, this is way better, but I think I'm going to ask for one more review, just to see how you handle the filename problem I mention below.

Thanks!

::: mail/base/content/msgMail3PaneWindow.js
@@ +1430,4 @@
>  
> +  let index = 0;
> +
> +  for (let msgUri of messageUris) {

Instead of manually keeping track of an index, what about doing something like:
for (let [index, msgUri] of messageUris.entries()) {
  …
}
?

@@ +1452,5 @@
> +    }
> +    else {
> +      msgFileName = msgFileName + messages[msgFileNameLowerCase];
> +    }
> +    

Nit: trailing spaces on an empty line.  (Also below.)

@@ +1453,5 @@
> +    else {
> +      msgFileName = msgFileName + messages[msgFileNameLowerCase];
> +    }
> +    
> +    msgFileName = msgFileName + ".eml";

Okay, so, I think this will fail if I have three messages with the titles:
  Test
  Test
and
  Test1

Perhaps we should also add an entry into messages for the message with the number at the end?

(Also, I think we might want to add a "-" in between the subject and the number, so that we'ld get "Test" and "Test-1" instead of "Test" and "Test1"…)

I apologize for not catching this sooner!

@@ +1482,5 @@
> +  QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsIFlavorDataProvider,
> +                       Components.interfaces.nsISupports]),
> +
> +  getFlavorData : function(aTransferable, aFlavor, aData, aDataLen) {
> +    if (aFlavor == "application/x-moz-file-promise") {

If you reversed this to:
if (aFlavor !== "application/x-moz-file-promise") {
  return;
}
then you could un-indent the rest of the function, which might be a little easier to read.  (If you don't feel like changing this, that's okay too.  :)

Comment 95

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c2369f6c3a35

Comment 96

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c2369f6c3a35

Comment 97

[Jonathan Meier]

Attachment: Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v5

Sorry for ignoring the "for (let [index, msgUri] of messageUris.entries())" thing. I just now saw, that you already suggested this before.

In this version of the patch, file name clashes shouldn't occur anymore.

Comment 98

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 8526386 [details] [diff] [review]
Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v5

Review of attachment 8526386 [details] [diff] [review]:
-----------------------------------------------------------------

Well, overall, r=me!  Thank you for the changes, and for your patience with the review process!  :)

::: mail/base/content/msgMail3PaneWindow.js
@@ +1427,5 @@
> +        messages[msgFileNameLowerCase]++;
> +        msgFileName = msgFileName + postfix;
> +        msgFileNameLowerCase = msgFileNameLowerCase + postfix;
> +      }
> +    }

So, I _think_ this code works, but wow was it ever hard to track through in my head…  Still, I think it works, and I'm not sure I could come up with a better algorithm.  :)

(As a followup bug, what do you think about adding some tests that verify it does what we think it should?)

Comment 99

[Thomas D. (:thomas8)]

Magnus, can this land so that users can finally drag multiple attachments into OS file folders?

Comment 100

[Guillaume .J]

Hello everyone ! :)

Some of the users of my organisation asked me about the possibility of drag and drop multiple attachments toward a folder. So i just wanted to say that this feature could be very usefull to ours users. So please dont forget it ^^

Thank you :)

Comment 101

[Magnus Melin [:mkmelin]]

Attachment: Bug_270292_thunderbird_messages_win_mac_v5.patch

Refreshed part 3. Unfortunately, seems to still just drop one attachment (at least on windows).

Comment 102

[Jonathan Meier]

Please note that part 2 enables drag and drop of multiple attachments and part 3 enables drag and drop of multiple messages.

Comment 103

[Jonathan Meier]

Attachment: [checked in, comment 106, Aurora comment 112] Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v6

Fixes the refreshment of comment 101 which did not work.

Comment 104

[Jonathan Meier]

Comment on attachment 8702665 [details] [diff] [review]
Bug_270292_thunderbird_messages_win_mac_v5.patch

Does not work. See fix in comment 103.

Comment 105

[Magnus Melin [:mkmelin]]

Comment on attachment 8702747 [details] [diff] [review]
[checked in, comment 106, Aurora comment 112] Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v6

Review of attachment 8702747 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, seems to work fine!

Comment 106

[Magnus Melin [:mkmelin]]

Folded and committed part2+3. https://hg.mozilla.org/comm-central/rev/ca2c0fd7c80d
Marking this fixed now, we should uplift it to 45. 

The seamonkey patches are not reviewed yet, and it could be cleaner to just clone this bug and have them land there after review.

Comment 107

[Magnus Melin [:mkmelin]]

Comment on attachment 8702747 [details] [diff] [review]
[checked in, comment 106, Aurora comment 112] Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v6

Approval Request Comment
[User impact if declined]: dragging multiple emails/attachments only drops one
[String/UUID change made/needed]: none

So this uplift is for https://hg.mozilla.org/comm-central/rev/ca2c0fd7c80d which is part2+part3.

Comment 108

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Magnus, these patches need to first land on moz-central (Nightly) before they can be uplifted to Beta or Aurora. Could you please set the right keywords for that to happen? Thanks!

Comment 109

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8702747 [details] [diff] [review]
[checked in, comment 106, Aurora comment 112] Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v6

I am pretty sure you want approval-comm

Comment 110

[Kent James (:rkent)]

Comment on attachment 8702747 [details] [diff] [review]
[checked in, comment 106, Aurora comment 112] Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v6

Let's at least get into aurora ASAP.

Comment 111

[Magnus Melin [:mkmelin]]

Yes indeed, wrong flag. The patch is for comm-cental

Comment 112

[Jorg K (CEST = GMT+2)]

Aurora:
https://hg.mozilla.org/releases/comm-aurora/rev/833446bffdd9

Comment 113

[Kent James (:rkent)]

(In reply to Jorg K (GMT+1) from comment #112)
> Aurora:
> https://hg.mozilla.org/releases/comm-aurora/rev/833446bffdd9

When you do aurora checkins, please also set the status-thunderbird-* flags in tracking_flags.

Comment 114

[Jorg K (CEST = GMT+2)]

Sorry. I'm new here ;-)
I assume we're tracking 45, 46 and perhaps 44, since there was a beta approval request.
I'm not sure we're doing a beta 44, personally I'd focus on a good beta 45 at the end of Jan. (2016-01-25).

Comment 115

[Jorg K (CEST = GMT+2)]

Comment on attachment 8702747 [details] [diff] [review]
[checked in, comment 106, Aurora comment 112] Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v6

I'm taking the liberty to clear the beta request. TB 45 beta is coming out in a few days, so there is no need to land this on TB 44 beta given that this is an ancient old bug.

Comment 116

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.