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

[Anton V. Tarasov]

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