1244533 - [e10s] Screen coordinates on the "dragend" event stopped working with e10s

Status: RESOLVED FIXED

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

Description

[Thibaut Courouble]

Attachment: test.html

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160127070712

Steps to reproduce:

Create an element with the draggable attribute set to true. Listen to the dragstart and dragend events.


Actual results:

On the dragend event, the event.screenX and event.screenY values are always zero.


Expected results:

event.screenX/Y should report the cursor coordinates. The test file works on FF 43 (can't test 44).

Note that there is a larger issue here (https://bugzilla.mozilla.org/show_bug.cgi?id=505521), which is that pageX/clientX/screenX don't work on drag events. "screenX" on dragend is the only one that used to work. With this bug it's now impossible to implement any kind of UI drag feature (like resizing) in Firefox using the Drag & Drop API.

A fix would be much appreciated.

Thanks.

Comment 1

[Alice0775 White]

could you attach results of about:support?

Comment 2

[Thibaut Courouble]

Attachment: about:support 45.0b1 OS X

Comment 3

[Thibaut Courouble]

Attachment: about:support 45.0a2 Win

Comment 4

[Alice0775 White]

I can reproduce the problem with e10s.

Comment 5

[Thibaut Courouble]

Right, didn't notice the A/B test. Good to know that this won't break when FF45 is released. Thanks!

Comment 6

[Neil Deakin]

> With this bug it's now impossible to implement any kind of UI drag feature (like resizing) in Firefox using the Drag & Drop API.

As an aside, resizing a UI element isn't something you would use the drag and drop API for.

Comment 7

[Thibaut Courouble]

(In reply to Neil Deakin from comment #6)
> As an aside, resizing a UI element isn't something you would use the drag
> and drop API for.

Why not? It works perfectly in all browsers that implement the DND API, except Firefox.

The only alternative I'm aware of is to use mousedown/mousemove/mouseleave, which requires more code. The spec even says:

> On a visual medium with a pointing device, a drag operation could be the default action of a mousedown event that is followed by a series of mousemove events, and the drop could be triggered by the mouse being released.

http://www.w3.org/html/wg/drafts/html/master/editing.html#dnd

PS: honestly don't mean to complain. I love the work you're doing, especially with e10s.

Comment 8

[Neil Deakin]

The dragend coordinate is set in the parent process by calling nsIDragService::SetDragEndPoint. nsBaseDragService::FireDragEventAtSource then assigns it to event.refPoint just before firing the event. This will need to get transferred over when propagating the dragend event to the child process. Maybe smaug can indicate the best way to do this?

Comment 9

[Jim Mathies [:jimm]]

waited to long, kickingt his back into triage.

Comment 10

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

Sorry, I process needinfos in general once I have time between review requests. And I've been behind the review queue for weeks now.

Comment 11

[Jim Mathies [:jimm]]

Niel, do you think this is something that should block e10s?

Comment 12

[Neil Deakin]

It regressed functionality and one cannot receive event coordinates at the source when the drop is complete, so it would be good to fix it.

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Hey Blake - I _think_ it sounds like we need to just send the mouse coordinates down to the content along with the message that dispatches the dragend event. Does that sound right? (smaug is swamped). If so, where does that message get sent, do you know?

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix

In investigating this bug, I ended up writing the fix.

Comment 15

[Neil Deakin]

I get build errors with this patch due to a caller of SendEndDragSession in nsBaseDragService.cpp not being updated.

Comment 16

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix v1.1

Sorry, I forgot to add that directory to the patch.

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/937ab6cd1b48

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/937ab6cd1b48

Comment 19

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8754446 [details] [diff] [review]
Fix v1.1

Approval Request Comment
[Feature/regressing bug #]: n/a
[User impact if declined]: Certain web sites won't work with e10s.
[Describe test coverage new/current, TreeHerder]: Not good test coverage.
[Risks and why]: Low risk, adds existing information to an existing event, using existing IPC channels.
[String/UUID change made/needed]: n/a

Comment 20

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8754446 [details] [diff] [review]
Fix v1.1

Improve e10s, taking it

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5f53adc1bdfa