455215 - [regression] drag and drop inside text input fields fails

Status: VERIFIED FIXED

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

Description

[philippe (part-time)]

STR:
1. type something in a textarea or inputfield
2. select a word
3. drag word to some other place in the text field

AR: nothing happens
ER: drop word to target area inside textfield

fails (Mac Builds)
Gecko/20080905020415 Minefield/3.1b1pre

works
Gecko/20080906041306 Minefield/3.1b1pre

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-09-05+02%3A04%3A05&enddate=2008-09-06+04%3A13%3A05

Not clear what caused this.

Comment 1

[philippe (part-time)]

Narrowing this down a little more:

works
Gecko/20080906021006 Minefield/3.1b1pre
Fails
Gecko/20080906041306 Minefield/3.1b1pre

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-09-06+02%3A10%3A06&enddate=2008-09-06+04%3A13%3A05

but it works again in
Gecko/20080907020345 Minefield/3.1b1pre
Gecko/20080908020603 Minefield/3.1b1pre
and fails
Gecko/20080909020334 Minefield/3.1b1pre

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-09-05+02%3A06%3A03&enddate=2008-09-09+02%3A03%3A34

And in the forums, it is claimed to be slightly older
http://forums.mozillazine.org/viewtopic.php?p=4466685#p4466685
(those dates would point to bug 356295)

Comment 2

[Alice0775 White]

See Bug 454843 in Window build.

Comment 3

[Cork]

The older range is as far as I tell Linux only and fits agents bug 455176.

Comment 4

[philippe (part-time)]

The 2008090803 SeaMonkey Mac build is not affected, but the 20080909002751 SM build fails.

Comment 5

[(mostly gone) XtC4UaLL [:xtc4uall]]

i *presume* (by looking at the range mentioned in comment 1) that this *could* be a fallout of fixing Bug 453520 and/or Bug 452787 -> CCing Neil for comment on this issue.

Comment 6

[Neil Deakin]

Attachment: check for elements inside an editor

This bug is caused because editors except to receive drag events yet aren't draggable.

Add a special case for content inside an editable element. One day, maybe we can rework the editor code to use the new API.

I'll have to see if I can come up with a test here.

Comment 7

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

Comment on attachment 338713 [details] [diff] [review]
check for elements inside an editor

>-      // Dispatch both the dragstart and draggesture events to the DOM
>+      // Dispatch both the dragstart and draggesture events to the DOM. For
>+      // elements in an editor, only fire the draggesture event so that the
>+      // editor code can handle it but content doesn't see a dragstart.
>       nsEventStatus status = nsEventStatus_eIgnore;
>-      nsEventDispatcher::Dispatch(targetContent, aPresContext, &startEvent, nsnull,
>-                                  &status);
>+      if (!isInEditor)
>+        nsEventDispatcher::Dispatch(targetContent, aPresContext, &startEvent, nsnull,
>+                                    &status);
Could you explain this? Why editor shouldn't see dragstart? Does it cancel the event?
If yes, why?

Comment 8

[Neil Deakin]

> Could you explain this? Why editor shouldn't see dragstart? Does it cancel the
> event?
> If yes, why?

HTML elements shouldn't receive drag events unless they are marked as draggable, as the spec specifies. So I don't want to fire dragstart on them. Instead I only allow the older draggesture event to fire so that editor code continues to receieve events (nsTextEditorDragListener::DragGesture is called in this case). When the editor code is updated to use the new API, we may be able to remove this special handling.

Comment 9

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

(In reply to comment #8)
> HTML elements shouldn't receive drag events unless they are marked as
> draggable, as the spec specifies.
Is that a bug in the spec?

Comment 10

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

According to HTML 5 Safari has some support for DnD. How does it handle form elements? What about contentEditable?

Comment 11

[Neil Deakin]

(In reply to comment #9)
> (In reply to comment #8)
> > HTML elements shouldn't receive drag events unless they are marked as
> > draggable, as the spec specifies.
> Is that a bug in the spec?

No, form elements themselves shouldn't be draggable. This patch is only made such that the existing editor dnd code is called, which uses the old api and works by adding a drag listener to the form element.

Safari's dnd support mostly mimics IE with a couple of minor additions. Neither implement the spec, but the spec was designed to be compatible. Safari supports dragging text in editable fields but does not fire the dragstart event.

Comment 12

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

Comment on attachment 338713 [details] [diff] [review]
check for elements inside an editor

Ok. I hope the dnd handling on form elements gets defined properly - eventually.

Comment 13

[Neil Deakin]

We could get clarification here. The spec does say something about firing the event for selections, but is vague about whether this applies to text selections.

Comment 14

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

What about dragging the selection inside contenteditable, does that work already?

Comment 15

[Neil Deakin]

(In reply to comment #14)
> What about dragging the selection inside contenteditable, does that work
> already?

Yes. The editor adds a listener which handles this. I think in the future we want to get rid of/combine this with the default browsing drag handler though.

Comment 16

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

Comment on attachment 338713 [details] [diff] [review]
check for elements inside an editor

Add a comment explaining how and why this code should be removed.

Comment 17

[philippe (part-time)]

verified with
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1pre) Gecko/20080926021000 Minefield/3.1b1pre

Comment 18

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

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/09aa56523a88