[PATCH 1/2] winex11: Invoke Key event handlers immediately, keeping pace with the XFilterEvent().
1. GCS_COMPSTR: XFilterEvent() True case, XIMPreeditDrawCallCallback() is invoked by XFilterEvent() internally. immediately processing. GCS_RESULTSTR: XFilterEvent() False case. X11DRV_XIMLookupChars() is invoked by KeyPress event handler. delayed processing. So, delayed event handler processing causes GCS_COMPSTR, GCS_RESULTSTR pair sequence inverting.
In Hangul IME, one key often generates GCS_COMPSTR, GCS_RESULTSTR pairs. The XIM server sends the correct order pair, but the message order inverted through the process_events(). Japanese input system can also produce similar situation.
2. Copying event variable does not guarantee complete state preservation. At the time of delayed processing, the global information that the event handler must refer to may be changed. Ex. XFilterEvent(), KeyPress event handler confirmed.
Signed-off-by: Byeongsik Jeon bsjeon@hanmail.net --- dlls/winex11.drv/event.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 170111e9c28..149069b0d29 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -445,6 +445,19 @@ static BOOL process_events( Display *display, Bool (*filter)(Display*, XEvent*,X else continue; /* filtered, ignore it */ } + if (event.type == KeyPress || event.type == KeyRelease) + { + if (prev_event.type) + { + queued |= call_event_handler( display, &prev_event ); + free_event_data( &prev_event ); + prev_event.type = 0; + action = MERGE_DISCARD; + } + queued |= call_event_handler( display, &event ); + continue; + } + get_event_data( &event ); if (prev_event.type) action = merge_events( &prev_event, &event ); switch( action )
IME_SetResultString() is a function called in a very common situation, ImmSetOpenStatus() is not appropriate HERE. In native opensource IME, IMC_SETOPENSTATUS cleans the Composition string of input context. The winex11.drv IMC_SETOPENSTATUS code is implemented to produce similar effects. So the common input process is BROKEN. Perhaps, I guess the original intention is IME_SetCompositionStatus().
This problem occurs in ibus-hangul, and it occurs very frequently. The ibus-hangul always delivers the XIM_PREEDIT_DONE message before XIM_COMMIT( ~= GCS_RESULTSTR).
Signed-off-by: Byeongsik Jeon bsjeon@hanmail.net --- dlls/winex11.drv/ime.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/dlls/winex11.drv/ime.c b/dlls/winex11.drv/ime.c index c1584930861..5f65281d214 100644 --- a/dlls/winex11.drv/ime.c +++ b/dlls/winex11.drv/ime.c @@ -1021,7 +1021,6 @@ void IME_SetResultString(LPWSTR lpResult, DWORD dwResultLen) LPINPUTCONTEXT lpIMC; HIMCC newCompStr; LPIMEPRIVATE myPrivate; - BOOL inComp;
imc = RealIMC(FROM_X11); lpIMC = ImmLockIMC(imc); @@ -1037,21 +1036,15 @@ void IME_SetResultString(LPWSTR lpResult, DWORD dwResultLen) lpIMC->hCompStr = newCompStr;
myPrivate = ImmLockIMCC(lpIMC->hPrivate); - inComp = myPrivate->bInComposition; - ImmUnlockIMCC(lpIMC->hPrivate); - - if (!inComp) - { - ImmSetOpenStatus(imc, TRUE); + if (!myPrivate->bInComposition) GenerateIMEMessage(imc, WM_IME_STARTCOMPOSITION, 0, 0); - }
GenerateIMEMessage(imc, WM_IME_COMPOSITION, 0, GCS_COMPSTR); GenerateIMEMessage(imc, WM_IME_COMPOSITION, lpResult[0], GCS_RESULTSTR|GCS_RESULTCLAUSE); GenerateIMEMessage(imc, WM_IME_ENDCOMPOSITION, 0, 0); + myPrivate->bInComposition = FALSE; + ImmUnlockIMCC(lpIMC->hPrivate);
- if (!inComp) - ImmSetOpenStatus(imc, FALSE);
ImmUnlockIMC(imc); }
On 4/4/22 10:49 PM, Byeongsik Jeon wrote:
GCS_COMPSTR: XFilterEvent() True case, XIMPreeditDrawCallCallback() is invoked by XFilterEvent() internally. immediately processing. GCS_RESULTSTR: XFilterEvent() False case. X11DRV_XIMLookupChars() is invoked by KeyPress event handler. delayed processing. So, delayed event handler processing causes GCS_COMPSTR, GCS_RESULTSTR pair sequence inverting.
In Hangul IME, one key often generates GCS_COMPSTR, GCS_RESULTSTR pairs. The XIM server sends the correct order pair, but the message order inverted through the process_events(). Japanese input system can also produce similar situation.
Copying event variable does not guarantee complete state preservation. At the time of delayed processing, the global information that the event handler must refer to may be changed. Ex. XFilterEvent(), KeyPress event handler confirmed.
Please ignore '2.'. Personally, while testing other issues, I got a misinterpretation.
"1." is still valid. We must prevent the change in call sequence between KeyPress, KeyRelease event handlers and XFilterEvent ().
Signed-off-by: Byeongsik Jeon bsjeon@hanmail.net
dlls/winex11.drv/event.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 170111e9c28..149069b0d29 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -445,6 +445,19 @@ static BOOL process_events( Display *display, Bool (*filter)(Display*, XEvent*,X else continue; /* filtered, ignore it */ }
if (event.type == KeyPress || event.type == KeyRelease){if (prev_event.type){queued |= call_event_handler( display, &prev_event );free_event_data( &prev_event );prev_event.type = 0;action = MERGE_DISCARD;}queued |= call_event_handler( display, &event );continue;}get_event_data( &event ); if (prev_event.type) action = merge_events( &prev_event, &event ); switch( action )
-
Byeongsik Jeon