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

Steps to reproduce:

1. Create a new message
2. Drag URL from browser (e.g. Firefox) URL bar to message compose window

Actual results:

3. The URL is attached to the message

Expected results:

4. The URL should be inserted as text (or, preferably, as a hyperlink) at the cursor position in the compose window.

• In previous TB versions (at least up to v78), a dragged URL was inserted as a hyperlink in the new message.

• The change of behaviour in TB91 may be related to https://bugzilla.mozilla.org/show_bug.cgi?id=1640760 ?

• In https://bugzilla.mozilla.org/show_bug.cgi?id=1625263#c18 , the type of file being dragged is being taken into consideration.
  This works for e.g. dragged images (where the option is presented to either attach or insert inline), but does not apply to dragged URL's.

• Note : this issue only concerns dragging from the URL bar ; dragging an URL from a webpage correctly inserts a hyperlink in the new message.

The end result of URL dragged into composition - an attachment which isn't pretty!

Confirming exactly as described on Win10, 91.1.2 (64-bit). And the outcome is neither pretty nor useful (see screenshot above).
So I'd guess we should just restore the previous behaviour that URLs should be added as a link when dragged into composition.
Alex, would this be hard to fix?

Screen shot showing process and result.
The 'Insert Inline' option does not appear when drag and drop hyperlink.

Found the issue! The overlay appears due to the text/x-moz-url data type, which we are using to see if we need to trigger the attachment overlay or not for when an attachment is dragged from a compose window (or message pane) to another.
This data type is also present when dragging a URL from the Firefox address bar.

We can fix this by checking for the text/x-moz-url-data which is only present for attachments and not URL texts.

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/3709ede8642c
Prevent the attachment overlay from showing if a text URL is dragged above the compose window. r=mkmelin

Comment on attachment 9243577 [details]
Bug 1732903 - Prevent the attachment overlay from showing if a text URL is dragged above the compose window. r=mkmelin

[Approval Request Comment]
Regression caused by (bug #): bug 1695641
User impact if declined: Text urls dragged from an address bar above the compose editor are handled like attachment instead of simple plain text/links.
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): low

Comment on attachment 9243577 [details]
Bug 1732903 - Prevent the attachment overlay from showing if a text URL is dragged above the compose window. r=mkmelin

[Triage Comment]
Approved for beta

This bug introduces a couple of annoying regressions.
Let's hold off on uplifting to beta.

Comment on attachment 9243577 [details]
Bug 1732903 - Prevent the attachment overlay from showing if a text URL is dragged above the compose window. r=mkmelin

[Triage Comment]
minus, per comment 11

Comment on attachment 9243577 [details]
Bug 1732903 - Prevent the attachment overlay from showing if a text URL is dragged above the compose window. r=mkmelin

[Triage Comment]
not yet on beta,so clearing flag

