Closed Bug 406362 Opened 14 years ago Closed 14 years ago
Mouse move events can be double processed, causes menu flicker and other issues
It is possible for mouse move events to be processed by more than one widget. Furthermore, rerouted events often use the wrong coordinates because we don't correct for the fact that event coords are relative to the window the event originally went to. This causes at least the following two problems: 1. Under certain conditions submenus may flicker when moving the mouse around within their menu item in the parent menu. This shows up most often in the default Places bookmark menu bar folder. 2. When using the mouse within a popup window, chrome and content items in the parent window can be activated at an offset from the popup menu such that you can see mouse over activations in other places on the page.
This patch fixes all of the issues described in the summary for this bug. The only thing I haven't done yet is investigate the potential impact on embedding clients.
Comment on attachment 291113 [details] [diff] [review] fix v1.0 I discovered a couple of bugs in this patch, both of which make it fail to build on Tiger (as opposed to Leopard). But these problems are minor (and easily corrected). And once I fixed them, all my other tests went just fine. 1) NSInteger is undefined in any of the ordinary locations on Tiger (/usr/include, /System/Library/Frameworks, /Developer/SDKs). The simplest fix for this is to replace every occurance of 'NSInteger' (in nsCocoaUtils.mm) with 'int'. (On Leopard, NSInteger is defined in NSObjCRuntime.h, and is meant to help the transition from 32-bits to 64-bits.) 2) The 'inline' keyword on screenLocationForEvent (also in nsCocoaUtils.mm) causes a linker error when building Minefield. The simplest fix is to get rid of it. For reasons that I haven't been able to figure out, neither of these problems happen when building Camino (on Tiger). My other tests were (on Leopard) to check that this bug's symptoms are fixed (they are), and (on both Tiger and Leopard) to run through various operations with the Places menu and with context menus. I tested in both Minefield and Camino, on both Tiger and Leopard.
Attachment #291113 - Flags: review?(smichaud) → review+
Address Steven's review comments.
Attachment #291113 - Attachment is obsolete: true
Moving to a P1. We'll do what we can to get this reviewed an in for beta 2.
Priority: P2 → P1
So clicks not over a rollup widget don't get handled by us? Shouldn't we be handling them and rolling up the widget? If not, how does the rollup happen, does Appkit tell us directly? Your C++ CocoaUtils functions should start Uppercase. Also, we generally try not to pollute the global namespace like this. The normal approach is to make them static methods of a class nsCocoaUtils or something. NSWindowList is guaranteed to return items in decreasing z-order? Please comment that if so. If not, there's a bug.
(In reply to comment #6) > So clicks not over a rollup widget don't get handled by us? Shouldn't we be > handling them and rolling up the widget? If not, how does the rollup happen, > does Appkit tell us directly? If a click is not over any window then it doesn't get handled by us. In that case we'll roll up in the resulting notification that app-level focus changed. If it is not over a rollup widget but it is over some other window, that window will get the click and do rollup. > Your C++ CocoaUtils functions should start Uppercase. Also, we generally try > not to pollute the global namespace like this. The normal approach is to make > them static methods of a class nsCocoaUtils or something. I'll post a new patch fixing this. > NSWindowList is guaranteed to return items in decreasing z-order? Please > comment that if so. If not, there's a bug. The list is in a particular z-order. The first item is the foremost.
Added a comment about the z-order and changed all nsCocoaUtils methods to public static members of nsCocoaUtils.
Attachment #291764 - Flags: superreview?(roc) → superreview+
landed on trunk
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.