357601 - Drag and drop from an external program to Firefox does not use the proper drop effect

Status: RESOLVED FIXED

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

Description

[Biju]

drag and drop convention

On MS-Win 3.1/95/98/2000/XP while you drag and drop 
* if you press "shift" key it act as "cut and paste"
* if you press "control" key it act as "copy and paste"

You can test this by dragging files in folders of WinExplorer 
or dragging text in Wordpad.

But on firefox while doing dragging text 
if you press "shift" key it removes the "+" 
from drag/drop mouse pointer, but not do a cut on source text.

Comment 1

[Phil Lacy]

The cursor is wrong then, correct?

Maybe we can use this bug to fix cursors to provide correct feedback.

If copy only is allowed or link only then the cursor should reflect that feedback to user regardless of modifier key, similar to windows other apps or IE.

drop targets should have their effectAllowed used as feedback.

Currently we don't use the link cursor either. Is that by design?

Maybe enn or gavin can confirm this is what we need.

Comment 2

[Ñull]

Using Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.6) Gecko/20091201 SUSE/3.5.6-3.1 Firefox/3.5.6 here. I have since some versions ago related problem. 

Only the cut and paste (shift drag) functionality of selected text is working on the present version, while the copy and paste functionality was lost somewhere. At ctrl drage the mouse pointer changes to symbol with a plus sign, but the text is not copied, it is moved (cut-paste). 

To reproduce the bug:
- Select a text in a textarea field
- hold the ctrl key while you drag the select text to another place in the text
- the mouse pointer will have a plus symbol
- releas the ctrl key.
- the text will now have been deleted from the former location and inserted at the new location, the same behaviour as shift drag.

Comment 3

[Brian R. Bondy [:bbondy]]

Confirmed this is how it should work in Windows:

1. Shift should always do a move operation (except for point 4)
2. Ctrl should always do a copy operation (except for point 4)
3. Ctrl + Shift should always do a link operation (except for point 4)
4. If one of the modifier keys is used, but that operation is not allowed, it should fall back to the allowed operation.
5. Preference should be done in this order: move, copy, link

Comment 4

[Brian R. Bondy [:bbondy]]

I created a separate task for dropping from Firefox to an eternal program and within Firefox since the task / fix is completely different from this task.

Bug 676464 - Drop behavior within Firefox and from Firefox into an external program does not respect drop effect.

Comment 5

[Brian R. Bondy [:bbondy]]

Attachment: Fix for effects when dragging from an external program into Firefox

The problem was that the drop effect should specify only the effect to use and not all available effects like a drag does.   

I think the problem for the sister bug is either inside nsDragService or a bit lower down the call stack into the non widget code.  Will investigate that in its bug ID.

Comment 6

[Brian R. Bondy [:bbondy]]

Attachment: Fix for effects when dragging from an external program into Firefox

Forgot to qrefresh, attached new patch.

Comment 7

[[e-mail]]

Comment on attachment 550604 [details] [diff] [review]
Fix for effects when dragging from an external program into Firefox

>   ProcessDrag(NS_DRAGDROP_DROP, grfKeyState, aPT, pdwEffect);
> 
>+  // Only a single effect should be specified on drop for the parameter pdwEffect.
I don't think this is the correct place, especially given bug 673080...

Comment 8

[Brian R. Bondy [:bbondy]]

Bug 673080 is still working with this patch but after looking into it again I agree that it is not the best place for the fix.  I'll fix and resubmit.

Comment 9

[Brian R. Bondy [:bbondy]]

Attachment: Fix for effects when dragging from an external program into Firefox

Review comments and additionally cleaned other logic and potential problems with wrong handling.

Comment 10

[[e-mail]]

OK, so on the whole the patch looks reasonable, but can you please explain why you need to change ProcessDrag?

Comment 11

[Brian R. Bondy [:bbondy]]

We call ProcessDrag ourselves inside the drop event so this comment does not hold true for drops:

> Now get the cached Drag effect from the drag service the data memeber should have been set by who ever handled the nsGUIEvent or nsIDOMEvent

So we were in error setting the effect to none on drops `effectively` clearing the work we did in GetGeckoDragAction:

> if (!canDrop)
> *pdwEffect = DROPEFFECT_NONE;

GetCanDrop was returning false because on previous calls during drag we would always set it back to false to clear the cached value.

> // Clear the cached value
> currSession->SetCanDrop(PR_FALSE);

Comment 12

[[e-mail]]

Comment on attachment 550677 [details] [diff] [review]
Fix for effects when dragging from an external program into Firefox

Bah, I was confused because this patch depends on the latest attachment to bug 673080 (which is not even in the dependency list) but I had an earlier one.

> void
> nsNativeDragTarget::ProcessDrag(PRUint32     aEventType,
>                                 DWORD        grfKeyState,
>                                 POINTL       ptl,
>-                                DWORD*       pdwEffect)
>+                                DWORD*       pdwEffect,
>+                                PRBool       aFromDrop)
I don't think you need an extra aFromDrop parameter.
Instead, just check if (aEventType != NS_DRAGDROP_DROP)
r=me with that fixed.

Comment 13

[Brian R. Bondy [:bbondy]]

Attachment: Fix for effects when dragging from an external program into Firefox w/ latest review comments

Comment 14

[Brian R. Bondy [:bbondy]]

This was pushed to try, results in progress:
http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=cde44aaad760

Comment 15

[Brian R. Bondy [:bbondy]]

Pushed to mozilla-inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/2c6b5f33faa6

Comment 16

[Ed Morley [:emorley]]

http://hg.mozilla.org/mozilla-central/rev/2c6b5f33faa6