Related: tdf#108795 a shared SvxAutoCorrect instance can not be stateful

An SvxAutoCorrect instance is shared via SvxAutoCorrCfg::Get().GetAutoCorrect().
Since commit 284eb106767d094fc5c547efd6c11cc390e3538a and following the
SvxAutoCorrect::bRunNext/HasRunNext() introduced a state whether a previously
inserted NO-BREAK SPACE should be removed again, depending on the next
character input. That does not work, for example, if
SvxAutoCorrect::DoAutoCorrect() is called from two different EditEngine
instances, like it is the case in the Calc input line and cell which are
synchronized; or any other two or more instances for that matter. The caller
has to pass and remember a flag that is maintained by SvxAutoCorrect.

Change-Id: I79a26d2ba44cc40771979a78b686c89f0c80b412
Reviewed-on: https://gerrit.libreoffice.org/41475
Reviewed-by: Eike Rathke <erack@redhat.com>
Tested-by: Jenkins <ci@libreoffice.org>

10 files changed, 60 insertions, 35 deletions

diff --git a/editeng/qa/unit/core-test.cxx b/editeng/qa/unit/core-test.cxx
index f806a056ff26..6345473d4257 100644
--- a/editeng/qa/unit/core-test.cxx
+++ b/editeng/qa/unit/core-test.cxx
@@ -353,9 +353,10 @@ void Test::testAutocorrect()
         OUString sInput("TEst-TEst");
         sal_Unicode const cNextChar(' ');
         OUString const sExpected("Test-Test ");
+        bool bNbspRunNext = false;
 
         TestAutoCorrDoc aFoo(sInput, LANGUAGE_ENGLISH_US);
-        aAutoCorrect.DoAutoCorrect(aFoo, sInput, sInput.getLength(), cNextChar, true);
+        aAutoCorrect.DoAutoCorrect(aFoo, sInput, sInput.getLength(), cNextChar, true, bNbspRunNext);
 
         CPPUNIT_ASSERT_EQUAL_MESSAGE("autocorrect", sExpected, aFoo.getResult());
     }
@@ -364,9 +365,10 @@ void Test::testAutocorrect()
         OUString sInput("TEst/TEst");
         sal_Unicode const cNextChar(' ');
         OUString const sExpected("Test/Test ");
+        bool bNbspRunNext = false;
 
         TestAutoCorrDoc aFoo(sInput, LANGUAGE_ENGLISH_US);
-        aAutoCorrect.DoAutoCorrect(aFoo, sInput, sInput.getLength(), cNextChar, true);
+        aAutoCorrect.DoAutoCorrect(aFoo, sInput, sInput.getLength(), cNextChar, true, bNbspRunNext);
 
         CPPUNIT_ASSERT_EQUAL_MESSAGE("autocorrect", sExpected, aFoo.getResult());
     }
@@ -376,9 +378,10 @@ void Test::testAutocorrect()
         OUString sInput("*foo");
         sal_Unicode const cNextChar('*');
         OUString const sExpected("foo");
+        bool bNbspRunNext = false;
 
         TestAutoCorrDoc aFoo(sInput, LANGUAGE_ENGLISH_US);
-        aAutoCorrect.DoAutoCorrect(aFoo, sInput, sInput.getLength(), cNextChar, true);
+        aAutoCorrect.DoAutoCorrect(aFoo, sInput, sInput.getLength(), cNextChar, true, bNbspRunNext);
 
         CPPUNIT_ASSERT_EQUAL(sExpected, aFoo.getResult());
     }
@@ -387,9 +390,10 @@ void Test::testAutocorrect()
         OUString sInput("Test. test");
         sal_Unicode const cNextChar(' ');
         OUString const sExpected("Test. Test ");
+        bool bNbspRunNext = false;
 
         TestAutoCorrDoc aFoo(sInput, LANGUAGE_ENGLISH_US);
