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

RESOLVED FIXED in Thunderbird 46.0

Status

defect
RESOLVED FIXED
15 years ago
a year ago

People

(Reporter: mcacker, Assigned: mr_sandman)

Tracking

(Blocks 2 bugs)

Trunk
Thunderbird 46.0
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird31-, thunderbird44 wontfix, thunderbird45 fixed, thunderbird46 fixed, thunderbird_esr38 wontfix)

Details

(Whiteboard: [gs] , URL)

Attachments

(6 attachments, 12 obsolete attachments)

3.02 KB, patch
Details | Diff | Splinter Review
747 bytes, patch
bwinton
: review+
squib
: feedback+
Details | Diff | Splinter Review
2.71 KB, patch
Details | Diff | Splinter Review
7.06 KB, patch
Details | Diff | Splinter Review
12.51 KB, patch
jimm
: review+
Details | Diff | Splinter Review
6.20 KB, patch
mkmelin
: review+
Details | Diff | Splinter Review
(Reporter)

Description

15 years ago
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

15 years ago
Doesn't matter if they're .jar files; I see the same behavior with multiple .jpg 
files, for instance.
Severity: normal → minor
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Hardware: PC → All

Comment 2

14 years ago
*** Bug 285293 has been marked as a duplicate of this bug. ***
The occasional file in use error is also reported here: bug 285296

Comment 4

14 years ago
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

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

Comment 6

14 years ago
This bug also happens in Seamonkey, moving to Core.
Assignee: mscott → nobody
Component: General → MailNews: Attachments
Product: Thunderbird → Core
Version: unspecified → Trunk

Comment 7

13 years ago
*** Bug 342405 has been marked as a duplicate of this bug. ***
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.
Severity: minor → normal
QA Contact: attachments
Product: Core → MailNews Core

Updated

10 years ago
Duplicate of this bug: 484666

Updated

10 years ago
Duplicate of this bug: 464644

Updated

10 years ago
Duplicate of this bug: 490797

Comment 12

10 years ago
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)?

Updated

10 years ago
Duplicate of this bug: 511861

Comment 14

10 years ago
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

10 years ago
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.
Depends on: 227305

Updated

10 years ago
Depends on: 513464

Updated

10 years ago
No longer depends on: 227305
Duplicate of this bug: 521012
Duplicate of this bug: 533417

Updated

9 years ago
Whiteboard: [gs]
Duplicate of this bug: 610963
Taking this, since I've been doing stuff in this part of the code anyway.
Assignee: nobody → squibblyflabbetydoo
Status: NEW → ASSIGNED
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

8 years ago
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?
Probably. I was waffling on whether we should ignore the error or yell at the user.

Comment 23

8 years ago
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

8 years ago
ignoring the error and creating a new bug, that is.
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?
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.
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.
Attachment #501528 - Attachment is obsolete: true
(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 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).
Attachment #501777 - Flags: review?(bienvenu)

Comment 30

