466379 - Need mouse-pointer coordinates in the dragend event

Status: VERIFIED FIXED

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

Description

[Asaf Romano (gone)]

In order to make the tab-to-window code a bit more encapsulated and much less buggy (esp. in order to implement behaviors as described in bug 465332 comment 9), I need screenX and screenY data for the dragend event, currently they're set to 0. I suppose a side effect might be negative clientX and clientY data when the drag ends outside the window, is that OK?

Comment 1

[Neil Deakin]

Screen coordinates of what? The dragend doesn't occur at a specific point, it just fires on an element.

Comment 2

[Asaf Romano (gone)]

Wherever the mouse was released (as with dragover).

Comment 3

[Asaf Romano (gone)]

Neil: did you see comment 2?

Comment 4

[Neil Deakin]

This is fairly simple for drops on the same window (assuming that the drop event coordinates are correct).

For drops on a different window, this is a bit harder due to the possibility of the coordinates/scales being different.

I can't answer how feasible this is for drops on other applications. It depends on whether the OS provides this information.

Whether an element should be able to receive coordinates outside its window is another question.

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Neil: we're looking for the screen co-ordinates from the operating system, I think, so that we can position the new window from a tear-off-tab.

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Neil: ping?

Comment 7

[Asaf Romano (gone)]

I don't need this immediately anymore (but it could certainly help a little, thus I'm keeping the bug open).

Comment 8

[Neil Deakin]

OK, I have a simple implementation on Mac, but it doesn't look to be possible to get the final coordinate on Linux or Windows when dropping on another application. I thought I had seen a means on Windows, but can't see it anymore. On Windows, at least, a hack might be to just get the current mouse position. Maybe Jim might know more.

Comment 9

[Jim Mathies [:jimm]]

(In reply to comment #8)
> OK, I have a simple implementation on Mac, but it doesn't look to be possible
> to get the final coordinate on Linux or Windows when dropping on another
> application. I thought I had seen a means on Windows, but can't see it anymore.
> On Windows, at least, a hack might be to just get the current mouse position.
> Maybe Jim might know more.

After we drop out of DoDragDrop we can get the cursor position in screen coordinates using GetCursorPos. There's some async code in there I'd have to look at to see where it comes into play. In typical drops though I don't believe it has any effect.

Comment 10

[Neil Deakin]

Attachment: implement this for windows and mac

Can't see a means to get the drag end position on Linux, but this works on Windows and Mac (although on Mac, the upper-left corner of the image is used rather than the mouse pointer position).

Changes the drag service api a bit, although not functions anyone should be calling.

Comment 11

[Neil Deakin]

Mano, does this patch work in some way for you?

Comment 12

[Asaf Romano (gone)]

I didn't test this yet, but this is the sort of change that I cannot rely on at all if it's not implemented on all supported platforms.

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Vlad: can we reconsider blocking here, as this really makes tab-drag-and-drop a lot easier for Mano to execute on, which is a blocker.

Comment 14

[Neil Deakin]

Attachment: simpler fix which doesn't change any interfaces

Comment 15

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

Why can't we get the current pointer position on X and use that just like we do for Windows?

Comment 16

[Neil Deakin]

Attachment: linux patch

Comment 17

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

Comment on attachment 372433 [details] [diff] [review]
linux patch

+    if (GDK_IS_DISPLAY(display)) {

I don't think we need this.

One thing I just noticed in the original patch that we should fix: mEndDragPoint and SetDragEndPoint should be nsIntPoint, since they're pixels.

Comment 18

[Mike Beltzner [:beltzner, not reading bugmail]]

This blocks a P1 blocker, and so is a P1 blocker. Is the patch safe for branch, or does it change interfaces?

Comment 19

[Mike Beltzner [:beltzner, not reading bugmail]]

Enn: can we get this landed on trunk and branch today, please?

Comment 20

[Neil Deakin]

Only the Linux patch (https://bugzilla.mozilla.org/attachment.cgi?id=372433) isn't checked in.

Comment 21

[Neil Deakin]

The linux patch was now checked into trunk (http://hg.mozilla.org/mozilla-central/rev/77a7c0eed364).

Comment 22

[Mike Beltzner [:beltzner, not reading bugmail]]

I see the windows+mac part going into 191 here:

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c3ab13ffbcd8

but no 191 changeset for the linux part yet. Is this really fixed1.9.1?

Comment 23

[Neil Deakin]

I haven't checked that in to 1.9.1 yet. The linux part probably should have been a different bug or have some indicator on the first patch that means 'fixed1.9.1'.

Comment 24

[Asaf Romano (gone)]

Neil, what blocks landing this on the branch?

Comment 25

[Asaf Romano (gone)]

Neil, please see https://bugzilla.mozilla.org/show_bug.cgi?id=475066#c69

Comment 26

[Neil Deakin]

I've now checked in the linux fix.

Comment 27

[Joe Drew (not getting mail)]

This seems to have caused a Txul regression; see http://groups.google.com/group/mozilla.dev.tree-management/browse_thread/thread/34dbe106fac69152#

Comment 28

[Daniel Holbert [:dholbert]]

The 1.9.1 checkin for this bug (which purportedly caused the Txul regression) was http://hg.mozilla.org/releases/mozilla-1.9.1/rev/970bee033ed0

Looks like the only changes in that cset that could affect WinXP are the s/nsPoint/nsIntPoint/ changes.... that doesn't seem particularly significant, but perhaps our nsPoint code was slightly more efficient or something?

Comment 29

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

No chance.

Comment 30

[Neil Deakin]

The patches here only run when dragging which cannot be done without human intervention, so the performance tests shouldn't be exercising this code.

Comment 31

[Nochum Sossonko [:Natch]]

Attachment: test file for verification.

Open this file and drag the div at the top of the page, you should get an alert telling you where the drag ended.

Comment 32

[Nochum Sossonko [:Natch]]

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090702 Minefield/3.6a1pre
and
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5

V. Fixed

Comment 33

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Nochum, those bugs should be verified on all platforms not only windows. I did this for OS X and Linux with your testcase. It looks good. Verified fixed too.