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] (Mozilla Japan)
:
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] (Mozilla Japan)
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] (Mozilla Japan)
bugs: review+
Details | Diff | Splinter Review
follow up patch (891 bytes, patch)
2012-02-22 23:18 PST, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Splinter Review

Description Masayuki Nakano [:masayuki] (Mozilla Japan) 2011-12-05 18:38:50 PST
This is actual cause of bug 706743. GTK widget sometimes fires mousemove event during drag.
Comment 1 Masayuki Nakano [:masayuki] (Mozilla Japan) 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 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 Masayuki Nakano [:masayuki] (Mozilla Japan) 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 Masayuki Nakano [:masayuki] (Mozilla Japan) 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 Olli Pettay [:smaug] (vacation Aug 25-28) 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 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-02-20 22:31:35 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/63861267f6a7

Thanks!
Comment 7 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 Olli Pettay [:smaug] (vacation Aug 25-28) 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 Ed Morley [:emorley] 2012-02-22 04:45:25 PST
https://hg.mozilla.org/mozilla-central/rev/63861267f6a7
Comment 10 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 Masayuki Nakano [:masayuki] (Mozilla Japan) 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 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-02-22 23:18:03 PST
Created attachment 599893 [details] [diff] [review]
follow up patch
Comment 13 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-02-23 20:42:23 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/ad54332581f8
Comment 14 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.