tdf#98512 Fix the Undo action of insert an input field.

Reviewed-on: https://gerrit.libreoffice.org/23055
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Oliver Specht <oliver.specht@cib.de>
(cherry picked from commit ed81bf39dd3431d28860fed6a2d4e8814126cc61)

tdf#98512 sw: remove duplicate undo history add

Follow-up to commit ed81bf39dd3431d28860fed6a2d4e8814126cc61.

In almost all cases SetAttr() and InsertHint() will already add
the history hint themselves, so this code in SwRegHistory::InsertItems()
looks somewhat pointless as it inserts a duplicate.

It turns out it's needed for hints that insert dummy characters, because
firstly for the hints-without-end the NoteInHistory() was not called, and
secondly when InsertText() inserts the dummy character it may actually
delete the hints array if there are no pre-existing hints, and a new
hints array will be created where the SwRegHistory isn't registered.

It's not obvious how the hints array actually can become empty in
SwTextNode::InsertText().

Let's fix that and assert instead that the history hint was inserted.

(cherry picked from commit b44e70e3d17c79eaf78f9bfe8bd0275d7ed9325f)

tdf#98512 sw: add unit test

... and fix the SAL_WARN not to warn spuriously.

(cherry picked from commit 20ae3d14187dacce2c2a1d2eab389e6110fe8442)

Change-Id: Ic951fc0d811e5cab39989285d34bdd2fff8f95fd
Reviewed-on: https://gerrit.libreoffice.org/24664
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>

4 files changed, 66 insertions, 8 deletions

diff --git a/sw/qa/extras/uiwriter/uiwriter.cxx b/sw/qa/extras/uiwriter/uiwriter.cxx
index c4adc563952b..9a3ce413fa1d 100644
--- a/sw/qa/extras/uiwriter/uiwriter.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter.cxx
@@ -108,6 +108,7 @@ public:
     void testFdo75110();
     void testFdo75898();
     void testFdo74981();
+    void testTdf98512();
     void testShapeTextboxSelect();
     void testShapeTextboxDelete();
     void testCp1000071();
@@ -199,6 +200,7 @@ public:
     CPPUNIT_TEST(testFdo75110);
     CPPUNIT_TEST(testFdo75898);
     CPPUNIT_TEST(testFdo74981);
+    CPPUNIT_TEST(testTdf98512);
     CPPUNIT_TEST(testShapeTextboxSelect);
     CPPUNIT_TEST(testShapeTextboxDelete);
     CPPUNIT_TEST(testCp1000071);
@@ -681,6 +683,44 @@ void SwUiWriterTest::testFdo74981()
     CPPUNIT_ASSERT(!pTextNode->HasHints());
 }
 