-        aAutoCorrect.DoAutoCorrect(aFoo, sInput, sInput.getLength(), cNextChar, true);
+        aAutoCorrect.DoAutoCorrect(aFoo, sInput, sInput.getLength(), cNextChar, true, bNbspRunNext);
 
         CPPUNIT_ASSERT_EQUAL_MESSAGE("autocorrect", sExpected, aFoo.getResult());
     }
@@ -399,9 +403,10 @@ void Test::testAutocorrect()
         OUString sInput("Test. \x01 test");
         sal_Unicode const cNextChar(' ');
         OUString const sExpected("Test. \x01 test ");
+        bool bNbspRunNext = false;
 
         TestAutoCorrDoc aFoo(sInput, LANGUAGE_ENGLISH_US);
-        aAutoCorrect.DoAutoCorrect(aFoo, sInput, sInput.getLength(), cNextChar, true);
+        aAutoCorrect.DoAutoCorrect(aFoo, sInput, sInput.getLength(), cNextChar, true, bNbspRunNext);
 
         CPPUNIT_ASSERT_EQUAL_MESSAGE("autocorrect", sExpected, aFoo.getResult());
     }
@@ -412,10 +417,11 @@ void Test::testAutocorrect()
         sal_Unicode const cNextChar('"');
         const sal_Unicode EXPECTED[] = { 'T', 0x01, 0x0201d };
         OUString sExpected(EXPECTED, SAL_N_ELEMENTS(EXPECTED));
+        bool bNbspRunNext = false;
 
         TestAutoCorrDoc aFoo(sInput, LANGUAGE_ENGLISH_US);
         aAutoCorrect.SetAutoCorrFlag(ChgQuotes, true);
-        aAutoCorrect.DoAutoCorrect(aFoo, sInput, sInput.getLength(), cNextChar, true);
+        aAutoCorrect.DoAutoCorrect(aFoo, sInput, sInput.getLength(), cNextChar, true, bNbspRunNext);
         fprintf(stderr, "text is %x\n", aFoo.getResult()[aFoo.getResult().getLength() - 1]);
 
         CPPUNIT_ASSERT_EQUAL_MESSAGE("autocorrect", sExpected, aFoo.getResult());
diff --git a/editeng/source/editeng/editeng.cxx b/editeng/source/editeng/editeng.cxx
index ed915b09a15f..287d219712ed 100644
--- a/editeng/source/editeng/editeng.cxx
+++ b/editeng/source/editeng/editeng.cxx
@@ -1270,10 +1270,9 @@ bool EditEngine::PostKeyEvent( const KeyEvent& rKeyEvent, EditView* pEditView, v
                     sal_Unicode nCharCode = rKeyEvent.GetCharCode();
                     pEditView->pImpEditView->DrawSelectionXOR();
                     // Autocorrection?
-                    SvxAutoCorrect* pAutoCorrect = SvxAutoCorrCfg::Get().GetAutoCorrect();
                     if ( ( pImpEditEngine->GetStatus().DoAutoCorrect() ) &&
                         ( SvxAutoCorrect::IsAutoCorrectChar( nCharCode ) ||
-                          pAutoCorrect->HasRunNext() ) )
+                          pImpEditEngine->IsNbspRunNext() ) )
                     {
                         aCurSel = pImpEditEngine->AutoCorrect(
                             aCurSel, nCharCode, !pEditView->IsInsertMode(), pFrameWin );
diff --git a/editeng/source/editeng/impedit.hxx b/editeng/source/editeng/impedit.hxx
index 9bcab368ee80..9cf5de4e22ab 100644
--- a/editeng/source/editeng/impedit.hxx
+++ b/editeng/source/editeng/impedit.hxx
@@ -538,6 +538,8 @@ private:
     bool            mbLastTryMerge:1;
     bool            mbReplaceLeadingSingleQuotationMark:1;
 
+    bool            mbNbspRunNext;  // can't be a bitfield as it is passed as bool&
+
 
     // Methods...
 
@@ -1053,6 +1055,10 @@ public:
         mark (apostrophe) or not (default is on) */
     void            SetReplaceLeadingSingleQuotationMark( bool bReplace ) { mbReplaceLeadingSingleQuotationMark = bReplace; }
     bool            IsReplaceLeadingSingleQuotationMark() const { return mbReplaceLeadingSingleQuotationMark; }
+
+    /** Whether last AutoCorrect inserted a NO-BREAK SPACE that may need to be removed again. */
+    bool            IsNbspRunNext() const { return mbNbspRunNext; }
+
     void Dispose();
 };
 
