Bug 85912 - ibus keyboarding solution - kmfl characters are not "swallowed" after completing the complex letter
Summary: ibus keyboarding solution - kmfl characters are not "swallowed" after complet...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
4.3.0.4 release
Hardware: Other Linux (All)
: medium normal
Assignee: Justin L
URL:
Whiteboard: target:4.4.0 target:4.3.5
Keywords: bibisected
Depends on:
Blocks:
 
Reported: 2014-11-05 10:34 UTC by Justin L
Modified: 2022-06-15 09:10 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Justin L 2014-11-05 10:34:39 UTC
We use KMFL (ibus) to type complex characters.  So, for example, the sequence "_;]" forms the letter "ɔ̱̈".   In LO 4.2.x, the three characters are "swallowed up" and replaced with the composed character.   In LO 4.3, only the last  character is swallowed up.   

Testing system:  Linux 13 Mint and LibreOffice 4.3.3.2.  Writer and Impress have the problem, but not Calc.

Linux 17 Mint with LO 4.3.3.2 does not exhibit the problem. 


 84b7f2bee1466b2b42cb32254abdf132dd20ed3c is the first bad commit
commit 84b7f2bee1466b2b42cb32254abdf132dd20ed3c
Author: Bjoern Michaelsen <bjoern.michaelsen@canonical.com>
Date:   Sat May 10 18:48:06 2014 +0000

    source-hash-362c8d67e1cb8920bf179b52c50b5997d32eb296
    
    commit 362c8d67e1cb8920bf179b52c50b5997d32eb296
    Author:     Pavel Janík <paveljanik@apache.org>
    AuthorDate: Tue Nov 26 20:36:34 2013 +0000
    Commit:     Caolán McNamara <caolanm@redhat.com>
    CommitDate: Mon Dec 2 10:27:09 2013 +0000
    
        WaE: compare unsigned values.
    
        (cherry picked from commit e215b94aea58527bf76db44f0985b467502d457b)

:100644 100644 5f1aa382121586f11afa19c011c4cd7426832998 5b641e0949034a17ebe37fcc5721bb8d681f918a M	ccache.log
:100644 100644 86509514b04fc0752ef874d90cb1d49af99be5fd d58a1fda1b6443fe603edcf1a5852106250dbb98 M	commitmsg
:100644 100644 a486bb4138808f28bde440febdff4f6491401607 cd43b5ba40bab2031582507c959155e323fe54a5 M	make.log
:040000 040000 9e1397d961b0dbc4692c41e5d120f26fe681132c 0bd8f3f42aca7a170c71b0cf68638adf30a4cae4 M	opt