+void SwUiWriterTest::testTdf98512()
+{
+    SwDoc* pDoc = createDoc();
+    SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell();
+    SwInputFieldType *const pType(static_cast<SwInputFieldType*>(
+                pWrtShell->GetFieldType(0, RES_INPUTFLD)));
+    SwInputField aField1(pType, OUString("foo"), OUString("bar"), INP_TXT, 0);
+    pWrtShell->Insert(aField1);
+    pWrtShell->SttEndDoc(/*bStt=*/true);
+    SwInputField aField2(pType, OUString("baz"), OUString("quux"), INP_TXT, 0);
+    pWrtShell->Insert(aField2);
+    pWrtShell->SttEndDoc(/*bStt=*/true);
+    pWrtShell->SetMark();
+    pWrtShell->SttEndDoc(/*bStt=*/false);
+    OUString const expected1(
+        OUStringLiteral1<CH_TXT_ATR_INPUTFIELDSTART>() + "foo" + OUStringLiteral1<CH_TXT_ATR_INPUTFIELDEND>());
+    OUString const expected2(
+        OUStringLiteral1<CH_TXT_ATR_INPUTFIELDSTART>() + "baz" + OUStringLiteral1<CH_TXT_ATR_INPUTFIELDEND>()
+        + expected1);
+    CPPUNIT_ASSERT_EQUAL(expected2, pWrtShell->getShellCursor(false)->GetText());
+    sw::UndoManager& rUndoManager = pDoc->GetUndoManager();
+    rUndoManager.Undo();
+    pWrtShell->SttEndDoc(/*bStt=*/true);
+    pWrtShell->SetMark();
+    pWrtShell->SttEndDoc(/*bStt=*/false);
+    CPPUNIT_ASSERT_EQUAL(expected1, pWrtShell->getShellCursor(false)->GetText());
+    rUndoManager.Redo();
+    pWrtShell->SttEndDoc(/*bStt=*/true);
+    pWrtShell->SetMark();
+    pWrtShell->SttEndDoc(/*bStt=*/false);
+    CPPUNIT_ASSERT_EQUAL(expected2, pWrtShell->getShellCursor(false)->GetText());
+    rUndoManager.Undo();
+    pWrtShell->SttEndDoc(/*bStt=*/true);
+    pWrtShell->SetMark();
+    pWrtShell->SttEndDoc(/*bStt=*/false);
+    CPPUNIT_ASSERT_EQUAL(expected1, pWrtShell->getShellCursor(false)->GetText());
+}
+
 void SwUiWriterTest::testShapeTextboxSelect()
 {
     SwDoc* pDoc = createDoc("shape-textbox.odt");
diff --git a/sw/source/core/txtnode/ndtxt.cxx b/sw/source/core/txtnode/ndtxt.cxx
index 85a83660f4b5..2145dcb37012 100644
--- a/sw/source/core/txtnode/ndtxt.cxx
+++ b/sw/source/core/txtnode/ndtxt.cxx
@@ -1944,6 +1944,7 @@ OUString SwTextNode::InsertText( const OUString & rStr, const SwIndex & rIdx,
 
     if ( HasHints() )
     {
+        bool const bHadHints(!m_pSwpHints->CanBeDeleted());
         bool bMergePortionsNeeded(false);
         for ( size_t i = 0; i < m_pSwpHints->Count() &&
                 rIdx >= m_pSwpHints->Get(i)->GetStart(); ++i )
@@ -2008,7 +2009,8 @@ OUString SwTextNode::InsertText( const OUString & rStr, const SwIndex & rIdx,
         {
             m_pSwpHints->MergePortions(*this);
         }
-        TryDeleteSwpHints();
+        SAL_WARN_IF(bHadHints && m_pSwpHints->CanBeDeleted(), "sw.core",
+                "SwTextNode::InsertText: unexpected loss of hints");
     }
 
     if ( HasWriterListeners() )
diff --git a/sw/source/core/txtnode/thints.cxx b/sw/source/core/txtnode/thints.cxx
index 19a0f777f8bb..9a588d8fc59d 100644
--- a/sw/source/core/txtnode/thints.cxx
+++ b/sw/source/core/txtnode/thints.cxx
@@ -3172,6 +3172,7 @@ bool SwpHints::TryInsertHint(
     if( !pHtEnd )
     {
         Insert( pHint );
+        NoteInHistory(pHint, true);
         CalcFlags();
 #ifdef DBG_UTIL
         if( !rNode.GetDoc()->IsInReading() )
diff --git a/sw/source/core/undo/rolbck.cxx b/sw/source/core/undo/rolbck.cxx
index fd439e76356d..36f0e5e521de 100644
--- a/sw/source/core/undo/rolbck.cxx
+++ b/sw/source/core/undo/rolbck.cxx
@@ -1376,9 +1376,9 @@ bool SwRegHistory::InsertItems( const SfxItemSet& rSet,
     if (!pTextNode)
         return false;
 
-    if ( pTextNode->GetpSwpHints() && m_pHistory )
+    if (m_pHistory)
     {
-        pTextNode->GetpSwpHints()->Register( this );
+        pTextNode->GetOrCreateSwpHints().Register(this);
     }
 
     const bool bInserted = pTextNode->SetAttr( rSet, nStart, nEnd, nFlags );
@@ -1393,14 +1393,29 @@ bool SwRegHistory::InsertItems( const SfxItemSet& rSet,
         pTextNode->GetpSwpHints()->DeRegister();
     }
 
+#ifndef NDEBUG
     if ( m_pHistory && bInserted )
     {
-        SwHistoryHint* pNewHstr = new SwHistoryResetAttrSet( rSet,
-                                    pTextNode->GetIndex(), nStart, nEnd );
-        // the NodeIndex might be moved!
-
-        m_pHistory->m_SwpHstry.push_back( pNewHstr );
+        SfxItemIter aIter(rSet);
+        for (SfxPoolItem const* pItem = aIter.FirstItem(); pItem; pItem = aIter.NextItem())
+        {   // check that the history recorded a hint to reset every item
+            sal_uInt16 const nWhich(pItem->Which());
+            RES_TXTATR const nExpected(
+                (isCHRATR(nWhich) || RES_TXTATR_UNKNOWN_CONTAINER == nWhich)
+                    ? RES_TXTATR_AUTOFMT
+                    : static_cast<RES_TXTATR>(nWhich));
+            if (RES_TXTATR_AUTOFMT == nExpected && 0 == nStart && pTextNode->Len() == nEnd)
+                continue; // special case, may get set on text node itself
+            assert(std::find_if(
+                m_pHistory->m_SwpHstry.begin(), m_pHistory->m_SwpHstry.end(),
+                [nExpected](SwHistoryHint *const pHint) -> bool {
+                    SwHistoryResetText const*const pReset(
+                            dynamic_cast<SwHistoryResetText const*>(pHint));
+                    return (pReset) ? pReset->GetWhich() == nExpected : false;
+                }) != m_pHistory->m_SwpHstry.end());
+        }
     }
+#endif
 
     return bInserted;
 }