diff --git a/editeng/source/editeng/impedit2.cxx b/editeng/source/editeng/impedit2.cxx
index eaec3beb77bd..b31066b8e660 100644
--- a/editeng/source/editeng/impedit2.cxx
+++ b/editeng/source/editeng/impedit2.cxx
@@ -107,7 +107,8 @@ ImpEditEngine::ImpEditEngine( EditEngine* pEE, SfxItemPool* pItemPool ) :
     bCallParaInsertedOrDeleted(false),
     bFirstWordCapitalization(true),
     mbLastTryMerge(false),
-    mbReplaceLeadingSingleQuotationMark(true)
+    mbReplaceLeadingSingleQuotationMark(true),
+    mbNbspRunNext(false)
 {
     pEditEngine         = pEE;
     pRefDev             = nullptr;
@@ -2571,7 +2572,7 @@ EditPaM ImpEditEngine::AutoCorrect( const EditSelection& rCurSel, sal_Unicode c,
         // FIXME: this _must_ be called with reference to the actual node text!
         OUString const& rNodeString(pNode->GetString());
         pAutoCorrect->DoAutoCorrect(
-            aAuto, rNodeString, nIndex, c, !bOverwrite, pFrameWin );
+            aAuto, rNodeString, nIndex, c, !bOverwrite, mbNbspRunNext, pFrameWin );
         aSel.Max().SetIndex( aAuto.GetCursor() );
 
         // #i78661 since the SvxAutoCorrect object used here is
diff --git a/editeng/source/misc/svxacorr.cxx b/editeng/source/misc/svxacorr.cxx
index d97961831540..51571e1af1ca 100644
--- a/editeng/source/misc/svxacorr.cxx
+++ b/editeng/source/misc/svxacorr.cxx
@@ -296,7 +296,6 @@ SvxAutoCorrect::SvxAutoCorrect( const OUString& rShareAutocorrFile,
                                 const OUString& rUserAutocorrFile )
     : sShareAutoCorrFile( rShareAutocorrFile )
     , sUserAutoCorrFile( rUserAutocorrFile )
-    , bRunNext( false )
     , eCharClassLang( LANGUAGE_DONTKNOW )
     , nFlags(SvxAutoCorrect::GetDefaultFlags())
     , cStartDQuote( 0 )
@@ -312,7 +311,6 @@ SvxAutoCorrect::SvxAutoCorrect( const SvxAutoCorrect& rCpy )
     : sShareAutoCorrFile( rCpy.sShareAutoCorrFile )
     , sUserAutoCorrFile( rCpy.sUserAutoCorrFile )
     , aSwFlags( rCpy.aSwFlags )
-    , bRunNext( false )
     , eCharClassLang(rCpy.eCharClassLang)
     , nFlags( rCpy.nFlags & ~(ChgWordLstLoad|CplSttLstLoad|WrdSttLstLoad))
     , cStartDQuote( rCpy.cStartDQuote )
@@ -630,7 +628,7 @@ bool SvxAutoCorrect::FnChgToEnEmDash(
 bool SvxAutoCorrect::FnAddNonBrkSpace(
                                 SvxAutoCorrDoc& rDoc, const OUString& rTxt,
                                 sal_Int32 nEndPos,
-                                LanguageType eLang )
+                                LanguageType eLang, bool& io_bNbspRunNext )
 {
     bool bRet = false;
 
@@ -689,11 +687,11 @@ bool SvxAutoCorrect::FnAddNonBrkSpace(
                     // Add the non-breaking space at the end pos
                     if ( bHasSpace )
                         rDoc.Insert( nPos, OUString(cNonBreakingSpace) );
-                    bRunNext = true;
+                    io_bNbspRunNext = true;
                     bRet = true;
                 }
                 else if ( chars.indexOf( cPrevChar ) != -1 )
-                    bRunNext = true;
+                    io_bNbspRunNext = true;
             }
         }
         else if ( cChar == '/' && nEndPos > 1 && rTxt.getLength() > (nEndPos - 1) )
