<AWT Dev> Request for review: 7154048 [macosx] At least drag twice, the toolbar can be dragged to the left side.

[Leonid Romanov]

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