tdf#91228: need to check the format's IsLockModified(), not the node's

commit 9f01951b858453684f2622541af0eb85d4544fc6 also did the extra
Remove/Add for Draw fly objects, and it turns out that that's actually
wrong because SwTextFlyCnt::SetAnchor() will set the anchor without
locking anything if it's a Draw object.  Replace it with a different
hack in SetAnchor() that applies only if it calls LockModify().

Thanks to Varun Dhall for creating a reproducer document.

Not sure if the LockModify() could be replaced completely, perhaps it's
just an optimization to avoid re-creating layout frames for the fly.

Change-Id: Ib3236f289c2c4202d48ac378a53ce02130d4ce2c

2 files changed, 21 insertions, 39 deletions

diff --git a/sw/source/core/txtnode/atrflyin.cxx b/sw/source/core/txtnode/atrflyin.cxx
index ee67fa2bfa06..78948542eba8 100644
--- a/sw/source/core/txtnode/atrflyin.cxx
+++ b/sw/source/core/txtnode/atrflyin.cxx
@@ -150,13 +150,19 @@ void SwTextFlyCnt::SetAnchor( const SwTextNode *pNode )
     SwPosition aPos( *pNode->StartOfSectionNode(), aIdx );
     SwFrameFormat* pFormat = GetFlyCnt().GetFrameFormat();
     SwFormatAnchor aAnchor( pFormat->GetAnchor() );
+    SwNode *const pOldNode(aAnchor.GetContentAnchor()
+            ? &aAnchor.GetContentAnchor()->nNode.GetNode()
+            : nullptr);
 
-    if( !aAnchor.GetContentAnchor() ||
-        !aAnchor.GetContentAnchor()->nNode.GetNode().GetNodes().IsDocNodes() ||
-        &aAnchor.GetContentAnchor()->nNode.GetNode() != static_cast<SwNode const *>(pNode) )
+    if (!pOldNode || !pOldNode->GetNodes().IsDocNodes() ||
+        pOldNode != static_cast<SwNode const *>(pNode))
+    {
         aPos.nNode = *pNode;
+    }
     else
-        aPos.nNode = aAnchor.GetContentAnchor()->nNode;
+    {
+        aPos.nNode = *pOldNode;
+    }
 
     aAnchor.SetType( FLY_AS_CHAR );        // default!
     aAnchor.SetAnchor( &aPos );
@@ -186,10 +192,17 @@ void SwTextFlyCnt::SetAnchor( const SwTextNode *pNode )
     {
         pFormat->LockModify();
         pFormat->SetFormatAttr( aAnchor );        // nur den Anker neu setzen
+        // tdf#91228 must notify the anchor nodes despite LockModify
+        assert(pOldNode);
+        pOldNode->RemoveAnchoredFly(pFormat);
+        aPos.nNode.GetNode().AddAnchoredFly(pFormat);
         pFormat->UnlockModify();
     }
     else
+    {
+        assert(!pFormat->IsModifyLocked()); // need to notify anchor node
         pFormat->SetFormatAttr( aAnchor );        // nur den Anker neu setzen
+    }
 
     // Am Node haengen u.a. abhaengige CntFrms.
     // Fuer jeden CntFrm wird ein SwFlyInCntFrm angelegt.
diff --git a/sw/source/core/txtnode/thints.cxx b/sw/source/core/txtnode/thints.cxx
index 54c8ab238420..669ab940d155 100644
--- a/sw/source/core/txtnode/thints.cxx
+++ b/sw/source/core/txtnode/thints.cxx
@@ -1276,45 +1276,19 @@ bool SwTextNode::InsertHint( SwTextAttr * const pAttr, const SetAttrMode nMode )
             {
                 SwTextFlyCnt *pFly = static_cast<SwTextFlyCnt *>(pAttr);
                 SwFrameFormat* pFormat = pAttr->GetFlyCnt().GetFrameFormat();
-
-                // In order to maintain data coherency, if the hint is a fly
-                // moved from a text node to another, we have to remove it from
-                // the first textnode then to add it to the new (this) textnode
-                const SwFormatAnchor* pAnchor = 0;
-                pFormat->GetItemState( RES_ANCHOR, false,
-                    reinterpret_cast<const SfxPoolItem**>(&pAnchor) );
-
-                SwIndex aIdx( this, pAttr->GetStart() );
-
-                bool bChangeFlyParentNode( false );
-                if (pAnchor &&
-                    pAnchor->GetAnchorId() == FLY_AS_CHAR &&
-                    pAnchor->GetContentAnchor() &&
-                    pAnchor->GetContentAnchor()->nNode != *this)
-                {
-                    assert(pAnchor->GetContentAnchor()->nNode.GetNode().IsTextNode());
-                    SwTextNode* textNode = pAnchor->GetContentAnchor()->nNode.GetNode().GetTextNode();
-
-                    if ( textNode->IsModifyLocked() )
-                    {
-                        //  Fly parent has changed but the FlyFormat is locked, so it will
-                        //  not be updated by SetAnchor (that calls Modify that updates
-                        //  relationships)
-                        textNode->RemoveAnchoredFly( pFormat );
-                        bChangeFlyParentNode = true;
-                    }
-                }
-
                 if( !(SetAttrMode::NOTXTATRCHR & nInsMode) )
                 {
-
                     // Wir muessen zuerst einfuegen, da in SetAnchor()
                     // dem FlyFrm GetStart() uebermittelt wird.
                     //JP 11.05.98: falls das Anker-Attribut schon richtig
                     // gesetzt ist, dann korrigiere dieses nach dem Einfuegen
                     // des Zeichens. Sonst muesste das immer  ausserhalb
                     // erfolgen (Fehleranfaellig !)
+                    const SwFormatAnchor* pAnchor = 0;
+                    pFormat->GetItemState( RES_ANCHOR, false,
+                        reinterpret_cast<const SfxPoolItem**>(&pAnchor) );
 
+                    SwIndex aIdx( this, pAttr->GetStart() );
                     const OUString c(GetCharOfTextAttr(*pAttr));
                     OUString const ins( InsertText(c, aIdx, nInsertFlags) );
                     if (ins.isEmpty())
@@ -1378,11 +1352,6 @@ bool SwTextNode::InsertHint( SwTextAttr * const pAttr, const SetAttrMode nMode )
                         return false;
                     }
                 }
-
-                // Finish relationships update now that SetAnchor has fixed part of it.
-                if (bChangeFlyParentNode)
-                    AddAnchoredFly( pFormat );
-
                 break;
             }