@@ -1234,10 +1232,10 @@ OUString SvxAutoCorrect::GetQuote( SvxAutoCorrDoc const & rDoc, sal_Int32 nInsPo
 
 void SvxAutoCorrect::DoAutoCorrect( SvxAutoCorrDoc& rDoc, const OUString& rTxt,
                                     sal_Int32 nInsPos, sal_Unicode cChar,
-                                    bool bInsert, vcl::Window const * pFrameWin )
+                                    bool bInsert, bool& io_bNbspRunNext, vcl::Window const * pFrameWin )
 {
-    bool bIsNextRun = bRunNext;
-    bRunNext = false;  // if it was set, then it has to be turned off
+    bool bIsNextRun = io_bNbspRunNext;
+    io_bNbspRunNext = false;  // if it was set, then it has to be turned off
 
     do{                                 // only for middle check loop !!
         if( cChar )
@@ -1276,7 +1274,7 @@ void SvxAutoCorrect::DoAutoCorrect( SvxAutoCorrDoc& rDoc, const OUString& rTxt,
             if ( IsAutoCorrFlag( AddNonBrkSpace ) )
             {
                 if ( NeedsHardspaceAutocorr( cChar ) &&
-                    FnAddNonBrkSpace( rDoc, rTxt, nInsPos, rDoc.GetLanguage( nInsPos ) ) )
+                    FnAddNonBrkSpace( rDoc, rTxt, nInsPos, rDoc.GetLanguage( nInsPos ), io_bNbspRunNext ) )
                 {
                     ;
                 }
diff --git a/include/editeng/svxacorr.hxx b/include/editeng/svxacorr.hxx
index 1f8e31287f7e..a7d03a9a971a 100644
--- a/include/editeng/svxacorr.hxx
+++ b/include/editeng/svxacorr.hxx
@@ -236,8 +236,6 @@ class EDITENG_DLLPUBLIC SvxAutoCorrect
     std::map<LanguageTag, sal_Int64> aLastFileTable;
     std::unique_ptr<CharClass> pCharClass;
 
-    bool bRunNext;
-
     LanguageType eCharClassLang;
 
     long nFlags;
@@ -274,13 +272,19 @@ public:
     SvxAutoCorrect( const SvxAutoCorrect& );
     virtual ~SvxAutoCorrect();
 
-    // Execute an AutoCorrect.
-    // Returns what has been executed, according to the above flags
+    /** Execute an AutoCorrect.
+        Returns what has been executed, according to the above auto correct flags.
+        @param  io_bNbspRunNext
+                Remembers if a NO-BREAK SPACE was added (eg. in "fr" language)
+                (set to <TRUE/>) at the last character input that may have to
+                be removed again depending on what character is following.
+     */
     // FIXME: this has the horrible flaw that the rTxt must be a reference
     // to the actual SwTxtNode/EditNode string because it inserts the character
     // in rDoc and expects that to side-effect rTxt
     void DoAutoCorrect( SvxAutoCorrDoc& rDoc, const OUString& rTxt,
-                           sal_Int32 nPos, sal_Unicode cInsChar, bool bInsert, vcl::Window const * pFrameWin = nullptr );
+                           sal_Int32 nPos, sal_Unicode cInsChar, bool bInsert, bool& io_bNbspRunNext,
+                           vcl::Window const * pFrameWin = nullptr );
 
     // Return for the autotext expansion the previous word,
     // AutoCorrect - corresponding algorithm
@@ -383,7 +387,7 @@ public:
                                 LanguageType eLang );
     bool FnAddNonBrkSpace( SvxAutoCorrDoc&, const OUString&,
                                 sal_Int32 nEndPos,
-                                LanguageType eLang );
+                                LanguageType eLang, bool& io_bNbspRunNext );
     bool FnSetINetAttr( SvxAutoCorrDoc&, const OUString&,
                                 sal_Int32 nSttPos, sal_Int32 nEndPos,
                                 LanguageType eLang );
