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 <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.scherba...@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.hal...@oracle.com >>> <mailto:manajit.hal...@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-8160144> >>> https://bugs.openjdk.java.net/browse/JDK-8160145 >>> <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.niy...@oracle.com >>>> <mailto:avik.niy...@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: <mailto:awt-dev@openjdk.java.net>awt-dev@openjdk.java.net >>>>> <mailto:awt-dev@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/ >>>>> <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-8158621> >>>>> >>>>> <https://bugs.openjdk.java.net/browse/JDK-8158485>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 >>>>> <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-8158389> >>>>> >>>>> <https://bugs.openjdk.java.net/browse/JDK-8158526>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-8158496> >>>>> >>>>> <https://bugs.openjdk.java.net/browse/JDK-8158362>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-8158512> >>>>> >>>>> <https://bugs.openjdk.java.net/browse/JDK-8156460>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 >>>>> <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 >>>> >>> >> >
