1207634 - Drag & drop a downloaded file from the Download Panel to the tab bar doesn't open a tab

Status: RESOLVED FIXED

(Core :: DOM: Copy & Paste and Drag & Drop, defect)

Description

[Loic]

BOTH non-e10s and e10s mode are affected.

STR:
1) Download a file that could be read directly in Firefox (.jpg, .pdf e.g.)
2) Click on the down arrow to open the Download Panel
3) Drag&drop the downloaded file into the tab bar to open it

Result: drop fails and no tab is open with the downloaded file.

Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=44986f66ee4b590f1a5d7781828477f06ba400b6&tochange=8c81e97e0604b64de8fcd3c84a7a13274bfab0f2

Suspected bug:
Neil Deakin — Bug 1121946, Implement e10 cursor drag feedback on Windows, r=jmathies

Comment 2

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

This is a recent regression. I can confirm that dragging+dropping files from download panel into tab bar works in FF40 but not FF41. However, since there are workarounds like double-click to open or open using File menu -> "Open File", I do not believe this is so critical to fix in a dot release. This is a wontfix for 41.

Tracked for 42+.

Comment 3

[Neil Deakin]

Attachment: Possible patch

The issue here is that the download panel starts a drag only allowing 'copy' and 'move' actions, but the tabbrowser only allows 'link' drops.

Comment 4

[Neil Deakin]

This is one possible fix, to just allow any type for drags. I haven't tested it though as something is wrong with my Windows builds. Some are being created with this patch at https://treeherder.mozilla.org/#/jobs?repo=try&revision=694ff474abf6

Note that bug 1207594 is a similar and related bug, but would not be fixed by this patch. There, I am guessing that the data being dragged is actually plaintext that happens to be a url. Windows doesn't allow data that isn't a link on targets that only accept links.

An alternate patch that would allow the tabstrip to accept 'copy' could fix both bugs. I will investigate.

Comment 6

[Kate Glazko]

Setting affected flag to match duplicate bug, and 44 is affected. Setting tracking for 44 consistency.

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Niel, what are your plans about this bug? Thanks

Comment 8

[Neil Deakin]

Attachment: Better patch

This patch instead makes the tabbrowser use the 'link' drop effect only if there is a link present. It also removes setting the effectAllowed which doesn't do anything (setting effectAllowed is only relevant to the dragstart event)

An alternate is to have this change (change 'link' to 'move' or 'copy' if no link is being dragged) done within the Windows drag code. I am unable to test this on Windows right now, but may be able to in a couple of days.

Comment 9

[Neil Deakin]

For beta, we can just back out bug 1121946, as it implements a feature which hasn't ever worked.

For others, I have several possible fixes and am trying to decide which one is better.

Comment 10

[Neil Deakin]

Comment on attachment 8666091 [details] [diff] [review]
Possible patch

After much thought I don't see why downloads don't allow the 'link' type. This is also the least risky fix despite not fixing bug 1207594.

Comment 11

[:Paolo Amadini]

Comment on attachment 8666091 [details] [diff] [review]
Possible patch

I'd like to test this locally first, but will not be able before next week.

Flagging Marco as well, in case he wants to get to this review first.

Comment 12

[Neil Deakin]

The patch in bug 1207594 fixes both bugs but is a more involved fix.

Comment 13

[:Paolo Amadini]

(In reply to Neil Deakin from comment #10)
> After much thought I don't see why downloads don't allow the 'link' type.

One consequence may be that dragging a file to the Desktop on Windows might create a link by default instead of actually moving the file from the folder where it was downloaded? Just thinking aloud here.

Comment 14

[Neil Deakin]

See comment 9.

Comment 15

[Sylvestre Ledru [:Sylvestre]]

As we are super late in the cycle (next build is RC), I think we will release 42 with this bug (like we did with 41). Bug 1121946 landed 4 months ago, not sure it is safe to backout it now.

Comment 16

[Neil Deakin]

> One consequence may be that dragging a file to the Desktop on Windows might
> create a link by default instead of actually moving the file from the folder
> where it was downloaded? Just thinking aloud here.

It should only do that if you press the modifier for a link.

Regardless since we don't need this for beta anymore, I think we can just go with the patch in 1207594.

Comment 17

[Neil Deakin]

This should be fixed now.

Comment 18

[:Paolo Amadini]

Comment on attachment 8666091 [details] [diff] [review]
Possible patch

I've tried this locally. Now that the other bug is fixed, this change doesn't make any difference even in the interactions with Windows Explorer, so clearing the review request.