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

[Alexander Scherbatiy]

Could you review the updated fix:
   http://cr.openjdk.java.net/~alexsch/7197619/webrev.03/

The only difference is that the formatting is updated for the line
    3176     if(isDeadKey){

Thanks,
Alexandr.


On 9/25/2012 10:46 AM, Leonid Romanov wrote:
> 3176     if(isDeadKey){
>
> Please, add space here. Otherwise looks OK, although I'm not an expert in
> this area.
>
>> -----Original Message-----
>> From: awt-dev-bounces at openjdk.java.net [mailto:awt-dev-
>> bounces at openjdk.java.net] On Behalf Of Alexander Scherbatiy
>> Sent: Thursday, September 20, 2012 6:48 PM
>> To: Oleg Pekhovskiy; awt-dev at openjdk.java.net; Alexey Utkin
>> Subject: Re:<AWT Dev>  [8] Review request for 7197619 Using modifiers for
>> the dead key detection on Windows
>>
>>
>> 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>(wke
>> y)));
>>> 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.
>>>>>>
>