If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Handle drag and drop of a download link

ASSIGNED
Unassigned

Status

()

Core
DOM: Core & HTML
ASSIGNED
5 years ago
3 years ago

People

(Reporter: evilpie, Unassigned)

Tracking

(Depends on: 1 bug, {dev-doc-needed})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
When you drag a link we the download attribute onto the desktop, we want to download this instead of just creating a link to the file. Bug 570164 is quite similar.
(Reporter)

Updated

5 years ago
Depends on: 570164, 676619
(Reporter)

Updated

5 years ago
No longer depends on: 676619
(Reporter)

Updated

5 years ago
Depends on: 676619
(Reporter)

Comment 1

5 years ago
I think this might actually be pretty easy. We already support something called x-moz-file-promise. And we even have documentation for it :)
https://developer.mozilla.org/en-US/docs/DragDrop/Recommended_Drag_Types#Dragging_files_to_an_operating_system_folder
(Reporter)

Comment 2

5 years ago
You can drag and drop files in Firefox on Linux, which makes this kind of painful to test and implement for me. But this should be relatively easy.
(Reporter)

Comment 3

5 years ago
*You can't

Comment 4

5 years ago
Well, at least image dnd from file system works on linux.

Comment 5

5 years ago
Only text and urls are supported for dropping on gtk currently.
(Reporter)

Comment 6

4 years ago
Created attachment 813271 [details] [diff] [review]
WIP patch

Now that I got drag and drop working on linux, I could actually implement this. I still have to iron out some of the details, but this works.
Assignee: nobody → evilpies
Status: NEW → ASSIGNED
Attachment #813271 - Flags: review?(bzbarsky)
Comment on attachment 813271 [details] [diff] [review]
WIP patch

>+  static void GetDownloadLink(nsIContent *aContent, nsIURI *aLinkURI,

Clearly bogus return value.  And needs to document aLinkURI, no?

And maybe call it IsDownloadLink?

>+++ b/content/base/src/nsContentAreaDragDrop.cpp

Someone familiar with this code should review it, please.

>+++ b/content/base/src/nsContentUtils.cpp
>+  // Check if there is an download attribute.

s/an/a/

>+  if (!aContent->GetAttr(kNameSpaceID_None, nsGkAtoms::download, aFileName))

Curlies around if body, please.

>+++ b/widget/gtk/nsDragService.cpp

Changes here are ok, but not obviously related...
Attachment #813271 - Flags: review?(neil)
Attachment #813271 - Flags: review?(bzbarsky)
Attachment #813271 - Flags: review+
(Reporter)

Updated

4 years ago
Keywords: dev-doc-needed
(Reporter)

Comment 8

4 years ago
Arguably having to pass the aLinkURI is a pretty bad API, but I am not sure how abstract that between the two cases. Maybe we should always use href?

Comment 9

4 years ago
Comment on attachment 813271 [details] [diff] [review]
WIP patch

>+      if (!nsContentUtils::GetDownloadLink(aDragNode, hrefURI, fileName)) {
>+        if (!mImage)
>+          return NS_OK;
This is very ugly, and possibly prone to error if someone needs to tweak the logic.
I tried to think of several approaches, but you probably don't want to accidentally download an image linked to a download that doesn't specify a file name, in which case I ended up with this approach:
if (mIsAnchor && nsContentUtils::GetDownloadLink(...)) {
  // this is a download, so promise to download it
} else if (mImage) {
  // promise the image
}
if (!fileName.IsEmpty()) {
  // we found something to download
}
Note that you don't now need the outer if (mImage || mIsAnchor) test.

>+      if (fileName.IsEmpty()) {
>+        nsAutoCString fileNameC;
>+        imgUrl->GetFileName(fileNameC);
Which URL is this? I don't see a relevant one handy. You might want the file from mUrlString, but I suppose you'd have to use string manipulation to extract it.

>diff --git a/widget/gtk/nsDragService.cpp b/widget/gtk/nsDragService.cpp
[Unnecessary change?]
(Reporter)

Updated

4 years ago
Assignee: evilpies → nobody

Comment 10

3 years ago
Could we not just duplicate what was used in Chromium to finish this?
You need to log in before you can comment on or make changes to this bug.