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

Oleg Pekhovskiy oleg.pekhovskiy at oracle.com
Thu Sep 27 06:24:13 PDT 2012


Looks good to me.

Oleg.

9/27/2012 5:06 PM, Alexander Scherbatiy wrote:
>
> 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.
>>>>>>>
>>
>




More information about the awt-dev mailing list