FS#5972 - OSX Crash when deleting marked text on input

Attached to Project: OpenTTD
Opened by Zydeco (Zydeco) - Sunday, 06 April 2014, 16:49 GMT
Last edited by Michael Lutz (michi_cc) - Sunday, 24 August 2014, 10:35 GMT
Type Bug
Category Core
Status Closed
Assigned To No-one
Operating System Mac OS X
Severity Critical
Priority Normal
Reported Version 1.4.0
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


If you have a keyboard layout with dead keys to compose accented characters, when you press an accent key, the accent symbol will appear in the input box with a red background, waiting for you to press another key. If you hit the delete key, it will crash instead of deleting the marked input.

I'm submitting a patch that prevents the crash, but it doesn't handle it in the same way as other mac apps: the delete key will insert a normal accent character with no key, same as if you press accent+space, instead of deleting the marked accent character, which is what text input does elsewhere.

It also crashes a lot with other input methods that rely heavily on marked text, like Chinese or Japanese, although Chinese seems to crash less after this patch, and it does let you delete marked text from pinyin input.
This task depends upon

Closed by  Michael Lutz (michi_cc)
Sunday, 24 August 2014, 10:35 GMT
Reason for closing:  Fixed
Additional comments about closing:  In r26758 (if the gods are merciful).
Comment by Zydeco (Zydeco) - Sunday, 06 April 2014, 16:53 GMT
easy steps to reproduce the crash:
- open console
- type a dead key (´ on a spanish keyboard, or option-E on UK or US keyboard)
- ´ will appear in the console, marked with a dark red background
- type backspace
Comment by George Koehler (kernigh2) - Friday, 15 August 2014, 22:34 GMT
I can reproduce this crash on my Mac, but I haven't tried the patch yet.
Comment by frosch (frosch) - Saturday, 16 August 2014, 10:42 GMT
I guess DiscardMarkedText should be called instead of DeleteChar, but I do not know what to call in case of InsertChar resp. whether that could be called at all during the marked-state.
Comment by Michael Lutz (michi_cc) - Saturday, 16 August 2014, 17:48 GMT
According to my fake OSX 10.7 I think the attached patch might be the proper solution. It doesn't handle the delete key like in e.g. TextEditor, but it is not because OTTD wouldn't delete the accent, but because OSX sends us an explicit insertText message with the accent after the delete. No idea what other apps do different there, but then OTTD only implements the minimal subset and in no way all the functionality of the real Cocoa text GUI control.
Comment by George Koehler (kernigh2) - Sunday, 17 August 2014, 03:46 GMT
The patch by Michael Lutz prevents the crash on my Mac. (I run OS X 10.4.11, but with --disable-cocoa-quickdraw because of FS#6087.)

But this patch only works around the problem, without fixing its cause. OpenTTD handles the events in the wrong order. If I type a dead key and then backspace, the Cocoa driver in src/video/cocoa uses this order:
1. Call HandleKeypress() with the backspace. This calls Textbuf:HandleKeyPress().
2. Call HandleTextInput(NULL, true) to delete the marked text. This calls Textbuf::InsertString().
3. Call HandleTextInput(..., false, ...) to insert the completed text from the IME.

But src/textbuf.cpp expects the other order:
1. Delete the marked text.
2. Insert the completed text.
3. Handle the backspace.

This conflict between src/video/cocoa and src/textbuf.cpp happens not only with backspace, but also with the left and right arrow keys. I guess src/video/cocoa calls HandleKeypress() and HandleTextInput() in the wrong order. I don't have a patch yet, but one might reverse the order if one would pass the backspace or arrow to the IME, and respond to NSTextInput messages (setMarkedText or insertText), before passing the same backspace or arrow to HandleKeypress().

fs5972.patch also has extra code to delete a character from the marked text. This is confusing (because the marked text no longer equals the text from the IME) but also meaningless (because the Cocoa driver is about to delete the whole marked text). So I am attaching a much simpler patch, just-ignore-keys.patch, which steals the work-around idea from fs5972.patch, but without the extra code.
Comment by Michael Lutz (michi_cc) - Sunday, 17 August 2014, 15:45 GMT
Simply ignoring the extra keys won't work as for example the Japanese IME relies on cursor movement.

The more or less "proper" alternative seems to be the lot longer patch attached. This implements more of the text editing API to handle what our edit box can do. Now deletion of an accent actually behaves as in TextEdit, and any IME that synthesizes cursor movement would work as well.
Comment by George Koehler (kernigh2) - Sunday, 17 August 2014, 18:39 GMT
Your patch is "complicated" but awesome! It works for me.

(I tried playing with the Mac's Japanese IME, too, but the hiragana and kanji display as question marks in OpenTTD. I might be using the wrong font or encoding.)
Comment by Michael Lutz (michi_cc) - Monday, 18 August 2014, 17:03 GMT
The default sprite font only has latin glyphs. To get a font with asian glyphs you either have to specify a font in openttd.cfg or switch the game to e.g. Japanese (and test your knowledge of OTTD's GUI :)