include/svl/poolitem.hxx | 4 +++- sfx2/source/control/statcach.cxx | 39 +++++++++------------------------------ svl/source/items/poolitem.cxx | 27 ++++++++++++++++++++++----- 3 files changed, 34 insertions(+), 36 deletions(-)
New commits: commit 6042e60d69c40d1f307710e60a278cb286d68603 Author: Armin Le Grand (Collabora) <[email protected]> AuthorDate: Wed Feb 5 18:55:51 2025 +0100 Commit: Armin Le Grand <[email protected]> CommitDate: Thu Feb 6 19:09:23 2025 +0100 tdf#164745 fix SfxStateCache Seems I did too much with tdf#162666 by also trying to remember invalid and disabled states in pLastItem, so went over it again. Also corrected SetVisibleState to also take action when disabled item. Not urgently necessary, but stumbled over it: The items used for INVALID_POOL_ITEM and DISABLED_POOL_ITEM were based on the same class. Not a problem technically, these are 'special' static items that represent a state, but comparing them would have claimed these to be equal what should not be the case. Thus I based each of these on an own derivation of SfxPoolItem. Change-Id: Ic9dd1a21772bb20b25ed8f7fa929da74edb79e42 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/181194 Tested-by: Jenkins Reviewed-by: Armin Le Grand <[email protected]> diff --git a/include/svl/poolitem.hxx b/include/svl/poolitem.hxx index 6d0105707077..440d152de9a0 100644 --- a/include/svl/poolitem.hxx +++ b/include/svl/poolitem.hxx @@ -122,8 +122,9 @@ enum class SfxItemType : sal_uInt16 CntUInt32ItemType, DatabaseMapItemType, DbuTypeCollectionItemType, + DisabledItemType, DriverPoolingSettingsItemType, - InvalidOrDisabledItemType, + InvalidItemType, MediaItemType, NameOrIndexType, OfaPtrItemType, @@ -562,6 +563,7 @@ class SfxItemPool; typedef struct _xmlTextWriter* xmlTextWriterPtr; class ItemInstanceManager; +// macro to define overloaded ItemType function, see explanations below #define DECLARE_ITEM_TYPE_FUNCTION(T) \ virtual SfxItemType ItemType() const override { return SfxItemType::T##Type; } diff --git a/sfx2/source/control/statcach.cxx b/sfx2/source/control/statcach.cxx index 4432f0bc37e6..34c2709af855 100644 --- a/sfx2/source/control/statcach.cxx +++ b/sfx2/source/control/statcach.cxx @@ -361,7 +361,8 @@ void SfxStateCache::SetVisibleState( bool bShow ) bItemVisible = bShow; if ( bShow ) { - if ( IsInvalidItem(pLastItem) || ( pLastItem == nullptr )) + // tdf#164745 also need to do this when disabled item + if ( nullptr == pLastItem || IsInvalidItem(pLastItem) || IsDisabledItem(pLastItem) ) { pState = new SfxVoidItem( nId ); bDeleteItem = true; @@ -431,39 +432,17 @@ void SfxStateCache::SetState_Impl static_cast<SfxDispatchController_Impl *>(pInternalController)->StateChanged( nId, eState, pState, &aSlotServ ); // Remember new value - if (!IsInvalidItem(pLastItem) && !IsDisabledItem(pLastItem)) + if ( !IsInvalidItem(pLastItem) && !IsDisabledItem(pLastItem) ) { - // tdf#162666 only delete if it *was* cloned delete pLastItem; + 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(); - } - } + // tdf#164745 clone when it exists and it's really an item + if ( pState && !IsInvalidItem(pState) && !IsDisabledItem(pState) ) + pLastItem = pState->Clone(); + else + pLastItem = nullptr; eLastState = eState; bItemDirty = false; diff --git a/svl/source/items/poolitem.cxx b/svl/source/items/poolitem.cxx index 2aa31be8db24..917a5e425a18 100644 --- a/svl/source/items/poolitem.cxx +++ b/svl/source/items/poolitem.cxx @@ -293,7 +293,9 @@ bool SfxPoolItem::areSame(const SfxPoolItem& rItem1, const SfxPoolItem& rItem2) namespace { -class InvalidOrDisabledItem final : public SfxPoolItem +// tdf#164745 make InvalidItem and DisabledItem two classes to +// avoi d that op== sees them as equal +class InvalidItem final : public SfxPoolItem { virtual bool operator==(const SfxPoolItem&) const override { return true; } virtual SfxPoolItem* Clone(SfxItemPool*) const override { return nullptr; } @@ -301,15 +303,30 @@ class InvalidOrDisabledItem final : public SfxPoolItem public: // make it StaticDefaultItem to process similar to these // which is plausible (never change and are not allowed to) - DECLARE_ITEM_TYPE_FUNCTION(InvalidOrDisabledItem) - InvalidOrDisabledItem() + DECLARE_ITEM_TYPE_FUNCTION(InvalidItem) + InvalidItem() : SfxPoolItem(0) { setStaticDefault(); } }; -InvalidOrDisabledItem aInvalidItem; -InvalidOrDisabledItem aDisabledItem; +class DisabledItem final : public SfxPoolItem +{ + virtual bool operator==(const SfxPoolItem&) const override { return true; } + virtual SfxPoolItem* Clone(SfxItemPool*) const override { return nullptr; } + +public: + // make it StaticDefaultItem to process similar to these + // which is plausible (never change and are not allowed to) + DECLARE_ITEM_TYPE_FUNCTION(DisabledItem) + DisabledItem() + : SfxPoolItem(0) + { + setStaticDefault(); + } +}; +InvalidItem aInvalidItem; +DisabledItem aDisabledItem; } SfxPoolItem const* const INVALID_POOL_ITEM = &aInvalidItem;
