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

Anthony Petrov anthony.petrov at oracle.com
Mon Apr 23 07:31:54 PDT 2012


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?

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



More information about the awt-dev mailing list