Last Comment Bug 357601 - Drag and drop from an external program to Firefox does not use the proper drop effect
: Drag and drop from an external program to Firefox does not use the proper dro...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Drag and Drop (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: mozilla9
Assigned To: Brian R. Bondy [:bbondy]
:
Mentors:
Depends on: 673080
Blocks: 676464 633160
  Show dependency treegraph
 
Reported: 2006-10-22 10:49 PDT by Biju
Modified: 2011-09-01 13:53 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix for effects when dragging from an external program into Firefox (4.17 KB, patch)
2011-08-03 22:09 PDT, Brian R. Bondy [:bbondy]
no flags Details | Diff | Review
Fix for effects when dragging from an external program into Firefox (4.12 KB, patch)
2011-08-03 22:14 PDT, Brian R. Bondy [:bbondy]
neil: review-
Details | Diff | Review
Fix for effects when dragging from an external program into Firefox (11.77 KB, patch)
2011-08-04 06:47 PDT, Brian R. Bondy [:bbondy]
neil: review+
Details | Diff | Review
Fix for effects when dragging from an external program into Firefox w/ latest review comments (9.48 KB, patch)
2011-08-11 19:51 PDT, Brian R. Bondy [:bbondy]
neil: review+
Details | Diff | Review

Description Biju 2006-10-22 10:49:15 PDT
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 2009-10-14 09:19:49 PDT
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 2010-01-21 13:33:10 PST
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] 2011-08-03 19:21:58 PDT
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] 2011-08-03 22:05:37 PDT
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] 2011-08-03 22:09:27 PDT
Created attachment 550602 [details] [diff] [review]
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] 2011-08-03 22:14:48 PDT
Created attachment 550604 [details] [diff] [review]
Fix for effects when dragging from an external program into Firefox

Forgot to qrefresh, attached new patch.
Comment 7 neil@parkwaycc.co.uk 2011-08-04 02:56:31 PDT
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] 2011-08-04 05:39:20 PDT
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] 2011-08-04 06:47:50 PDT
Created attachment 550677 [details] [diff] [review]
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 neil@parkwaycc.co.uk 2011-08-06 09:34:51 PDT
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] 2011-08-08 06:08:41 PDT
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 neil@parkwaycc.co.uk 2011-08-11 08:51:54 PDT
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] 2011-08-11 19:51:02 PDT
Created attachment 552579 [details] [diff] [review]
Fix for effects when dragging from an external program into Firefox w/ latest review comments
Comment 14 Brian R. Bondy [:bbondy] 2011-08-31 13:38:15 PDT
This was pushed to try, results in progress:
http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=cde44aaad760
Comment 15 Brian R. Bondy [:bbondy] 2011-09-01 07:21:52 PDT
Pushed to mozilla-inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/2c6b5f33faa6
Comment 16 Ed Morley [:emorley] 2011-09-01 13:53:39 PDT
http://hg.mozilla.org/mozilla-central/rev/2c6b5f33faa6

Note You need to log in before you can comment on or make changes to this bug.