Hi, Oleg,
I'm not the expert in this area, it would be fine if Dmitry or Naoto
could also take a look. Anyway, here are some comments from my side:
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.
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.
Minor comments:
3. There is no need to call ImmReleaseContext() if hIMC is NULL in
AwtComponent::WmImeSetContext().
4. In WM_ACTIVATE handler, I would first handle WM_IME_ENDCOMPOSITION,
then call ImmReleaseContext().
5. In the same WM_ACTIVATE handler, what's the reason of direct call to
DefWindowProc() instead of regular ::SendMessage()?
Thanks,
Artem
On 4/19/2012 5:41 PM, Oleg Pekhovskiy wrote:
Hi!
Please review the fix for this CR:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7024749
Webrev:
http://cr.openjdk.java.net/~bagiras/8/7024749.1
The idea of this fix based on IMM interaction theory (from MSDN) and
actual implementation in AWT.
AWT creates its own IME context. It is shared between all AWT windows.
This context is associated with window each time it receives WM_ACTIVATE
message (which surplus but not critical).
WM_IME_SETCONTEXT and WM_IME_NOTIFY (with INM_OPENSTATUSWINDOW) messages
apply to all AWT windows and usually are sent to the top-level window.
So there is no need to send them to the focus proxy.
That's why I removed CallProxyDefWindowProc() from their handlers in
AwtComponent::WindowProc().
I also removed switch cases for those messages from
AwtFrame::ProxyWindowProc() because they didn't make any sense there.
Thus, only IME messages between WM_IME_STARTCOMPOSITION and
WM_IME_ENDCOMPOSITION are sent to the proxy focus owner - that's a
typing period.
I changed all places where IME context was used according to MSDN notes:
http://msdn.microsoft.com/en-us/library/windows/desktop/dd318639(v=vs.85).aspx
"The application retrieves the handle by using the ImmGetContext
function. It can use the retrieved handle in subsequent calls to the IMM
functions
to retrieve and set IME values, such as the composition window style,
the composition style, and the status window position.
Once the application has finished using the context, it must release the
context using the ImmReleaseContext function."
All changes were tested by the regression tests in 'test/java/awt/Focus'
and all JCK tests.
I also tested them on hierarchy of owned windows with different
enabled/disabled states.
No bad influence was found. From that point of view it is safe.
Thanks,
Oleg
