<AWT Dev> <AWT dev>[9] Review request for JDK-8156460 [macosx] Test case javax/swing/JPopupMenu/6827786/bug6827786.java fails

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 7/5/2016 2:24 PM, Manajit Halder wrote:
> Hi Alexandr,
>
> I feel it is not a problem to use "if (leftAltKeyPressed == YES)” and 
>  "if (altGRPressed == NO)” as the variable leftAltKeyPressed is 
> defined as static BOOL and anytime during the execution the variable 
> would be either YES or NO.
>
> Also I found following files in AWT project using the same way for 
> BOOL variables as used by me:
> ImageSurfaceData.m
> QuartzRenderer.m
> QuartzSurfaceData.m
> ThreadUtilities.m
>
> Please go through the link which talks about BOOL scalar types.
> https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/FoundationTypesandCollections/FoundationTypesandCollections.html#//apple_ref/doc/uid/TP40011210-CH7-SW1 
>
>
> Thanks,
> Manajit
>
>> On 30-Jun-2016, at 12:25 am, Alexandr Scherbatiy 
>> <alexandr.scherbatiy at oracle.com 
>> <mailto:alexandr.scherbatiy at oracle.com>> wrote:
>>
>> On 6/28/2016 11:14 AM, Manajit Halder wrote:
>>> Hi All,
>>>
>>> Gentle remainder. Please review the changes.
>>    It is better to use "if (leftAltKeyPressed)" instead of "if 
>> (leftAltKeyPressed == YES)" and "if (!altGRPressed)" instead of "if 
>> (altGRPressed == NO)".
>>
>>   Thanks,
>>   Alexandr.
>>>
>>> Thanks,
>>> Manajit
>>>
>>>> On 25-Jun-2016, at 7:46 pm, Manajit Halder 
>>>> <manajit.halder at oracle.com <mailto:manajit.halder at oracle.com>> wrote:
>>>>
>>>> Hi All,
>>>>
>>>> The code was changed on the same lines in one file after the first 
>>>> review was generated. A new review is generated after taking an 
>>>> update of the code.
>>>> Fix wise the webrev.00 and webrev.01 are same.
>>>>
>>>> Please review webrev.01
>>>> http://cr.openjdk.java.net/~mhalder/8156460/webrev.01/ 
>>>> <http://cr.openjdk.java.net/%7Emhalder/8156460/webrev.01/>
>>>>
>>>> Also note that along with the previous 10 issues as mentioned in 
>>>> the first review mail below another two new issues created 2 days 
>>>> ago also gets resolved by this fix.
>>>> The 2 new issues are:
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8160144
>>>> https://bugs.openjdk.java.net/browse/JDK-8160145
>>>>
>>>> Thank you Avik for your comment. The lines were moved up to 
>>>> maintain the order of modifier values in increasing order.
>>>>
>>>> Thanks,
>>>> Manajit
>>>>
>>>>
>>>>> On 21-Jun-2016, at 12:10 pm, Avik Niyogi <avik.niyogi at oracle.com 
>>>>> <mailto:avik.niyogi at oracle.com>> wrote:
>>>>>
>>>>> Hi,
>>>>> The fix looks good to me.
>>>>> A small query though, line 281 - 290 is required at that position, 
>>>>> looks like it was moved.
>>>>>
>>>>> With Regards,
>>>>> Avik Niyogi
>>>>>> *From:*Manajit Halder
>>>>>> *Sent:*Monday, June 20, 2016 1:56 AM
>>>>>> *To:*Sergey Bylokhov; Semyon Sadetsky
>>>>>> *Cc:*awt-dev at openjdk.java.net
>>>>>> *Subject:*<AWT Dev> <AWT dev>[9] Review request for JDK-8156460 
>>>>>> [macosx] Test case javax/swing/JPopupMenu/6827786/bug6827786.java 
>>>>>> fails
>>>>>> Hi All,
>>>>>> Please review the regression fix for issue JDK-8156460 
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8156460> which fixes 
>>>>>> below mentioned test cases.
>>>>>> <http://cr.openjdk.java.net/%7Emhalder/8156460/webrev.00/>http://cr.openjdk.java.net/~mhalder/8156460/webrev.00/
>>>>>> This fix resolves the following 3 JCK failures and 7 test failures:
>>>>>> JCK tests:
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8158621>https://bugs.openjdk.java.net/browse/JDK-8158621
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8158485>https://bugs.openjdk.java.net/browse/JDK-8158485
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8158501>https://bugs.openjdk.java.net/browse/JDK-8158501
>>>>>> Jtreg tests:
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8158389>https://bugs.openjdk.java.net/browse/JDK-8158389
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8158526>https://bugs.openjdk.java.net/browse/JDK-8158526
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8158496>https://bugs.openjdk.java.net/browse/JDK-8158496
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8158362>https://bugs.openjdk.java.net/browse/JDK-8158362
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8158512>https://bugs.openjdk.java.net/browse/JDK-8158512
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8156460>https://bugs.openjdk.java.net/browse/JDK-8156460
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8158377>https://bugs.openjdk.java.net/browse/JDK-8158377
>>>>>> Reason of failure:
>>>>>> The modifier value calculation was wrong.
>>>>>> Note that with this fix the 
>>>>>> test /java/awt/keyboard/AllKeyCode/AllKeyCode.java will fail due 
>>>>>> to the reason that pressing number (0 to 9) after pressing arrow 
>>>>>> keys( up, down, left and right) will generate corresponding 
>>>>>> Numpad keys code for number keys (0 to 9). Whereas if the arrow 
>>>>>> key are pressed after number keys are pressed then there is no 
>>>>>> problem. An issue will be created for this issue once this fix is 
>>>>>> accepted.
>>>>>> Thanks,
>>>>>> Manajit
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20160705/d0440913/attachment-0001.html>