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

[Leonid Romanov]

Looks good.

> -----Original Message-----
> From: Alexander Scherbatiy [mailto:alexandr.scherbatiy at oracle.com]
> Sent: Thursday, September 27, 2012 5:07 PM
> To: Leonid Romanov; 'Oleg Pekhovskiy'; awt-dev at openjdk.java.net
> 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.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.
> >>>>>>
> >