1676094 - Can no longer drag and drop links as attachments [regression]

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Pierre Ossman]

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:82.0) Gecko/20100101 Firefox/82.0

Steps to reproduce:

1. Open new message
2. Drag link to .zip file from Firefox to attachment box

Actual results:

Attachment box was displayed, but nothing was added.

Expected results:

.zip file was added as an attachment.

Comment 1

[Pierre Ossman]

Seen with thunderbird-78.3.1-1.fc31.x86_64.

This is a regression. Previous version 68.11.0 worked just fine.

This is a major hindrance for the work flow here where we attach files from an internal web page regularly. A workaround would be much appreciated.

Comment 2

[Pierre Ossman]

Also note that using "Attach Web Page" still works, although it is not nearly as convenient and it also uses the entire URL as the filename and not just the final path component.

Comment 3

[gene smith]

Yes, works OK on 68.10.0 but broken on a daily 83.0a1. I don't have a 78 installed but you are probably right that it's broken there too. Confirming as a probable regression.

Comment 4

[Pierre Ossman]

Any insight in to what might have happened? Any better way to work around it?

Comment 5

[Khushil Mistry [:khushil324]]

Attachment: Bug-1676094_fix-DnD-attachments-Message-Compose-0.patch

Comment 6

[Magnus Melin [:mkmelin]]

Comment on attachment 9188068 [details] [diff] [review]
Bug-1676094_fix-DnD-attachments-Message-Compose-0.patch

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

::: mail/components/compose/content/messengercompose.xhtml
@@ +2140,4 @@
>               ondragover="envelopeDragObserver.onDragOver(event);"
>               ondrop="envelopeDragObserver.onDrop(event);"
>               ondragexit="envelopeDragObserver.onDragExit(event);">
> +      <hbox id="msgheaderstoolbar-box" flex="1" style="flex: 1;">

If the flex is needed it should go into the css file. Why are they needed for this?

Comment 7

[Khushil Mistry [:khushil324]]

It's showing strange behavior. Without flex:1 styling, drop event is not getting fired.

Comment 8

[Khushil Mistry [:khushil324]]

Attachment: Bug-1676094_fix-DnD-attachments-Message-Compose-1.patch

Comment 9

[Magnus Melin [:mkmelin]]

Comment on attachment 9188136 [details] [diff] [review]
Bug-1676094_fix-DnD-attachments-Message-Compose-1.patch

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

I go to https://www.learningcontainer.com/sample-zip-files/ in firefox, then try to drag a link to the attachments box in a compose window. Doesn't seem to do anything?

Comment 10

[Khushil Mistry [:khushil324]]

I am able to do it in the given link URL as well as Zip files. Can you try to drag and drop links/attachments from any other mail in the Thunderbird to the message compose window?

Comment 11

[gene smith]

For me I can drag/drop these OK with 68 but not with the patch applied to trunk:
https://file-examples.com/index.php/text-files-and-archives-download/
With patch applied, nothing appears in the attachment box when I drop a link.

Comment 12

[Magnus Melin [:mkmelin]]

Comment on attachment 9188136 [details] [diff] [review]
Bug-1676094_fix-DnD-attachments-Message-Compose-1.patch

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

I think there's some misunderstanding. 
What's desired is that one can drag a link into *the attachment bucket* to add it as an attachment. 

I don't see how this patch could do that. It's modifying the mail-recipients-area drop handling. But the attachmentBucket is outside of that the mail-recipients-area

Comment 13

[Pierre Ossman]

Is someone still looking at this? It's unfortunately a major nuisance here. :/

Comment 14

[Lasana Murray]

I don't understand exactly what's broken here. Are you trying to:

1. drag an actual link from the browser to the message compose window and have it show up as a "Web Page" attachment? The work around for that is the the menu Attach > Web Page.

2. or are you trying to drag a zip file downloaded via the browser and have it attached to a message?

Comment 15

[Pierre Ossman]

The first one. And Attach > Web Page is indeed a work around, albeit a cumbersome one. It's extra steps, primarily because it behaves differently with regards to naming. It includes the full URL, whilst dragging just keeps the basename.

Comment 16

[Lasana Murray]

Attachment: bug1683460.patch

