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

[dmitry markov]

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