@@ -396,8 +400,6 @@ public:
                             sal_Int32 nSttPos, sal_Int32 nEndPos,
                             LanguageType eLang );
 
-    bool                HasRunNext() { return bRunNext; }
-
     static long         GetDefaultFlags();
 
 // returns sal_True for characters where the function
diff --git a/sw/inc/editsh.hxx b/sw/inc/editsh.hxx
index df4d0de6e148..411c8eed3921 100644
--- a/sw/inc/editsh.hxx
+++ b/sw/inc/editsh.hxx
@@ -947,6 +947,8 @@ public:
      character attribute dialog. */
     sal_uInt16 GetScalingOfSelectedText() const;
 
+    bool IsNbspRunNext() const { return m_bNbspRunNext; }
+
     /// Ctor/Dtor.
     SwEditShell( SwDoc&, vcl::Window*, const SwViewOption *pOpt );
 
@@ -957,6 +959,16 @@ public:
 private:
     SwEditShell(const SwEditShell &) = delete;
     const SwEditShell &operator=(const SwEditShell &) = delete;
+
+    /* TODO: this flag may have to be invalidated / reset to false at various
+     * places if it was true and the edit cursor position changes. It's somehow
+     * overkill though because it can only be true if a NO-BREAK SPACE was
+     * inserted by the last DoAutoCorrect() call (in French language), any
+     * subsequent call will reset it anyway and just if the cursor is
+     * positioned behind "x :" and the next character inserted is not a space
+     * the existing nb-space will be removed. Bear this in mind if that problem
+     * arises. */
+    bool m_bNbspRunNext;    ///< NO-BREAK SPACE state flag passed to and maintained by SvxAutoCorrect::DoAutoCorrect()
 };
 
 inline const sfx2::LinkManager& SwEditShell::GetLinkManager() const
diff --git a/sw/source/core/edit/autofmt.cxx b/sw/source/core/edit/autofmt.cxx
index 2dcae5d94d73..7ce2f7d0a068 100644
--- a/sw/source/core/edit/autofmt.cxx
+++ b/sw/source/core/edit/autofmt.cxx
@@ -1831,6 +1831,7 @@ void SwAutoFormat::AutoCorrect( sal_Int32 nPos )
     sal_Int32 nSttPos, nLastBlank = nPos;
     bool bFirst = m_aFlags.bCapitalStartSentence, bFirstSent = bFirst;
     sal_Unicode cChar = 0;
+    bool bNbspRunNext = false;
 
     CharClass& rAppCC = GetAppCharClass();
 
@@ -1984,7 +1985,7 @@ void SwAutoFormat::AutoCorrect( sal_Int32 nPos )
                                            : LANGUAGE_SYSTEM;
 
                     SetRedlineText( STR_AUTOFMTREDL_NON_BREAK_SPACE );
-                    if ( pATst->FnAddNonBrkSpace( aACorrDoc, *pText, nPos, eLang ) )
+                    if ( pATst->FnAddNonBrkSpace( aACorrDoc, *pText, nPos, eLang, bNbspRunNext ) )
                         --nPos;
                 }
                 break;