This seems to be broken along the same reasons as bug 1683460. The mime expected for links is "x-moz-url" but they can come in as "text/uri-list" too.

The use of the link display as the attachment name depends on the source of the data transfer including it in the url data, separated from the actual url via a newline. This looks like a mozilla specific convention for "x-moz-url" and is unlikely to be available when receiving "text/uri-list".

Comment 17

[Lasana Murray]

A side effect of this patch is, the link gets included in the attachment list but it also gets added to the editor area. This wasn't a problem when the attachment area was confined to a small box. Anyway around that Alex, Magnus?

Comment 18

[Pierre Ossman]

(In reply to Lasana Murray from comment #16)

The use of the link display as the attachment name depends on the source of the data transfer including it in the url data, separated from the actual url via a newline. This looks like a mozilla specific convention for "x-moz-url" and is unlikely to be available when receiving "text/uri-list".

That's unfortunate. Could this be added as a feature request in that case? The full URL is likely not of much value to the recipient so a simple filename is very much preferred.

And I assume Firefox has stopped providing "x-moz-url" complete so it's not a format Thunderbird can prefer over "text/uri-list"?

Comment 19

[Alessandro Castellani [:aleca]]

Comment on attachment 9209333 [details] [diff] [review]
bug1683460.patch

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

You need to also extend this condition in order to make the new attachment overlay appear: https://searchfox.org/comm-central/rev/5428d2836831a284a682b6d1268f489ba1502e4c/mail/components/compose/content/MsgComposeCommands.js#8003-8007

Once you do that, the external file-link dragged will be handled correctly.

You can ping me directly on this for a review as I recently worked on this section, and also to remove a bit of workload from Magnus review queue :D

Comment 20

[Lasana Murray]

Attachment: bug1683460v2.patch

I changed it to display for anything in the flavours list.

Comment 21

[Lasana Murray]

Attachment: bug1683460v2.patch

On second thought, just the text/uri-list content type. Bringing up the overlay obscures drop addresses in the To fields etc.

Comment 22

[Alessandro Castellani [:aleca]]

Comment on attachment 9210051 [details] [diff] [review]
bug1683460v2.patch

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

Great work, this is a good and scalable solution.
Just a little typo in the comment and then it's good to go.

::: mail/components/compose/content/MsgComposeCommands.js
@@ +7622,5 @@
>    "text/x-moz-url",
>  ];
>  
> +// Content types that should the overlay should
> +let overlayFlavors = ["text/x-moz-message", "text/uri-list"];

Great solution!
Little nit comment typo.
Let's write something like: Array of content types that should trigger the attachment overlay.

Comment 23

[Lasana Murray]

Attachment: bug1676094.patch

Fix typo.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/c1c9a00072a5
Support text/uri-list in compose window drag and drop. r=aleca

Comment 25

[Pierre Ossman]

Nice, thanks!

Did this also include handling of the name? Or should I file a separate entry for this?

Comment 26

[Lasana Murray]

(In reply to Pierre Ossman from comment #25)

Nice, thanks!

Did this also include handling of the name? Or should I file a separate entry for this?

You can but I don't think there is much that could be done about the link name. The code to detect it is still there but the content type of the drop is entirely dependent on the source and you can probably expect the more standard text/uri-list than x-moz-url in most cases.

Comment 27

[Rob Lemley [:rjl]]

Comment on attachment 9210742 [details] [diff] [review]
bug1676094.patch

[Triage Comment]
Required dependency for bug 1699051.

Comment 28

[Rob Lemley [:rjl]]

Thunderbird 88.0b2:
https://hg.mozilla.org/releases/comm-beta/rev/aaa6bead19f8

Comment 29

[Pierre Ossman]

I've upgraded to Thunderbird 91.9.0, and I'm afraid the issue is still not resolved. :/

Can we get a reopen on this? Or should I file a new bug?

Comment 30

[Pierre Ossman]

Note that this regressed again in 91.3. Before that, it did indeed work. See this downstream report:

https://bugzilla.redhat.com/show_bug.cgi?id=2035271

Comment 31

[Alessandro Castellani [:aleca]]

This was completely reworked and fixed in 102, in bug 1766073.
We will release 102 next month, so there's no need for an uplift.

Comment 32

[Pierre Ossman]

Great! Thanks for the update.