atrflyin.cxx Modify no more

the fragile tests depending on specific order of objects any a bit
unfortunate ...

Change-Id: Ib74ec2a69c95e6ca859d7c75796081ac889ac32d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/107647
Tested-by: Jenkins
Reviewed-by: Bjoern Michaelsen <bjoern.michaelsen@libreoffice.org>

11 files changed, 122 insertions, 136 deletions

diff --git a/sw/qa/core/txtnode/txtnode.cxx b/sw/qa/core/txtnode/txtnode.cxx
index 1c4d48b04598..3874f0f858c7 100644
--- a/sw/qa/core/txtnode/txtnode.cxx
+++ b/sw/qa/core/txtnode/txtnode.cxx
@@ -80,7 +80,7 @@ CPPUNIT_TEST_FIXTURE(SwCoreTxtnodeTest, testTextBoxNodeSplit)
     SwWrtShell* pWrtShell = pShell->GetWrtShell();
     pWrtShell->SttEndDoc(/*bStart=*/false);
     // Without the accompanying fix in place, this would have crashed in
-    // SwFlyAtContentFrame::Modify().
+    // SwFlyAtContentFrame::SwClientNotify().
     pWrtShell->SplitNode();
 }
 
diff --git a/sw/qa/extras/mailmerge/mailmerge.cxx b/sw/qa/extras/mailmerge/mailmerge.cxx
index 17d92d3789ac..824c4bbd60e5 100644
--- a/sw/qa/extras/mailmerge/mailmerge.cxx
+++ b/sw/qa/extras/mailmerge/mailmerge.cxx
@@ -397,7 +397,7 @@ DECLARE_FILE_MAILMERGE_TEST(testMissingDefaultLineColor, "missing-default-line-c
     executeMailMerge();
     // The document was created by LO version which didn't write out the default value for line color
     // (see XMLGraphicsDefaultStyle::SetDefaults()).
-    uno::Reference<beans::XPropertySet> xPropertySet(getShape(1), uno::UNO_QUERY);
+    uno::Reference<beans::XPropertySet> xPropertySet(getShape(4), uno::UNO_QUERY);
     // Lines do not have a line color.
     CPPUNIT_ASSERT( !xPropertySet->getPropertySetInfo()->hasPropertyByName( "LineColor" ));
     SwXTextDocument* pTextDoc = dynamic_cast<SwXTextDocument *>(mxComponent.get());
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport.cxx
index 26151ebcc0c9..6275ee8948d4 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport.cxx
@@ -467,31 +467,31 @@ DECLARE_OOXMLEXPORT_EXPORTONLY_TEST(testMsoPosition, "bnc884615-mso-position.doc
         // We write the frames out in different order than they were read, so check it's the correct
         // textbox first by checking width. These tests may need reordering if that gets fixed.
         OUString style1 = getXPath(doc, "/w:ftr/w:p/w:r[3]/mc:AlternateContent/mc:Fallback/w:pict/v:rect", "style");
-        CPPUNIT_ASSERT( style1.indexOf( ";width:531pt;" ) >= 0 );
-        CPPUNIT_ASSERT( style1.indexOf( ";mso-position-vertical-relative:page" ) >= 0 );
-        CPPUNIT_ASSERT( style1.indexOf( ";mso-position-horizontal-relative:page" ) >= 0 );
+        CPPUNIT_ASSERT( style1.indexOf( ";width:36pt;" ) >= 0 );
+        CPPUNIT_ASSERT( style1.indexOf( ";mso-position-horizontal-relative:text" ) >= 0 );
+        CPPUNIT_ASSERT( style1.indexOf( ";mso-position-vertical-relative:text" ) >= 0 );
         OUString style2 = getXPath(doc, "/w:ftr/w:p/w:r[4]/mc:AlternateContent/mc:Fallback/w:pict/v:rect", "style");
         CPPUNIT_ASSERT( style2.indexOf( ";width:549pt;" ) >= 0 );
         CPPUNIT_ASSERT( style2.indexOf( ";mso-position-vertical-relative:text" ) >= 0 );
         CPPUNIT_ASSERT( style2.indexOf( ";mso-position-horizontal:center" ) >= 0 );
         CPPUNIT_ASSERT( style2.indexOf( ";mso-position-horizontal-relative:text" ) >= 0 );
         OUString style3 = getXPath(doc, "/w:ftr/w:p/w:r[5]/mc:AlternateContent/mc:Fallback/w:pict/v:rect", "style");
-        CPPUNIT_ASSERT( style3.indexOf( ";width:36pt;" ) >= 0 );
-        CPPUNIT_ASSERT( style3.indexOf( ";mso-position-horizontal-relative:text" ) >= 0 );
-        CPPUNIT_ASSERT( style3.indexOf( ";mso-position-vertical-relative:text" ) >= 0 );
+        CPPUNIT_ASSERT( style3.indexOf( ";width:531pt;" ) >= 0 );
+        CPPUNIT_ASSERT( style3.indexOf( ";mso-position-vertical-relative:page" ) >= 0 );
+        CPPUNIT_ASSERT( style3.indexOf( ";mso-position-horizontal-relative:page" ) >= 0 );
     }
 
     xmlDocUniquePtr doc = parseExport("word/header1.xml");
     OUString style1 = getXPath(doc, "/w:hdr/w:p/w:r[2]/mc:AlternateContent/mc:Fallback/w:pict/v:rect", "style");
-    CPPUNIT_ASSERT( style1.indexOf( ";width:335.75pt;" ) >= 0 );
+    CPPUNIT_ASSERT( style1.indexOf( ";width:138.15pt;" ) >= 0 );
     CPPUNIT_ASSERT( style1.indexOf( ";mso-position-horizontal-relative:page" ) >= 0 );
     CPPUNIT_ASSERT( style1.indexOf( ";mso-position-vertical-relative:page" ) >= 0 );
     OUString style2 = getXPath(doc, "/w:hdr/w:p/w:r[3]/mc:AlternateContent/mc:Fallback/w:pict/v:rect", "style");
-    CPPUNIT_ASSERT( style2.indexOf( ";width:138.15pt;" ) >= 0 );
+    CPPUNIT_ASSERT( style2.indexOf( ";width:163.8pt;" ) >= 0 );
     CPPUNIT_ASSERT( style2.indexOf( ";mso-position-horizontal-relative:page" ) >= 0 );
     CPPUNIT_ASSERT( style2.indexOf( ";mso-position-vertical-relative:page" ) >= 0 );
     OUString style3 = getXPath(doc, "/w:hdr/w:p/w:r[4]/mc:AlternateContent/mc:Fallback/w:pict/v:rect", "style");
-    CPPUNIT_ASSERT( style3.indexOf( ";width:163.8pt;" ) >= 0 );
+    CPPUNIT_ASSERT( style3.indexOf( ";width:335.75pt;" ) >= 0 );
     CPPUNIT_ASSERT( style3.indexOf( ";mso-position-horizontal-relative:page" ) >= 0 );
     CPPUNIT_ASSERT( style3.indexOf( ";mso-position-vertical-relative:page" ) >= 0 );
 
diff --git a/sw/source/core/inc/flyfrm.hxx b/sw/source/core/inc/flyfrm.hxx
index fec9a4936e5b..52295f340c28 100644
--- a/sw/source/core/inc/flyfrm.hxx
+++ b/sw/source/core/inc/flyfrm.hxx
@@ -75,6 +75,7 @@ class SW_DLLPUBLIC SwFlyFrame : public SwLayoutFrame, public SwAnchoredObject
 protected:
     // Predecessor/Successor for chaining with text flow
     SwFlyFrame *m_pPrevLink, *m_pNextLink;
+   static const SwFormatAnchor* GetAnchorFromPoolItem(const SfxPoolItem& rItem);
 
 private:
     // It must be possible to block Content-bound flys so that they will be not
diff --git a/sw/source/core/inc/flyfrms.hxx b/sw/source/core/inc/flyfrms.hxx
index a401e4abf0af..cb9ef4f7a415 100644
--- a/sw/source/core/inc/flyfrms.hxx
+++ b/sw/source/core/inc/flyfrms.hxx
@@ -154,9 +154,8 @@ public:
 };
 
 // Flys that are bound to Content but not in Content
