<AWT Dev> [9] Review Request: JDK-8032872: [macosx] Cannot select from JComboBox in a JWindow

[dmitry markov]

Anton,

I updated the test. New version is located at 
http://cr.openjdk.java.net/~dmarkov/8032872/jdk9/webrev.03/

Thanks,
Dmitry

On 05/03/2014 13:13, Anton V. Tarasov wrote:
> Hi Dmitry,
>
> Actually, I meant to add a _simple_ case to the test, or otherwise it 
> becomes overly complicated. Sorry, if I didn't make that clear.
>
> So, the simple case is to have a hierarchy: frame -> window-1 -> 
> window-2, where window-1 is grabbed. Then you click in window-2 and 
> check if it doesn't cause ungrab event. Right?
>
> Thanks,
> Anton.
>
> On 05.03.2014 12:55, dmitry markov wrote:
>> Hi Anton,
>>
>> I updated java/awt/Window/Grab/GrabTest.java as you requested. Please 
>> find new webrev here: 
>> http://cr.openjdk.java.net/~dmarkov/8032872/jdk9/webrev.02/
>>
>> Thanks,
>> Dmitry
>>
>> On 04/03/2014 19:49, Anton V. Tarasov wrote:
>>> Hi Dmitry,
>>>
>>> The fix looks fine for me, but I'm still voting for adding this case 
>>> to java/awt/Window/Grab/GrabTest.java
>>>
>>> Thanks,
>>> Anton.
>>>
>>> On 04.03.2014 17:26, dmitry markov wrote:
>>>> Hello,
>>>>
>>>> Could you review the updated fix, please?
>>>> webrev: http://cr.openjdk.java.net/~dmarkov/8032872/jdk9/webrev.01/
>>>> - Fixed problem in changeFocusedWindow()
>>>> - Added description for getOwnerFrameDialog()
>>>>
>>>> Thanks,
>>>> Dmitry
>>>>
>>>> On 03/03/2014 20:19, Sergey Bylokhov wrote:
>>>>> On 3/3/14 6:15 PM, Anton V. Tarasov wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> The fix looks fine for me. The usage of getOwnerFrameDialog() in 
>>>>>> the mentioned cases indeed seems incorrect. I've looked at the 
>>>>>> rest of the code and found yet another incorrect usage, in 
>>>>>> LWWindowPeer.changeFocusedWindow line 1265. Please, fix it the 
>>>>>> same way. All the other use cases of the method should be fine as 
>>>>>> they relate to an activation (a simple window can't be an active 
>>>>>> window).
>>>>> Then it would be good to add appropriate javadoc to 
>>>>> getOwnerFrameDialog to mention that it returns owner which can be 
>>>>> activated(Frame or D
>>>>> ialog and not a WIndow).
>>>>>>
>>>>>> Also, I'd recommend to add a new testcase to 
>>>>>> java/awt/Window/Grab/GrabTest.java
>>>>>>
>>>>>> Thanks,
>>>>>> Anton.
>>>>>>
>>>>>> On 03.03.2014 16:49, Sergey Bylokhov wrote:
>>>>>>> On 3/3/14 2:22 PM, dmitry markov wrote:
>>>>>>>> If I get it right, getOwnerFrameDialog() is designed for 
>>>>>>>> another purpose. Also it is NOT only used in notifyMouseEvent() 
>>>>>>>> and notifyNCMouseDown().
>>>>>>> Probably other places don't work also? I see that these usages 
>>>>>>> are related to the focus staff. Anton do you know why 
>>>>>>> LWWindowPeer.getOwnerFrameDialog() checks Frame and Dialog only?
>>>>>>>> So I think we should stay this method as is to avoid any 
>>>>>>>> problems in future.
>>>>>>>>
>>>>>>>> It is really necessary to add new method isOneOfOwnersOf(). BTW 
>>>>>>>> this approach is already used on Windows platform, see 
>>>>>>>> awt_Window.cpp for details.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Dmitry
>>>>>>>>
>>>>>>>> On 03/03/2014 13:54, Petr Pchelko wrote:
>>>>>>>>> Hello, Dmitry.
>>>>>>>>>
>>>>>>>>> I've investigated a similar issue a while ago 
>>>>>>>>> (https://bugs.openjdk.java.net/browse/JDK-8029686), could you 
>>>>>>>>> please check if this issue is also resolved by your fix?
>>>>>>>>>
>>>>>>>>>> In other words the current implementation assumes that the 
>>>>>>>>>> grabbingWindow must be an instance of Frame or Dialog and 
>>>>>>>>>> does not handle the case when the grabbingWindow is JWindow.
>>>>>>>>> When I was investigating this I did not understand why that 
>>>>>>>>> was done that way. Do you know the reason?
>>>>>>>>> May be it's better not in introduce a new function but replace 
>>>>>>>>> the getOwnerFrameDialog with your new implementation?
>>>>>>>>>
>>>>>>>>> Thank you.
>>>>>>>>> With best regards. Petr.
>>>>>>>>>
>>>>>>>>> On 03.03.2014, at 13:45, dmitry markov 
>>>>>>>>> <dmitry.markov at oracle.com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Sergey,
>>>>>>>>>>
>>>>>>>>>> The current implementation of 
>>>>>>>>>> LWWindowPeer.getOwnerFrameDialog() may return an instance of 
>>>>>>>>>> Frame or Dialog. The returned value used to check whether the 
>>>>>>>>>> grabbingWindow is owner of mouse event target or not.
>>>>>>>>>>
>>>>>>>>>> If JComboBox is added to JFrame, the grabbingWindow is JFrame 
>>>>>>>>>> and getOwnerFrameDialog() returns the same JFrame object, 
>>>>>>>>>> (i.e. the check passes).
>>>>>>>>>> If JCombobox is added to JWindow which is owned by JFrame, 
>>>>>>>>>> the grabbingWindow is JWindow but getOwnerFrameDialog() 
>>>>>>>>>> returns the JFrame, (i.e. the check fails).
>>>>>>>>>>
>>>>>>>>>> In other words the current implementation assumes that the 
>>>>>>>>>> grabbingWindow must be an instance of Frame or Dialog and 
>>>>>>>>>> does not handle the case when the grabbingWindow is JWindow.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Dmitry
>>>>>>>>>>
>>>>>>>>>> On 03/03/2014 13:01, Sergey Bylokhov wrote:
>>>>>>>>>>> Hi, Dmitry.
>>>>>>>>>>> Why the problem is reproduced in JWindow? Why it works in 
>>>>>>>>>>> JFrame?
>>>>>>>>>>>
>>>>>>>>>>> On 3/3/14 10:40 AM, dmitry markov wrote:
>>>>>>>>>>>> Hello,
>>>>>>>>>>>>
>>>>>>>>>>>> Could you review the fix for jdk9, please?
>>>>>>>>>>>>
>>>>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8032872
>>>>>>>>>>>> webrev: 
>>>>>>>>>>>> http://cr.openjdk.java.net/~dmarkov/8032872/jdk9/webrev.00/
>>>>>>>>>>>>
>>>>>>>>>>>> Problem description: It is impossible to make a selection 
>>>>>>>>>>>> in JComboBox added to JWindow via the mouse. The problem is 
>>>>>>>>>>>> caused by incorrect mouse events handling in LWWindowPeer 
>>>>>>>>>>>> class. When LWWindowPeer receives a mouse event intended 
>>>>>>>>>>>> for a popup window, it checks whether the current 
>>>>>>>>>>>> grabbingWindow is owner of the popup using 
>>>>>>>>>>>> getOwnerFrameDialog() method. This approach always fails 
>>>>>>>>>>>> for the JComboBox added to JWindow. As a result an 
>>>>>>>>>>>> UngrabEvent is send to the popup window.
>>>>>>>>>>>>
>>>>>>>>>>>> Fix: Introduce new private method 
>>>>>>>>>>>> LWWindowPeer.isOneOfOwnersOf(LWWindowPeer peer). The method 
>>>>>>>>>>>> will be invoked on grabbingWindow object to test whether it 
>>>>>>>>>>>> is owner of current mouse event target or not. The usage of 
>>>>>>>>>>>> getOwnerFrameDialog() should be replaced by 
>>>>>>>>>>>> isOneOfOwnersOf() in LWWindowPeer.notifyMouseEvent() and 
>>>>>>>>>>>> LWWindowPeer.NotifyNCMouseDown().
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Dmitry
>>>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>