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

[Anthony Petrov]

On 4/24/2012 3:18 PM, Alexander Scherbatiy wrote:
> On 4/23/2012 6:31 PM, Anthony Petrov wrote:
>> Hi Alexander,
>>
>> Thanks! The fix looks fine to me.
>>
>> I appreciate the newly added tests, however, I haven't reviewed them 
>> thoroughly. One question though, do the tests pass on other platforms 
>> (Win & X11) as well?
> 
>    Yes. The tests pass on the Mac OS X, Windows 7 and Linux Ubuntu.

Thanks for verifying that! I'm fine with the fix (incl. the latest 
version with the mouseIsOver flag initialized.)

--
best regards,
Anthony

> 
>    Thanks,
>    Alexandr.
> 
>>
>> -- 
>> best regards,
>> Anthony
>>
>> On 04/23/12 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.
>>>
>