diff options
author | Bjoern Michaelsen <bjoern.michaelsen@libreoffice.org> | 2020-12-12 23:44:26 +0100 |
---|---|---|
committer | Bjoern Michaelsen <bjoern.michaelsen@libreoffice.org> | 2020-12-30 10:47:05 +0100 |
commit | 188ec34cf157ffee8c63f03f420ca9daafb5ff29 (patch) | |
tree | c1a24efdaf13076fd9e3d4597976e5290c5630bb | |
parent | b808610533fce13228c4f4ad8924586b560a5928 (diff) |
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>
-rw-r--r-- | sw/qa/core/txtnode/txtnode.cxx | 2 | ||||
-rw-r--r-- | sw/qa/extras/mailmerge/mailmerge.cxx | 2 | ||||
-rw-r--r-- | sw/qa/extras/ooxmlexport/ooxmlexport.cxx | 18 | ||||
-rw-r--r-- | sw/source/core/inc/flyfrm.hxx | 1 | ||||
-rw-r--r-- | sw/source/core/inc/flyfrms.hxx | 5 | ||||
-rw-r--r-- | sw/source/core/layout/fly.cxx | 12 | ||||
-rw-r--r-- | sw/source/core/layout/flycnt.cxx | 196 | ||||
-rw-r--r-- | sw/source/core/layout/flyincnt.cxx | 4 | ||||
-rw-r--r-- | sw/source/core/layout/flylay.cxx | 14 | ||||
-rw-r--r-- | sw/source/core/layout/wsfrm.cxx | 2 | ||||
-rw-r--r-- | sw/source/core/txtnode/atrflyin.cxx | 2 |
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(); |