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.

(cherry picked from commit fae87e03ea3829718ec0381ed3b04ceb52c23720)

Conflicts:
	sw/source/core/txtnode/atrflyin.cxx
	sw/source/core/txtnode/thints.cxx

Added test for redline with as-char frame

fixed by commits 4dd2e61 and fae87e0

Reviewed-by: Michael Stahl <mstahl@redhat.com>
(cherry picked from commit f36ac1aa3bef5ba218f3dae24f260ce7e4afba95)

Change-Id: Ib3236f289c2c4202d48ac378a53ce02130d4ce2c
Reviewed-on: https://gerrit.libreoffice.org/16323
Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>
Tested-by: Miklos Vajna <vmiklos@collabora.co.uk>

4 files changed, 32 insertions, 39 deletions

diff --git a/sw/qa/extras/odfexport/data/redlineTextFrame.odt b/sw/qa/extras/odfexport/data/redlineTextFrame.odt
new file mode 100644
index 000000000000..0986c3792fdb
--- /dev/null
+++ b/sw/qa/extras/odfexport/data/redlineTextFrame.odt
diff --git a/sw/qa/extras/odfexport/odfexport.cxx b/sw/qa/extras/odfexport/odfexport.cxx
index df5da9c89248..a7c1a18bd54c 100644
--- a/sw/qa/extras/odfexport/odfexport.cxx
+++ b/sw/qa/extras/odfexport/odfexport.cxx
@@ -37,6 +37,15 @@ public:
 
 #define DECLARE_ODFEXPORT_TEST(TestName, filename) DECLARE_SW_ROUNDTRIP_TEST(TestName, filename, Test)
 
+DECLARE_ODFEXPORT_TEST(testredlineTextFrame, "redlineTextFrame.odt")
+{
+    //Note this is for a crash test
+    //Counting the Number of Frames and checking with the expected count
+    uno::Reference<text::XTextFramesSupplier> xTextFramesSupplier(mxComponent, uno::UNO_QUERY);
+    uno::Reference<container::XIndexAccess> xIndexAccess(xTextFramesSupplier->getTextFrames(), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), xIndexAccess->getCount());
+}
+
 DECLARE_ODFEXPORT_TEST(testFdo38244, "fdo38244.odt")
 {
     // See ooxmlexport's testFdo38244().
diff --git a/sw/source/core/txtnode/atrflyin.cxx b/sw/source/core/txtnode/atrflyin.cxx
index 7dc780e5d64b..08acb3540eeb 100644
--- a/sw/source/core/txtnode/atrflyin.cxx
+++ b/sw/source/core/txtnode/atrflyin.cxx
@@ -151,12 +151,19 @@ void SwTxtFlyCnt::SetAnchor( const SwTxtNode *pNode )
     SwFrmFmt* pFmt = GetFlyCnt().GetFrmFmt();
     SwFmtAnchor aAnchor( pFmt->GetAnchor() );
 
-    if( !aAnchor.GetCntntAnchor() ||
-        !aAnchor.GetCntntAnchor()->nNode.GetNode().GetNodes().IsDocNodes() ||
-        &aAnchor.GetCntntAnchor()->nNode.GetNode() != (SwNode*)pNode )
+    SwNode *const pOldNode(aAnchor.GetCntntAnchor()
+            ? &aAnchor.GetCntntAnchor()->nNode.GetNode()
+            : nullptr);
+
+    if (!pOldNode || !pOldNode->GetNodes().IsDocNodes() ||
+        pOldNode != static_cast<SwNode const *>(pNode))
+    {
         aPos.nNode = *pNode;
+    }
     else
-        aPos.nNode = aAnchor.GetCntntAnchor()->nNode;
+    {
+        aPos.nNode = *pOldNode;
+    }
 
     aAnchor.SetType( FLY_AS_CHAR );        // default!
     aAnchor.SetAnchor( &aPos );
@@ -186,10 +193,17 @@ void SwTxtFlyCnt::SetAnchor( const SwTxtNode *pNode )
     {
         pFmt->LockModify();
         pFmt->SetFmtAttr( aAnchor );        // nur den Anker neu setzen
+        // tdf#91228 must notify the anchor nodes despite LockModify
+        assert(pOldNode);
+        pOldNode->RemoveAnchoredFly(pFmt);
+        aPos.nNode.GetNode().AddAnchoredFly(pFmt);
         pFmt->UnlockModify();
     }
     else
+    {
+        assert(!pFmt->IsModifyLocked()); // need to notify anchor node
         pFmt->SetFmtAttr( 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 37b0c261aa9b..a03b391a7440 100644
--- a/sw/source/core/txtnode/thints.cxx
+++ b/sw/source/core/txtnode/thints.cxx
@@ -1277,44 +1277,19 @@ bool SwTxtNode::InsertHint( SwTxtAttr * const pAttr, const SetAttrMode nMode )
             {
                 SwTxtFlyCnt *pFly = (SwTxtFlyCnt *)pAttr;
                 SwFrmFmt* pFmt = pAttr->GetFlyCnt().GetFrmFmt();
-
-                // 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 SwFmtAnchor* pAnchor = 0;
-                pFmt->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->GetCntntAnchor() &&
-                    pAnchor->GetCntntAnchor()->nNode != *this)
-                {
-                    assert(pAnchor->GetCntntAnchor()->nNode.GetNode().IsTxtNode());
-                    SwTxtNode* textNode = pAnchor->GetCntntAnchor()->nNode.GetNode().GetTxtNode();
-
-                    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( pFmt );
-                        bChangeFlyParentNode = true;
-                    }
-                }
-
                 if( !(nsSetAttrMode::SETATTR_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 SwFmtAnchor* pAnchor = 0;
+                    pFmt->GetItemState( RES_ANCHOR, false,
+                        (const SfxPoolItem**)&pAnchor );
+
+                    SwIndex aIdx( this, pAttr->GetStart() );
                     const OUString c(GetCharOfTxtAttr(*pAttr));
                     OUString const ins( InsertText(c, aIdx, nInsertFlags) );
                     if (ins.isEmpty())
@@ -1378,11 +1353,6 @@ bool SwTxtNode::InsertHint( SwTxtAttr * const pAttr, const SetAttrMode nMode )
                         return false;
                     }
                 }
-
-                // Finish relationships update now that SetAnchor has fixed part of it.
-                if (bChangeFlyParentNode)
-                    AddAnchoredFly( pFmt );
-
                 break;
             }