<AWT Dev> [8] Review request for 7197619 Using modifiers for the dead key detection on Windows

[Oleg Pekhovskiy]

Looks good to me.

Thanks,
Oleg

9/20/2012 6:47 PM, Alexander Scherbatiy wrote:
>
> Could you review the updated fix:
>    http://cr.openjdk.java.net/~alexsch/7197619/webrev.02/
>
> - cast to BOOL type is fixed for the isDeadKey
> - isDeadKey parameter is passed by reference in the 
> WindowsKeyToJavaChar method
>
> Thanks,
> Alexandr.
>
> On 9/19/2012 8:16 PM, Oleg Pekhovskiy wrote:
>> Hi Alexander,
>>
>> let me advice you further changes:
>>
>> 3397 UINT AwtComponent::WindowsKeyToJavaChar(UINT wkey, UINT 
>> modifiers, TransOps ops, BOOL *isDeadKey)
>> 3398 {
>> 3399     static Hashtable transTable("VKEY translations");
>> 3400     static Hashtable deadKeyFlagTable("Dead Key Flags");
>> 3401     *isDeadKey = FALSE;
>> 3402
>> 3403     // Try to translate using last saved translation
>> 3404     if (ops == LOAD) {
>> 3405        void* deadKeyFlag = 
>> deadKeyFlagTable.remove(reinterpret_cast<void*>(static_cast<INT_PTR>(wkey)));
>> 3406        void* value = 
>> transTable.remove(reinterpret_cast<void*>(static_cast<INT_PTR>(wkey)));
>> 3407        if (value != NULL) {
>> 3408            *isDeadKey = 
>> static_cast<UINT>(reinterpret_cast<BOOL>(deadKeyFlag));
>> 3409            return 
>> static_cast<UINT>(reinterpret_cast<INT_PTR>(value));
>> 3410        }
>> 3411     }
>>
>> 1. Casting at row 3408 should be corrected to:
>>
>> static_cast<BOOL>(reinterpret_cast<INT_PTR>(deadKeyFlag))
>>
>>
>> 2. Seems like it makes sense to use a reference instead of a pointer 
>> for:
>>
>> BOOL *isDeadKey
>>
>> as this is a required parameter.
>>
>> Thanks,
>> Oleg
>>
>> 9/19/2012 5:42 PM, Alexander Scherbatiy wrote:
>>>
>>> Could you review the updated fix:
>>>   http://cr.openjdk.java.net/~alexsch/7197619/webrev.01/
>>>
>>> - The WindowsKeyToJavaChar method returns a char as it was before 
>>> the fix
>>> - ToAsciiEx is changed to ToUnicodeEx as it is suggested in the 
>>> comment.
>>>
>>>  Thanks,
>>>  Alexandr.
>>>
>>>
>>> On 9/14/2012 9:11 PM, Oleg Pekhovskiy wrote:
>>>> Hi Alexander,
>>>>
>>>> here are my comments:
>>>> 1. Why did you decide to make WindowsKeyToJavaChar void? Maybe it 
>>>> takes sense to leave signature closer to its original look?
>>>> 2. Seems like WindowsKeyToJavaChar method could be simplifed 
>>>> (ToAsciiEx -> ToUnicodeEx, remove MultiByteToWideChar),
>>>> like this:
>>>>
>>>> 3504     WORD wChar[2];
>>>> 3505     UINT scancode = ::MapVirtualKey(wkey, 0);
>>>> 3506     int converted = ::ToUnicodeEx(wkey, scancode, keyboardState,
>>>> 3507&wChar, 2, 0, GetKeyboardLayout());
>>>> 3508
>>>> 3509     UINT translation;
>>>> 3510     BOOL deadKeyFlag = (converted == 2);
>>>> 3511
>>>> 3512     // Dead Key
>>>> 3513     if (converted<  0) {
>>>> 3514         translation = java_awt_event_KeyEvent_CHAR_UNDEFINED;
>>>> 3515     } else
>>>> 3516     // No translation available -- try known conversions or 
>>>> else punt.
>>>> 3517     if (converted == 0) {
>>>> 3518         if (wkey == VK_DELETE) {
>>>> 3519             translation = '\177';
>>>> 3520         } else
>>>> 3521         if (wkey>= VK_NUMPAD0&&  wkey<= VK_NUMPAD9) {
>>>> 3522             translation = '0' + wkey - VK_NUMPAD0;
>>>> 3523         } else {
>>>> 3524             translation = java_awt_event_KeyEvent_CHAR_UNDEFINED;
>>>> 3525         }
>>>> 3526     } else
>>>> 3527     // the caller expects a Unicode character.
>>>> 3528     if (converted>  0) {
>>>> 3533         translation = wChar[0];
>>>> 3534     }
>>>>
>>>>
>>>> Thanks,
>>>> Oleg
>>>>
>>>>
>>>> 9/11/2012 5:24 PM, Alexander Scherbatiy wrote:
>>>>>
>>>>> bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7197619
>>>>> webrev: http://cr.openjdk.java.net/~alexsch/7197619/webrev.00/
>>>>>
>>>>> Only a virtual key code is used for the dead key detection on 
>>>>> Windows in the current implementation.
>>>>> This does not take into account that dead keys can be pressed with 
>>>>> modifiers like ctrl+alt+2 (caron dead key) on Hungarian keyboard.
>>>>>
>>>>> The fix gets isDeadKey flag and a character from the ToAsciiEx 
>>>>> method and uses them for the windows dead key to java key 
>>>>> translation.
>>>>>
>>>>> Thanks,
>>>>> Alexandr.
>>>>>
>>>>
>>>
>>
>