Hi Wolfgang and Fred I have had another attempt at this patch, trying to address some of your concerns.
On 20/02/2011, at 19:51 PM, Wolfgang Lux wrote: >> Christopher also took create care to release the captured mouse on all >> possible paths. This is very important, as otherwise the application >> could stop responding. But now consider that somebody else works on this >> method in the future. The person might easily not know about that >> necessity and accidentally break stuff. Even in this patch the mouse >> sometimes gets release after a tracking in a submenu. I don't have a >> clue how this is supposed to work. We always have to release the mouse >> before handling over control to a different window. This of course is >> easy to fix for now. But who will remember this for future changes? Actually, IMHO *shouldn't* release the mouse before handing control over to another window while we are still tracking the mouse, as it introduces a possible (but tiny) race condition where another app could capture the mouse. When we capture the mouse in the first window, we get events for all the windows sent to GNUstep, and GNUstep just passes them all along. All the internal calls for other windows to capture/release mouse are just ignored by X11 (it just returns a error value to the caller). The last (outermost) call should release the mouse. >> As much as I like the idea of this patch, I think it is too fragile code >> to be added. We should think hard on how to restructure the code here to >> make it less fragile and then we might be able to safely add a mouse >> capture to it. > > The safe solution is of course to move the body of this method without the > code for capturing the mouse into a new private method -_trackWithEvent: and > to define trackWithEvent: as a thin wrapper around the new method, which > performs mouse capturing. E.g., something like I've created a new patch incorporating Wolfgang's suggestion, which seems like an elegant way to solve the mouse capture problem without interspersing the method with capture/release calls. It can be found here: https://savannah.gnu.org/patch/index.php?7470 Cheers Chris -------- Christopher Armstrong [email protected] _______________________________________________ Gnustep-dev mailing list [email protected] http://lists.gnu.org/mailman/listinfo/gnustep-dev