Re-requesting approval for comm-beta for this (I can't change the flag) now that bug 1735459 fixed the few regressions introduced by it.
Both this patch and the one in bug 1735459 should be uplifted together for both beta and 91.

Comment on attachment 9243577 [details]
Bug 1732903 - Prevent the attachment overlay from showing if a text URL is dragged above the compose window. r=mkmelin

[Triage Comment]
Approved for beta

Rob, "Both this patch and the one in bug 1735459 should be uplifted together for both beta and 91."

Thunderbird 94.0b4:
https://hg.mozilla.org/releases/comm-beta/rev/6479a1830ad1

Comment on attachment 9243577 [details]
Bug 1732903 - Prevent the attachment overlay from showing if a text URL is dragged above the compose window. r=mkmelin

[Triage Comment]
approved for esr91

Could you prepare a patch for 91.3 please?

Thunderbird 91.3.0:
https://hg.mozilla.org/releases/comm-esr91/rev/529d4c5962b5

Comment 19 should not have been pushed.... backout:
https://hg.mozilla.org/releases/comm-esr91/rev/6c08fcfc4ff80f3bf547ccc4a2f9174ec5ea9a57

This is the 91 variation.

The patches in bug 1735459 and bug 1737386 need to be applied at the same time as this one and they all apply cleanly on top of each other on 91.
Pinging Rob to be sure he sees this.

Thunderbird 91.3.0:
https://hg.mozilla.org/releases/comm-esr91/rev/03b24d910d7e

Not working in W10 91.3.2 or 95b3: drag and drop of links from browser are still added as attachments.

Wonderful, this issue doesn't reproduce on Linux, but it does on Windows and macOS.

This code is the culprit: https://searchfox.org/comm-central/rev/1471a7836a2abc68c373e51b739407ce151d84fd/mail/components/compose/content/MsgComposeCommands.js#8259-8270

On Linux, the dataList is empty, instead on Windows and macOS, the dataList contains the text/uri-list flavor.

These are the data types on Linux when dragging a URL:
["application/x-moz-file", "text/html", "text/x-moz-url", "text/plain", "Files"]

Instead on Windows and macOS, the data types are:
["text/html", "text/x-moz-url", "text/uri-list", "text/plain"]

Why is this happening now all of a sudden? And why the platform inconsistencies?

Also, why are we allowing attaching URLs on the first place?
https://searchfox.org/comm-central/rev/1471a7836a2abc68c373e51b739407ce151d84fd/mail/components/compose/content/MsgComposeCommands.js#8308-8328

Is this a feature we want to keep that makes sense for some specific cases?

In TB 78, dragging a URL to the body inserts a link, dragging it to the header area/attachment bucket adds the content of the web page as a HTML attachment. A further issue is that in TB 78 the name/filename of the attachment are the websites title, in TB 91 they are the URL. In general, unplanned changes to established behavior are considered a regression. Removing a feature just because it broke doesn't appear to be a good reason IMHO. As for the "pretty" argument (comment #2): That depends on the webpage that is attached.

Thanks newsfan for the info.

Personally, I think that dragging a webpage URL to attach it as HTML page is very very strange and unusual, but if some of our users are used to this feature and find it useful, we should keep it.

So, I guess the approach here is to show both containers (Add as attachment, add inline) for when a text/x-moz-url is dragged.

If someone wants the content of a URL attached, they should use copy paste.
I think if they drag a url only a link should be inserted.

That is, for web pages. IIRC the real issue though is that some people may want to drag a pdf link (e.g. from intranet ), and then attach the pdf. Which seems like a fairly valid request. (But let's not mix in web pages, which is probably about never what anyone wants to do.)

I'm not sure where this leaves us.

This is indeed tricky.
I'll see if I can detect if the dragged link points to a real file and not just a page and handle that.

Maybe if there's a file extension we can return both "Add as attachment" and "Add inline" overlay so the user can decide. But if it's just a link that doesn't point to any file, not show the attachment overlay at all.

I will also create some tests for this scenario to be sure we're completely covered.

After investigating further and testing various scenarios, I'd like to propose the removal of this feature, which is the ability to attach a dragged URL as a file.

Rationale

• We can assume it's more common to wanting to attach a link when dragging a URL, than attaching the file that link points to.
• Attaching a file directly from a link should be discouraged as users should first download the file they want to attach to be sure it's safe and correct.
• We have issues with URL pointing to images.

Images issue
If users drag a URL that points to an image, we would have 3 potential scenarios the users might want to interact with:

• Add the image as attachment.
• Append the image inline in the editor.
• Add the URL of the image as text in the editor.

These 3 scenarios are not currently supported and we would need to increase the complexity of the code to detect if the dragged test is a link that points to an attachable file, not a simple webpage, and if the file is an image, and return a different overlay.

Therefore, I propose the removal of this feature and only handle links as links.
Thoughts?

Yeah I'm onboard with dropping support for that.

Hmmm. This was working before, and it's broken now, so it's a regression. We have established that this was regressed by Bug 1695641. So I think it should be possible to restore this useful feature? I agree with newsfan's comment 28:

Removing a feature just because it broke doesn't appear to be a good reason IMHO.

(In reply to Magnus Melin [:mkmelin] from comment #30)

That is, for web pages. IIRC the real issue though is that some people may want to drag a pdf link (e.g. from intranet ), and then attach the pdf. Which seems like a fairly valid request. (But let's not mix in web pages, which is probably about never what anyone wants to do.)

Given that even Magnus was considering this a "fairly valid request", it's certainly worth trying harder.
It's not just about PDFs, it could be other files from location bar or link URLs, images, zip files, office documents.
I imagine that for e.g. the intranet case mentioned by Magnus, it makes a lot of sense wanting to attach the actual file to the email, because it won't be available from the link outside the intranet. Otoh, having to save each file before attaching is a significant hassle.

(In reply to Alessandro Castellani [:aleca] from comment #32)

After investigating further and testing various scenarios, I'd like to propose the removal of this feature, which is the ability to attach a dragged URL as a file.

Rationale

• We can assume it's more common to wanting to attach a link when dragging a URL, than attaching the file that link points to.

Generally maybe (although we do not have any data), but that won't help the enterprise-intranet scenario where attaching PDFs from intranet URLs might be the order of the day.

• Attaching a file directly from a link should be discouraged as users should first download the file they want to attach to be sure it's safe and correct.

Attaching a file directly from a link would look extremely convenient, and we should strive to be as convenient as possible - that's the only reason to use Thunderbird. Safety and correctness is out of scope, that's user's responsibility. Instead, we should default to getting a snapshot copy of anything attached at the time of attaching. Plus, for the case reported here (location bar URL), the file would typically already be displayed. Forcing users into downloading a file is much more cumbersome compared to attaching directly from the link. Not feasible for enterprise use.

• We have issues with URL pointing to images.

That would look like a minor ux-implementation-level issue.

Images issue
If users drag a URL that points to an image, we would have 3 potential scenarios the users might want to interact with:

• Add the image as attachment.
• Append the image inline in the editor.
• Add the URL of the image as text in the editor.

These 3 scenarios are not currently supported and we would need to increase the complexity of the code to detect if the dragged test is a link that points to an attachable file, not a simple webpage, and if the file is an image, and return a different overlay.

Hmm, not exactly. We already support 2 out of 3 scenarios for image URLs, only the "insert as link" is missing.
We don't need to special case HTML documents, nobody has complained about that (and it's likely less used).
If we do want to special case HTML documents, I don't think detecting HTML documents could be all that hard?
If I use Ctrl+S from Firefox to save an HTML file, image file, pdf file, each of which displayed in browser from an URL, Firefox always suggests the correct extension even if the URL doesn't have the file extension. So it would look like FF has a way of determining the type.
We're already special-casing images, so adding a 3rd drop target (inline-text) for images from URL should be possible.

Adjusting summary to describe the regression correctly.

Somehow this bug has stalled. In TB 78, only text/x-moz-url was treated here
https://searchfox.org/comm-esr78/rev/ba95b27c546ce348adadd41ce57d2a944df198ad/mail/components/compose/content/MsgComposeCommands.js#7773
to allow for the insertion of a link target content as attachment. text/uri-list wasn't treated at all:
https://searchfox.org/comm-esr78/search?q=text%2Furi-list&path=&case=false&regexp=false

If insertion of a link target content as attachment is to be removed as per the consent in comment #33, the treatment of both flavours can just be removed. This fixes dragging a link onto a compose window and it also fixes dragging an image from a received message (bug 1753331). This patch addresses both issues:
https://github.com/Betterbird/thunderbird-patches/blob/main/91/bugs/1753331-fix-drag-image-from-message.patch

Simple solution that also fixes bug 1753331:

• Links dragged onto the compose window get inserted as links.
• Attachments of other messages get added as attachment.
• Dragging an embedded image from another message to a composition embeds it again.
  Looks like that covers all the cases. We've been deploying this for a few months now.

Since this bug has gone through the release uplift process once already, I suggest closing this bug and moving the recently added patches into a new bug.

(In reply to Alessandro Castellani [:aleca] from comment #39)

Thanks for the input and the contribution, but this removes the
functionality of allowing users to upload a dragged link as an attachment.

Indeed, this appeared to be the consensus that was finally summarised in comment #33. In any case, if your solution is more sophisticated, please ensure that these three cases are covered:

• Links dragged onto the compose window get inserted as links (or attachments, if you want to keep supporting that)
• Attachments of other messages get added as attachment, including images (for bonus points, allow both inline and attachment for images)
• Dragging an embedded image from another message to a composition embeds it again. Bug 1753331. This can't be added as attachment since the image is missing the filename in most cases.

Also, using if (Services.io.getProtocolHandler(scheme) instanceof Ci.nsIMsgMessageFetchPartService) { is the way to go to test for parts from other messages, see prior art here.

(In reply to Rob Lemley [:rjl] from comment #40)

Since this bug has gone through the release uplift process once already, I suggest closing this bug and moving the recently added patches into a new bug.

Sounds good, Rob. Thanks for the heads up. I'll do that and Cc everyone that is part of this bug.

(In reply to Rachel Martin from comment #41)

Indeed, this appeared to be the consensus that was finally summarised in comment #33.

True, that was the original consensus, but Thomas raised good points and if we can keep this dualism as a usable feature, I think we should try.

In any case, if your solution is more sophisticated, please ensure that these three cases are covered:

"Sophisticated" is a bit of a stretch :P

• Links dragged onto the compose window get inserted as links (or attachments, if you want to keep supporting that)
• Attachments of other messages get added as attachment, including images (for bonus points, allow both inline and attachment for images)
• Dragging an embedded image from another message to a composition embeds it again. Bug 1753331. This can't be added as attachment since the image is missing the filename in most cases.

These are exactly the scenarios I'm trying to handle, thanks for the list.
Indeed, Bug 1753331 should be potentially solved by my patch, maybe we could move things there.

Also, using if (Services.io.getProtocolHandler(scheme) instanceof Ci.nsIMsgMessageFetchPartService) { is the way to go to test for parts from other messages, see prior art here.

Great, thanks for the info!

Comment on attachment 9273385 [details] [diff] [review]
1753331-fix-drag-image-from-message.patch

We're on favour of maintaining the original functionality of attaching the the target of the dragged link on request. The patch implemented the "other" consensus of dropping that function.

Closing this in favor of bug 1766073.

Comment on attachment 9273379 [details]
Bug 1732903 - Enable proper handling of attachable URLs dragged on the composer. r=mkmelin

Revision D144389 was moved to bug 1766073. Setting attachment 9273379 [details] to obsolete.