editeng/source/editeng/editdoc.cxx | 1 editeng/source/editeng/eerdll.cxx | 21 +++---- include/editeng/eeitem.hxx | 13 ++-- include/svl/itemset.hxx | 10 +-- include/svl/poolitem.hxx | 23 +++---- sfx2/source/control/bindings.cxx | 3 - sfx2/source/control/ctrlitem.cxx | 2 sfx2/source/control/dispatch.cxx | 4 - sfx2/source/control/shell.cxx | 2 sfx2/source/control/statcach.cxx | 6 +- sfx2/source/control/unoctitm.cxx | 6 +- sfx2/source/doc/objxtor.cxx | 2 sfx2/source/statbar/stbitem.cxx | 2 sfx2/source/toolbox/tbxitem.cxx | 2 svl/source/items/itemset.cxx | 108 +++++++++++++------------------------ svl/source/items/poolitem.cxx | 9 +-- svl/source/items/voiditem.cxx | 3 - sw/source/core/attr/swatrset.cxx | 10 +-- sw/source/uibase/docvw/romenu.cxx | 2 19 files changed, 95 insertions(+), 134 deletions(-)
New commits: commit 38072fd7eb7d53237efbe0d8bacc7db1c4f3131c Author: Armin Le Grand (allotropia) <armin.le.grand.ext...@allotropia.de> AuthorDate: Mon Jan 22 19:18:32 2024 +0100 Commit: Armin Le Grand <armin.le.gr...@me.com> CommitDate: Tue Jan 23 10:30:59 2024 +0100 ITEM: Solve SfxVoidItem(0) situation An instance of SfxVoidItem(0) was used to signal the SfxItemState::DISABLED. This was done not only using WhichID == 0, but using isVoidItem() at the SfxPoolItem. Unfortunately this mixes up with usages of SfxVoidItems, mostly for UI stuff/Slots. This also means that all the time an SfxVoidItem had to be cloned/delete when when added/removed from ItemSet or ItemHolder. Much more action than e.g. for INVALID_POOL_ITEM which we already use by havong just a simple ptr to a single static instance of an Item. Disabled should do the same thing. Unfortunately also the functionality was mixed with non-SfxItemState::DISABLED purposes and these were very hard to be separated. But the current solution works now after some quirks doing that. It even oes no more need the isVoidItem() flag at the SfxPoolItem. Change-Id: I99f03db144f541ae4ea35f3775b3b3d58a375a81 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162414 Tested-by: Jenkins Reviewed-by: Armin Le Grand <armin.le.gr...@me.com> diff --git a/editeng/source/editeng/editdoc.cxx b/editeng/source/editeng/editdoc.cxx index 1bd6504b2374..0e26a5802a00 100644 --- a/editeng/source/editeng/editdoc.cxx +++ b/editeng/source/editeng/editdoc.cxx @@ -207,7 +207,6 @@ const SfxItemInfo aItemInfos[EDITITEMCOUNT] = { SID_ATTR_CHAR_CTL_POSTURE, SFX_ITEMINFOFLAG_NONE }, // EE_CHAR_ITALIC_CTL { SID_ATTR_CHAR_EMPHASISMARK, SFX_ITEMINFOFLAG_NONE }, // EE_CHAR_EMPHASISMARK { SID_ATTR_CHAR_RELIEF, SFX_ITEMINFOFLAG_NONE }, // EE_CHAR_RELIEF - { 0, SFX_ITEMINFOFLAG_NONE }, // EE_CHAR_RUBI_DUMMY { 0, SFX_ITEMINFOFLAG_SUPPORT_SURROGATE }, // EE_CHAR_XMLATTRIBS { SID_ATTR_CHAR_OVERLINE, SFX_ITEMINFOFLAG_NONE }, // EE_CHAR_OVERLINE { SID_ATTR_CHAR_CASEMAP, SFX_ITEMINFOFLAG_NONE }, // EE_CHAR_CASEMAP diff --git a/editeng/source/editeng/eerdll.cxx b/editeng/source/editeng/eerdll.cxx index 9e3e8c4cf8c5..d93eded8cbf7 100644 --- a/editeng/source/editeng/eerdll.cxx +++ b/editeng/source/editeng/eerdll.cxx @@ -137,19 +137,18 @@ DefItems::DefItems() rDefItems[44] = new SvxPostureItem( ITALIC_NONE, EE_CHAR_ITALIC_CTL ); rDefItems[45] = new SvxEmphasisMarkItem( FontEmphasisMark::NONE, EE_CHAR_EMPHASISMARK ); rDefItems[46] = new SvxCharReliefItem( FontRelief::NONE, EE_CHAR_RELIEF ); - rDefItems[47] = new SfxVoidItem( EE_CHAR_RUBI_DUMMY ); - rDefItems[48] = new SvXMLAttrContainerItem( EE_CHAR_XMLATTRIBS ); - rDefItems[49] = new SvxOverlineItem( LINESTYLE_NONE, EE_CHAR_OVERLINE ); - rDefItems[50] = new SvxCaseMapItem( SvxCaseMap::NotMapped, EE_CHAR_CASEMAP ); - rDefItems[51] = new SfxGrabBagItem( EE_CHAR_GRABBAG ); - rDefItems[52] = new SvxColorItem( COL_AUTO, EE_CHAR_BKGCOLOR ); + rDefItems[47] = new SvXMLAttrContainerItem( EE_CHAR_XMLATTRIBS ); + rDefItems[48] = new SvxOverlineItem( LINESTYLE_NONE, EE_CHAR_OVERLINE ); + rDefItems[49] = new SvxCaseMapItem( SvxCaseMap::NotMapped, EE_CHAR_CASEMAP ); + rDefItems[50] = new SfxGrabBagItem( EE_CHAR_GRABBAG ); + rDefItems[51] = new SvxColorItem( COL_AUTO, EE_CHAR_BKGCOLOR ); // Features - rDefItems[53] = new SfxVoidItem( EE_FEATURE_TAB ); - rDefItems[54] = new SfxVoidItem( EE_FEATURE_LINEBR ); - rDefItems[55] = new SvxColorItem( COL_RED, EE_FEATURE_NOTCONV ); - rDefItems[56] = new SvxFieldItem( SvxFieldData(), EE_FEATURE_FIELD ); + rDefItems[52] = new SfxVoidItem( EE_FEATURE_TAB ); + rDefItems[53] = new SfxVoidItem( EE_FEATURE_LINEBR ); + rDefItems[54] = new SvxColorItem( COL_RED, EE_FEATURE_NOTCONV ); + rDefItems[55] = new SvxFieldItem( SvxFieldData(), EE_FEATURE_FIELD ); - assert(EDITITEMCOUNT == 57 && "ITEMCOUNT changed, adjust DefItems!"); + assert(EDITITEMCOUNT == 56 && "ITEMCOUNT changed, adjust DefItems!"); // Init DefFonts: GetDefaultFonts( *static_cast<SvxFontItem*>(rDefItems[EE_CHAR_FONTINFO - EE_ITEMS_START]), diff --git a/include/editeng/eeitem.hxx b/include/editeng/eeitem.hxx index f862244865c1..92c7830af0cc 100644 --- a/include/editeng/eeitem.hxx +++ b/include/editeng/eeitem.hxx @@ -123,14 +123,13 @@ constexpr TypedWhichId<SvxPostureItem> EE_CHAR_ITALIC_CJK (EE_CHAR_S constexpr TypedWhichId<SvxPostureItem> EE_CHAR_ITALIC_CTL (EE_CHAR_START+24); constexpr TypedWhichId<SvxEmphasisMarkItem> EE_CHAR_EMPHASISMARK (EE_CHAR_START+25); constexpr TypedWhichId<SvxCharReliefItem> EE_CHAR_RELIEF (EE_CHAR_START+26); -constexpr TypedWhichId<SfxVoidItem> EE_CHAR_RUBI_DUMMY (EE_CHAR_START+27); -constexpr TypedWhichId<SvXMLAttrContainerItem> EE_CHAR_XMLATTRIBS (EE_CHAR_START+28); -constexpr TypedWhichId<SvxOverlineItem> EE_CHAR_OVERLINE (EE_CHAR_START+29); -constexpr TypedWhichId<SvxCaseMapItem> EE_CHAR_CASEMAP (EE_CHAR_START+30); -constexpr TypedWhichId<SfxGrabBagItem> EE_CHAR_GRABBAG (EE_CHAR_START+31); -constexpr TypedWhichId<SvxColorItem> EE_CHAR_BKGCOLOR (EE_CHAR_START+32); +constexpr TypedWhichId<SvXMLAttrContainerItem> EE_CHAR_XMLATTRIBS (EE_CHAR_START+27); +constexpr TypedWhichId<SvxOverlineItem> EE_CHAR_OVERLINE (EE_CHAR_START+28); +constexpr TypedWhichId<SvxCaseMapItem> EE_CHAR_CASEMAP (EE_CHAR_START+29); +constexpr TypedWhichId<SfxGrabBagItem> EE_CHAR_GRABBAG (EE_CHAR_START+30); +constexpr TypedWhichId<SvxColorItem> EE_CHAR_BKGCOLOR (EE_CHAR_START+31); -constexpr sal_uInt16 EE_CHAR_END (EE_CHAR_START + 32); +constexpr sal_uInt16 EE_CHAR_END (EE_CHAR_START + 31); constexpr sal_uInt16 EE_FEATURE_START (EE_CHAR_END + 1); constexpr sal_uInt16 EE_FEATURE_TAB (EE_FEATURE_START + 0); diff --git a/include/svl/itemset.hxx b/include/svl/itemset.hxx index 691bbd3718c9..8762c77e142f 100644 --- a/include/svl/itemset.hxx +++ b/include/svl/itemset.hxx @@ -246,9 +246,9 @@ public: { return HasItem(sal_uInt16(nWhich), reinterpret_cast<const SfxPoolItem**>(ppItem)); } void DisableItem(sal_uInt16 nWhich) - { DisableItem_ForWhichID(nWhich); } + { DisableOrInvalidateItem_ForWhichID(true, nWhich); } void InvalidateItem(sal_uInt16 nWhich) - { InvalidateItem_ForWhichID(nWhich); } + { DisableOrInvalidateItem_ForWhichID(false, nWhich); } sal_uInt16 ClearItem( sal_uInt16 nWhich = 0); void ClearInvalidItems(); void InvalidateAllItems(); // HACK(via nWhich = 0) ??? @@ -315,10 +315,8 @@ private: void MergeItem_Impl(const SfxPoolItem **ppFnd1, const SfxPoolItem *pFnd2, bool bIgnoreDefaults); // split version(s) of InvalidateItem/DisableItem for input types WhichID and Offset - void InvalidateItem_ForWhichID(sal_uInt16 nWhich); - void InvalidateItem_ForOffset(sal_uInt16 nOffset); - void DisableItem_ForWhichID(sal_uInt16 nWhich); - void DisableItem_ForOffset(sal_uInt16 nOffset); + void DisableOrInvalidateItem_ForWhichID(bool bDsiable, sal_uInt16 nWhich); + void DisableOrInvalidateItem_ForOffset(bool bDisable, sal_uInt16 nOffset); // split version(s) of GetItemStateImpl for input types WhichID and Offset SfxItemState GetItemState_ForWhichID( SfxItemState eState, sal_uInt16 nWhich, bool bSrchInParent, const SfxPoolItem **ppItem) const; diff --git a/include/svl/poolitem.hxx b/include/svl/poolitem.hxx index 813e8c421cc4..ab909c4edfec 100644 --- a/include/svl/poolitem.hxx +++ b/include/svl/poolitem.hxx @@ -131,33 +131,28 @@ class SVL_DLLPUBLIC SfxPoolItem // bitfield for Item attributes that are Item-Dependent - // Item is a SfxVoidItem (used for SfxItemState::DISABLED, - // but unfortunately also for some slot stuff with - // Which != 0) -> needs cleanup - bool m_bIsVoidItem : 1; // bit 0 - // Item is registered at some Pool as default. // m_bStaticDefault: direct Pool Item (CAUTION: // these are not really 'static', but should be) // -> needs cleanup // m_bPoolDefault: set by user using SetPoolDefaultItem // those should be better called 'UserDefault' - bool m_bStaticDefault : 1; // bit 1 - bool m_bPoolDefault : 1; // bit 2 + bool m_bStaticDefault : 1; // bit 0 + bool m_bPoolDefault : 1; // bit 1 // Item is derived from SfxSetItem -> is Pool-dependent - bool m_bIsSetItem : 1; // bit 3 + bool m_bIsSetItem : 1; // bit 2 // Defines if the Item can be shared/RefCounted else it will be cloned. // Default is true - as it should be for all Items. It is needed by some // SW items, so protected to let them set it in constructor. If this could // be fixed at that Items we may remove this again. - bool m_bShareable : 1; // bit 4 + bool m_bShareable : 1; // bit 3 protected: #ifdef DBG_UTIL // this flag will make debugging item stuff much simpler - bool m_bDeleted : 1; // bit 5 + bool m_bDeleted : 1; // bit 4 #endif private: @@ -168,7 +163,6 @@ private: } protected: - void setIsVoidItem() { m_bIsVoidItem = true; } void setStaticDefault() { m_bStaticDefault = true; } void setPoolDefault() { m_bPoolDefault = true; } void setIsSetItem() { m_bIsSetItem = true; } @@ -186,7 +180,6 @@ public: sal_uInt32 getSerialNumber() const { return m_nSerialNumber; } #endif - bool isVoidItem() const { return m_bIsVoidItem; } bool isStaticDefault() const { return m_bStaticDefault; } bool isPoolDefault() const { return m_bPoolDefault; } bool isSetItem() const { return m_bIsSetItem; } @@ -311,12 +304,18 @@ inline bool IsPooledItem( const SfxPoolItem *pItem ) } SVL_DLLPUBLIC extern SfxPoolItem const * const INVALID_POOL_ITEM; +SVL_DLLPUBLIC extern SfxPoolItem const * const DISABLED_POOL_ITEM; inline bool IsInvalidItem(const SfxPoolItem *pItem) { return pItem == INVALID_POOL_ITEM; } +inline bool IsDisabledItem(const SfxPoolItem *pItem) +{ + return pItem == DISABLED_POOL_ITEM; +} + SVL_DLLPUBLIC bool areSfxPoolItemPtrsEqual(const SfxPoolItem* pItem1, const SfxPoolItem* pItem2); class SVL_DLLPUBLIC SfxPoolItemHint final : public SfxHint diff --git a/sfx2/source/control/bindings.cxx b/sfx2/source/control/bindings.cxx index 5a318d11b77a..549a9fc45e67 100644 --- a/sfx2/source/control/bindings.cxx +++ b/sfx2/source/control/bindings.cxx @@ -1203,8 +1203,7 @@ void SfxBindings::UpdateControllers_Impl SfxItemPool::IsSlot(rFound.nWhichId) ) { // no Status or Default but without Pool - SfxVoidItem aVoid(0); - rCache.SetState( SfxItemState::UNKNOWN, &aVoid ); + rCache.SetState( SfxItemState::UNKNOWN, DISABLED_POOL_ITEM ); } else if ( SfxItemState::DISABLED == eState ) rCache.SetState(SfxItemState::DISABLED, nullptr); diff --git a/sfx2/source/control/ctrlitem.cxx b/sfx2/source/control/ctrlitem.cxx index 9a8c6e0203a2..82ee78cb5dde 100644 --- a/sfx2/source/control/ctrlitem.cxx +++ b/sfx2/source/control/ctrlitem.cxx @@ -293,7 +293,7 @@ SfxItemState SfxControllerItem::GetItemState ? SfxItemState::DISABLED : IsInvalidItem(pState) ? SfxItemState::DONTCARE - : pState->isVoidItem() && !pState->Which() + : IsDisabledItem(pState) ? SfxItemState::UNKNOWN : SfxItemState::DEFAULT; } diff --git a/sfx2/source/control/dispatch.cxx b/sfx2/source/control/dispatch.cxx index a73fd763cd18..3b52f7ae322e 100644 --- a/sfx2/source/control/dispatch.cxx +++ b/sfx2/source/control/dispatch.cxx @@ -1696,7 +1696,7 @@ bool SfxDispatcher::FillState_(const SfxSlotServer& rSvr, SfxItemSet& rState, pItem; pItem = aIter.NextItem() ) { - if ( !IsInvalidItem(pItem) && !pItem->isVoidItem() ) + if ( !IsInvalidItem(pItem) && !IsDisabledItem(pItem) ) { sal_uInt16 nSlotId = rState.GetPool()->GetSlotId(pItem->Which()); SAL_INFO_IF( @@ -1997,7 +1997,7 @@ SfxItemState SfxDispatcher::QueryState( sal_uInt16 nSID, css::uno::Any& rAny ) else { css::uno::Any aState; - if ( !aItem.getItem()->isVoidItem() ) + if ( !IsDisabledItem(aItem.getItem()) ) { sal_uInt16 nSubId( 0 ); SfxItemPool& rPool = pShell->GetPool(); diff --git a/sfx2/source/control/shell.cxx b/sfx2/source/control/shell.cxx index 4d0ae2d66b85..4f4ba99ce3ee 100644 --- a/sfx2/source/control/shell.cxx +++ b/sfx2/source/control/shell.cxx @@ -515,7 +515,7 @@ SfxPoolItemHolder SfxShell::GetSlotState { if ( pStateSet ) pStateSet->ClearItem(nSlotId); - return SfxPoolItemHolder(rPool, new SfxVoidItem(0), true); + return SfxPoolItemHolder(rPool, DISABLED_POOL_ITEM); } // bItemStateSet && eState >= SfxItemState::DEFAULT diff --git a/sfx2/source/control/statcach.cxx b/sfx2/source/control/statcach.cxx index 2346333fd06a..7ce4f11e3ff2 100644 --- a/sfx2/source/control/statcach.cxx +++ b/sfx2/source/control/statcach.cxx @@ -82,6 +82,7 @@ void SAL_CALL BindDispatch_Impl::statusChanged( const css::frame::FeatureStateE std::unique_ptr<SfxPoolItem> pItem; sal_uInt16 nId = pCache->GetId(); SfxItemState eState = SfxItemState::DISABLED; + const SfxPoolItem* pArg(nullptr); if ( !aStatus.IsEnabled ) { // default @@ -128,18 +129,19 @@ void SAL_CALL BindDispatch_Impl::statusChanged( const css::frame::FeatureStateE else pItem.reset( new SfxVoidItem( nId ) ); } + pArg = pItem.get(); } else { // DONTCARE status - pItem.reset( new SfxVoidItem(0) ); + pArg = DISABLED_POOL_ITEM; eState = SfxItemState::UNKNOWN; } for ( SfxControllerItem *pCtrl = pCache->GetItemLink(); pCtrl; pCtrl = pCtrl->GetItemLink() ) - pCtrl->StateChangedAtToolBoxControl( nId, eState, pItem.get() ); + pCtrl->StateChangedAtToolBoxControl( nId, eState, pArg ); } } diff --git a/sfx2/source/control/unoctitm.cxx b/sfx2/source/control/unoctitm.cxx index cd50e70231e7..f138eca95af6 100644 --- a/sfx2/source/control/unoctitm.cxx +++ b/sfx2/source/control/unoctitm.cxx @@ -670,7 +670,7 @@ void SfxDispatchController_Impl::dispatch( const css::util::URL& aURL, { if (const SfxBoolItem* pBoolItem = dynamic_cast<const SfxBoolItem*>(aItem.getItem())) bSuccess = pBoolItem->GetValue(); - else if ( !aItem.getItem()->isVoidItem() ) + else if ( !IsDisabledItem(aItem.getItem()) ) bSuccess = true; // all other types are true } // else bSuccess = false look to line 664 it is false @@ -733,7 +733,7 @@ void SfxDispatchController_Impl::dispatch( const css::util::URL& aURL, aEvent.State = css::frame::DispatchResultState::FAILURE; aEvent.Source = static_cast<css::frame::XDispatch*>(pDispatch); - if ( bSuccess && aItem && !aItem.getItem()->isVoidItem() ) + if ( bSuccess && aItem && !IsDisabledItem(aItem.getItem()) ) { sal_uInt16 nSubId( 0 ); if ( eMapUnit == MapUnit::MapTwip ) @@ -834,7 +834,7 @@ void SfxDispatchController_Impl::StateChanged( sal_uInt16 nSID, SfxItemState eSt return; css::uno::Any aState; - if ( ( eState >= SfxItemState::DEFAULT ) && pState && !IsInvalidItem( pState ) && !pState->isVoidItem() ) + if ( ( eState >= SfxItemState::DEFAULT ) && pState && !IsInvalidItem( pState ) && !IsDisabledItem(pState) ) { // Retrieve metric from pool to have correct sub ID when calling QueryValue sal_uInt16 nSubId( 0 ); diff --git a/sfx2/source/doc/objxtor.cxx b/sfx2/source/doc/objxtor.cxx index 0d2f6b91af43..88c13826e6b1 100644 --- a/sfx2/source/doc/objxtor.cxx +++ b/sfx2/source/doc/objxtor.cxx @@ -590,7 +590,7 @@ bool SfxObjectShell::PrepareClose aPoolItem = pFrame->GetBindings().ExecuteSynchron( SID_SAVEDOC, ppArgs ); } - if (!aPoolItem || aPoolItem.getItem()->isVoidItem() ) + if (!aPoolItem || IsDisabledItem(aPoolItem.getItem()) ) return false; if ( auto pBoolItem = dynamic_cast< const SfxBoolItem *>( aPoolItem.getItem() ) ) if ( !pBoolItem->GetValue() ) diff --git a/sfx2/source/statbar/stbitem.cxx b/sfx2/source/statbar/stbitem.cxx index 6e9ee1bcaa95..6ac8a1a62f9c 100644 --- a/sfx2/source/statbar/stbitem.cxx +++ b/sfx2/source/statbar/stbitem.cxx @@ -376,7 +376,7 @@ void SfxStatusBarControl::StateChangedAtStatusBarControl pBar->SetItemText( nSID, pStr->GetValue() ); else { - DBG_ASSERT( eState != SfxItemState::DEFAULT || pState->isVoidItem(), + DBG_ASSERT( eState != SfxItemState::DEFAULT || IsDisabledItem(pState), "wrong SfxPoolItem subclass in SfxStatusBarControl" ); pBar->SetItemText( nSID, OUString() ); } diff --git a/sfx2/source/toolbox/tbxitem.cxx b/sfx2/source/toolbox/tbxitem.cxx index bf01be5e1921..78a6e5df33ab 100644 --- a/sfx2/source/toolbox/tbxitem.cxx +++ b/sfx2/source/toolbox/tbxitem.cxx @@ -234,7 +234,7 @@ SfxItemState SfxToolBoxControl::GetItemState( ? SfxItemState::DISABLED : IsInvalidItem(pState) ? SfxItemState::DONTCARE - : pState->isVoidItem() && !pState->Which() + : IsDisabledItem(pState) ? SfxItemState::UNKNOWN : SfxItemState::DEFAULT; } diff --git a/svl/source/items/itemset.cxx b/svl/source/items/itemset.cxx index 17862d3ea71b..a7db03c68932 100644 --- a/svl/source/items/itemset.cxx +++ b/svl/source/items/itemset.cxx @@ -293,6 +293,11 @@ SfxPoolItem const* implCreateItemEntry(SfxItemPool& rPool, SfxPoolItem const* pS // just use pSource which equals INVALID_POOL_ITEM return pSource; + if (IsDisabledItem(pSource)) + // SfxItemState::DISABLED aka invalid item + // just use pSource which equals DISABLED_POOL_ITEM + return pSource; + // CAUTION: static default items are not *that* static as it seems // (or: should be). If they are freed with the Pool (see // ::ReleaseDefaults) they will be deleted. Same is true for @@ -306,7 +311,8 @@ SfxPoolItem const* implCreateItemEntry(SfxItemPool& rPool, SfxPoolItem const* pS if (0 == pSource->Which()) { - // These *should* be SfxVoidItem(0) the only Items with 0 == WhichID, + // There should be no Items with 0 == WhichID, but there are some + // constructed for dialog return values AKA result (look for SetReturnValue) // these need to be cloned (currently...) if (bPassingOwnership) return pSource; @@ -441,9 +447,15 @@ void implCleanupItemEntry(SfxPoolItem const* pSource) // nothing to do for invalid item entries return; + if (IsDisabledItem(pSource)) + // SfxItemState::DISABLED aka invalid item + // just use pSource which equals DISABLED_POOL_ITEM + return; + if (0 == pSource->Which()) { - // These *should* be SfxVoidItem(0) the only Items with 0 == WhichID + // There should be no Items with 0 == WhichID, but there are some + // constructed for dialog return values AKA result (look for SetReturnValue) // and need to be deleted delete pSource; return; @@ -612,8 +624,8 @@ void SfxItemSet::checkRemovePoolRegistration(const SfxPoolItem* pItem) // no Item, done return; - if (IsInvalidItem(pItem) || pItem->isVoidItem() || 0 == pItem->Which()) - // checks IsInvalidItem/SfxVoidItem(0) + if (IsInvalidItem(pItem) || IsDisabledItem(pItem)) + // checks IsInvalidItem/IsDisabledItem return; if (SfxItemPool::IsSlot(pItem->Which())) @@ -641,8 +653,8 @@ void SfxItemSet::checkAddPoolRegistration(const SfxPoolItem* pItem) // no Item, done return; - if (IsInvalidItem(pItem) || pItem->isVoidItem() || 0 == pItem->Which()) - // checks IsInvalidItem/SfxVoidItem(0) + if (IsInvalidItem(pItem) || IsDisabledItem(pItem)) + // checks IsInvalidItem/IsDisabledItem return; if (SfxItemPool::IsSlot(pItem->Which())) @@ -669,7 +681,7 @@ sal_uInt16 SfxItemSet::ClearSingleItem_ForOffset( sal_uInt16 nOffset ) { assert(nOffset < TotalCount()); const_iterator aEntry(begin() + nOffset); - assert(nullptr == *aEntry || IsInvalidItem(*aEntry) || (*aEntry)->isVoidItem() || 0 != (*aEntry)->Which()); + assert(nullptr == *aEntry || IsInvalidItem(*aEntry) || IsDisabledItem(*aEntry) || 0 != (*aEntry)->Which()); if (nullptr == *aEntry) // no entry, done @@ -789,7 +801,7 @@ SfxItemState SfxItemSet::GetItemState_ForOffset( sal_uInt16 nOffset, const SfxPo // Different ones are present return SfxItemState::DONTCARE; - if (pCandidate->isVoidItem()) + if (IsDisabledItem(pCandidate)) // Item is Disabled return SfxItemState::DISABLED; @@ -840,10 +852,9 @@ const SfxPoolItem* SfxItemSet::PutImplAsTargetWhich(const SfxPoolItem& rItem, sa const SfxPoolItem* SfxItemSet::PutImpl(const SfxPoolItem& rItem, bool bPassingOwnership) { - if (0 == rItem.Which()) + if (IsDisabledItem(&rItem)) { - // no action needed: this *should* be SfxVoidItem(0) the only Items - // with 0 == WhichID -> only to be used by SfxItemSet::DisableItem + // no action needed: IsDisabledItem if (bPassingOwnership) delete &rItem; return nullptr; @@ -933,7 +944,7 @@ bool SfxItemSet::Put(const SfxItemSet& rSource, bool bInvalidAsDefault) } else { - InvalidateItem_ForWhichID(nWhich); + DisableOrInvalidateItem_ForWhichID(false, nWhich); } } else @@ -998,7 +1009,7 @@ void SfxItemSet::PutExtended break; case SfxItemState::DONTCARE: - InvalidateItem_ForWhichID(nWhich); + DisableOrInvalidateItem_ForWhichID(false, nWhich); break; default: @@ -1025,7 +1036,7 @@ void SfxItemSet::PutExtended break; case SfxItemState::DONTCARE: - InvalidateItem_ForWhichID(nWhich); + DisableOrInvalidateItem_ForWhichID(false, nWhich); break; default: @@ -1264,7 +1275,7 @@ const SfxPoolItem& SfxItemSet::Get( sal_uInt16 nWhich, bool bSrchInParent) const return GetPool()->GetDefaultItem(nWhich); } #ifdef DBG_UTIL - if ((*aFoundOne)->isVoidItem()) + if (IsDisabledItem(*aFoundOne)) SAL_INFO("svl.items", "SFX_WARNING: Getting disabled Item"); #endif return **aFoundOne; @@ -1598,8 +1609,8 @@ void SfxItemSet::MergeValues( const SfxItemSet& rSet ) void SfxItemSet::MergeValue(const SfxPoolItem& rAttr, bool bIgnoreDefaults) { - if (0 == rAttr.Which()) - // seems to be SfxVoidItem(0), nothing to do + if (IsDisabledItem(&rAttr)) + // is IsDisabledItem, nothing to do return; const sal_uInt16 nOffset(GetRanges().getOffsetFromWhich(rAttr.Which())); @@ -1610,17 +1621,17 @@ void SfxItemSet::MergeValue(const SfxPoolItem& rAttr, bool bIgnoreDefaults) } } -void SfxItemSet::InvalidateItem_ForWhichID(sal_uInt16 nWhich) +void SfxItemSet::DisableOrInvalidateItem_ForWhichID(bool bDisable, sal_uInt16 nWhich) { const sal_uInt16 nOffset(GetRanges().getOffsetFromWhich(nWhich)); if (INVALID_WHICHPAIR_OFFSET != nOffset) { - InvalidateItem_ForOffset(nOffset); + DisableOrInvalidateItem_ForOffset(bDisable, nOffset); } } -void SfxItemSet::InvalidateItem_ForOffset(sal_uInt16 nOffset) +void SfxItemSet::DisableOrInvalidateItem_ForOffset(bool bDisable, sal_uInt16 nOffset) { // check and assert from invalid offset. The caller is responsible for // ensuring a valid offset (see callers, all checked & safe) @@ -1634,65 +1645,22 @@ void SfxItemSet::InvalidateItem_ForOffset(sal_uInt16 nOffset) } else { - // entry is set - if (IsInvalidItem(*aFoundOne)) - // already invalid item, done + if (bDisable && IsDisabledItem(*aFoundOne)) + // already disabled item, done return; - // cleanup entry - checkRemovePoolRegistration(*aFoundOne); - implCleanupItemEntry(*aFoundOne); - } - - // set new entry - *aFoundOne = INVALID_POOL_ITEM; -} - -void SfxItemSet::DisableItem_ForWhichID(sal_uInt16 nWhich) -{ - const sal_uInt16 nOffset(GetRanges().getOffsetFromWhich(nWhich)); - - if (INVALID_WHICHPAIR_OFFSET != nOffset) - { - DisableItem_ForOffset(nOffset); - } -} - -void SfxItemSet::DisableItem_ForOffset(sal_uInt16 nOffset) -{ - // check and assert from invalid offset. The caller is responsible for - // ensuring a valid offset (see callers, all checked & safe) - assert(nOffset < TotalCount()); - const_iterator aFoundOne(begin() + nOffset); - - if (nullptr == *aFoundOne) - { - // entry goes from nullptr -> invalid - ++m_nCount; - } - else - { - // entry is set - if ((*aFoundOne)->isVoidItem() && 0 == (*aFoundOne)->Which()) + if (!bDisable && IsInvalidItem(*aFoundOne)) // already invalid item, done return; - } - // prepare new entry - const SfxPoolItem* pNew(implCreateItemEntry(*GetPool(), new SfxVoidItem(0), true)); - // Notification-Callback - if(m_aCallback) - { - m_aCallback(*aFoundOne, pNew); + // cleanup entry + checkRemovePoolRegistration(*aFoundOne); + implCleanupItemEntry(*aFoundOne); } - // cleanup entry (remove only) - checkRemovePoolRegistration(*aFoundOne); - implCleanupItemEntry(*aFoundOne); - // set new entry - *aFoundOne = pNew; + *aFoundOne = bDisable ? DISABLED_POOL_ITEM : INVALID_POOL_ITEM; } sal_uInt16 SfxItemSet::GetWhichByOffset( sal_uInt16 nOffset ) const diff --git a/svl/source/items/poolitem.cxx b/svl/source/items/poolitem.cxx index 3b0ebd6fa882..60de354cdeba 100644 --- a/svl/source/items/poolitem.cxx +++ b/svl/source/items/poolitem.cxx @@ -494,7 +494,6 @@ SfxPoolItem::SfxPoolItem(sal_uInt16 const nWhich) #ifdef DBG_UTIL , m_nSerialNumber(nUsedSfxPoolItemCount) #endif - , m_bIsVoidItem(false) , m_bStaticDefault(false) , m_bPoolDefault(false) , m_bIsSetItem(false) @@ -642,7 +641,7 @@ bool SfxPoolItem::areSame(const SfxPoolItem* pItem1, const SfxPoolItem* pItem2) { if (pItem1 == pItem2) // pointer compare, this handles already - // nullptr, INVALID_POOL_ITEM, SfxVoidItem + // nullptr, INVALID_POOL_ITEM, DISABLED_POOL_ITEM // and if any Item is indeed handed over twice return true; @@ -689,14 +688,16 @@ bool SfxPoolItem::areSame(const SfxPoolItem& rItem1, const SfxPoolItem& rItem2) namespace { -class InvalidItem final : public SfxPoolItem +class InvalidOrDisabledItem final : public SfxPoolItem { virtual bool operator==(const SfxPoolItem&) const override { return true; } virtual SfxPoolItem* Clone(SfxItemPool*) const override { return nullptr; } }; -InvalidItem aInvalidItem; +InvalidOrDisabledItem aInvalidItem; +InvalidOrDisabledItem aDisabledItem; } SfxPoolItem const* const INVALID_POOL_ITEM = &aInvalidItem; +SfxPoolItem const* const DISABLED_POOL_ITEM = &aDisabledItem; /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/svl/source/items/voiditem.cxx b/svl/source/items/voiditem.cxx index 32057e1e2c75..2091359e7bb5 100644 --- a/svl/source/items/voiditem.cxx +++ b/svl/source/items/voiditem.cxx @@ -25,19 +25,16 @@ SfxPoolItem* SfxVoidItem::CreateDefault() { return new SfxVoidItem(0); } SfxVoidItem::SfxVoidItem(sal_uInt16 which) : SfxPoolItem(which) { - setIsVoidItem(); } SfxVoidItem::SfxVoidItem(const SfxVoidItem& rCopy) : SfxPoolItem(rCopy.Which()) { - setIsVoidItem(); } SfxVoidItem::SfxVoidItem(SfxVoidItem&& rOrig) : SfxPoolItem(rOrig) { - setIsVoidItem(); } bool SfxVoidItem::operator==(const SfxPoolItem& rCmp) const diff --git a/sw/source/core/attr/swatrset.cxx b/sw/source/core/attr/swatrset.cxx index 09c04b43f635..6e52f4af3d51 100644 --- a/sw/source/core/attr/swatrset.cxx +++ b/sw/source/core/attr/swatrset.cxx @@ -105,8 +105,8 @@ void SwAttrSet::changeCallback(const SfxPoolItem* pOld, const SfxPoolItem* pNew) if (nullptr != pOld) { - // do not handle if an invalid item is involved - if (IsInvalidItem(pOld)) + // do not handle if an invalid or disabled item is involved + if (IsInvalidItem(pOld) || IsDisabledItem(pOld)) return; // get WhichID from pOld @@ -115,8 +115,8 @@ void SwAttrSet::changeCallback(const SfxPoolItem* pOld, const SfxPoolItem* pNew) if (nullptr != pNew) { - // do not handle if an invalid item is involved - if (IsInvalidItem(pNew)) + // do not handle if an invalid or disabled item is involved + if (IsInvalidItem(pNew) || IsDisabledItem(pNew)) return; if (0 == nWhich) @@ -127,7 +127,7 @@ void SwAttrSet::changeCallback(const SfxPoolItem* pOld, const SfxPoolItem* pNew) } // all given items are valid. If we got no WhichID != 0 then - // pOld == pNew == nullptr or SfxVoidItem(0) and we have no + // pOld == pNew == nullptr or IsDisabledItem and we have no // valid input. Also not needed if !IsWhich (aka > SFX_WHICH_MAX) if (0 == nWhich || !SfxItemPool::IsWhich(nWhich)) return; diff --git a/sw/source/uibase/docvw/romenu.cxx b/sw/source/uibase/docvw/romenu.cxx index 05d352f181c3..bcbe76f9ef39 100644 --- a/sw/source/uibase/docvw/romenu.cxx +++ b/sw/source/uibase/docvw/romenu.cxx @@ -60,7 +60,7 @@ void SwReadOnlyPopup::Check( sal_uInt16 nMID, sal_uInt16 nSID, SfxDispatcher con m_xMenu->EnableItem(nMID); if (_pItem) { - m_xMenu->CheckItem(nMID, !_pItem->isVoidItem() && + m_xMenu->CheckItem(nMID, !IsDisabledItem(_pItem.get()) && dynamic_cast< const SfxBoolItem *>( _pItem.get() ) != nullptr && static_cast<SfxBoolItem*>(_pItem.get())->GetValue()); //remove full screen entry when not in full screen mode