<Swing Dev> <Swing dev>[13] Review request for JDK-8216971: [macosx swing] For JCheckBoxMenuItem actionPerformed() is called twice, when apple.laf.useScreenMenuBar=true and modifier is InputEvent.META_DOWN_MASK

[Prasanta Sadhukhan]

fix looks fine to me but there are indentation issue both in fix and in 
testcase. Please rectify it.

Regards
Prasanta
On 12-Mar-19 6:46 PM, Krishna Addepalli wrote:
> Hi Manajit,
>
> Thanks for the clarification. The fix looks ok to me.
>
> Thanks,
> Krishna
>
>> On 12-Mar-2019, at 3:36 PM, Manajit Halder <manajit.halder at oracle.com 
>> <mailto:manajit.halder at oracle.com>> wrote:
>>
>> Lines 74 to 82 explains why we need to ignore this call. This method 
>> should be ignored if it is called as a result of user pressing  a 
>> shortcut and the window containing the menu is not minimized.
>>
>> Regards,
>> Manajit
>>
>>
>>> On 12-Mar-2019, at 3:16 PM, Krishna Addepalli 
>>> <krishna.addepalli at oracle.com <mailto:krishna.addepalli at oracle.com>> 
>>> wrote:
>>>
>>> Hi Manajit,
>>>
>>> Thanks for the clarification. I think you should add some more 
>>> comments around the statement at line86, to explain in more detail, 
>>> about why to ignore this call.
>>>
>>> Thanks,
>>> Krishna
>>>
>>>> On 11-Mar-2019, at 2:25 PM, Manajit Halder 
>>>> <manajit.halder at oracle.com <mailto:manajit.halder at oracle.com>> wrote:
>>>>
>>>> Hi Krishna,
>>>>
>>>> Thanks for the review comment.
>>>>
>>>> The key mapping is done by method setKeyEquivalent on fMenuItem 
>>>> (object of the NSMenuItem) in the same file.
>>>>
>>>> As discussed with you, the second key event is the problem here, 
>>>> and is caused only when System property 
>>>> “apple.laf.useScreenMenuBar” is set to true. The extra event is 
>>>> generated in the handleAction method and my proposed fix is solving 
>>>> this issue. The difference with other look and feel setting or when 
>>>> “apple.laf.useScreenMenuBar” is set to “false” is that handleAction 
>>>> method is not called. I have verified and found that the META_MASK 
>>>> and CTRL_MASK are only set when “apple.laf.useScreenMenuBar” is set 
>>>> to true and not in case of it is false. Also verified with “metal” 
>>>> look and feel and found the MASKS are not set and handleAction 
>>>> method is not called and hence the extra key event is not generated.
>>>>
>>>> Please let me know if you have any other query.
>>>>
>>>> Regards,
>>>> Manajit
>>>>
>>>>
>>>>
>>>>> On 05-Mar-2019, at 4:52 PM, Krishna Addepalli 
>>>>> <krishna.addepalli at oracle.com 
>>>>> <mailto:krishna.addepalli at oracle.com>> wrote:
>>>>>
>>>>> Hi Manajit,
>>>>>
>>>>> Per our discussion, The cause of the problem is : 1), Key Event 
>>>>> being sent from the OS to the application - which the Java layer 
>>>>> processes it correctly
>>>>> 2) The Mac OS calling the handleAction function directly on the 
>>>>> NSMenutItem - although as per your description, there is no code 
>>>>> which maps the hot key to this widget in the native layer.
>>>>> Ideally, since the OS is recognising the key combination, that key 
>>>>> event should not be delivered again to the application. Or, it 
>>>>> should be that the key event is not recognised and hence delivered 
>>>>> to the application.
>>>>>
>>>>> Can you check why in this case, we are getting the key event as 
>>>>> well as the handleAction from the OS?
>>>>>
>>>>> Thanks,
>>>>> Krishna
>>>>>
>>>>>> On 23-Feb-2019, at 9:14 PM, Manajit Halder 
>>>>>> <manajit.halder at oracle.com <mailto:manajit.halder at oracle.com>> wrote:
>>>>>>
>>>>>> Hi All,
>>>>>>
>>>>>> Please review the fix for JDK13.
>>>>>>
>>>>>> Bug:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8216971
>>>>>>
>>>>>> Webrev:
>>>>>> http://cr.openjdk.java.net/~mhalder/8216971/webrev.00/ 
>>>>>> <http://cr.openjdk.java.net/%7Emhalder/8216971/webrev.00/>
>>>>>>
>>>>>> Fix:
>>>>>> actionPerformed() was called twice due to wrong handling of key 
>>>>>> down event in method handleAction, which is corrected with this fix.
>>>>>> This change was added during fix of issue JDK-8152492. Apart from 
>>>>>> the changes required to fix the problem, code related to finding out
>>>>>> eventKey is removed as eventKey is no more used now.
>>>>>>
>>>>>> Note:
>>>>>> This issue is regression of bug 8152492, which was introduced in 
>>>>>> JDK release 9b120.
>>>>>>
>>>>>> Regards,
>>>>>> Manajit
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/swing-dev/attachments/20190315/aad235d8/attachment-0001.html>