sw/source/core/inc/UndoAttribute.hxx | 1
sw/source/core/layout/atrfrm.cxx | 61 +++++++++++++++++++++--------------
sw/source/core/undo/unattr.cxx | 11 ++----
sw/source/core/unocore/unoframe.cxx | 6 +--
sw/source/ui/frmdlg/frmpage.cxx | 4 +-
sw/source/uibase/frmdlg/frmmgr.cxx | 2 -
sw/source/uibase/shells/basesh.cxx | 2 -
7 files changed, 51 insertions(+), 36 deletions(-)
New commits:
commit 3db0ff0c49159f9856c259167256f6c5f89931e0
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Thu Mar 30 14:17:18 2023 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Fri Mar 31 05:52:47 2023 +0000
add some asserts in SwFormatAnchor
Which flushes out some dodgy behaviour.
Particularly in SwUndoFormatAttr, which was storing a sal_Int32 content
offset in a sal_uInt16 page field.
In this case, rather just store the value on SwUndoFormatAttr itself, the
same way it does for other things.
Change-Id: I5890353f5d51d82037bf7e0e77a16cf3b0dff565
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/149764
Tested-by: Noel Grandin <noel.gran...@collabora.co.uk>
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
diff --git a/sw/source/core/inc/UndoAttribute.hxx
b/sw/source/core/inc/UndoAttribute.hxx
index 178a17f5a2fd..7fbbb5b522cd 100644
--- a/sw/source/core/inc/UndoAttribute.hxx
+++ b/sw/source/core/inc/UndoAttribute.hxx
@@ -87,6 +87,7 @@ class SwUndoFormatAttr final : public SwUndo
friend class SwUndoDefaultAttr;
OUString m_sFormatName;
std::optional<SfxItemSet> m_oOldSet; // old attributes
+ sal_Int32 m_nAnchorContentOffset;
SwNodeOffset m_nNodeIndex;
const sal_uInt16 m_nFormatWhich;
const bool m_bSaveDrawPt;
diff --git a/sw/source/core/layout/atrfrm.cxx b/sw/source/core/layout/atrfrm.cxx
index b53683e780e2..b976f6ee1377 100644
--- a/sw/source/core/layout/atrfrm.cxx
+++ b/sw/source/core/layout/atrfrm.cxx
@@ -1560,20 +1560,27 @@ void SwFormatHoriOrient::dumpAsXml(xmlTextWriterPtr
pWriter) const
(void)xmlTextWriterEndElement(pWriter);
}
-// Partially implemented inline in hxx
SwFormatAnchor::SwFormatAnchor( RndStdIds nRnd, sal_uInt16 nPage )
: SfxPoolItem( RES_ANCHOR ),
m_eAnchorId( nRnd ),
m_nPageNumber( nPage ),
// OD 2004-05-05 #i28701# - get always new increased order number
m_nOrder( ++s_nOrderCounter )
-{}
+{
+ assert( m_eAnchorId == RndStdIds::FLY_AT_PARA
+ || m_eAnchorId == RndStdIds::FLY_AS_CHAR
+ || m_eAnchorId == RndStdIds::FLY_AT_PAGE
+ || m_eAnchorId == RndStdIds::FLY_AT_FLY
+ || m_eAnchorId == RndStdIds::FLY_AT_CHAR);
+ // only FLY_AT_PAGE should have a valid page
+ assert( m_eAnchorId == RndStdIds::FLY_AT_PAGE || nPage == 0 );
+}
SwFormatAnchor::SwFormatAnchor( const SwFormatAnchor &rCpy )
: SfxPoolItem( RES_ANCHOR )
, m_oContentAnchor( rCpy.m_oContentAnchor )
- , m_eAnchorId( rCpy.GetAnchorId() )
- , m_nPageNumber( rCpy.GetPageNum() )
+ , m_eAnchorId( rCpy.m_eAnchorId )
+ , m_nPageNumber( rCpy.m_nPageNumber )
// OD 2004-05-05 #i28701# - get always new increased order number
, m_nOrder( ++s_nOrderCounter )
{
@@ -1585,22 +1592,23 @@ SwFormatAnchor::~SwFormatAnchor()
void SwFormatAnchor::SetAnchor( const SwPosition *pPos )
{
+ if (!pPos)
+ {
+ m_oContentAnchor.reset();
+ return;
+ }
// anchor only to paragraphs, or start nodes in case of
RndStdIds::FLY_AT_FLY
// also allow table node, this is used when a table is selected and is
converted to a frame by the UI
- assert(!pPos
- || (RndStdIds::FLY_AT_FLY == m_eAnchorId &&
pPos->GetNode().GetStartNode())
+ assert((RndStdIds::FLY_AT_FLY == m_eAnchorId &&
pPos->GetNode().GetStartNode())
|| (RndStdIds::FLY_AT_PARA == m_eAnchorId &&
pPos->GetNode().GetTableNode())
|| pPos->GetNode().GetTextNode());
- if (pPos)
- m_oContentAnchor.emplace(*pPos);
- else
- m_oContentAnchor.reset();
+ // verify that the SwPosition being passed to us is not screwy
+ assert(!pPos->nContent.GetContentNode()
+ || &pPos->nNode.GetNode() == pPos->nContent.GetContentNode());
+ m_oContentAnchor.emplace(*pPos);
// Flys anchored AT paragraph should not point into the paragraph content
- if (m_oContentAnchor &&
- ((RndStdIds::FLY_AT_PARA == m_eAnchorId) || (RndStdIds::FLY_AT_FLY ==
m_eAnchorId)))
- {
+ if ((RndStdIds::FLY_AT_PARA == m_eAnchorId) || (RndStdIds::FLY_AT_FLY ==
m_eAnchorId))
m_oContentAnchor->nContent.Assign( nullptr, 0 );
- }
}
SwNode* SwFormatAnchor::GetAnchorNode() const
@@ -1633,8 +1641,8 @@ SwFormatAnchor& SwFormatAnchor::operator=(const
SwFormatAnchor& rAnchor)
{
if (this != &rAnchor)
{
- m_eAnchorId = rAnchor.GetAnchorId();
- m_nPageNumber = rAnchor.GetPageNum();
+ m_eAnchorId = rAnchor.m_eAnchorId;
+ m_nPageNumber = rAnchor.m_nPageNumber;
// OD 2004-05-05 #i28701# - get always new increased order number
m_nOrder = ++s_nOrderCounter;
m_oContentAnchor = rAnchor.m_oContentAnchor;
@@ -1647,8 +1655,8 @@ bool SwFormatAnchor::operator==( const SfxPoolItem& rAttr
) const
assert(SfxPoolItem::operator==(rAttr));
SwFormatAnchor const& rFormatAnchor(static_cast<SwFormatAnchor
const&>(rAttr));
// OD 2004-05-05 #i28701# - Note: <mnOrder> hasn't to be considered.
- return ( m_eAnchorId == rFormatAnchor.GetAnchorId() &&
- m_nPageNumber == rFormatAnchor.GetPageNum() &&
+ return ( m_eAnchorId == rFormatAnchor.m_eAnchorId &&
+ m_nPageNumber == rFormatAnchor.m_nPageNumber &&
// compare anchor: either both do not point into a textnode or
// both do (valid m_oContentAnchor) and the positions are equal
(m_oContentAnchor == rFormatAnchor.m_oContentAnchor) );
@@ -1674,7 +1682,7 @@ bool SwFormatAnchor::QueryValue( uno::Any& rVal,
sal_uInt8 nMemberId ) const
case MID_ANCHOR_ANCHORTYPE:
text::TextContentAnchorType eRet;
- switch (GetAnchorId())
+ switch (m_eAnchorId)
{
case RndStdIds::FLY_AT_CHAR:
eRet = text::TextContentAnchorType_AT_CHARACTER;
@@ -1749,10 +1757,12 @@ bool SwFormatAnchor::PutValue( const uno::Any& rVal,
sal_uInt8 nMemberId )
case text::TextContentAnchorType_AT_CHARACTER:
eAnchor = RndStdIds::FLY_AT_CHAR;
break;
- //case text::TextContentAnchorType_AT_PARAGRAPH:
- default:
+ case text::TextContentAnchorType_AT_PARAGRAPH:
eAnchor = RndStdIds::FLY_AT_PARA;
break;
+ default:
+ eAnchor = RndStdIds::FLY_AT_PARA; // just to keep some
compilers happy
+ assert(false);
}
SetType( eAnchor );
}
@@ -1762,9 +1772,9 @@ bool SwFormatAnchor::PutValue( const uno::Any& rVal,
sal_uInt8 nMemberId )
sal_Int16 nVal = 0;
if((rVal >>= nVal) && nVal > 0)
{
- SetPageNum( nVal );
- if (RndStdIds::FLY_AT_PAGE == GetAnchorId())
+ if (RndStdIds::FLY_AT_PAGE == m_eAnchorId)
{
+ SetPageNum( nVal );
// If the anchor type is page and a valid page number
// is set, the content position has to be deleted to not
// confuse the layout (frmtool.cxx). However, if the
@@ -1772,6 +1782,11 @@ bool SwFormatAnchor::PutValue( const uno::Any& rVal,
sal_uInt8 nMemberId )
// be kept.
m_oContentAnchor.reset();
}
+ else
+ {
+ assert(false && "cannot set page number on this anchor
type");
+ bRet = false;
+ }
}
else
bRet = false;
diff --git a/sw/source/core/undo/unattr.cxx b/sw/source/core/undo/unattr.cxx
index ac57fe8adcba..ca6a53f71e99 100644
--- a/sw/source/core/undo/unattr.cxx
+++ b/sw/source/core/undo/unattr.cxx
@@ -383,24 +383,23 @@ void SwUndoFormatAttr::SaveFlyAnchor( const SwFormat *
pFormat, bool bSvDrwPt )
const SwFormatAnchor& rAnchor =
m_oOldSet->Get( RES_ANCHOR, false );
- if( !rAnchor.GetAnchorNode() )
+ if( !rAnchor.GetAnchorNode() || rAnchor.GetAnchorId() ==
RndStdIds::FLY_AT_PAGE)
return;
- sal_Int32 nContent = 0;
switch( rAnchor.GetAnchorId() ) {
case RndStdIds::FLY_AS_CHAR:
case RndStdIds::FLY_AT_CHAR:
- nContent = rAnchor.GetAnchorContentOffset();
+ m_nAnchorContentOffset = rAnchor.GetAnchorContentOffset();
[[fallthrough]];
case RndStdIds::FLY_AT_PARA:
case RndStdIds::FLY_AT_FLY:
m_nNodeIndex = rAnchor.GetAnchorNode()->GetIndex();
break;
default:
- return;
+ assert(false);
}
- SwFormatAnchor aAnchor( rAnchor.GetAnchorId(), nContent );
+ SwFormatAnchor aAnchor( rAnchor.GetAnchorId(), 0 );
m_oOldSet->Put( aAnchor );
}
@@ -431,7 +430,7 @@ bool
SwUndoFormatAttr::RestoreFlyAnchor(::sw::UndoRedoContext & rContext)
SwPosition aPos( *pNd );
if ((RndStdIds::FLY_AS_CHAR == rAnchor.GetAnchorId()) ||
(RndStdIds::FLY_AT_CHAR == rAnchor.GetAnchorId())) {
- aPos.SetContent( rAnchor.GetPageNum() );
+ aPos.SetContent( m_nAnchorContentOffset );
if ( aPos.GetContentIndex() >
pNd->GetTextNode()->GetText().getLength()) {
// #i35443# - invalid position.
// Thus, anchor attribute not restored
diff --git a/sw/source/core/unocore/unoframe.cxx
b/sw/source/core/unocore/unoframe.cxx
index a55e4d976a7c..ff9e06e93219 100644
--- a/sw/source/core/unocore/unoframe.cxx
+++ b/sw/source/core/unocore/unoframe.cxx
@@ -183,12 +183,12 @@ bool
BaseFrameProperties_Impl::FillBaseProperties(SfxItemSet& rToSet, const SfxI
// always add an anchor to the set
SwFormatAnchor aAnchor ( rFromSet.Get ( RES_ANCHOR ) );
{
- const ::uno::Any* pAnchorPgNo;
- if(GetProperty(RES_ANCHOR, MID_ANCHOR_PAGENUM, pAnchorPgNo))
- bRet &= static_cast<SfxPoolItem&>(aAnchor).PutValue(*pAnchorPgNo,
MID_ANCHOR_PAGENUM);
const ::uno::Any* pAnchorType;
if(GetProperty(RES_ANCHOR, MID_ANCHOR_ANCHORTYPE, pAnchorType))
bRet &= static_cast<SfxPoolItem&>(aAnchor).PutValue(*pAnchorType,
MID_ANCHOR_ANCHORTYPE);
+ const ::uno::Any* pAnchorPgNo;
+ if(GetProperty(RES_ANCHOR, MID_ANCHOR_PAGENUM, pAnchorPgNo))
+ bRet &= static_cast<SfxPoolItem&>(aAnchor).PutValue(*pAnchorPgNo,
MID_ANCHOR_PAGENUM);
}
rToSet.Put(aAnchor);
diff --git a/sw/source/ui/frmdlg/frmpage.cxx b/sw/source/ui/frmdlg/frmpage.cxx
index 94c86e95df5e..74ccc1a51809 100644
--- a/sw/source/ui/frmdlg/frmpage.cxx
+++ b/sw/source/ui/frmdlg/frmpage.cxx
@@ -1099,7 +1099,7 @@ bool SwFramePage::FillItemSet(SfxItemSet *rSet)
OSL_ENSURE( pSh , "shell not found");
if (pSh)
{
- SwFormatAnchor aAnc( eAnchorId, pSh->GetPhyPageNum() );
+ SwFormatAnchor aAnc( eAnchorId, eAnchorId ==
RndStdIds::FLY_AT_PAGE ? pSh->GetPhyPageNum() : 0 );
bRet = nullptr != rSet->Put( aAnc );
}
}
@@ -1767,7 +1767,7 @@ DeactivateRC SwFramePage::DeactivatePage(SfxItemSet *
_pSet)
if (pSh)
{
RndStdIds eAnchorId = GetAnchor();
- SwFormatAnchor aAnc( eAnchorId, pSh->GetPhyPageNum() );
+ SwFormatAnchor aAnc( eAnchorId, eAnchorId ==
RndStdIds::FLY_AT_PAGE ? pSh->GetPhyPageNum() : 0 );
_pSet->Put( aAnc );
}
}
diff --git a/sw/source/uibase/frmdlg/frmmgr.cxx
b/sw/source/uibase/frmdlg/frmmgr.cxx
index d135f5f2d24b..729892398997 100644
--- a/sw/source/uibase/frmdlg/frmmgr.cxx
+++ b/sw/source/uibase/frmdlg/frmmgr.cxx
@@ -227,7 +227,7 @@ void SwFlyFrameAttrMgr::SetAnchor( RndStdIds eId )
sal_uInt16 nPhyPageNum, nVirtPageNum;
m_pOwnSh->GetPageNum( nPhyPageNum, nVirtPageNum );
- m_aSet.Put( SwFormatAnchor( eId, nPhyPageNum ) );
+ m_aSet.Put( SwFormatAnchor( eId, RndStdIds::FLY_AT_PAGE == eId ?
nPhyPageNum : 0 ) );
if ((RndStdIds::FLY_AT_PAGE == eId) || (RndStdIds::FLY_AT_PARA == eId) ||
(RndStdIds::FLY_AT_CHAR == eId)
|| (RndStdIds::FLY_AT_FLY == eId))
{
diff --git a/sw/source/uibase/shells/basesh.cxx
b/sw/source/uibase/shells/basesh.cxx
index 07a44e6d78be..c4154b00de7e 100644
--- a/sw/source/uibase/shells/basesh.cxx
+++ b/sw/source/uibase/shells/basesh.cxx
@@ -1474,7 +1474,7 @@ void SwBaseShell::Execute(SfxRequest &rReq)
rSh.ChgAnchor(eSet);
else if (rSh.IsFrameSelected())
{
- SwFormatAnchor aAnc(eSet, rSh.GetPhyPageNum());
+ SwFormatAnchor aAnc(eSet, eSet == RndStdIds::FLY_AT_PAGE ?
rSh.GetPhyPageNum() : 0);
SfxItemSet
aSet(SwFEShell::makeItemSetFromFormatAnchor(GetPool(), aAnc));
rSh.SetFlyFrameAttr(aSet);
}
