707859 - sometimes fires mousemove event after dragstart

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

This is actual cause of bug 706743. GTK widget sometimes fires mousemove event during drag.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm, my analysis is wrong. The unexpected mousemove events are caused by synthesized events but I'm not sure why it comes on gtk2 build remarkably.

Comment 2

[Neil Deakin]

Drag and drop is asynchronous on linux, so you're probably seeing the mousemove event that caused the drag. Now that tooltips are using system events, the mousemove still progagates to them. The fix here is probably to stop propagation entirely when a drag begins.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Enn:

When I researched bug 706743, I saw the native event dispatcher of nsWindow in the callstack. However, now, I cannot see it. But I can see synthesized mousemove events. I'll post a patch for it after bug 706743.

Anyway, bug 706743 is a regression and lower risky patch. I hope that we should take only it for Mozilla 11 due to the end of the cycle will end 12/20. I want to fix this bug in 12's cycle. Would you review the patch in bug 706743?

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch

Strangely, I don't see any unexpected mousemove events during drag with this patch. I'm not sure why I couldn't see same bug on non-Linux platforms. I guess that there are some difference in the timer implementation between Linux and the others.

I have no idea to test this patch. I think that we have no API to call the PresShell's SynthesizeMouseMove() directly.

Comment 5

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

Comment on attachment 597035 [details] [diff] [review]
Patch


> PresShell::ProcessSynthMouseMoveEvent(bool aFromScroll)
> {
>+  // If drag sesstion has started, we shouldn't synthesize mousemove event.
sesstion?

>+  nsCOMPtr<nsIDragService> dragService =
>+    do_GetService("@mozilla.org/widget/dragservice;1");
>+  if (dragService) {
>+    nsCOMPtr<nsIDragSession> dragSession;
>+    dragService->GetCurrentSession(getter_AddRefs(dragSession));
>+    if (dragSession) {
>+      return;
>+    }
>+  }

Use nsContentUtils::GetDragSession();
nsCOMPtr<nsIDragSession> dragSession = nsContentUtils::GetDragSession();
if (dragSession) {
  return;
}

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/63861267f6a7

Thanks!

Comment 7

[Timothy Nikkel (:tnikkel)]

Comment on attachment 597035 [details] [diff] [review]
Patch

So just returning in ProcessSynthMouseMoveEvent will keep mSynthMouseMoveEvent as a refresh driver observer and it will get notified again on the next refresh driver tick. Is that what we want here?

Comment 8

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

Ah, removing refresh driver observer could be then useful.
Shouldn't be a major problem though.

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/63861267f6a7

Comment 10

[Timothy Nikkel (:tnikkel)]

It depends, do we want to delay the synth mouse move until the drag is over, or cancel it?

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

we should cancel it, if it's needed, it should be fired by the latest mouse cursor position at drag canceled. I'll post a follow up patch. Thank you for your pointing out.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: follow up patch

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ad54332581f8

Comment 14

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/ad54332581f8