459323 - Drag and Drop in input controls is broken

Status: VERIFIED FIXED

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

Description

[Robert Stopp (textrinum)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9.0.3) Gecko/2008092417 Firefox/3.0.3
Build Identifier: 

Drag and drop in input controls in a current nightly results in unloading the current page and trying to resolve the the dropped fragment against DNS and search engines.
It seems that every input control is handled like the adressbar.
The behaviour exist max. two weeks, before this change drag and drop was complete broken.

Reproducible: Always

Comment 1

[Elmar Ludwig]

Confirming on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b2pre) Gecko/20081007 Minefield/3.1b2pre ID:20081007173602.

Comment 3

[IU]

Works: http://hg.mozilla.org/mozilla-central/rev/1630d60e624e
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20081006
Minefield/3.1b1pre

Broken: http://hg.mozilla.org/mozilla-central/rev/55366228f456
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20081007
Minefield/3.1b1pre

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Seems most likely to be bug 458070 or bug 455311, does it not?

Comment 5

[Boris Zbarsky [:bzbarsky]]

There's no way bug 455311 causes this, especially not on Linux: it's a Windows-only change.  Bug 458070 certainly could, though!

Comment 6

[Elmar Ludwig]

I just verified that backing out the patch for bug 458070 fixes this bug as well as bug 459334, so Boris is right.

Comment 7

[Robert Stopp (textrinum)]

