Hi Oleg,

I'm Ok with the kbd focus changes.

Thanks,
Anton.

On 22.06.2012 20:28, Oleg Pekhovskiy wrote:
Hi,

Please review the second version of fix for CR:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7024749

Webrev:
http://cr.openjdk.java.net/~bagiras/8/7024749.2

It resolves applet's IME typing problems that existed with the first fix.

Here are my answers to Artem's comments:

1. I see no reason to change ImmGetContext() to ImmGetHWnd(). Everywhere in the code ImmGetHWnd() is followed by ImmGetContext(), and these two calls can easily be combined into a single method in AwtComponent. ImmGetHWnd() & ImmGetContext() return HWND and HIMC accordingly that are used BOTH in ImmReleaseContext().

2. Comment about focus proxy in AwtComponent::OpenCandidateWindow() is now obsolete. Instead, you need to add a comment why we send WM_IME_NOTIFY to this component (GetHWnd()), not to its focus proxy.
It's not obsolete now, because I returned GetProxyFocusOwner() there.

3. There is no need to call ImmReleaseContext() if hIMC is NULL in 
AwtComponent::WmImeSetContext().
Thanks, I changed that.

4. In WM_ACTIVATE handler, I would first handle WM_IME_ENDCOMPOSITION, then 
call ImmReleaseContext().
Seems like it would be better to release IMM Context for avoiding its 
simultaneous usage.

5. In the same WM_ACTIVATE handler, what's the reason of direct call to DefWindowProc() instead of regular ::SendMessage()?
It's not my code, so I left that as is.

PS: this fix leads to the regression for 'test/java/awt/Focus/AppletInitialFocusTest', but I was not able to fix that yet (ready to file a separate CR).

Thanks,
Oleg

Reply via email to