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;

Reply via email to