(In reply to comment #6)
> I just verified that backing out the patch for bug 458070 fixes this bug as
> well as bug 459334, so Boris is right.

Elmar: Is it possible to drag a selection from inside an textarea/input?


By working on this: Imo it would be better if a selection in the adressbar is dragged to another place in the adressbar (even with Ctrl-key pressed), the original value + new location of the dropped fragment would be the result.

Comment 8

[Elmar Ludwig]

(In reply to comment #7)
> Elmar: Is it possible to drag a selection from inside an textarea/input?

Yes.

> By working on this: Imo it would be better if a selection in the adressbar is
> dragged to another place in the adressbar (even with Ctrl-key pressed), the
> original value + new location of the dropped fragment would be the result.

Feel free to file an enhancement request for this (if there isn't already one).

Comment 9

[Asaf Romano (gone)]

Attachment: Remove the usage of the dragdrop event and nsIDOMDragListener interface

I'm handling the places code in other bugs.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 342734 [details] [diff] [review]
Remove the usage of the dragdrop event and nsIDOMDragListener interface

> nsContentAreaDragDrop::HookupTo(nsIDOMEventTarget *inAttachPoint,
>                                 nsIWebNavigation* inNavigator)
> {
>+  printf("\n\n\n\n\nIn HookupTo %d %d\n\n\n", inAttachPoint != 0, inNavigator != 0);
Remove this.

> NS_IMETHODIMP
> nsContentAreaDragDrop::HandleEvent(nsIDOMEvent *event)
> {
Make sure that event is really a drag event.
So you need to QI to nsIDOMDragEvent

>+  nsresult DragOver(nsIDOMEvent* aMouseEvent);
>+  nsresult Drop(nsIDOMEvent* aMouseEvent);
...then you could change these to take nsIDOMDragEvent as a parameter, if needed.

> nsTextEditorDragListener::HandleEvent(nsIDOMEvent* aEvent)
> {
Same here, you need to check that aEvent is nsIDOMDragEvent

> nsresult
> nsPluginInstanceOwner::HandleEvent(nsIDOMEvent* aEvent)
And also here. And actually, because all the drag events
get handled the same way, you don't need to compare eventType if
aEvent implements nsIDOMDragEvent

>-  gToolbox.addEventListener("draggesture", onToolbarDragGesture, false);
>+  gToolbox.addEventListener("dragstart", onToolbarDragStart, false);
>   gToolbox.addEventListener("dragover", onToolbarDragOver, false);
>-  gToolbox.addEventListener("dragexit", onToolbarDragExit, false);
>-  gToolbox.addEventListener("dragdrop", onToolbarDragDrop, false);
>+  gToolbox.addEventListener("dragleave", onToolbarDragLeave, false);
>+  gToolbox.addEventListener("drop", onToolbarDrop, false);
Here you add listener for dragstart

> function removeToolboxListeners()
> {
>-  gToolbox.removeEventListener("draggesture", onToolbarDragGesture, false);
>+  gToolbox.removeEventListener("draggesture", onToolbarDragStart, false);
>   gToolbox.removeEventListener("dragover", onToolbarDragOver, false);
>-  gToolbox.removeEventListener("dragexit", onToolbarDragExit, false);
>-  gToolbox.removeEventListener("dragdrop", onToolbarDragDrop, false);
>+  gToolbox.removeEventListener("dragexit", onToolbarDragLeave, false);
>+  gToolbox.removeEventListener("dragdrop", onToolbarDrop, false);
...but don't remove it here.

Those fixed and mochitest or browser tests added, r=me

Comment 11

[Elmar Ludwig]

(In reply to comment #10)
> Here you add listener for dragstart
> [...]
> ...but don't remove it here.

Same for "dragleave" and "drop", BTW.

Comment 12

[Asaf Romano (gone)]

Attachment: patch

Comment 13

[Boris Zbarsky [:bzbarsky]]

Er... if you feel the need to wrap aURI, why not aWhat and aValue?

Comment 14

[Asaf Romano (gone)]

Sorry, the changes to feedwriter.js belong to bug 360529.

Comment 15

[Asaf Romano (gone)]

Er, bug 454363.

Comment 16

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 343057 [details] [diff] [review]
patch

Yay!! We should get rid of more interfaces like this. (AddEventListenerByIID needs to die IMHO)

Comment 17

[Asaf Romano (gone)]

I'm going to land this ASAP, but without tests. I couldn't figure out how to test this in mochitest. I tried to do what test_dragstart does (note it doesn't test full d&d either). Simply dispatching mousedown+mousemove on the drag-able element and mousemove on the target element didn't generate dragover events. I'm pretty sure I'm also running here into a 100% reproducible case for bug 455871 since D&D seems to get completely broken when running this test, don't ask me how.

Comment 20

[zengbl]

I reported Bug 460559, then I found it is marked as a duplicate of this bug. So
I post another case which probably belongs here too.

Environment:
Firefox 3.1b1, Windows xp

Summary:
Drag and drop behavior is different. It depends on the selecting method .

Steps:
1. Get a page with a textarea, e.g. Bug 459323 (just this page)
2a. 
  2a1. Hold left button and move the cursor through to select some text
  2a2. Drop the selection in the textarea, e.g. Additional Comments box in the
bottom of the page
  Result of 2a: The selection is copied to the drop point

2b. 
  2b1. Double/triple click to select some text
  2b2. Drop the selection in the textarea
  Result of 2b: The current page is refreshed to a new URL

Other infomation:
In the case 2a and 2b, it doesn't matter whether the selected text is from
outside or inside the textarea.

Comment 21

[Asaf Romano (gone)]

I checked this in yesterday.

Comment 22

[Robert Stopp (textrinum)]

The problem with the unloading page is fixed but now there is no insert caret while dragging - this happens on all drag and drop operations and even in designmode, drag and drop from inside an input control is not possible on Windows.

Comment 23

[Robert Stopp (textrinum)]

I had the problem with the invisible insert caret some time before. This was caused by the "drag" - event on a designmode document.

Comment 24

[Asaf Romano (gone)]

Marking FIXED again, don't reopen this bug unless you can tell for sure that the describe behavior regressed from the same patch. If the caret issue is a result of this bug, file a bug blocking it, or otherwise just file a bug.

Comment 25

[IU]

This bug is not really fixed -- only partially.

I can drag-and-drop into a text filed, but I can neither: (1) drag-and-drop within a text area nor (2) drag-and-drop from a text area.

Comment 26

[Robert Stopp (textrinum)]

(In reply to comment #24)
> Marking FIXED again, don't reopen this bug unless you can tell for sure that
> the describe behavior regressed from the same patch. If the caret issue is a
> result of this bug, file a bug blocking it, or otherwise just file a bug.

Can you drag a selection from inside an input control?

Comment 27

[Elmar Ludwig]

(In reply to comment #25)
> This bug is not really fixed -- only partially.

Yes, I see this as well. I also noticed what Robert mentioned: the caret is
missing while dragging over a text area or input field.

I just tested the last tinderbox build from before the checkin of this patch
and the first one containing this patch and can confirm that all three problems
were caused by this checkin.

Comment 28

[Elmar Ludwig]

Filed bug 460595 and bug 460596 on the regressions mentioned by IU and Robert.

Comment 29

[Asaf Romano (gone)]

I checked in a fix for some typos i did, which probably caused this bustage: http://hg.mozilla.org/mozilla-central/rev/2090e8775575

Let me know if things are still buggy in the next build.

Comment 30

[Elmar Ludwig]

This fixes the caret problems for me, but dragging a text selection from an input control still does not work in a current build.

Comment 31

[:Gavin Sharp [email: [e-mail]]]

This landed on 1.9.1:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/4e0ce3d295c2

Comment 32

[Tony Chung [:tchung]]

(In reply to comment #25)
> This bug is not really fixed -- only partially.
> 
> I can drag-and-drop into a text filed, but I can neither: (1) drag-and-drop
> within a text area nor (2) drag-and-drop from a text area.

Using STR here, verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5;
en-US; rv:1.9.2a1pre) Gecko/20090122 Minefield/3.2a1pre 
and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre)
Gecko/20090122 Shiretoko/3.1b3pre

Comment 33

[Jose Fandos]

Is this fixed in branch? From comment #32 it looks as it is, but testing with current branch I can drag this same text to the search box just sitting below and it doesn't get moved, just copied. So it's not fixed, unless I'm misunderstanding what the bug is for.

Comment 34

[Boris Zbarsky [:bzbarsky]]

Jose, this bug is described in comment 0.  If you drag and drop and the page doesn't unload, this bug is fixed.