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

[Leonid Romanov]

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.
> >>>>
> >>>
> >>
> >