Thanks! I'm OK with the fix. On 26.04.2012, at 17:47, Alexander Scherbatiy wrote:
> On 4/25/2012 2:46 PM, Leonid Romanov wrote: >> it is not obvious what isTopmostWindowUnderMouse is supposed to check >> without reading its source code. >> It would be great if you could add a brief comment describing method's >> purpose. Otherwise, the fix looks good. > > I have added the following comment: > // checks that this window is under the mouse cursor and this point is not > overlapped by others windows > > Here is the updated version: > http://cr.openjdk.java.net/~alexsch/7154048/webrev.03/ > > Thanks, > Alexandr. > >> On 24.04.2012, at 15:22, Alexander Scherbatiy wrote: >> >>> On 4/23/2012 7:39 PM, Leonid Romanov wrote: >>>> Hi, >>>> I can't find where you set initial value of mouseIsOver variable. Does >>>> Objective-C guarantee that it gets some defined default value? >>> I really missed this part. >>> >>> Please review the new version: >>> http://cr.openjdk.java.net/~alexsch/7154048/webrev.02/ >>> >>> The only thing that is changed is the initialization of the mouseIsOver >>> variable to false in the -initWithRect: method of the AWTView class. >>> >>> Thanks, >>> Alexandr. >>> >>>> On 23.04.2012, at 15:08, Alexander Scherbatiy wrote: >>>> >>>>> Thank you for the review. >>>>> >>>>> Here is the new version: >>>>> http://cr.openjdk.java.net/~alexsch/7154048/webrev.01/ >>>>> >>>>> 1. The synthesizeMouseEnteredExitedEvents method is added as a native >>>>> method to the CPlatformWindow class. >>>>> Now it is invoked only in places that programmatically change a window >>>>> size: >>>>> - nativeSetNSWindowBounds method from the AWTWindow >>>>> - setVisible and setWindowState methods from the CPlatformWindow >>>>> >>>>> 2. The objective-c code is formatted. >>>>> >>>>> 3. I do not think that setting the lastMouseEventPeer after sending the >>>>> MOUSE_ENTERED event in the LWWindowPeer class can affect some logic in >>>>> the peer. >>>>> The postEvent method just post the MOUSE_ENTERED events to the queue. >>>>> It does not use the lastMouseEventPeer variable and there is no a >>>>> recursion that invokes the dispatchMouseEvent method again. >>>>> >>>>> 4. Dragging a window under a panel should not generate mouse >>>>> entered/exited events for components. However the events should be >>>>> generated if the window is moved out of the frame or moved in to the >>>>> frame. So one more condition that checks is the mouse crosess the frame >>>>> borders are added to the dispatchMouseEvent method from the LWWindowPeer >>>>> class. >>>>> The DragWindowOutOfFrameTest test is added that these events are >>>>> properly generated. >>>>> >>>>> Thanks, >>>>> Alexandr. >>>>> >>>>> On 4/19/2012 5:14 PM, Anthony Petrov wrote: >>>>>> Hi Alexander, >>>>>> >>>>>> 1. I don't think that it's a good idea to add >>>>>> synthesizeMouseEnteredExitedEvents calls to CWrapper methods. These >>>>>> methods are supposed to perform direct calls to Cocoa API w/o any >>>>>> side-effects. They may be used for windows that even aren't AWT windows, >>>>>> and as such sending them the synthesizeMouseEnteredExitedEvents message >>>>>> is useless, and just doesn't seem right from CWrapper's purpose >>>>>> perspective. You may want to introduce >>>>>> CPlatformWindow._synthesizeMouseEnteredExitedEvents() native method that >>>>>> would call this native method, and then add a call to it where needed in >>>>>> Java code. >>>>>> >>>>>> 2. Please follow formatting guidelines and reformat lines like this: >>>>>> >>>>>>> if(id != MouseEvent.MOUSE_DRAGGED){ >>>>>> to read as >>>>>> >>>>>> if (id != MouseEvent.MOUSE_DRAGGED) { >>>>>> >>>>>> instead. I see lots of such mis-formatted if() statements all over your >>>>>> code. >>>>>> >>>>>> 3. In LWWindowPeer.java you are now setting the lastMouseEventPeer after >>>>>> sending the MOUSE_ENTERED event. Before your fix it's been set earlier. >>>>>> Can this change affect some logic in the peer code while processing >>>>>> ENTERED events at a user event handler? >>>>>> >>>>>> -- >>>>>> best regards, >>>>>> Anthony >>>>>> >>>>>> On 04/18/12 19:40, Alexander Scherbatiy wrote: >>>>>>> Please review a fix for CR 7154048. >>>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7154048 >>>>>>> >>>>>>> webrev: http://cr.openjdk.java.net/~alexsch/7154048/webrev.00/ >>>>>>> >>>>>>> Let's see the following test case: >>>>>>> - Frame contains two components JLabel and JButton >>>>>>> - The JLabel component has a mouse listener >>>>>>> mousePressed: create a Window under the mouse click >>>>>>> mouseDragged: drag the created window >>>>>>> mouseReleased: close the Window >>>>>>> - A user clicks on the JLabel component, drags the mouse to the JButton >>>>>>> component and releases the mouse button >>>>>>> >>>>>>> The current JDK 8 implementation shows the following events on Mac OS X: >>>>>>> -------------------------------------------------------- >>>>>>> mouse pressed: javax.swing.JLabel >>>>>>> mouse exited: javax.swing.JLabel >>>>>>> mouse entered: javax.swing.JLabel >>>>>>> mouse dragged: javax.swing.JLabel >>>>>>> mouse exited: javax.swing.JLabel >>>>>>> mouse entered: javax.swing.JButton >>>>>>> mouse dragged: javax.swing.JLabel >>>>>>> mouse exited: javax.swing.JButton >>>>>>> mouse entered: Drag Window >>>>>>> mouse exited: Drag Window >>>>>>> mouse entered: javax.swing.JButton >>>>>>> mouse released: javax.swing.JButton >>>>>>> -------------------------------------------------------- >>>>>>> >>>>>>> There are several issues: >>>>>>> 1) The window does not receive the mouse entered event when it is >>>>>>> created under the mouse >>>>>>> 2) There are JLabel exited/JButton entered events during the window >>>>>>> dragging >>>>>>> 3) JLabel does not receive the mouse released event >>>>>>> >>>>>>> The fix synthesizes the mouse entered/exited events manually if they are >>>>>>> not received. >>>>>>> >>>>>>> The entered/exited events synthesizing is added to setFrame, toFront, >>>>>>> toBack, and zoom methods of the AWTWindow and CWrapper classes. >>>>>>> >>>>>>> There is an option to add the events synthesizing to the windowDidResize >>>>>>> notification. However this notification is sent when a window size is >>>>>>> changed in both cases, programmatically and when user is resized the >>>>>>> window. So in a lot of case there is no need for the our use case events >>>>>>> generation. >>>>>>> >>>>>>> The LWWindowPeer class is updated to not generate extra mouse enter/exit >>>>>>> events during the mouse dragging. >>>>>>> >>>>>>> Tho automated tests are added. >>>>>>> >>>>>>> Thanks, >>>>>>> Alexandr. >
