<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

[Manajit Halder]

Thanks Prasanta,

I have corrected the indentation issue in both files. Please review the modified webrev.
http://cr.openjdk.java.net/~mhalder/8216971/webrev.01/ <http://cr.openjdk.java.net/~mhalder/8216971/webrev.01/>

Regards,
Manajit


> On 15-Mar-2019, at 11:32 AM, Prasanta Sadhukhan <prasanta.sadhukhan at oracle.com> wrote:
> 
> 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 <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/925e3421/attachment-0001.html>