sfx2/source/control/bindings.cxx | 2 +
sfx2/source/control/statcach.cxx | 46 +++++++++++++++++++++++++++++++++------
sfx2/source/inc/statcach.hxx | 2 -
svl/source/items/voiditem.cxx | 5 ++++
4 files changed, 47 insertions(+), 8 deletions(-)
New commits:
commit 288d720e4940d8fdd71715e6d339765e90716931
Author: Armin Le Grand (allotropia) <armin.le.grand.ext...@allotropia.de>
AuthorDate: Thu Oct 31 13:38:30 2024 +0100
Commit: Armin Le Grand <armin.le.gr...@me.com>
CommitDate: Thu Oct 31 17:22:09 2024 +0100
tdf#162666 Make SfxStateCache handle Items correctly
SfxStateCache had problems processing the extra Item
states disabled/invalid. Note that due to not using a
Pool here it is not possible to use an ItemSet or an
ItemHolder, those would automatically handle these
cases correctly.
In this cache, this currently needs to be done
handish. Also note that because of that this may break
again when changes in the Item/ItemSet/ItemHolder
mechanism may be done.
Change-Id: I007bc25c727e6432062fa0d2af8215a912cb2fff
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/175871
Reviewed-by: Armin Le Grand <armin.le.gr...@me.com>
Tested-by: Jenkins
diff --git a/sfx2/source/control/bindings.cxx b/sfx2/source/control/bindings.cxx
index af9129e455c1..718df9bd9887 100644
--- a/sfx2/source/control/bindings.cxx
+++ b/sfx2/source/control/bindings.cxx
@@ -1207,6 +1207,8 @@ void SfxBindings::UpdateControllers_Impl
SfxItemPool::IsSlot(rFound.nWhichId) )
{
// no Status or Default but without Pool
+ // tdf#162666 note that use DISABLED_POOL_ITEM needs to be
+ // handled correctly in the cache, see comments there
rCache.SetState( SfxItemState::UNKNOWN, DISABLED_POOL_ITEM );
}
else if ( SfxItemState::DISABLED == eState )
diff --git a/sfx2/source/control/statcach.cxx b/sfx2/source/control/statcach.cxx
index 037a38253a31..4432f0bc37e6 100644
--- a/sfx2/source/control/statcach.cxx
+++ b/sfx2/source/control/statcach.cxx
@@ -127,6 +127,8 @@ void SAL_CALL BindDispatch_Impl::statusChanged( const
css::frame::FeatureStateE
pItem->PutValue( aAny, 0 );
}
else
+ // tdf#162666 nId should not be zero. This will now create
+ // a SAL_INFO in SfxVoidItem::SfxVoidItem
pItem.reset( new SfxVoidItem( nId ) );
}
pArg = pItem.get();
@@ -203,8 +205,13 @@ SfxStateCache::SfxStateCache( sal_uInt16 nFuncId ):
SfxStateCache::~SfxStateCache()
{
DBG_ASSERT( pController == nullptr && pInternalController == nullptr,
"there are still Controllers registered" );
- if ( !IsInvalidItem(pLastItem) )
+
+ if ( !IsInvalidItem(pLastItem) && !IsDisabledItem(pLastItem) )
+ {
+ // tdf#162666 only delete if it *was* cloned
delete pLastItem;
+ }
+
if ( mxDispatch.is() )
mxDispatch->Release();
}
@@ -424,15 +431,40 @@ void SfxStateCache::SetState_Impl
static_cast<SfxDispatchController_Impl
*>(pInternalController)->StateChanged( nId, eState, pState, &aSlotServ );
// Remember new value
- if ( !IsInvalidItem(pLastItem) )
+ if (!IsInvalidItem(pLastItem) && !IsDisabledItem(pLastItem))
{
+ // tdf#162666 only delete if it *was* cloned
delete pLastItem;
- pLastItem = nullptr;
}
- if ( pState && !IsInvalidItem(pState) )
- pLastItem = pState->Clone();
- else
- pLastItem = nullptr;
+
+ // always reset to nullptr
+ pLastItem = nullptr;
+
+ if ( nullptr != pState)
+ {
+ if (IsInvalidItem(pState))
+ {
+ // tdf#162666 if invalid, use INVALID_POOL_ITEM
+ pLastItem = INVALID_POOL_ITEM;
+ }
+ else if (IsDisabledItem(pState))
+ {
+ // tdf#162666 if disabled, use DISABLED_POOL_ITEM
+ pLastItem = DISABLED_POOL_ITEM;
+ }
+ else
+ {
+ // tdf#162666 in all other cases, clone the Item. Note that
+ // due to not using a Pool here it is not possible to use a
+ // ItemSet or a ItemHolder, those would automatically handle
+ // these cases correctly. In this cache, this currently needs
+ // to be done handish. Also note that this may break again
+ // when changes in the Item/ItemSet/ItemHolder mechanism may
+ // be done
+ pLastItem = pState->Clone();
+ }
+ }
+
eLastState = eState;
bItemDirty = false;
}
diff --git a/sfx2/source/inc/statcach.hxx b/sfx2/source/inc/statcach.hxx
index 388e184f6653..2e648bd4b7fb 100644
--- a/sfx2/source/inc/statcach.hxx
+++ b/sfx2/source/inc/statcach.hxx
@@ -67,7 +67,7 @@ friend class BindDispatch_Impl;
css::uno::Reference < css::frame::XDispatch > xMyDispatch;
SfxControllerItem* pController; // Pointer to first bound
Controller (interlinked with each other)
SfxSlotServer aSlotServ; // SlotServer, SlotPtr = 0 -> not
on Stack
- SfxPoolItem* pLastItem; // Last sent Item, never -1
+ const SfxPoolItem* pLastItem; // Last sent Item, never -1
SfxItemState eLastState; // Last sent State
bool bCtrlDirty:1; // Update Controller?
bool bSlotDirty:1; // Present Function, must be
updated
diff --git a/svl/source/items/voiditem.cxx b/svl/source/items/voiditem.cxx
index f62e693272c8..ba72003a4149 100644
--- a/svl/source/items/voiditem.cxx
+++ b/svl/source/items/voiditem.cxx
@@ -19,12 +19,17 @@
#include <svl/voiditem.hxx>
#include <libxml/xmlwriter.h>
+#include <sal/log.hxx>
SfxPoolItem* SfxVoidItem::CreateDefault() { return new SfxVoidItem(0); }
SfxVoidItem::SfxVoidItem(sal_uInt16 which)
: SfxPoolItem(which, SfxItemType::SfxVoidItemType)
{
+#ifdef DBG_UTIL
+ if (0 == which)
+ SAL_INFO("svl.items", "ITEM: SfxVoidItem with 0 == WhichID gets
constructed (!)");
+#endif
}
SfxVoidItem::SfxVoidItem(const SfxVoidItem& rCopy)
