Loic Reporter
	Description • 9 years ago

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

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 2 • 9 years ago

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+.

	Neil Deakin Assignee
	Comment 3 • 9 years ago

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

	Neil Deakin Assignee
	Comment 4 • 9 years ago

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.

	Kate Glazko
	Comment 6 • 9 years ago

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

	Sylvestre Ledru [:Sylvestre]
	Comment 7 • 9 years ago

Niel, what are your plans about this bug? Thanks

	Neil Deakin Assignee
	Comment 8 • 9 years ago

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.

	Neil Deakin Assignee
	Comment 9 • 9 years ago

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.

	Neil Deakin Assignee
	Comment 10 • 9 years ago

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.

	:Paolo Amadini
	Comment 11 • 9 years ago

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.

	Neil Deakin Assignee
	Comment 12 • 9 years ago

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

	:Paolo Amadini
	Comment 13 • 9 years ago

(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.

	Neil Deakin Assignee
	Comment 14 • 9 years ago

See comment 9.

	Sylvestre Ledru [:Sylvestre]
	Comment 15 • 9 years ago

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.

	Neil Deakin Assignee
	Comment 16 • 9 years ago

> 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.

	Neil Deakin Assignee
	Comment 17 • 9 years ago

This should be fixed now.

	:Paolo Amadini
	Comment 18 • 9 years ago

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.