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

[Anton V. Tarasov]

Thanks, Dmitry! Looks great.

Regards,
Anton.

On 05.03.2014 15:09, dmitry markov wrote:
> 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
>>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>