include/svl/poolitem.hxx | 4 ++- sfx2/source/control/statcach.cxx | 39 ++++++++----------------------------- svl/source/items/poolitem.cxx | 27 ++++++++++++++++++++----- sw/source/core/txtnode/txtatr2.cxx | 21 +++++++++---------- 4 files changed, 44 insertions(+), 47 deletions(-)
New commits: commit 04d761d965e7d8d6b3a51ee1558e7f739360f58f Author: Noel Grandin <noelgran...@gmail.com> AuthorDate: Sun Dec 22 22:35:09 2024 +0200 Commit: Xisco Fauli <xiscofa...@libreoffice.org> CommitDate: Mon Feb 10 11:31:19 2025 +0100 convert some warnings in SwTextCharFormat to assert Change-Id: I665db920f11108c29ac0cbbf44090f6ff4847e1d Reviewed-on: https://gerrit.libreoffice.org/c/core/+/179178 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk> Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/181265 diff --git a/sw/source/core/txtnode/txtatr2.cxx b/sw/source/core/txtnode/txtatr2.cxx index c18a9f6d4e23..e57e612fad1f 100644 --- a/sw/source/core/txtnode/txtatr2.cxx +++ b/sw/source/core/txtnode/txtatr2.cxx @@ -75,11 +75,10 @@ SwTextCharFormat::~SwTextCharFormat( ) void SwTextCharFormat::TriggerNodeUpdate(const sw::LegacyModifyHint& rHint) { const auto nWhich = rHint.GetWhich(); - SAL_WARN_IF( - !isCHRATR(nWhich) && - RES_OBJECTDYING != nWhich && - RES_ATTRSET_CHG != nWhich && - RES_FMT_CHG != nWhich, "sw.core", "SwTextCharFormat::TriggerNodeUpdate: unknown hint type"); + assert( (isCHRATR(nWhich) || + RES_OBJECTDYING == nWhich || + RES_ATTRSET_CHG == nWhich || + RES_FMT_CHG == nWhich) && "SwTextCharFormat::TriggerNodeUpdate: unknown hint type"); if(m_pTextNode) { @@ -185,8 +184,8 @@ void SwTextINetFormat::SwClientNotify(const SwModify&, const SfxHint& rHint) return; auto pLegacy = static_cast<const sw::LegacyModifyHint*>(&rHint); const auto nWhich = pLegacy->GetWhich(); - OSL_ENSURE(isCHRATR(nWhich) || (RES_OBJECTDYING == nWhich) - || (RES_ATTRSET_CHG == nWhich) || (RES_FMT_CHG == nWhich), + assert((isCHRATR(nWhich) || (RES_OBJECTDYING == nWhich) + || (RES_ATTRSET_CHG == nWhich) || (RES_FMT_CHG == nWhich)) && "SwTextINetFormat::SwClientNotify: unknown hint."); if(!m_pTextNode) return; @@ -225,10 +224,10 @@ void SwTextRuby::SwClientNotify(const SwModify&, const SfxHint& rHint) return; auto pLegacy = static_cast<const sw::LegacyModifyHint*>(&rHint); const auto nWhich = pLegacy->GetWhich(); - SAL_WARN_IF( !isCHRATR(nWhich) - && (RES_OBJECTDYING == nWhich) - && (RES_ATTRSET_CHG == nWhich) - && (RES_FMT_CHG == nWhich), "sw.core", "SwTextRuby::SwClientNotify(): unknown legacy hint"); + assert( (isCHRATR(nWhich) + || (RES_OBJECTDYING == nWhich) + || (RES_ATTRSET_CHG == nWhich) + || (RES_FMT_CHG == nWhich)) && "SwTextRuby::SwClientNotify(): unknown legacy hint"); if(!m_pTextNode) return; SwUpdateAttr aUpdateAttr(GetStart(), *GetEnd(), nWhich); commit 04693b1b6e54de341e82bab0455be340dd0fceb0 Author: Armin Le Grand (Collabora) <armin.le.gr...@me.com> AuthorDate: Wed Feb 5 18:55:51 2025 +0100 Commit: Xisco Fauli <xiscofa...@libreoffice.org> CommitDate: Mon Feb 10 11:31:05 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 <armin.le.gr...@me.com> Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/181229 diff --git a/include/svl/poolitem.hxx b/include/svl/poolitem.hxx index 2324c7a35c6e..e9d455eeed83 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, @@ -566,6 +567,7 @@ class SfxItemSet; 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;