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

[Sergey Bylokhov]

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


-- 
Best regards, Sergey.