Often with ^C/Gdb attach I get spurious mouse events for 30 seconds. I can resolve this quickly this by copy pasting garbage into my console however this has been bugging me for over a year now. I spoke with Jeff and he found in bug 611068 that disabling mouse hooking solve this problem. Mouse hooking was made to work around bug 368077 and bug 339945. I tried removing this and it made the spurious mouse events problem go away and I could still not reproduce bug 368077 and bug 339945. Steven, do you think it's safe to remove this? (CC'ed people whom I've discussed this issue with)

> Steven, do you think it's safe to remove this? I've been meaning to deal with this for a while, but it kept falling through the cracks. I'll take a look today or tomorrow.

I already had a patch under way when I saw your comment. It fix the problem described above and I can't reproduce the bugs that this patch was meant to fix (on 10.6).

(forgot qref) I can post a try build if you'd like. I tried to reproduce the bugs by: 1) Open youtube.com (lots of link) 2) Focus unrelated secondary window (Instruments in my case) 3) Right click a link, notice that pop up appears and mouse over event work, unrelated window retains focus. 4) Selecting menu appears performs the action while the window retains focus. Everything works as expected.

Sorry for bugspam :(

By the way, Benoit, what OS versions did you test on? I'll need to test on all the ones we support in current code (10.5, 10.6 and 10.7).

I tested on 10.6, it was buried in my comment above. If we agree above removing the code I also can test on 10.5-7. Should I start a try build?

Testing comes before removing code :-) Go ahead and start a tryserver build, if you like. I'm already doing my own build (which should be finished a lot quicker).

(In reply to Steven Michaud from comment #7) > Testing comes before removing code :-) > > Go ahead and start a tryserver build, if you like. I'm already doing my own > build (which should be finished a lot quicker). I'll wait on your results. Take your time, it's low priority so no need to drop other important tasks.

As it happens my other work is mostly blocked, so I currently do have the time.

Comment on attachment 571760 [details] [diff] [review] patch v2 (more removal) Benoit, I've already noticed one fatal problem with your patch, testing only on 10.6: If, while FF is in the background, you right-click on its browser window, a context menu correctly comes up without FF having been made the foreground app. But that context menu should disappear if you left-click anywhere outside of FF (on the Desktop or in another app). That doesn't happen. Tomorrow I'll spend more time testing, and see if I find any more problems. I'll also try to find some other way than using event taps to fix the problem I found. I personally hate event taps -- they've been buggy ever since we started using them. But we had good reasons to use them.

(In reply to Steven Michaud from comment #10) > But that context menu should disappear if you left-click anywhere > outside of FF (on the Desktop or in another app). That doesn't happen. As far as I know, this is the one and only scenario that we need the event tap for. If we don't want to break it, we could probably fix this bug by changing things in such a way that the event tap is only active while (gRollupWidget && gRollupListener && ![NSApp isActive]). When this condition is not fulfilled, we'll take the early exit in EventTapCallback anyway. In other words, we could set up the event tap when a context menu is opened (while ![NSApp isActive]) and destroy it when it closes. gRollupListener and gRollupWidget are only modified inside nsCocoaWindow::CaptureRollupEvents, so this should actually be pretty simple. Benoit, do you want to do this? By the way, this should be done on top of the patch in bug 696745 in order to avoid conflicts.

Sure, it may have to wait until next week.

Benoit, if you're going to continue working on this you should test using a Carbon event monitor (installing a Carbon event handler on what's returned by GetEventMonitorTarget()). Apparently we switched from that to using event taps because of a problem in OS X 10.4. But (of course) we no longer need to support OS X 10.4. See http://developer.apple.com/legacy/mac/library/#documentation/Carbon/Reference/Carbon_Event_Manager_Ref/Reference/reference.html. There's also a way to use Cocoa to monitor events (http://developer.apple.com/library/mac/#documentation/Cocoa/Conceptual/EventOverview/MonitoringEvents/MonitoringEvents.html), but that's only available on OS X 10.6 and up. As best I can tell from the docs Carbon event monitors still work, and do support 64-bit code. It's true that the entire Carbon Event Manager is deprecated (is considered "legacy"), and its methods may disappear at some point (or more likely continue to exist as undocumented methods). But by that time it should be possible to use Cocoa event monitors on all our supported versions of OS X.

I've now tested disabling/removing event taps on all three supported versions of OS X -- 10.5.8, 10.6.8 and 10.7.2. I didn't find any new problems. But I did find the problem I've already mentioned on all three versions of OS X, and more details on it (including a variant of it). For the sake of clarity and completeness, here are the two variants: A) Context menu raised in the background doesn't disappear when left-clicking or middle-clicking (aka other-clicking) on the desktop or in another app. 1) Right-click on an FF browser window while FF is running in the background. A context menu will appear, but the app will remain in the background. This is correct. 2) Left-click or middle-click on the desktop or on another app's window. The FF context menu should disappear, but it doesn't. B) An autoscroll icon that appears in the background doesn't disappear when left-clicking or middle-clicking on the desktop or in another app. 1) Run FF and visit a site that gives the browser window a vertical or a horizontal scroll bar (or both). Then put FF in the background (by left-clicking the desktop or another app's window). 2) Middle-click on the FF browser window. 3) Left-click or middle-click on the desktop or on another app's window. The autoscroll icon should disappear, but it doesn't.

This fixes the problems in Comment 14 on 10.6. If you're happy with this solution I'll test with 10.5 and 10.7 then ask for r?. Is this closely what you had in mind Markus? I'm not sure what you mean by testing with Carbon event monitors in Comment 13, if you give me more details I can try it out.

> I'm not sure what you mean by testing with Carbon event monitors in > Comment 13, if you give me more details I can try it out. Event taps have been a royal pain. So I think we should stop using them completely, if at all possible. Sometime this week I'll test Carbon event monitors myself, and also Cocoa event monitors (which I suspect are a thin wrapper around Carbon event monitors). If my tests work out, I'll take this bug. Otherwise I'll review and test your patch.

(In reply to Benoit Girard (:BenWa) from comment #15) > Is this closely what you had in mind Markus? Almost. I'd put the call to UnregisterAllProcessMouseEventHandlers into nsCocoaWindow::CaptureRollupEvents instead (for aDoRollup == false), so that the monitor is also unregistered when the context menu is closed via other means, for example by Cmd+tabbing into the background Firefox app.

Right, I figured we could use the next event to unregister and that would be roughly equivalent. If you'd like I can move it to CaptureRollupEvents.

Is there any updates on this? This makes me cry every day. :(

Sorry, I've been too busy with topcrashers :-( I'll try to get to it next week.

(In reply to Ehsan Akhgari [:ehsan] from comment #19) > Is there any updates on this? This makes me cry every day. :( I too cry every day because of this bug. :(

Work around for now, it's lovely to debug without this problem.

Comment on attachment 718630 [details] [diff] [review] Work around Wonderful! This is fine with me. I keep trying to find time to look for a real fix, but am always pre-empted by more important bugs. Too bad I didn't think of this workaround myself.

Marking leave-open because we want a better fix. This probably has user impact if crashing maybe? https://hg.mozilla.org/integration/mozilla-inbound/rev/2b4e9accde67

This addresses comment 17.

Comment on attachment 821524 [details] [diff] [review] v4: Only set up the event tap while a context menu is open and our app is not active Looks fine to me.

This should be fixed now. Please reopen if anybody runs into it again.

Thank you for fixing this! I haven't had any problems since this landed, and I'm very happy about that!