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.
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. >
