Closed Bug 357601 Opened 16 years ago Closed 11 years ago

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

Categories

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

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla9

People

(Reporter: BijuMailList, Assigned: bbondy)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 3 obsolete files)

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.
Summary: drag and drop convention → Drag and drop behavior should differ if Ctrl or Shift is held (copy vs. cut)
QA Contact: drag-drop
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.
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.
Assignee: nobody → netzen
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
Summary: Drag and drop behavior should differ if Ctrl or Shift is held (copy vs. cut) → Drag and drop from Firefox to an external program does not use the proper drop effect
Blocks: 676464
Summary: Drag and drop from Firefox to an external program does not use the proper drop effect → Drag and drop from an external program to Firefox does not use the proper drop effect
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.
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.
Attachment #550602 - Flags: review?(neil)
Forgot to qrefresh, attached new patch.
Attachment #550602 - Attachment is obsolete: true
Attachment #550604 - Flags: review?(neil)
Attachment #550602 - Flags: review?(neil)
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...
Attachment #550604 - Flags: review?(neil) → review-
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.
Review comments and additionally cleaned other logic and potential problems with wrong handling.
Attachment #550604 - Attachment is obsolete: true
Attachment #550677 - Flags: review?(neil)
OK, so on the whole the patch looks reasonable, but can you please explain why you need to change ProcessDrag?
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);
Blocks: 633160
Depends on: 673080
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.
Attachment #550677 - Flags: review?(neil) → review+
Attachment #552579 - Flags: review?(neil) → review+
This was pushed to try, results in progress:
http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=cde44aaad760
http://hg.mozilla.org/mozilla-central/rev/2c6b5f33faa6
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.