@@ -2041,7 +2042,7 @@ void SwAutoFormat::AutoCorrect( sal_Int32 nPos )
             if ( m_aFlags.bAddNonBrkSpace )
             {
                 SetRedlineText( STR_AUTOFMTREDL_NON_BREAK_SPACE );
-                pATst->FnAddNonBrkSpace( aACorrDoc, *pText, nPos, eLang );
+                pATst->FnAddNonBrkSpace( aACorrDoc, *pText, nPos, eLang, bNbspRunNext );
             }
 
             if( ( m_aFlags.bChgOrdinalNumber &&
diff --git a/sw/source/core/edit/edws.cxx b/sw/source/core/edit/edws.cxx
index 46e5e48687fe..3a719b11736f 100644
--- a/sw/source/core/edit/edws.cxx
+++ b/sw/source/core/edit/edws.cxx
@@ -35,12 +35,13 @@
 
 // masqueraded copy constructor
 SwEditShell::SwEditShell( SwEditShell& rEdSH, vcl::Window *pWindow )
-    : SwCursorShell( rEdSH, pWindow )
+    : SwCursorShell( rEdSH, pWindow ),
+    m_bNbspRunNext(false)   // TODO: would copying that make sense? only if editing continues
 {
 }
 
 SwEditShell::SwEditShell( SwDoc& rDoc, vcl::Window *pWindow, const SwViewOption *pOptions )
-    : SwCursorShell( rDoc, pWindow, pOptions )
+    : SwCursorShell( rDoc, pWindow, pOptions ), m_bNbspRunNext(false)
 {
     if (0 < officecfg::Office::Common::Undo::Steps::get())
     {
@@ -257,7 +258,7 @@ void SwEditShell::AutoCorrect( SvxAutoCorrect& rACorr, bool bInsert,
     OUString const& rNodeText(pTNd->GetText());
     rACorr.DoAutoCorrect( aSwAutoCorrDoc,
                     rNodeText, pCursor->GetPoint()->nContent.GetIndex(),
-                    cChar, bInsert, GetWin() );
+                    cChar, bInsert, m_bNbspRunNext, GetWin() );
     if( cChar )
         SaveTableBoxContent( pCursor->GetPoint() );
     EndAllAction();
diff --git a/sw/source/uibase/docvw/edtwin.cxx b/sw/source/uibase/docvw/edtwin.cxx
index 5d432fd11fa5..41c8b841a6e6 100644
--- a/sw/source/uibase/docvw/edtwin.cxx
+++ b/sw/source/uibase/docvw/edtwin.cxx
@@ -2430,8 +2430,7 @@ KEYINPUT_CHECKTABLE_INSDEL:
                 }
 
                 const bool bIsAutoCorrectChar =  SvxAutoCorrect::IsAutoCorrectChar( aCh );
-                const bool bRunNext = pACorr != nullptr && pACorr->HasRunNext();
-                if( !aKeyEvent.GetRepeat() && pACorr && ( bIsAutoCorrectChar || bRunNext ) &&
+                if( !aKeyEvent.GetRepeat() && pACorr && ( bIsAutoCorrectChar || rSh.IsNbspRunNext() ) &&
                         pACfg->IsAutoFormatByInput() &&
                     (( pACorr->IsAutoCorrFlag( ChgWeightUnderl ) &&
                         ( '*' == aCh || '_' == aCh ) ) ||
@@ -2443,7 +2442,7 @@ KEYINPUT_CHECKTABLE_INSDEL:
                     if( '\"' != aCh && '\'' != aCh )        // only call when "*_"!
                         rSh.UpdateAttr();
                 }
-                else if( !aKeyEvent.GetRepeat() && pACorr && ( bIsAutoCorrectChar || bRunNext ) &&
+                else if( !aKeyEvent.GetRepeat() && pACorr && ( bIsAutoCorrectChar || rSh.IsNbspRunNext() ) &&
                         pACfg->IsAutoFormatByInput() &&
                     pACorr->IsAutoCorrFlag( CapitalStartSentence | CapitalStartWord |
                                             ChgOrdinalNumber | AddNonBrkSpace |