8 years ago
(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

8 years ago
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.
(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.
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

8 years ago
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

8 years ago
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.
Attachment #501777 - Flags: review?(bienvenu)
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

8 years ago
(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 khuey@kylehuey.com seems to have done significant work in that area.

Updated

8 years ago
Duplicate of this bug: 634720
Duplicate of this bug: 642168

Updated

8 years ago
Duplicate of this bug: 647277

Updated

8 years ago
Duplicate of this bug: 681218
Hi

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

Updated

7 years ago
Duplicate of this bug: 753706

Comment 45

7 years ago
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?
(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.
Assignee: squibblyflabbetydoo → nobody
Status: ASSIGNED → NEW

Updated

7 years ago
Summary: Unable to drag multiple attachments to a folder → Unable to drag multiple attachments to OS file folder

Comment 47

7 years ago
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...
Flags: needinfo?(khuey)

Comment 48

7 years ago
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?
(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.
Flags: needinfo?(khuey)

Comment 50

6 years ago
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

6 years ago
I have this bug since i'm using Thunderbird.
Is this problem so hard to resolve?
(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

6 years ago
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] (khuey@mozilla.com) 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.
Flags: needinfo?(squibblyflabbetydoo)
Whiteboard: [gs] → [gs][patchlove][has draft patch; tbd: comment 49]
(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)).
Flags: needinfo?(squibblyflabbetydoo)

Comment 55

6 years ago
(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

6 years ago
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.)
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.

Updated

6 years ago
See Also: → 634720

Comment 58

5 years ago
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.
(Assignee)

Comment 59

5 years ago
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.
(Assignee)

Comment 60

5 years ago
This second patch enables DnD of multiple attachments in an OS file folder from within Thunderbird on Windows and Mac OS X.
(Assignee)

Comment 61

5 years ago
This third patch enables DnD of multiple messages in an OS file folder from within Thunderbird on Windows and Mac OS X.
(Assignee)

Comment 62

5 years ago
This fourth patch enables DnD of multiple attachments in an OS file folder from within SeaMonkey on Windows and Mac OS X.
(Assignee)

Comment 63

5 years ago
This fifth patch enables DnD of multiple messages in an OS file folder from within SeaMonkey on Mac OS X.
(Assignee)

Comment 64

5 years ago
Attachment #8410692 - Attachment is obsolete: true
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.
Flags: needinfo?(squibblyflabbetydoo)
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 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.

Updated

5 years ago
Flags: needinfo?(squibblyflabbetydoo)
(Assignee)

Comment 68

5 years ago
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?
Attachment #8410686 - Attachment is obsolete: true
Attachment #8414754 - Flags: review?(squibblyflabbetydoo)
(Assignee)

Comment 69

5 years ago
This is a new version based on the review in Comment 67.
Attachment #8410689 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #8414758 - Flags: review?(squibblyflabbetydoo)
(Assignee)

Updated

5 years ago
Attachment #8410680 - Attachment description: Core fix (nsDataObjectCollection) → Part 1: Core fix (nsDataObjectCollection)
(Assignee)

Comment 70

5 years ago
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.
Attachment #8410691 - Attachment is obsolete: true
(Assignee)

Comment 71

5 years ago
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.
Attachment #8410695 - Attachment is obsolete: true
(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

5 years ago
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

5 years ago
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).

Updated

5 years ago
Whiteboard: [gs][patchlove][has draft patch; tbd: comment 49] → [gs]

Updated

5 years ago
Blocks: 634720

Comment 75

5 years ago
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.
No longer blocks: 634720
Flags: needinfo?(squibblyflabbetydoo)

Comment 76

5 years ago
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.
Blocks: 634720
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.
Flags: needinfo?(squibblyflabbetydoo)

Comment 78

5 years ago
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' (bugzilla-daemon@mozilla.org) - 2014-05-25 0014.eml
[Bug 270292] Unable to drag multiple attachments to OS file folder - 'Bugzilla@Mozilla' (bugzilla-daemon@mozilla.org) - 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 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!
Attachment #8410680 - Flags: review?(jmathies)
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';
Attachment #8414754 - Flags: review?(squibblyflabbetydoo) → review-
(Assignee)

Comment 81

5 years ago
(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

5 years ago
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.
(Assignee)

Comment 83

5 years ago
(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 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.
Attachment #8410680 - Flags: review?(jmathies) → review-
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 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?
Attachment #8414758 - Flags: review?(squibblyflabbetydoo)
Attachment #8414758 - Flags: review?(mkmelin+mozilla)
Attachment #8414758 - Flags: feedback+

Updated

5 years ago
Attachment #8414754 - Flags: review- → feedback+
(Assignee)

Comment 87

5 years ago
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.
Attachment #8410680 - Attachment is obsolete: true
Attachment #8479412 - Flags: review?(jmathies)
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.
Attachment #8479412 - Flags: review?(jmathies) → review+
Assignee: nobody → mr_sandman
Status: NEW → ASSIGNED
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
Attachment #8414758 - Flags: review?(mkmelin+mozilla)
Whiteboard: [gs] → [gs] [checkin needed: m-c - Part 1: Core fix (nsDataObjectCollection) v2]
(Assignee)

Comment 90

5 years ago
Fixed issues pointed out in Comment 89.
Attachment #8414758 - Attachment is obsolete: true
Attachment #8490765 - Flags: review?(bwinton)
(Assignee)

Updated

5 years ago
Attachment #8414754 - Flags: review?(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.
Attachment #8414754 - Flags: review?(bwinton) → review+
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?)
Attachment #8490765 - Flags: review?(bwinton) → review-
(Assignee)

Comment 93

5 years ago
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.
Attachment #8490765 - Attachment is obsolete: true
Attachment #8516699 - Flags: review?(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.  :)
Attachment #8516699 - Flags: review?(bwinton) → review-
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2369f6c3a35
Keywords: checkin-needed
Whiteboard: [gs] [checkin needed: m-c - Part 1: Core fix (nsDataObjectCollection) v2] → [gs]
Attachment #8479412 - Attachment description: Part 1: Core fix (nsDataObjectCollection) v2 → [checked in] Part 1: Core fix (nsDataObjectCollection) v2
(Assignee)

Comment 97

5 years ago
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.
Attachment #8516699 - Attachment is obsolete: true
Attachment #8526386 - Flags: review?(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?)
Attachment #8526386 - Flags: review?(bwinton) → review+

Comment 99

4 years ago
Magnus, can this land so that users can finally drag multiple attachments into OS file folders?
Flags: needinfo?(mkmelin+mozilla)

Updated

4 years ago
Summary: Unable to drag multiple attachments to OS file folder → Unable to drag and drop multiple attachments to OS file folder (e.g. Desktop)

Comment 100

4 years ago
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 :)
Refreshed part 3. Unfortunately, seems to still just drop one attachment (at least on windows).
Attachment #8526386 - Attachment is obsolete: true
Flags: needinfo?(mkmelin+mozilla)
(Assignee)

Comment 102

3 years ago
Please note that part 2 enables drag and drop of multiple attachments and part 3 enables drag and drop of multiple messages.
Flags: needinfo?(mkmelin+mozilla)
(Assignee)

Comment 104

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

Does not work. See fix in comment 103.
Attachment #8702665 - Attachment is obsolete: true
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!
Attachment #8702747 - Flags: review?(mkmelin+mozilla) → review+
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Flags: needinfo?(mkmelin+mozilla)
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 46.0
Attachment #8702747 - Attachment description: Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v6 → [checked in] Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v6
Attachment #8414754 - Attachment description: Part 2: DnD of multiple attachments in Thunderbird on Windows and Mac OS X v2 → [checked in] Part 2: DnD of multiple attachments in Thunderbird on Windows and Mac OS X v2
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.
Attachment #8702747 - Flags: approval-mozilla-beta?
Attachment #8702747 - Flags: approval-mozilla-aurora?
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!
Flags: needinfo?(mkmelin+mozilla)
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
Attachment #8702747 - Flags: approval-mozilla-beta?
Attachment #8702747 - Flags: approval-mozilla-aurora?
Attachment #8702747 - Flags: approval-comm-beta?
Attachment #8702747 - Flags: approval-comm-aurora?
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.
Attachment #8702747 - Flags: approval-comm-aurora? → approval-comm-aurora+
Yes indeed, wrong flag. The patch is for comm-cental
Flags: needinfo?(mkmelin+mozilla)
(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

3 years ago
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).

Updated

3 years ago
Attachment #8414754 - Attachment description: [checked in] Part 2: DnD of multiple attachments in Thunderbird on Windows and Mac OS X v2 → [checked in, comment 106, Aurora comment 112] Part 2: DnD of multiple attachments in Thunderbird on Windows and Mac OS X v2

Updated

3 years ago
Attachment #8702747 - Attachment description: [checked in] Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v6 → [checked in, comment 106, Aurora comment 112] Part 3: DnD of multiple messages in Thunderbird on Windows and Mac OS X v6

Updated

3 years ago
Attachment #8479412 - Attachment description: [checked in] Part 1: Core fix (nsDataObjectCollection) v2 → [checked into M-C, comment 96] Part 1: Core fix (nsDataObjectCollection) v2

Comment 115

3 years ago
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.
Attachment #8702747 - Flags: approval-comm-beta?
Blocks: 1268474

Updated

3 years ago
No longer blocks: 1268474
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.