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

RESOLVED FIXED in mozilla9

Status

()

Core
Drag and Drop
RESOLVED FIXED
11 years ago
6 years ago

People

(Reporter: Biju, Assigned: bbondy)

Tracking

(Blocks: 1 bug)

Trunk
mozilla9
x86
Windows XP
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

11 years ago
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.

Updated

11 years ago
Summary: drag and drop convention → Drag and drop behavior should differ if Ctrl or Shift is held (copy vs. cut)
QA Contact: drag-drop

Comment 1

8 years ago
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

8 years ago
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)

Updated

6 years ago
Assignee: nobody → netzen
(Assignee)

Comment 3

6 years ago
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
(Assignee)

Updated

6 years ago
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
(Assignee)

Updated

6 years ago
Blocks: 676464
(Assignee)

Updated

6 years ago
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
(Assignee)

Comment 4

6 years ago
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.
(Assignee)

Comment 5

6 years ago
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.
Attachment #550602 - Flags: review?(neil)
(Assignee)

Comment 6

6 years ago
Created attachment 550604 [details] [diff] [review]
Fix for effects when dragging from an external program into Firefox

Forgot to qrefresh, attached new patch.
Attachment #550602 - Attachment is obsolete: true
Attachment #550604 - Flags: review?(neil)
Attachment #550602 - Flags: review?(neil)

Comment 7

6 years ago
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-
(Assignee)

Comment 8

6 years ago
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.
(Assignee)

Comment 9

6 years ago
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.
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?
(Assignee)

Comment 11

6 years ago
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);
(Assignee)

Updated

6 years ago
Blocks: 633160
(Assignee)

Updated

6 years ago
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+
(Assignee)

Comment 13

6 years ago
Created attachment 552579 [details] [diff] [review]
Fix for effects when dragging from an external program into Firefox w/ latest review comments
Attachment #550677 - Attachment is obsolete: true
Attachment #552579 - Flags: review?(neil)

Updated

6 years ago
Attachment #552579 - Flags: review?(neil) → review+
(Assignee)

Comment 14

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

Comment 15

6 years ago
Pushed to mozilla-inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/2c6b5f33faa6
http://hg.mozilla.org/mozilla-central/rev/2c6b5f33faa6
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.