tdf#158783 Correct compares of SwTOXMark Items
That item was never 'pooled', so operator== was not really
ever used. It just compared the 'type', so pretty many
instances were assumed to be equal, what is wrong.
We discussed to implement it (there is quite some content),
but we came to the point that it's only safe to say
instances are equal when same instance -> fallback to ptr
compare.
This came into play since I identified/changed many (160?)
places where SfxPoolItems were ptr-compared when doing that
paradigm change in Items. This leads to the two methods
'areSfxPoolItemPtrsEqual' which just makes ptr compare and
'SfxPoolItem::areSame' which also will use op==. For the
initial adaption I chose the wrong function adapting
places where SwTOXMark were involved.
Change-Id: I7df029ad4542719681b1455de17ed5990d248395
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161963
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
Tested-by: Armin Le Grand <Armin.Le.Grand@me.com>
(cherry picked from commit d22a86089edfcadbef5231525a2947b954f4784e)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164996
Tested-by: Jenkins
Reviewed-by: Adolfo Jayme Barrientos <fitojb@ubuntu.com>
diff --git a/sw/source/core/tox/tox.cxx b/sw/source/core/tox/tox.cxx
index 04f43e5..b29bafd 100644
--- a/sw/source/core/tox/tox.cxx
+++ b/sw/source/core/tox/tox.cxx
@@ -148,7 +148,13 @@ void SwTOXMark::RegisterToTOXType(SwTOXType& rType)
bool SwTOXMark::operator==( const SfxPoolItem& rAttr ) const
{
assert(SfxPoolItem::operator==(rAttr));
return m_pType == static_cast<const SwTOXMark&>(rAttr).m_pType;
// tdf#158783 this item was never 'pooled', so operator== was not really
// ever used. We discussed to implement it (there is quite some
// content), but we came to the point that it's only safe to say
// instances are equal when same instance -> fallback to ptr compare.
// NOTE: Do *not* use areSfxPoolItemPtrsEqual here, with DBG_UTIL
// active the contol/test code there would again call operator==
return this == &rAttr;
}
SwTOXMark* SwTOXMark::Clone( SfxItemPool* ) const
diff --git a/sw/source/ui/index/swuiidxmrk.cxx b/sw/source/ui/index/swuiidxmrk.cxx
index 39443f7..8a6f74b8 100644
--- a/sw/source/ui/index/swuiidxmrk.cxx
+++ b/sw/source/ui/index/swuiidxmrk.cxx
@@ -287,19 +287,20 @@ void SwIndexMarkPane::InitControls()
bool bShow = false;
pMoveMark = &m_pSh->GotoTOXMark( *pMark, TOX_PRV );
if (!SfxPoolItem::areSame( pMoveMark, pMark ))
// tdf#158783 ptr compare OK for SwTOXMark (more below)
if (!areSfxPoolItemPtrsEqual( pMoveMark, pMark ))
{
m_pSh->GotoTOXMark( *pMoveMark, TOX_NXT );
bShow = true;
}
m_xPrevBT->set_sensitive(!SfxPoolItem::areSame(pMoveMark, pMark));
m_xPrevBT->set_sensitive(!areSfxPoolItemPtrsEqual(pMoveMark, pMark));
pMoveMark = &m_pSh->GotoTOXMark( *pMark, TOX_NXT );
if (!SfxPoolItem::areSame( pMoveMark, pMark ))
if (!areSfxPoolItemPtrsEqual( pMoveMark, pMark ))
{
m_pSh->GotoTOXMark( *pMoveMark, TOX_PRV );
bShow = true;
}
m_xNextBT->set_sensitive(!SfxPoolItem::areSame(pMoveMark, pMark));
m_xNextBT->set_sensitive(!areSfxPoolItemPtrsEqual(pMoveMark, pMark));
if( bShow )
{
m_xPrevBT->show();
@@ -308,19 +309,19 @@ void SwIndexMarkPane::InitControls()
}
pMoveMark = &m_pSh->GotoTOXMark( *pMark, TOX_SAME_PRV );
if (!SfxPoolItem::areSame( pMoveMark, pMark ))
if (!areSfxPoolItemPtrsEqual( pMoveMark, pMark ))
{
m_pSh->GotoTOXMark( *pMoveMark, TOX_SAME_NXT );
bShow = true;
}
m_xPrevSameBT->set_sensitive(!SfxPoolItem::areSame(pMoveMark, pMark));
m_xPrevSameBT->set_sensitive(!areSfxPoolItemPtrsEqual(pMoveMark, pMark));
pMoveMark = &m_pSh->GotoTOXMark( *pMark, TOX_SAME_NXT );
if (!SfxPoolItem::areSame( pMoveMark, pMark ))
if (!areSfxPoolItemPtrsEqual( pMoveMark, pMark ))
{
m_pSh->GotoTOXMark( *pMoveMark, TOX_SAME_PRV );
bShow = true;
}
m_xNextSameBT->set_sensitive(!SfxPoolItem::areSame(pMoveMark, pMark));
m_xNextSameBT->set_sensitive(!areSfxPoolItemPtrsEqual(pMoveMark, pMark));
if( bShow )
{
m_xNextSameBT->show();
@@ -894,25 +895,26 @@ void SwIndexMarkPane::UpdateDialog()
if( m_xPrevBT->get_visible() )
{
const SwTOXMark* pMoveMark = &m_pSh->GotoTOXMark( *pMark, TOX_PRV );
if (!SfxPoolItem::areSame( pMoveMark, pMark ))
// tdf#158783 ptr compare OK for SwTOXMark (more below)
if (!areSfxPoolItemPtrsEqual( pMoveMark, pMark ))
m_pSh->GotoTOXMark( *pMoveMark, TOX_NXT );
m_xPrevBT->set_sensitive( !SfxPoolItem::areSame(pMoveMark, pMark) );
m_xPrevBT->set_sensitive( !areSfxPoolItemPtrsEqual(pMoveMark, pMark) );
pMoveMark = &m_pSh->GotoTOXMark( *pMark, TOX_NXT );
if (!SfxPoolItem::areSame( pMoveMark, pMark ))
if (!areSfxPoolItemPtrsEqual( pMoveMark, pMark ))
m_pSh->GotoTOXMark( *pMoveMark, TOX_PRV );
m_xNextBT->set_sensitive( !SfxPoolItem::areSame(pMoveMark, pMark) );
m_xNextBT->set_sensitive( !areSfxPoolItemPtrsEqual(pMoveMark, pMark) );
}
if (m_xPrevSameBT->get_visible())
{
const SwTOXMark* pMoveMark = &m_pSh->GotoTOXMark( *pMark, TOX_SAME_PRV );
if (!SfxPoolItem::areSame( pMoveMark, pMark ))
if (!areSfxPoolItemPtrsEqual( pMoveMark, pMark ))
m_pSh->GotoTOXMark( *pMoveMark, TOX_SAME_NXT );
m_xPrevSameBT->set_sensitive( !SfxPoolItem::areSame(pMoveMark, pMark) );
m_xPrevSameBT->set_sensitive( !areSfxPoolItemPtrsEqual(pMoveMark, pMark) );
pMoveMark = &m_pSh->GotoTOXMark( *pMark, TOX_SAME_NXT );
if (!SfxPoolItem::areSame( pMoveMark, pMark ))
if (!areSfxPoolItemPtrsEqual( pMoveMark, pMark ))
m_pSh->GotoTOXMark( *pMoveMark, TOX_SAME_PRV );
m_xNextSameBT->set_sensitive( !SfxPoolItem::areSame(pMoveMark, pMark) );
m_xNextSameBT->set_sensitive( !areSfxPoolItemPtrsEqual(pMoveMark, pMark) );
}
const bool bEnable = !m_pSh->HasReadonlySel();
@@ -1013,7 +1015,8 @@ void SwIndexMarkPane::ReInitDlg(SwWrtShell& rWrtShell, SwTOXMark const * pCurTOX
if(pCurTOXMark)
{
for(sal_uInt16 i = 0; i < m_pTOXMgr->GetTOXMarkCount(); i++)
if (SfxPoolItem::areSame(m_pTOXMgr->GetTOXMark(i), pCurTOXMark))
// tdf#158783 ptr compare OK for SwTOXMark (more below)
if (areSfxPoolItemPtrsEqual(m_pTOXMgr->GetTOXMark(i), pCurTOXMark))
{
m_pTOXMgr->SetCurTOXMark(i);
break;
diff --git a/sw/source/uibase/index/toxmgr.cxx b/sw/source/uibase/index/toxmgr.cxx
index 8b8ff6d..c7cd813 100644
--- a/sw/source/uibase/index/toxmgr.cxx
+++ b/sw/source/uibase/index/toxmgr.cxx
@@ -47,7 +47,8 @@ void SwTOXMgr::DeleteTOXMark()
if( m_pCurTOXMark )
{
pNext = const_cast<SwTOXMark*>(&m_pSh->GotoTOXMark( *m_pCurTOXMark, TOX_NXT ));
if (SfxPoolItem::areSame( pNext, m_pCurTOXMark ))
// tdf#158783 ptr compare OK for SwTOXMark (more below)
if (areSfxPoolItemPtrsEqual( pNext, m_pCurTOXMark ))
pNext = nullptr;
m_pSh->DeleteTOXMark( m_pCurTOXMark );
