For Linux, KMFL (ibus) takes a sequence of keystrokes and converts them into a complex character. In Calc, when editing an existing line of text, the character AFTER the cursor is moved in front of the inserted complex character. For example typing ";n" produces "ŋ". If I am editing "Hello World" and have the cursor in front of W, the buggy end result will be "Hello Wŋorld" instead of "Hello ŋWorld" Ṭhe problem occurs in oldest-bibisect-43 (and in OpenOffice), so marking as pre-bibisect 3.5. An assert in the development version (dbggui.cxx:817ish) causes calc to exit when doing the "hello ŋWorld" test. I removed the assert and confirmed that this bug still exists in 5.0 development tree. A comment from potentially related bug 85912 might be helpful here: KMFL (and other inputs methods) need to be able to delete text that has been previously entered. There are couple of ways to do this, one being to use a system call like "delete surrounding text". Another method is to simulate the backspace key. The former method is the preferred method because it gives better control over what is deleted. However applications have to be written to support "surrounding text". Apps are supposed to report to ibus whether they support "surrounding text" calls and if they don't, then kmfl falls back to simulating the backspace key. There is a work around (confirmed that it also works for this bug). If you tell libreoffice to interface with ibus using xim rather than the "GTK ibus connector" it currently uses, then calc will "not assert". If you want to test it out to see if it works for you, try the following from a terminal: XMODIFIERS=@im=ibus GTK_IM_MODULE=xim libreoffice --writer instructions for installing kmfl can be found here: http://linux.lsdev.sil.org/wiki/index.php/Installing_KMFL_on_Ubuntu
Created attachment 116069 [details] definition file for kmfl - defines mapping for ";n"
patch submitted to avoid the assert: https://gerrit.libreoffice.org/15960 This is basically a stab in the dark. It works and is used by several other IM functions, but the concept of SolarMutex guards is a black box to me. The assert avoidance is only needed for development builds anyway, so not a critical patch for resolving this bug.
patch submitted to resolve the incorrect placement of the composed character: https://gerrit.libreoffice.org/15961
There is still an edge case that doesn't work properly. The very first cell that is edited with a call to IMDeleteSurrounding does not work properly. That is because it is not recognized as having the focus. Step1. Type 123456 in a cell. Step2. In the input line, position the cursor between 34 Step3. Enter a kmfl sequence, like ;;~a (for ä̃). the result is 123;;~ä̃456. This is a different problem from the reported problem. The cursor position is OK (because deleteText was not called), but the composing keystrokes are not swallowed up (also becase deleteText was not called). I have a patch submitted with resolves this: https://gerrit.libreoffice.org/15962 but I am not sure that this is the best way to fix it.
Justin Luth committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=8bf66416a834a503309d4d0f3298d65f321f360e tdf#91641 EditLine should have focus on accessibleText init. It will be available in 5.1.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Ho hum =) So - we can take a SolarMutex in these IM methods - if we are sure that ibus is not doing the right thing and there is no hope of fixing it there - but ... that's far less than ideal. Ideally the ibus input method plugin should be calling: GDK_THREADS_ENTER(); do_stuff(); GDK_THREADS_LEAVE(); in the right places. Where 'right' means - almost certainly in response to any dbus message - which will happen in an input handler from the main-loop. And 'wrong' means - you mustn't take this non-recursive mutex anywhere you're called by gtk+ itself (which clearly holds the toolkit lock while it is executing). Yes - there is some 'deprecated' nonsense about these methods - but there is basically no other way to achieve thread-safety for LibreOffice cf. /** * GDK_THREADS_ENTER: * * This macro marks the beginning of a critical section in which GDK and * GTK+ functions can be called safely and without causing race * conditions. Only one thread at a time can be in such a critial * section. The macro expands to a no-op if #G_THREADS_ENABLED has not * been defined. Typically gdk_threads_enter() should be used instead of * this macro. * * Deprecated:3.6: Use g_main_context_invoke(), g_idle_add() and related * functions if you need to schedule GTK+ calls from other threads. */ #define GDK_THREADS_ENTER() gdk_threads_enter() So - please do use those. For now - if we insist on banging our head futiley against the wall in an attempt to work around whatever bit of the brokenness that we happen to be able to see ;-) (which is only part of it - LibreOffice does use gtk+ from multiple threads so the only safe way is as above). Then yes we can add SolarMutexGuard aMutex; to each of the input-method methods. Lets have a typedef: IBusThreadingNeedsFixGuard aGuard; or something and a link to this bug next to the typedef; my hope would be to remove this later =) I'd like to see the complete set of input-methods instrumented if that's possible ? Does that sound sensible ? it's -really- awesome to have you working on this - many thanks for raising the issue, and submitting patches too =)
(In reply to Michael Meeks from comment #6) If the SolarMutex solution is not ideal, it can be ignored, since it only affects testing with the development builds. (I'm still just a newbie LibreOffice-only programmer trying to tackle bugs that specifically affect us, so I didn't really understand much of what you said)-:
Justin Luth committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=6c76a0a84785b48c29687e6d31b8229a230563d8 tdf#91641 adjust cursor when deleting preceding characters It will be available in 5.1.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Justin Luth committed a patch related to this issue. It has been pushed to "libreoffice-5-0": http://cgit.freedesktop.org/libreoffice/core/commit/?id=227e7f903fee64d63b48bd31707887ba28a019bc&h=libreoffice-5-0 tdf#91641 adjust cursor when deleting preceding characters It will be available in 5.0.0.3. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Justin Luth committed a patch related to this issue. It has been pushed to "libreoffice-4-4": http://cgit.freedesktop.org/libreoffice/core/commit/?id=6f4c31eb757cd19311b23ce06fdbbe9268f9cd16&h=libreoffice-4-4 tdf#91641 adjust cursor when deleting preceding characters It will be available in 4.4.5. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.