Re: [PATCH] winex11.drv: Fix IME_SetResultString for Korean(hangul) IME.

[Alex Kwak]

Hi

I resubmitted the problems related to this by dividing them into 4 patches.

All CJK have tested it on their own, but in my imagination, the problem 
seems to have been solved.

I tried to create a test, but ImSetCompositionStringW seems to be set 
regardless of dwIndex, so I can't implement it well.

I'll have to take a look when I have time.

[PATCH v2] winex11.drv: call XIMReset on CPS_COMPLETE

https://source.winehq.org/patches/data/231256

[PATCH v2] riched20: Send CPS_COMPLETE from mouse click event.

https://source.winehq.org/patches/data/231251

[PATCH v2] winex11.drv: Call ImeSetCompositionString after IME_SetResultString

https://source.winehq.org/patches/data/231236

[PATCH v2] riched20: Handle GCS_COMPSTR, GCS_CURSORPOS on WM_IME_COMPOSITION.

https://source.winehq.org/patches/data/231055

Regards,

Alex Kwak

22. 3. 26. 19:44에 Akihiro Sagawa 이(가) 쓴 글:
> On Sun, 20 Mar 2022 21:17:27 +0900, Alex Kwak wrote:
> > In a special case for Hangul(Korean), when Composition is completed and
> > this function is called, the new Composition disappears. so, calling
> > IME_SetCompositionString again for makes composition will be
> > starting.
>
> Please add Wine-Bug header before Signed-off-by line.
> > Signed-off-by: Alex Kwak
> >
> > dlls/winex11.drv/ime.c    |  4 ++--
> >   dlls/winex11.drv/x11drv.h | 27 +++++++++++++++++++++++++++
> >   dlls/winex11.drv/xim.c    | 13 +++++++++++++
> >   3 files changed, 42 insertions(+), 2 deletions(-)
> >
> > diff --git a/dlls/winex11.drv/ime.c b/dlls/winex11.drv/ime.c
> > index c1584930861..f7827b31635 100644
> > --- a/dlls/winex11.drv/ime.c
> > +++ b/dlls/winex11.drv/ime.c
> > @@ -1050,8 +1050,8 @@ void IME_SetResultString(LPWSTR lpResult, DWORD dwResultLen)
> >       GenerateIMEMessage(imc, WM_IME_COMPOSITION, lpResult[0], GCS_RESULTSTR|GCS_RESULTCLAUSE);
> >       GenerateIMEMessage(imc, WM_IME_ENDCOMPOSITION, 0, 0);
> > - if (!inComp)
> > -
> >    ImmSetOpenStatus(imc, FALSE);
> > -
> >
> > myPrivate->bInComposition = FALSE;
> > -
> >
> > ImmSetOpenStatus(imc, FALSE);
> >
> > ImmUnlockIMC(imc);
> > }
>
> I'm not sure about this change.
> It always turns off IME open status. Since composition might be
> continuous at this moment, it can't be set unconditionally.
> If we should do so for some reasons, could you explain in detail?
> > diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h
> > index f389f3e0836..8ce3a7e8528 100644
> > --- a/dlls/winex11.drv/x11drv.h
> > +++ b/dlls/winex11.drv/x11drv.h
>
> [..]
> > --- a/dlls/winex11.drv/xim.c
> > +++ b/dlls/winex11.drv/xim.c
> > @@ -117,6 +117,19 @@ void X11DRV_XIMLookupChars( const char *str, DWORD count )
> >   IME_SetResultString(wcOutput, dwOutput);
> >   HeapFree(GetProcessHeap(), 0, wcOutput);
> > -
> > -
> >
> > /*
> > -
> > * In a special case for Hangul(Korean), when Composition is completed and
> > -
> > * this function is called, the new Composition disappears. so, calling
> > -
> > * IME_SetCompositionString again for makes composition will be
> > -
> > * starting.
> > -
> > */
> > -
> >
> > if (CompositionString && dwCompStringLength >= sizeof(WCHAR) &&
> > -
> >    IsHangul(((const WCHAR*) CompositionString)[0]))
> > -
> >
> > {
> > -
> >    IME_SetCompositionString(SCS_SETSTR, CompositionString,
> > -
> >                             dwCompStringLength, NULL, 0);
> > -
> >
> > }
> > }
> >
> > static BOOL XIMPreEditStateNotifyCallback(XIC xic, XPointer p, XPointer data)
>
> Could you revert Hangul(Korean) specific changes? This issue seems not
> to be Korean specific because I can occasionally reproduce it with
> Japanese input method.
>
> From my point of view, the main cause of this issue is that the order of
> PreeditCallbacks and XmbLookupString() aren't stable. The current wine's
> implementation seems to rely on getting the result of XmbLookupString()
> before PreeditCallback for the next clause. However, when one or more
> clauses are left after commiting a clause, PreeditCallback for the left
> clauses sometimes (or always in Hangul?) called before getting the
> result of XmbLookupString.
> What do you think about these XIM situations? Does your patch works
> well for both cases?
>
> Regards,
> Akihiro Sagawa