-class SwFlyAtContentFrame : public SwFlyFreeFrame
+class SwFlyAtContentFrame final: public SwFlyFreeFrame
 {
-protected:
     virtual void MakeAll(vcl::RenderContext* pRenderContext) override;
 
     // #i28701#
@@ -168,7 +167,7 @@ protected:
         #i28701#
     */
     virtual void RegisterAtCorrectPage() override;
-    virtual void Modify( const SfxPoolItem*, const SfxPoolItem* ) override;
+    virtual void SwClientNotify(const SwModify&, const SfxHint&) override;
 
 public:
     // #i28701#
diff --git a/sw/source/core/layout/fly.cxx b/sw/source/core/layout/fly.cxx
index 5dd10dee4eae..b45509d295de 100644
--- a/sw/source/core/layout/fly.cxx
+++ b/sw/source/core/layout/fly.cxx
@@ -2927,4 +2927,16 @@ SwTwips SwFlyFrame::CalcContentHeight(const SwBorderAttrs *pAttrs, const SwTwips
     return nHeight;
 }
 
+const SwFormatAnchor* SwFlyFrame::GetAnchorFromPoolItem(const SfxPoolItem& rItem)
+{
+    switch(rItem.Which())
+    {
+        case RES_ATTRSET_CHG:
+            return static_cast<const SwAttrSetChg*>(&rItem)->GetChgSet()->GetItem(RES_ANCHOR, false);
+        case RES_ANCHOR:
+            return static_cast<const SwFormatAnchor*>(&rItem);
+        default:
+            return nullptr;
+    }
+}
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/layout/flycnt.cxx b/sw/source/core/layout/flycnt.cxx
index 4dc2db5921d7..0b159ea1a743 100644
--- a/sw/source/core/layout/flycnt.cxx
+++ b/sw/source/core/layout/flycnt.cxx
@@ -79,123 +79,107 @@ SwFlyAtContentFrame::SwFlyAtContentFrame( SwFlyFrameFormat *pFormat, SwFrame* pS
 
 // #i28701#
 
-void SwFlyAtContentFrame::Modify( const SfxPoolItem* pOld, const SfxPoolItem *pNew )
+void SwFlyAtContentFrame::SwClientNotify(const SwModify&, const SfxHint& rHint)
 {
-    const SwFormatAnchor *pAnch = nullptr;
-
-    if (pNew)
+    auto pLegacy = dynamic_cast<const sw::LegacyModifyHint*>(&rHint);
+    if(!pLegacy)
+        return;
+    const SwFormatAnchor* pAnch = pLegacy->m_pNew ? GetAnchorFromPoolItem(*pLegacy->m_pNew) : nullptr;
+    if(!pAnch)
     {
-        const sal_uInt16 nWhich = pNew->Which();
-        if( RES_ATTRSET_CHG == nWhich && SfxItemState::SET ==
-            static_cast<const SwAttrSetChg*>(pNew)->GetChgSet()->GetItemState( RES_ANCHOR, false,
-                reinterpret_cast<const SfxPoolItem**>(&pAnch) ))
-            ;       // The anchor pointer is set at GetItemState!
-
-        else if( RES_ANCHOR == nWhich )
-        {
-            //Change anchor, I move myself to a new place.
-            //The anchor type must not change, this is only possible using
-            //SwFEShell.
-            pAnch = static_cast<const SwFormatAnchor*>(pNew);
-        }
+        SwFlyFrame::Modify(pLegacy->m_pOld, pLegacy->m_pNew);
+        return;
     }
-
-    if( pAnch )
+    OSL_ENSURE(pAnch->GetAnchorId() == GetFormat()->GetAnchor().GetAnchorId(),
+            "Illegal change of anchor type.");
+
+    //Unregister, get hold of a new anchor and attach it
+    SwRect aOld(GetObjRectWithSpaces());
+    SwPageFrame* pOldPage = FindPageFrame();
+    const SwFrame* pOldAnchor = GetAnchorFrame();
+    SwContentFrame* pContent = const_cast<SwContentFrame*>(static_cast<const SwContentFrame*>(GetAnchorFrame()));
+    AnchorFrame()->RemoveFly(this);
+
+    const bool bBodyFootnote = (pContent->IsInDocBody() || pContent->IsInFootnote());
+
+    // Search the new anchor using the NodeIdx; the relation between old
+    // and new NodeIdx determines the search direction
+    const SwNodeIndex aNewIdx(pAnch->GetContentAnchor()->nNode);
+    SwNodeIndex aOldIdx(pContent->IsTextFrame()
+            // sw_redlinehide: can pick any node here, the compare with
+            // FrameContainsNode should catch it
+            ? *static_cast<SwTextFrame *>(pContent)->GetTextNodeFirst()
+            : *static_cast<SwNoTextFrame *>(pContent)->GetNode());
+
+    //fix: depending on which index was smaller, searching in the do-while
+    //loop previously was done forward or backwards respectively. This however
+    //could lead to an infinite loop. To at least avoid the loop, searching
+    //is now done in only one direction. Getting hold of a frame from the node
+    //is still possible if the new anchor could not be found. Chances are
+    //good that this will be the correct one.
+    // consider the case that at found anchor frame candidate already a
+    // fly frame of the given fly format is registered.
+    // consider, that <pContent> is the already
+    // the new anchor frame.
+    bool bFound(FrameContainsNode(*pContent, aNewIdx.GetIndex()));
+    const bool bNext = !bFound && aOldIdx < aNewIdx;
+    while(pContent && !bFound)
     {
-        OSL_ENSURE( pAnch->GetAnchorId() == GetFormat()->GetAnchor().GetAnchorId(),
-                "Illegal change of anchor type. " );
-
-        //Unregister, get hold of a new anchor and attach it
-        SwRect aOld( GetObjRectWithSpaces() );
-        SwPageFrame *pOldPage = FindPageFrame();
-        const SwFrame *pOldAnchor = GetAnchorFrame();
-        SwContentFrame *pContent = const_cast<SwContentFrame*>(static_cast<const SwContentFrame*>(GetAnchorFrame()));
-        AnchorFrame()->RemoveFly( this );
-
-        const bool bBodyFootnote = (pContent->IsInDocBody() || pContent->IsInFootnote());
-
-        // Search the new anchor using the NodeIdx; the relation between old
-        // and new NodeIdx determines the search direction
-        const SwNodeIndex aNewIdx( pAnch->GetContentAnchor()->nNode );
-        SwNodeIndex aOldIdx( pContent->IsTextFrame()
-                // sw_redlinehide: can pick any node here, the compare with
-                // FrameContainsNode should catch it
-                ? *static_cast<SwTextFrame *>(pContent)->GetTextNodeFirst()
-                : *static_cast<SwNoTextFrame *>(pContent)->GetNode() );
-
-        //fix: depending on which index was smaller, searching in the do-while
-        //loop previously was done forward or backwards respectively. This however
-        //could lead to an infinite loop. To at least avoid the loop, searching
-        //is now done in only one direction. Getting hold of a frame from the node
-        //is still possible if the new anchor could not be found. Chances are
-        //good that this will be the correct one.
-        // consider the case that at found anchor frame candidate already a
-        // fly frame of the given fly format is registered.
-        // consider, that <pContent> is the already
-        // the new anchor frame.
-        bool bFound( FrameContainsNode(*pContent, aNewIdx.GetIndex()) );
-        const bool bNext = !bFound && aOldIdx < aNewIdx;
-        while ( pContent && !bFound )
+        do
         {
-            do
-            {
-                if ( bNext )
-                    pContent = pContent->GetNextContentFrame();
-                else
-                    pContent = pContent->GetPrevContentFrame();
-            } while ( pContent &&
-                      ( bBodyFootnote != ( pContent->IsInDocBody() ||
-                                           pContent->IsInFootnote() ) ) );
-            if ( pContent )
-                bFound = FrameContainsNode(*pContent, aNewIdx.GetIndex());
-
-            // check, if at found anchor frame candidate already a fly frame
-            // of the given fly frame format is registered.
-            if (bFound && pContent && pContent->GetDrawObjs())
+            if(bNext)
+                pContent = pContent->GetNextContentFrame();
+            else
+                pContent = pContent->GetPrevContentFrame();
+        } while(pContent &&
+                  (bBodyFootnote != (pContent->IsInDocBody() || pContent->IsInFootnote())));
+        if(pContent)
+            bFound = FrameContainsNode(*pContent, aNewIdx.GetIndex());
+
+        // check, if at found anchor frame candidate already a fly frame
+        // of the given fly frame format is registered.
+        if(bFound && pContent && pContent->GetDrawObjs())
+        {
+            SwFrameFormat* pMyFlyFrameFormat(&GetFrameFormat());
+            SwSortedObjs &rObjs = *pContent->GetDrawObjs();
+            for(SwAnchoredObject* rObj : rObjs)
             {
-                SwFrameFormat* pMyFlyFrameFormat( &GetFrameFormat() );
-                SwSortedObjs &rObjs = *pContent->GetDrawObjs();
-                for(SwAnchoredObject* rObj : rObjs)
+                SwFlyFrame* pFlyFrame = dynamic_cast<SwFlyFrame*>(rObj);
+                if (pFlyFrame &&
+                     &(pFlyFrame->GetFrameFormat()) == pMyFlyFrameFormat)
                 {
-                    SwFlyFrame* pFlyFrame = dynamic_cast<SwFlyFrame*>(rObj);
-                    if ( pFlyFrame &&
-                         &(pFlyFrame->GetFrameFormat()) == pMyFlyFrameFormat )
-                    {
-                        bFound = false;
-                        break;
-                    }
+                    bFound = false;
+                    break;
                 }
             }
         }
-        if ( !pContent )
-        {
-            SwContentNode *pNode = aNewIdx.GetNode().GetContentNode();
-            std::pair<Point, bool> const tmp(pOldAnchor->getFrameArea().Pos(), false);
-            pContent = pNode->getLayoutFrame(getRootFrame(), nullptr, &tmp);
-            OSL_ENSURE( pContent, "New anchor not found" );
-        }
-        //Flys are never attached to a follow, but always on the master which
-        //we are going to search now.
-        SwContentFrame* pFlow = pContent;
-        while ( pFlow->IsFollow() )
-            pFlow = pFlow->FindMaster();
-        pContent = pFlow;
-
-        //and *puff* it's attached...
-        pContent->AppendFly( this );
-        if ( pOldPage && pOldPage != FindPageFrame() )
-            NotifyBackground( pOldPage, aOld, PrepareHint::FlyFrameLeave );
-
-        //Fix(3495)
-        InvalidatePos_();
-        InvalidatePage();
-        SetNotifyBack();
-        // #i28701# - reset member <maLastCharRect> and
-        // <mnLastTopOfLine> for to-character anchored objects.
-        ClearCharRectAndTopOfLine();
     }
-    else
-        SwFlyFrame::Modify( pOld, pNew );
+    if(!pContent)
+    {
+        SwContentNode *pNode = aNewIdx.GetNode().GetContentNode();
+        std::pair<Point, bool> const tmp(pOldAnchor->getFrameArea().Pos(), false);
+        pContent = pNode->getLayoutFrame(getRootFrame(), nullptr, &tmp);
+        OSL_ENSURE(pContent, "New anchor not found");
+    }
+    //Flys are never attached to a follow, but always on the master which
+    //we are going to search now.
+    SwContentFrame* pFlow = pContent;
+    while(pFlow->IsFollow())
+        pFlow = pFlow->FindMaster();
+    pContent = pFlow;
+
+    //and *puff* it's attached...
+    pContent->AppendFly( this );
+    if(pOldPage && pOldPage != FindPageFrame())
+        NotifyBackground(pOldPage, aOld, PrepareHint::FlyFrameLeave);
+
+    //Fix(3495)
+    InvalidatePos_();
+    InvalidatePage();
+    SetNotifyBack();
+    // #i28701# - reset member <maLastCharRect> and
+    // <mnLastTopOfLine> for to-character anchored objects.
+    ClearCharRectAndTopOfLine();
 }
 
 //We need some helper classes to monitor the oscillation and a few functions
diff --git a/sw/source/core/layout/flyincnt.cxx b/sw/source/core/layout/flyincnt.cxx
index 69417d7f07ee..c5c647e0738a 100644
--- a/sw/source/core/layout/flyincnt.cxx
+++ b/sw/source/core/layout/flyincnt.cxx
@@ -90,7 +90,7 @@ void SwFlyInContentFrame::SetRefPoint( const Point& rPoint,
     }
 }
 
-void SwFlyInContentFrame::SwClientNotify(const SwModify&, const SfxHint& rHint)
+void SwFlyInContentFrame::SwClientNotify(const SwModify& rMod, const SfxHint& rHint)
 {
     auto pLegacy = dynamic_cast<const sw::LegacyModifyHint*>(&rHint);
     if(!pLegacy)
@@ -130,7 +130,7 @@ void SwFlyInContentFrame::SwClientNotify(const SwModify&, const SfxHint& rHint)
     }
     if(aSuperArgs.second)
     {
-        SwFlyFrame::Modify(aSuperArgs.first, aSuperArgs.second);
+        SwFlyFrame::SwClientNotify(rMod, sw::LegacyModifyHint(aSuperArgs.first, aSuperArgs.second));
         if(GetAnchorFrame())
             AnchorFrame()->Prepare(PrepareHint::FlyFrameAttributesChanged, GetFormat());
     }
diff --git a/sw/source/core/layout/flylay.cxx b/sw/source/core/layout/flylay.cxx
index 2a084e67ad71..4159be4b5617 100644
--- a/sw/source/core/layout/flylay.cxx
+++ b/sw/source/core/layout/flylay.cxx
@@ -728,19 +728,9 @@ SwFlyLayFrame::SwFlyLayFrame( SwFlyFrameFormat *pFormat, SwFrame* pSib, SwFrame
 void SwFlyLayFrame::SwClientNotify(const SwModify&, const SfxHint& rHint)
 {
     auto pLegacy = dynamic_cast<const sw::LegacyModifyHint*>(&rHint);
-    if(!pLegacy)
+    if(!pLegacy || !pLegacy->m_pNew)
         return;
-    const SwFormatAnchor* pAnch = nullptr;
-    switch(pLegacy->GetWhich())
-    {
-        case RES_ATTRSET_CHG:
-        {
-            pAnch = static_cast<const SwAttrSetChg*>(pLegacy->m_pNew)->GetChgSet()->GetItem(RES_ANCHOR, false);
-            break;
-        }
-        case RES_ANCHOR:
-            pAnch = static_cast<const SwFormatAnchor*>(pLegacy->m_pNew);
-    }
+    const auto pAnch = GetAnchorFromPoolItem(*pLegacy->m_pNew);
 
     if(!pAnch)
     {
diff --git a/sw/source/core/layout/wsfrm.cxx b/sw/source/core/layout/wsfrm.cxx
index 8d64d23fc7dd..57b4d433a242 100644
--- a/sw/source/core/layout/wsfrm.cxx
+++ b/sw/source/core/layout/wsfrm.cxx
@@ -4371,7 +4371,7 @@ static void UnHideRedlines(SwRootFrame & rLayout,
                                 pObject->InvalidateObjPos();
                             }
                         }
-                        // SwFlyAtContentFrame::Modify() always appends to
+                        // SwFlyAtContentFrame::SwClientNotify() always appends to
                         // the master frame, so do the same here.
                         // (RemoveFootnotesForNode must be called at least once)
                         if (!pFrame->IsFollow())
diff --git a/sw/source/core/txtnode/atrflyin.cxx b/sw/source/core/txtnode/atrflyin.cxx
index d04da840d8e4..49c9d1d19670 100644
--- a/sw/source/core/txtnode/atrflyin.cxx
+++ b/sw/source/core/txtnode/atrflyin.cxx
@@ -209,7 +209,7 @@ void SwTextFlyCnt::SetAnchor( const SwTextNode *pNode )
             SwFormatAnchor aTextBoxAnchor(pTextBox->GetAnchor());
             aTextBoxAnchor.SetAnchor(aAnchor.GetContentAnchor());
 
-            // SwFlyAtContentFrame::Modify() assumes the anchor has a matching layout frame, which
+            // SwFlyAtContentFrame::SwClientNotify() assumes the anchor has a matching layout frame, which
             // may not be the case when we're in the process of a node split, so block
             // notifications.
             bool bIsInSplitNode = pNode->GetpSwpHints() && pNode->GetpSwpHints()->IsInSplitNode();