[8] Review request for 7197619 Using modifiers for the dead key detection on Windows (original) (raw)

Leonid Romanov leonid.romanov at oracle.com
Thu Sep 27 03:14:03 PDT 2012

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: [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: [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(reinterpretcast<void*>(staticcast(wke >> y))); >>> 3406 void* value = >>> transTable.remove(reinterpretcast<void*>(staticcast(wkey))); >>> 3407 if (value != NULL) { >>> 3408 *isDeadKey = >>> staticcast(reinterpretcast(deadKeyFlag)); >>> 3409 return >>> staticcast(reinterpretcast(value)); >>> 3410 } >>> 3411 } >>> >>> 1. Casting at row 3408 should be corrected to: >>> >>> staticcast(reinterpretcast(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 = javaawteventKeyEventCHARUNDEFINED; >>>>> 3515 } else >>>>> 3516 // No translation available -- try known conversions or >>>>> else punt. >>>>> 3517 if (converted == 0) { >>>>> 3518 if (wkey == VKDELETE) { >>>>> 3519 translation = '\177'; >>>>> 3520 } else >>>>> 3521 if (wkey>= VKNUMPAD0&& wkey<= VKNUMPAD9) {_ _>>>>> 3522 translation = '0' + wkey - VKNUMPAD0; >>>>> 3523 } else { >>>>> 3524 translation = javaawteventKeyEventCHARUNDEFINED; >>>>> 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/viewbug.do?bugid=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