458613 - event.relatedTarget is not set for dragexit/dragleave events?

Status: RESOLVED FIXED

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

Description

[Asaf Romano (gone)]

While testing and hacking tabbbrowser, I figured that event.realatedTarget is never set for the dragexit/dragleave events.

Comment 1

[Neil Deakin]

Attachment: check for drag events for relatedTarget and button properties

Asaf, does this work for you?

Comment 2

[Asaf Romano (gone)]

Nope, relatedTarget is still set to null.

Plus, and i don't know if that's related, effectAllowed for this event is unset.

Comment 3

[Neil Deakin]

Mano, do you have a testcase for relatedTarget?

Comment 4

[Asaf Romano (gone)]

I tested within the dragleave listener of tabbrowser. I don't have a standalone testcase, I could probably write one if that helps.

Note that tabbrowser won't rely on this api after my patch lands (I'm using coordinates checks instead for some other reasons). This used to work previously though, assuuming someone tested that code path.

Comment 5

[Martijn Wargers (dead)]

Attachment: simple testcase for dragend event handler

Comment 6

[Neil Deakin]

Martijn, what is this test checking? relatedTarget should only apply to dragleave and dragenter, and be null for all other drag events.

Comment 7

[Martijn Wargers (dead)]

Attachment: testcase

Sorry, I didn't know that.
I was confused. I was thinking that dragend needed a relatedTarget to fix the bugs that bug 225680 caused.

But apparently that can be done with dragleave too, perhaps, if I understand this code correctly:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.xml#2277

Comment 8

[Martijn Wargers (dead)]

The patch attached to this bug works fine for the testcase.

Comment 9

[Neil Deakin]

Comment on attachment 343418 [details] [diff] [review]
check for drag events for relatedTarget and button properties

OK, whether this fixes mano's issue or not, this is still a suitable patch for relatedTarget

Comment 10

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

Based on HTML 5 .relatedTarget should be null by default for drag events.
Of course if relatedTarget is explicitly set using .initDragEvent, then
.relatedTarget shouldn't be null.

Btw, seems like we have some old version of initDragEvent(...), not the one
HTML 5 specifies.

Comment 11

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

So as such the patch is ok, but ESM shouldn't set relatedTarget for drag events.

Comment 12

[Neil Deakin]

It seems that the spec changed very recently -- before drag events were just UIEvents so didn't have a relatedTarget. The relatedTarget should be updated for the dragenter/dragleave events to mirror the mouse ones (likely this is an oversight in the spec).

Comment 13

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

Neil, could you bring up this issue in whatwg?

Comment 14

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

Comment on attachment 343418 [details] [diff] [review]
check for drag events for relatedTarget and button properties

Anyway, I'd like to see the right kind of .initDragEvent()

Comment 15

[Neil Deakin]

(In reply to comment #14)
> (From update of attachment 343418 [details] [diff] [review])
> Anyway, I'd like to see the right kind of .initDragEvent()

What does that have to do with this bug?

Comment 16

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

With initDragEvent scripts can set .relatedTarget to script generated
events.

Comment 17

[Neil Deakin]

Attachment: something like this

Comment 18

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

Comment on attachment 352216 [details] [diff] [review]
something like this

You could add some comment to those commented-out tests. (I know, not about this bug)

Looks good, r=me.

Please ask Hixie/WhatWG about .relatedTarget on system generated dnd events.

Comment 19

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

Comment on attachment 352216 [details] [diff] [review]
something like this

Is this actually missing a change to nsDOMClassInfo.cpp where DragEvents should be marked to be also MouseEvents?

Comment 20

[Neil Deakin]

(In reply to comment #19)
> (From update of attachment 352216 [details] [diff] [review])
> Is this actually missing a change to nsDOMClassInfo.cpp where DragEvents should
> be marked to be also MouseEvents?

You asked that once before I think ;) DragEvent inherits from MouseEvent.

Comment 21

[Stephane]

testcase not working anymore on "Mozilla/5.0 (Windows NT 6.1; WOW64; rv:54.0) Gecko/20100101 Firefox/54.0", the relatedTarget is always null (but Chrome 60 now returns a valid relatedTarget)

Comment 22

[thedavidmeister]

i can see this is not working (always null) in 57.0.4 (64-bit) on mac OS X.

Makes it impossible to see if the dragleave event is just for a nested child or the whole parent.

Comment 23

[Neil Deakin]

That issue is bug 1420590 which is to be fixed in Firefox 59.

Comment 24

[thedavidmeister]

ok awesome thanks!

i've been looking into enter/leave counting such as explained in https://stackoverflow.com/questions/10253663/how-to-detect-the-dragleave-event-in-firefox-when-dragging-outside-the-window as a potential workaround in the meantime