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

Still

74         } finally {

in testcase not fixed. Please fix it before pushing...No need to send 
webrev.

Regards
Prasanta
On 18-Mar-19 4:12 PM, Manajit Halder wrote:
> Thanks Prasanta for the review. I have corrected the indentation 
> problem. Please review the updated webrev.
> http://cr.openjdk.java.net/~mhalder/8216971/webrev.02/ 
> <http://cr.openjdk.java.net/%7Emhalder/8216971/webrev.02/>
>
> Regards,
> Manajit
>
>> On 18-Mar-2019, at 11:21 AM, Prasanta Sadhukhan 
>> <prasanta.sadhukhan at oracle.com 
>> <mailto:prasanta.sadhukhan at oracle.com>> wrote:
>>
>> I guess l94-111 indentation in CMenuItem.m and l69 Robot robot = new 
>> Robot(); in testcase
>>
>> is still not fixed.
>>
>> Regards
>> Prasanta
>> On 15-Mar-19 4:51 PM, Manajit Halder wrote:
>>> 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/%7Emhalder/8216971/webrev.01/>
>>>
>>> Regards,
>>> Manajit
>>>
>>>
>>>> On 15-Mar-2019, at 11:32 AM, Prasanta Sadhukhan 
>>>> <prasanta.sadhukhan at oracle.com 
>>>> <mailto: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
>>>>>>>>>>
>>>>>>>>>> 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/20190318/40d75007/attachment-0001.html>