<AWT Dev>  Review Request: 8143054 [macosx] KeyEvent modifiers do not contain information about mouse buttons
Sergey.Bylokhov at oracle.com
Wed Dec 23 16:33:50 UTC 2015
Thanks for review, see my comments inline.
On 23/12/15 18:37, Semyon Sadetsky wrote:
> 1. That would be nice to test that modifiers are exactly as expected to
> ensure that no extra modifier bits are set.
Yes, but according to the spec it is not guaranteed that some other
modifiers are set or not, so I just check the modifiers which must be
set in this case only.
> 2. Test summary and author are omitted
I am not sure what I can add to the summary, other than information
which already existed in the bug description. The author tag is
undesired in the openjdk and its usage is not strictly necessary.
> 3. In the test you are creating a new window for each button. That is
> not optimal for test execution time.
It is done intentionally to skip any side effects caused by the previous
keyup/keydown, so each time it is new frame, texteditor, listeners etc.
> 4. Test fails on Linux, but you've filed a separate bug already. That
> should be ok.
> On 11/18/2015 6:50 PM, Sergey Bylokhov wrote:
>> Please review the fix for jdk9.
>> On macosx when we create a KeyEvent we ignore the mouse state of all
>> mouse buttons, which means that mouse modifiers are missing.
>> The only KeyEvent is affected, because when we convert NS modifiers to
>> java modifiers we use nsToJavaKeyModifiers(), but in all other cases
>> we use nsToJavaMouseModifiers() which is the same but adds mouse
>> In the fix:
>> - The button parameter of nsToJavaMouseModifiers() was removed because
>> it was unused.
>> - nsToJavaMouseModifiers() was renamed to nsToJavaModifiers() and now
>> is used when keyEvent is generated.
>> - nsToJavaKeyModifiers() was removed because it unused now.
>> The similar bug for extended(id>=3) mouse buttons was filed for linux:
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8143054
>> Webrev can be found at:
Best regards, Sergey.
More information about the awt-dev