Last Comment Bug 707859 - sometimes fires mousemove event after dragstart
: sometimes fires mousemove event after dragstart
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Event Handling (show other bugs)
: Trunk
: x86_64 Linux
-- normal (vote)
: mozilla13
Assigned To: Masayuki Nakano [:masayuki]
:
: Andrew Overholt [:overholt]
Mentors:
data:text/html,<style>%23p:hover { ba...
Depends on:
Blocks: 706743
  Show dependency treegraph
 
Reported: 2011-12-05 18:38 PST by Masayuki Nakano [:masayuki]
Modified: 2012-02-24 02:48 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (1.15 KB, patch)
2012-02-14 08:01 PST, Masayuki Nakano [:masayuki]
bugs: review+
Details | Diff | Splinter Review
follow up patch (891 bytes, patch)
2012-02-22 23:18 PST, Masayuki Nakano [:masayuki]
bugs: review+
Details | Diff | Splinter Review

Description User image Masayuki Nakano [:masayuki] 2011-12-05 18:38:50 PST
This is actual cause of bug 706743. GTK widget sometimes fires mousemove event during drag.
Comment 1 User image Masayuki Nakano [:masayuki] 2011-12-05 19:21:23 PST
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 User image Neil Deakin 2011-12-07 12:29:59 PST
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 User image Masayuki Nakano [:masayuki] 2011-12-08 22:51:15 PST
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 User image Masayuki Nakano [:masayuki] 2012-02-14 08:01:51 PST
Created attachment 597035 [details] [diff] [review]
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 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2012-02-20 11:26:08 PST
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 User image Masayuki Nakano [:masayuki] 2012-02-20 22:31:35 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/63861267f6a7

Thanks!
Comment 7 User image Timothy Nikkel (:tnikkel) 2012-02-21 18:42:54 PST
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 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2012-02-22 04:36:15 PST
Ah, removing refresh driver observer could be then useful.
Shouldn't be a major problem though.
Comment 9 User image Ed Morley [:emorley] 2012-02-22 04:45:25 PST
https://hg.mozilla.org/mozilla-central/rev/63861267f6a7
Comment 10 User image Timothy Nikkel (:tnikkel) 2012-02-22 15:01:23 PST
It depends, do we want to delay the synth mouse move until the drag is over, or cancel it?
Comment 11 User image Masayuki Nakano [:masayuki] 2012-02-22 18:16:33 PST
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 User image Masayuki Nakano [:masayuki] 2012-02-22 23:18:03 PST
Created attachment 599893 [details] [diff] [review]
follow up patch
Comment 13 User image Masayuki Nakano [:masayuki] 2012-02-23 20:42:23 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/ad54332581f8
Comment 14 User image Marco Bonardo [::mak] 2012-02-24 02:48:08 PST
https://hg.mozilla.org/mozilla-central/rev/ad54332581f8

Note You need to log in before you can comment on or make changes to this bug.