We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 3006

Summary: SDL_SendEditingText and SDL_SendKeyboardText cut off IME input more than 32 bytes.
Product: SDL Reporter: Hak Matsuda <hakuro>
Component: eventsAssignee: Sam Lantinga <slouken>
Status: ASSIGNED --- QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2 CC: alex, hqm03ster, icculus, ttakah+sdl, xenotron007
Version: 2.0.3   
Hardware: x86   
OS: Other   
Attachments: patch to implement SendEditEX for 3006, 3421, 4782, and 4783 for windows and fcitx (not yet for ibus)
patch to implement SendEditEX for bugs 3006, 3421, 4782, and 4783 on windows and fcitx (not yet for ibus)

Description Hak Matsuda 2015-06-10 03:03:46 UTC
In SDL_SendEditingText and SDL_SendKeyboardText, it cut off an input string from IME more than 32 bytes via SDL_utf8strlcpy().
The size is hard limit for Asian IMEs since it can easily generates strings more than 32 bytes.

Rather than just cutting off a remaining bytes, having an API to retrieve a rest of 
string is an ideal.
Comment 1 Sam Lantinga 2015-06-15 02:36:09 UTC
How big are the strings likely to be?
One solution to this is to have the application free those strings. Another solution would be to keep some number of them cached and valid, but that could cause an application to crash or see the wrong text if the events are not handled for a long time.
Comment 2 Hak Matsuda 2015-06-15 03:35:24 UTC
In case of Japanese, it's likely in a range of 64 bytes ~ 256 bytes in most of use cases (I would imagine it's similar in other languages that has a context based character transform mechanism).

I would prefer former option rather than to have a chance to lose them.
Comment 3 Ryan C. Gordon 2015-06-16 04:48:20 UTC
(In reply to Sam Lantinga from comment #1)
> How big are the strings likely to be?
> One solution to this is to have the application free those strings. Another
> solution would be to keep some number of them cached and valid, but that
> could cause an application to crash or see the wrong text if the events are
> not handled for a long time.

I haven't looked, but could we just split them up into multiple TEXTINPUT events? That would keep the existing API/ABI intact.

--ryan.
Comment 4 Sam Lantinga 2015-06-16 06:32:30 UTC
SendKeyboardText could, but the editing text is an in-place temporary string and each event replaces the previous text.
Comment 5 Hak Matsuda 2015-06-16 18:13:36 UTC
If it's easier to fix, adding new definition of SDL_TEXTEDITING_EXTRA to send extra bytes (multiple times depending on the length) in addition to existing message and let the receiver to concatenate them, would work?
Comment 6 Alex Baines 2015-09-29 18:29:51 UTC
In the iBus code, I worked around this by sending multiple input / editing events in a row if the text is more than 32 bytes.

You'd have to concatenate all the editing events that were received in a single PumpEvents / PollEvent loop to get the full replacement editing string.

Not sure that is the best approach though.
Comment 7 Sam Lantinga 2016-10-08 00:53:01 UTC
*** Bug 2857 has been marked as a duplicate of this bug. ***
Comment 8 hqm03ster 2016-10-08 03:41:01 UTC
I think one could just add an SDL_TEXTEDITING_EX event which appends text to those sent by SDL_TEXTEDITING and updates the cursor accordingly. That way, existing applications could just ignore that and work with the old clamped text.
Comment 9 Sam Lantinga 2016-10-08 08:36:02 UTC
We could also do the same thing that we do for drag-and-drop events, where we create a new event that's disabled by default, and the text is dynamically allocated and needs to be freed by the code processing the event.
Comment 10 xenotron007 2019-02-08 02:10:26 UTC
Is there a consensus regarding the solution?

This latter proposal is probably the best option. If we have to add new events anyway (due to binary compatibility) and have to adjust the app code to handle them then we shouldn't make our lives more difficult by creating new events that have to cooperate with the old ones.

How to allow the opt-in? A function call (like SDL_EnableTextInput2()) that turns all SDL_TEXTEDITING and SDL_TEXTINPUT events into their v2 form (SDL_TEXTEDITING_2 and SDL_TEXTINPUT_2?) or an alternative like an SDL_StartTextInput2() function so different parts of the codebase can choose from v1 or v2 events by calling either SDL_StartTextInput() or SDL_StartTextInput2(). That way older parts of the app could still use the old SDL_StartTextInput() and work with the older events.

Having both versions of the events in the same application might add more trouble than benefit due to the centralised nature of event handling in a lot of SDL apps so having a global switch is probably a safer choice.
Comment 11 tamo 2019-09-01 19:35:57 UTC
*** Bug 4774 has been marked as a duplicate of this bug. ***
Comment 12 tamo 2019-09-01 19:39:01 UTC
(In reply to tamo from comment #11)
> *** Bug 4774 has been marked as a duplicate of this bug. ***

But 4774 has a buildable patch to malloc the text (and thus needs freeing).
Comment 13 tamo 2019-09-02 14:43:49 UTC
If you should add another event to push extra information on editing event, please consider separating readingstring from composition (editing) text.

Currently SDL puts readingstring into composition text at cursor's position.
http://hg.libsdl.org/SDL/file/08db6a6f6c23/src/video/windows/SDL_windowskeyboard.c#l771
But they are different things. I don't think they should be mixed.

For example, target range, which I suggested you add to the event in bug 4783, refers to raw composition text, not the mixed text. If readingstring were a separate field, the patch for bug 4783 would have been a little simpler.

Composition is composition, reading is reading. I hope they will be provided in their whole length, as individual variables.
Comment 14 tamo 2019-09-08 03:41:06 UTC
Created attachment 3958 [details]
patch to implement SendEditEX for 3006, 3421, 4782, and 4783 for windows and fcitx (not yet for ibus)

(In reply to Alex Baines from comment #6)
> In the iBus code, I worked around this by sending multiple input / editing
> events in a row if the text is more than 32 bytes.

As mentioned in comment #4, the current SendEditingText implementations for ibus and fcitx are broken. They try to iterate a long editing text, but it doesn't work.

I wrote an editx for fcitx.

Although this patch includes some lines for ibus, they must be broken, bogus, because I haven't successfully set up ibus on my WSL ubuntu and I haven't tested them at all.
Comment 15 tamo 2019-09-10 01:06:26 UTC
Created attachment 3962 [details]
patch to implement SendEditEX for bugs 3006, 3421, 4782, and 4783 on windows and fcitx (not yet for ibus)

(In reply to tamo from comment #14)
> I wrote an editx for fcitx.

I made it a little more readable.
I tested it with testime. (This patch modifies it so that it underlines the target text. Also, space key is disabled because fcitx uses Ctrl+space to trigger IME. Press backspace to exit.)

The following statement is still true.

> Although this patch includes some lines for ibus, they must be broken,
> bogus, because I haven't successfully set up ibus on my WSL ubuntu and I
> haven't tested them at all.