libsdl-org / SDL

Simple Directmedia Layer
https://libsdl.org
zlib License
9.97k stars 1.84k forks source link

IME_SendEditingEvent() always calls SDL_SendEditingText() with incorrect arguments #3384

Closed SDLBugzilla closed 1 year ago

SDLBugzilla commented 3 years ago

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: HG 2.0 Reported for operating system, platform: Windows 10, All

Comments on the original bug report:

On 2019-09-01 19:32:58 +0000, tamo wrote:

First, see events/SDL_keyboard_c.h, line 56-57:

/ Send editing text for selected range from start to end / extern int SDL_SendEditingText(const char *text, int start, int end);

But see video/windows/SDL_windowskeyboard.c, line 778:

SDL_SendEditingText(s, videodata->ime_cursor + (int)SDL_wcslen(videodata->ime_readingstring), 0);

The line should be:

SDL_SendEditingText(s, videodata->ime_cursor, (int)SDL_wcslen(videodata->ime_readingstring));

or

SDL_SendEditingText(s, len, (int)SDL_wcslen(videodata->ime_readingstring));

Otherwise, "start" and "end" are always incorrect.

-- Background: https://github.com/kivy/kivy/pull/6498

On 2019-09-01 20:02:50 +0000, tamo wrote:

Oops, "end" should be "len+wcslen(reading)".

SDL_SendEditingText(s, len, len + (int)SDL_wcslen(videodata->ime_readingstring));

On 2019-09-01 20:15:29 +0000, tamo wrote:

No, reading events/SDL_keyboard.c and others, the last arg is not "end" but "length." So events/SDL_keyboard.c is incorrect, too.

On 2019-09-01 20:24:24 +0000, tamo wrote:

Created attachment 3940 not "end" but "length", and fix "start" and "length"

Fix both header and code

On 2019-09-01 20:30:22 +0000, tamo wrote:

Created attachment 3941 fix scope

the previous patch didn't build

On 2019-09-01 20:32:21 +0000, tamo wrote:

Created attachment 3942 fix it with correct indent

fix indent

On 2019-09-02 07:31:09 +0000, tamo wrote:

Created attachment 3943 fix header and indent

The header is incorrect. "readingstring" is just a pronunciation, not selection.

In fact, Japanese IMEs hardly use readingstring. (I've never imagined that.)

On 2019-09-02 07:42:42 +0000, tamo wrote:

Created attachment 3944 picture of IME composition

What I really wanted are, things like GetCompositionTargetRange and GetImeTextSpans in https://github.com/chromium/chromium/blob/master/ui/base/ime/win/imm32_manager.cc code.

So, at least two numbers should be given to the event: target_start and target_end.

See the attached picure. The red range is the "target" and the yellow spans are "clauses" in IME terms. The target range is necessary for Japanese. (That's what I thought "readingstring" was.)

Anyway, that's another issue.

slouken commented 1 year ago

SDL 2.0 is now in maintenance mode, and all inactive issues are being closed. If this issue is impacting you, please feel free to reopen it with additional information.