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.

Change-Id: If8e0ff1fe28a162f144149a536a7566f94219c0d

3 files changed, 26 insertions, 11 deletions

diff --git a/sw/source/core/txtnode/ndtxt.cxx b/sw/source/core/txtnode/ndtxt.cxx
index 07e6027f43c1..ecc1d37a8c13 100644
--- a/sw/source/core/txtnode/ndtxt.cxx
+++ b/sw/source/core/txtnode/ndtxt.cxx
@@ -2004,7 +2004,8 @@ OUString SwTextNode::InsertText( const OUString & rStr, const SwIndex & rIdx,
         {
             m_pSwpHints->MergePortions(*this);
         }
-        TryDeleteSwpHints();
+        SAL_WARN_IF(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 6b5abd809b68..d6cc66eda27d 100644
--- a/sw/source/core/txtnode/thints.cxx
+++ b/sw/source/core/txtnode/thints.cxx
@@ -3173,6 +3173,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 5c9e68428612..4b62d49b79cd 100644
--- a/sw/source/core/undo/rolbck.cxx
+++ b/sw/source/core/undo/rolbck.cxx
@@ -1375,9 +1375,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 );
@@ -1392,16 +1392,29 @@ bool SwRegHistory::InsertItems( const SfxItemSet& rSet,
         pTextNode->GetpSwpHints()->DeRegister();
     }
 
-    // if (m_pHistory->Count()) return true, then it shouldn't to be pushed into
-    // the m_pHistory->m_SwpHstry again, or Undo Action will do the same thing twice
-    if ( m_pHistory && bInserted && !m_pHistory->Count() )
+#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;
 }