<AWT Dev> Review request for 6191390: Action Event triggered by list does not reflect the modifiers properly on win32

[Sergey Bylokhov]

Can you please confirm that this is the only place where we create 
ActionEvent w/o modifiers? What about awt_Button.cpp and others?

On 28.03.16 17:56, Ambarish Rapte wrote:
> Hi Sergey,
>
> 	Thanks for the review comments,
>
> 	As you pointed out,
> 	1. ALT_GRAPH_MASK is not needed.
> 	2. Should use ActionEvent masks, instead of InputEvent.
> 	
> 	=> Both the above are corrected in webrev.01,
> 	http://cr.openjdk.java.net/~arapte/6191390/webrev.01/
>
> 	Please review the updated webrev.01
>
> Regards,
> Ambarish
> 	
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Friday, March 25, 2016 3:48 AM
> To: Ambarish Rapte; Semyon Sadetsky; Prasanta Sadhukhan; awt-dev at openjdk.java.net
> Subject: Re: Review request for 6191390: Action Event triggered by list does not reflect the modifiers properly on win32
>
> Hi, Ambarish.
> You add a correct comment to the method:
> "modifiers provided by ActionEvent class should be set"
>
> But the method itself sets the modifiers from the InputEvent. Yes in most cases the id is the same, but for example the ActionEvent class has no ALT_GRAPH_MASK. Are you sure that it should be set?
>
> On 24.03.16 12:56, Ambarish Rapte wrote:
>> Hi,
>>
>>                   Please review fix for JDK9,
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-6191390
>>
>>                   Webrev:
>> http://cr.openjdk.java.net/~arapte/6191390/webrev.00/
>>
>> Issue:
>>
>>                   Only on windows:
>>
>> ActionEvent received by ActionListener, does not contain modifiers
>> specific to ActionEvent class.
>>
>> On Linux & Mac:
>>
>> Correct modifiers are received.
>>
>> Cause:
>>
>>                   The actionEvent created in windows is created only
>> with Extended modifiers.
>>
>> Fix:
>>
>>                   While creating ActionEvent, added code to include
>> ActionEvent modifiers in addition to earlier included extended modifiers.
>>
>> Verification:
>>
>>                   The new test passes on Windows with fix, & also on
>> ubuntu & mac.
>>
>>                   No Regression & JCK test fails due to this change.
>>
>> Regards,
>>
>> Ambarish
>>
>
>
> --
> Best regards, Sergey.
>


-- 
Best regards, Sergey.