git bisect log
# bad: [423a84c4f7068853974887d98442bc2a2d0cc91b] source-hash-c15927f20d4727c3b8de68497b6949e72f9e6e9e
# good: [752769ad0d2179e17ea0a08cc9004df7b890305b] source-hash-60c64b437c6678dd1d3fa3a6fc2b7da0480890d4
git bisect start 'latest' 'last42onmaster'
# bad: [4fcd68ce4979f85fda4568f4b419a4b41d07345f] source-hash-2c4621c87ed3a7b19de195c21494c9a381e72b2e
git bisect bad 4fcd68ce4979f85fda4568f4b419a4b41d07345f
# bad: [0d4c20a601a3cfff27d6685d0e81463086bd9d74] source-hash-f1b1e73227471192682d303a58618ca8bd65a74d
git bisect bad 0d4c20a601a3cfff27d6685d0e81463086bd9d74
# skip: [18ee045c7e35e5ae98cffaafd56fb6fb37d7afcf] source-hash-fe506f34f2dccb6562935fe4dfbc1fe6d609dec8
git bisect skip 18ee045c7e35e5ae98cffaafd56fb6fb37d7afcf
# bad: [9fe7b44f1975d64e3009c31341187c53c8e3a2b8] source-hash-7f5494f3c4bf14209a119c6b21c02e10075503ae
git bisect bad 9fe7b44f1975d64e3009c31341187c53c8e3a2b8
# skip: [c2e967de61ee4b1291703af64dac729a8ada79fb] source-hash-17dab5bf8efb3fd676e6854474b199b681d0dc28
git bisect skip c2e967de61ee4b1291703af64dac729a8ada79fb
# skip: [c2e967de61ee4b1291703af64dac729a8ada79fb] source-hash-17dab5bf8efb3fd676e6854474b199b681d0dc28
git bisect skip c2e967de61ee4b1291703af64dac729a8ada79fb
# skip: [92abe3d8366d452263401a2a61d6d7fa894cbfbd] source-hash-9c2a085a45f9acb305fb85367bede360982e71cb
git bisect skip 92abe3d8366d452263401a2a61d6d7fa894cbfbd
# good: [3f7dffadbdabcc8730fd19598afa9f5f70dca5b5] source-hash-2abcff25137c7c9af007554c97a4512319ec2e4d
git bisect good 3f7dffadbdabcc8730fd19598afa9f5f70dca5b5
# good: [3f7dffadbdabcc8730fd19598afa9f5f70dca5b5] source-hash-2abcff25137c7c9af007554c97a4512319ec2e4d
git bisect good 3f7dffadbdabcc8730fd19598afa9f5f70dca5b5
# skip: [bb138a397a1f29902ce6c69bb161254832081623] source-hash-acf233e2bb259759c9167a32e17fcf13e1f54f6c
git bisect skip bb138a397a1f29902ce6c69bb161254832081623
# bad: [306d62ec4b911895f08f2bb8efefebed7ac795f0] source-hash-735bd120c9ee2d9bb3514907936c27efb75d7282
git bisect bad 306d62ec4b911895f08f2bb8efefebed7ac795f0
# bad: [306d62ec4b911895f08f2bb8efefebed7ac795f0] source-hash-735bd120c9ee2d9bb3514907936c27efb75d7282
git bisect bad 306d62ec4b911895f08f2bb8efefebed7ac795f0
# bad: [159e65f9cccfc8e4774d7a60f00876c07fc7c82a] source-hash-a0be5278c24efcc9a6f22fe5398d780b0744f8ce
git bisect bad 159e65f9cccfc8e4774d7a60f00876c07fc7c82a
# bad: [159e65f9cccfc8e4774d7a60f00876c07fc7c82a] source-hash-a0be5278c24efcc9a6f22fe5398d780b0744f8ce
git bisect bad 159e65f9cccfc8e4774d7a60f00876c07fc7c82a
# good: [277ceb260ef1f2482baf3c86e9df2787d318b957] source-hash-4f94f16ba15218e5e7a9eb4d72ddb4cb62884dbb
git bisect good 277ceb260ef1f2482baf3c86e9df2787d318b957
# good: [30cde618212ecaf5725321372bd1b8339f8e2b9f] source-hash-137f872aa8e6e598e7c7ed1ffa4d21e580e22bdb
git bisect good 30cde618212ecaf5725321372bd1b8339f8e2b9f
# bad: [76a4a4db1a4efbead04287a436e59006505bc705] source-hash-5b03bc8a4d92fee2fdfdca4917b321985feb930a
git bisect bad 76a4a4db1a4efbead04287a436e59006505bc705
# bad: [76a4a4db1a4efbead04287a436e59006505bc705] source-hash-5b03bc8a4d92fee2fdfdca4917b321985feb930a
git bisect bad 76a4a4db1a4efbead04287a436e59006505bc705
# bad: [84b7f2bee1466b2b42cb32254abdf132dd20ed3c] source-hash-362c8d67e1cb8920bf179b52c50b5997d32eb296
git bisect bad 84b7f2bee1466b2b42cb32254abdf132dd20ed3c


I expect the bug occurred in "Integrate branch of IAccessible2" 
http://cgit.freedesktop.org/libreoffice/core/commit/?id=76c549eb01dcb7b5bf28a271ce00e386f3d388ba
Comment 1 Justin L 2014-11-05 18:26:06 UTC
Confirmed that the problem also exists in Ubuntu 12.04.  

Confirmed that the break is in:

author	Steve Yin <steve_y@apache.org>	2013-11-29 13:03:27 (GMT)
committer	Caolán McNamara <caolanm@redhat.com>	2013-12-02 10:25:33 (GMT)
commit 76c549eb01dcb7b5bf28a271ce00e386f3d388ba
Comment 2 Doug Rintoul 2014-11-05 19:22:17 UTC
This is similar to a bug in Chrome.

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. Unfortunately, Libreoffice 4.3 reports erroneously that it supports  "surrounding text" when in fact, it doesn't.

There is a work around. If you tell libreoffice to interface with ibus using xim rather than the "GTK ibus connector" it currently uses, then kmfl will delete characters using simulated backspaces. 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

If that works, then you can create a launcher for google-chrome with the following as the Command:

env XMODIFIERS=@im=ibus GTK_IM_MODULE=xim libreoffice --writer %U

calc and impress have similar issues. Note that writer 4.2.x did not have this issue with writer.
Comment 3 Justin L 2014-11-07 08:15:18 UTC
I'm pretty sure the bug is in the changes made to sw/source/core/access/accpara.cxx

Marking as confirmed based on Doug's comments, and on emails I have received since posting this bug.
Comment 4 Justin L 2014-11-07 15:26:16 UTC
No problem with Windows 2003, keyman and LO 4.3.3.2

The problem comes from the removal of this line:

    // If this object has the focus, then it is remembered by the map itself.
    nOldCaretPos = GetCaretPos();

SwAccessibleParagraph::SwAccessibleParagraph(
        SwAccessibleMap& rInitMap,
        const SwTxtFrm& rTxtFrm )
    : SwClient( const_cast<SwTxtNode*>(rTxtFrm.GetTxtNode()) ) // #i108125#
    , SwAccessibleContext( &rInitMap, AccessibleRole::PARAGRAPH, &rTxtFrm )
    , sDesc()
    , pPortionData( NULL )
    , pHyperTextData( NULL )
    , nOldCaretPos( -1 )
    , bIsHeading( sal_False )
    , aSelectionHelper( *this )
    , mpParaChangeTrackInfo( new SwParaChangeTrackingInfo( rTxtFrm ) ) // #i108125#
{
    SolarMutexGuard aGuard;

    bIsHeading = IsHeading();
    SetName( OUString() ); // set an empty accessibility name for paragraphs

    // If this object has the focus, then it is remembered by the map itself.
    nOldCaretPos = GetCaretPos();
}
Comment 5 Justin L 2014-11-07 19:41:19 UTC
Reviewed OpenOffice code as well.  The missing lines exist (but are commented out) in the current AOO code.  The commenting out occurred when Steven Ying implemented his huge "AOO IA2 enabled draft version 1" change on 27 Sep 2013.  https://github.com/apache/openoffice/commit/0deba7fbda3d9908785c25a443701a293b6f4e71#diff-50d752f41bb880abd1094d09d9e1a7fc

The current AOO comment is 
// If this object has the focus, then it is remembered by the map itself.
// not necessary to remember this pos here. Generally, the pos will be updated in invalidateXXX method, which may fire the
//Focus event based on the difference of new & old caret pos.
//nOldCaretPos = GetCaretPos();

Since this is in a call to the constructor, I see no reason not to re-implement this. There doesn't seem to be a good reason for why it was removed in the first place.
Comment 6 Justin L 2014-11-08 16:52:20 UTC
Confirmed that the bug also exists in Apache OpenOffice, and that uncommenting the one line fixes that one too.

Bug-fix submitted for review:  https://gerrit.libreoffice.org/#/c/12311
Comment 7 Commit Notification 2014-11-11 16:41:18 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=817da76529aa39f641d76805d429b09681348811

fdo#85912 Delete surrounding text failing for input method calls regression.

It will be available in 4.4.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.
Comment 8 Justin L 2014-11-11 17:28:40 UTC
Cherry-picked for 4.3 branch
https://gerrit.libreoffice.org/#/c/12365/
Comment 9 Justin L 2014-11-12 12:22:01 UTC
accepted into the 4.3 branch.   Should be available with 4.3.5 in Dec 2014.
Comment 11 Robinson Tryon (qubit) 2015-12-17 08:38:47 UTC Comment hidden (obsolete)
Comment 12 Michael Weghorn 2022-06-15 09:10:03 UTC
(In reply to Commit Notification from comment #7)
> Justin Luth committed a patch related to this issue.
> It has been pushed to "master":
> 
> http://cgit.freedesktop.org/libreoffice/core/commit/
> ?id=817da76529aa39f641d76805d429b09681348811
> 
> fdo#85912 Delete surrounding text failing for input method calls regression.
> 
> It will be available in 4.4.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.

I have a pending Gerrit change essentially reverting that commit at https://gerrit.libreoffice.org/c/core/+/135868/1 because it causes issues with a11y and I'm unable the reproduce the original issue described here, s. commit message in that change for details. Any input appreciated.