let's make Impress crash less - SfxStyleFamily edition

Commit bcb41235deaf4b7ca90522bda3ba21a686819e6e - in addition to
introducing the enum SfxStyleFamily - inconsistently converted one
call of DrawDocShell::SetStyleFamily to use enum SfxStyleFamily
instead of a mysterious "5", but did not adapt all of the code that
extracts the SfxUInt16Item that subsequently extracts this value and
expects a number 0-5.

Since it's clearly inexcusably stupid to have 2 different public sets
of integers identifying styles, make the usage of the second "array
index" integers private to templdlg.cxx; the SfxUInt16 item now always
contains enum SfxStyleFamily.

(regression from bcb41235deaf4b7ca90522bda3ba21a686819e6e)

(cherry picked from commit 75c39b903f06b656293edf20ec4a5173a0755fb6)

sfx2: SfxCommonTemplateDialog_Impl can't even decide if...
...no family selection should be represented as "0" or "0xffff".
(cherry picked from commit 813a8944bdb337a0018208381aa30118151872ff)

Change-Id: I333575c504277c2046f8f5a6b36ae3f86b3b3201
Reviewed-on: https://gerrit.libreoffice.org/28569
Reviewed-by: Noel Grandin <noelgrandin@gmail.com>
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>

9 files changed, 53 insertions, 52 deletions

diff --git a/include/sfx2/templdlg.hxx b/include/sfx2/templdlg.hxx
index 94d213409676..9ffded71a851 100644
--- a/include/sfx2/templdlg.hxx
+++ b/include/sfx2/templdlg.hxx
@@ -28,14 +28,6 @@
 #include <sfx2/childwin.hxx>
 #include <sfx2/basedlgs.hxx>
 
-namespace SfxTemplate
-{
-    // converts from SFX_STYLE_FAMILY Ids to 1-5
-    sal_uInt16 SFX2_DLLPUBLIC SfxFamilyIdToNId(SfxStyleFamily nFamily);
-    // converts from 1-5 to SFX_STYLE_FAMILY Ids
-    SfxStyleFamily SFX2_DLLPUBLIC NIdToSfxFamilyId(sal_uInt16 nId);
-}
-
 class SfxTemplateDialog_Impl;
 
 class SFX2_DLLPUBLIC SfxTemplatePanelControl : public vcl::Window
diff --git a/sc/source/ui/view/formatsh.cxx b/sc/source/ui/view/formatsh.cxx
index b7af7e0ec8ae..9236462520b8 100644
--- a/sc/source/ui/view/formatsh.cxx
+++ b/sc/source/ui/view/formatsh.cxx
@@ -217,7 +217,7 @@ void ScFormatShell::GetStyleState( SfxItemSet& rSet )
                 pTabViewShell->GetViewFrame()->GetBindings().QueryState(SID_STYLE_FAMILY, pItem);
                 SfxUInt16Item* pFamilyItem = dynamic_cast<SfxUInt16Item*>(pItem.get());
 
-                bool bPage = pFamilyItem && SfxStyleFamily::Page == SfxTemplate::NIdToSfxFamilyId(pFamilyItem->GetValue());
+                bool bPage = pFamilyItem && SfxStyleFamily::Page == static_cast<SfxStyleFamily>(pFamilyItem->GetValue());
 
                 if ( bProtected || bPage )
                     rSet.DisableItem( nSlotId );
@@ -232,7 +232,7 @@ void ScFormatShell::GetStyleState( SfxItemSet& rSet )
                 std::unique_ptr<SfxPoolItem> pItem;
                 pTabViewShell->GetViewFrame()->GetBindings().QueryState(SID_STYLE_FAMILY, pItem);
                 SfxUInt16Item* pFamilyItem = dynamic_cast<SfxUInt16Item*>(pItem.get());
-                bool bPage = pFamilyItem && SfxStyleFamily::Page == SfxTemplate::NIdToSfxFamilyId(pFamilyItem->GetValue());
+                bool bPage = pFamilyItem && SfxStyleFamily::Page == static_cast<SfxStyleFamily>(pFamilyItem->GetValue());
 
                 if ( bProtected && !bPage )
                     rSet.DisableItem( nSlotId );
diff --git a/sd/source/ui/view/drviewsf.cxx b/sd/source/ui/view/drviewsf.cxx
index 15bb40e9aa6c..66ee19e2dd42 100644
--- a/sd/source/ui/view/drviewsf.cxx
+++ b/sd/source/ui/view/drviewsf.cxx
@@ -491,7 +491,7 @@ void DrawViewShell::GetAttrState( SfxItemSet& rSet )
                 std::unique_ptr<SfxPoolItem> pItem;
                 GetViewFrame()->GetBindings().QueryState(SID_STYLE_FAMILY, pItem);
                 SfxUInt16Item* pFamilyItem = dynamic_cast<SfxUInt16Item*>(pItem.get());
-                if (pFamilyItem && SfxTemplate::NIdToSfxFamilyId(pFamilyItem->GetValue()) == SD_STYLE_FAMILY_PSEUDO)
+                if (pFamilyItem && static_cast<SfxStyleFamily>(pFamilyItem->GetValue()) == SD_STYLE_FAMILY_PSEUDO)
                     rSet.Put(SfxBoolItem(nWhich,false));
                 else
                 {
@@ -506,7 +506,7 @@ void DrawViewShell::GetAttrState( SfxItemSet& rSet )
                 std::unique_ptr<SfxPoolItem> pItem;
                 GetViewFrame()->GetBindings().QueryState(SID_STYLE_FAMILY, pItem);
                 SfxUInt16Item* pFamilyItem = dynamic_cast<SfxUInt16Item*>(pItem.get());
-                if (pFamilyItem && SfxTemplate::NIdToSfxFamilyId(pFamilyItem->GetValue()) == SD_STYLE_FAMILY_PSEUDO)
+                if (pFamilyItem && static_cast<SfxStyleFamily>(pFamilyItem->GetValue()) == SD_STYLE_FAMILY_PSEUDO)
                 {
                     rSet.DisableItem(nWhich);
                 }
@@ -518,7 +518,7 @@ void DrawViewShell::GetAttrState( SfxItemSet& rSet )
                 std::unique_ptr<SfxPoolItem> pItem;
                 GetViewFrame()->GetBindings().QueryState(SID_STYLE_FAMILY, pItem);
                 SfxUInt16Item* pFamilyItem = dynamic_cast<SfxUInt16Item*>(pItem.get());
-                if (pFamilyItem && SfxTemplate::NIdToSfxFamilyId(pFamilyItem->GetValue()) == SD_STYLE_FAMILY_PSEUDO)
+                if (pFamilyItem && static_cast<SfxStyleFamily>(pFamilyItem->GetValue()) == SD_STYLE_FAMILY_PSEUDO)
                     rSet.DisableItem(nWhich);
             }
             break;
@@ -533,11 +533,11 @@ void DrawViewShell::GetAttrState( SfxItemSet& rSet )
                 SfxUInt16Item* pFamilyItem = dynamic_cast<SfxUInt16Item*>(pItem.get());
                 if (pFamilyItem)
                 {
-                    if (SfxTemplate::NIdToSfxFamilyId(pFamilyItem->GetValue()) == SD_STYLE_FAMILY_PSEUDO)
+                    if (static_cast<SfxStyleFamily>(pFamilyItem->GetValue()) == SD_STYLE_FAMILY_PSEUDO)
                     {
                         rSet.DisableItem(nWhich);
                     }
-                    else if (SfxTemplate::NIdToSfxFamilyId(pFamilyItem->GetValue()) == SD_STYLE_FAMILY_GRAPHICS)
+                    else if (static_cast<SfxStyleFamily>(pFamilyItem->GetValue()) == SD_STYLE_FAMILY_GRAPHICS)
                     {
                         if (!mpDrawView->AreObjectsMarked())
                         {
diff --git a/sd/source/ui/view/outlnvsh.cxx b/sd/source/ui/view/outlnvsh.cxx
index c875ce3de6d0..1bfc7717573e 100644
--- a/sd/source/ui/view/outlnvsh.cxx
+++ b/sd/source/ui/view/outlnvsh.cxx
@@ -1479,7 +1479,7 @@ void OutlineViewShell::GetAttrState( SfxItemSet& rSet )
                 std::unique_ptr<SfxPoolItem> pItem;
                 GetViewFrame()->GetBindings().QueryState(SID_STYLE_FAMILY, pItem);
                 SfxUInt16Item* pFamilyItem = dynamic_cast<SfxUInt16Item*>(pItem.get());
-                if (pFamilyItem && SfxTemplate::NIdToSfxFamilyId(pFamilyItem->GetValue()) == SD_STYLE_FAMILY_PSEUDO)
+                if (pFamilyItem && static_cast<SfxStyleFamily>(pFamilyItem->GetValue()) == SD_STYLE_FAMILY_PSEUDO)
                 {
                     SfxItemSet aSet(*rSet.GetPool(), SID_STATUS_LAYOUT, SID_STATUS_LAYOUT);
                     GetStatusBarState(aSet);
diff --git a/sd/source/ui/view/viewshe3.cxx b/sd/source/ui/view/viewshe3.cxx
index a23c752b5670..dfff1e88a689 100644
--- a/sd/source/ui/view/viewshe3.cxx
+++ b/sd/source/ui/view/viewshe3.cxx
@@ -89,7 +89,7 @@ void  ViewShell::GetMenuState( SfxItemSet &rSet )
 {
     if( SfxItemState::DEFAULT == rSet.GetItemState( SID_STYLE_FAMILY ) )
     {
-        sal_uInt16 nFamily = (sal_uInt16)GetDocSh()->GetStyleFamily();
+        SfxStyleFamily const nFamily = GetDocSh()->GetStyleFamily();
 
         SdrView* pDrView = GetDrawView();
 
@@ -108,7 +108,7 @@ void  ViewShell::GetMenuState( SfxItemSet &rSet )
             }
         }
 
-        rSet.Put(SfxUInt16Item(SID_STYLE_FAMILY, nFamily ));
+        rSet.Put(SfxUInt16Item(SID_STYLE_FAMILY, static_cast<sal_uInt16>(nFamily)));
     }
 
     if(SfxItemState::DEFAULT == rSet.GetItemState(SID_GETUNDOSTRINGS))
diff --git a/sfx2/source/dialog/templdlg.cxx b/sfx2/source/dialog/templdlg.cxx
index 9e437500eb21..ca53f4c48a41 100644
--- a/sfx2/source/dialog/templdlg.cxx
+++ b/sfx2/source/dialog/templdlg.cxx
@@ -644,6 +644,38 @@ SvTreeListEntry* FillBox_Impl(SvTreeListBox* pBox,
     return pTreeListEntry;
 }
 
+
+namespace SfxTemplate
+{
+    // converts from SFX_STYLE_FAMILY Ids to 1-6
+    sal_uInt16 SfxFamilyIdToNId(SfxStyleFamily nFamily)
+    {
+        switch ( nFamily )
+        {
+            case SfxStyleFamily::Char:   return 1;
+            case SfxStyleFamily::Para:   return 2;
+            case SfxStyleFamily::Frame:  return 3;
+            case SfxStyleFamily::Page:   return 4;
+            case SfxStyleFamily::Pseudo: return 5;
+            default:                     return 0xffff;
+        }
+    }
+
+    // converts from 1-6 to SFX_STYLE_FAMILY Ids
+    SfxStyleFamily NIdToSfxFamilyId(sal_uInt16 nId)
+    {
+        switch (nId)
+        {
+            case 1: return SfxStyleFamily::Char;
+            case 2: return SfxStyleFamily::Para;
+            case 3: return SfxStyleFamily::Frame;
+            case 4: return SfxStyleFamily::Page;
+            case 5: return SfxStyleFamily::Pseudo;
+            default: return SfxStyleFamily::All;
+        }
+    }
+}
+
 // Constructor
 
 SfxCommonTemplateDialog_Impl::SfxCommonTemplateDialog_Impl( SfxBindings* pB, vcl::Window* pW, bool )
@@ -884,35 +916,6 @@ SfxCommonTemplateDialog_Impl::~SfxCommonTemplateDialog_Impl()
     aFilterLb.disposeAndClear();
 }
 
-namespace SfxTemplate
-{
-    sal_uInt16 SfxFamilyIdToNId(SfxStyleFamily nFamily)
-    {
-        switch ( nFamily )
-        {
-            case SfxStyleFamily::Char:   return 1;
-            case SfxStyleFamily::Para:   return 2;
-            case SfxStyleFamily::Frame:  return 3;
-            case SfxStyleFamily::Page:   return 4;
-            case SfxStyleFamily::Pseudo: return 5;
-            default:                      return 0;
-        }
-    }
-
-    SfxStyleFamily NIdToSfxFamilyId(sal_uInt16 nId)
-    {
-        switch (nId)
-        {
-            case 1: return SfxStyleFamily::Char;
-            case 2: return SfxStyleFamily::Para;
-            case 3: return SfxStyleFamily::Frame;
-            case 4: return SfxStyleFamily::Page;
-            case 5: return SfxStyleFamily::Pseudo;
-            default: return SfxStyleFamily::All;
-        }
-    }
-}
-
 // Helper function: Access to the current family item
 const SfxStyleFamilyItem *SfxCommonTemplateDialog_Impl::GetFamilyItem_Impl() const
 {
@@ -1697,12 +1700,14 @@ IMPL_LINK_TYPED( SfxCommonTemplateDialog_Impl, FilterSelectHdl, ListBox&, rBox,
 // Select-Handler for the Toolbox
 void SfxCommonTemplateDialog_Impl::FamilySelect(sal_uInt16 nEntry)
 {
+    assert((0 < nEntry && nEntry <= MAX_FAMILIES) || 0xffff == nEntry);
     if( nEntry != nActFamily )
     {
         CheckItem( nActFamily, false );
         nActFamily = nEntry;
         SfxDispatcher* pDispat = pBindings->GetDispatcher_Impl();
-        SfxUInt16Item aItem( SID_STYLE_FAMILY, nEntry );
+        SfxUInt16Item const aItem(SID_STYLE_FAMILY,
+                static_cast<sal_uInt16>(SfxTemplate::NIdToSfxFamilyId(nEntry)));
         pDispat->ExecuteList(SID_STYLE_FAMILY, SfxCallMode::SYNCHRON, { &aItem });
         pBindings->Invalidate( SID_STYLE_FAMILY );
         pBindings->Update( SID_STYLE_FAMILY );
@@ -2525,8 +2530,10 @@ IMPL_LINK_TYPED( SfxTemplateDialog_Impl, MenuSelectHdl, Menu*, pMenu, bool)
     return false;
 }
 
-void SfxCommonTemplateDialog_Impl::SetFamily( sal_uInt16 nId )
+void SfxCommonTemplateDialog_Impl::SetFamily(SfxStyleFamily const nFamily)
 {
+    sal_uInt16 const nId(SfxTemplate::SfxFamilyIdToNId(nFamily));
+    assert((0 < nId && nId <= MAX_FAMILIES) || 0xffff == nId);
     if ( nId != nActFamily )
     {
         if ( nActFamily != 0xFFFF )
diff --git a/sfx2/source/dialog/tplcitem.cxx b/sfx2/source/dialog/tplcitem.cxx
index 714a2aaaa0d2..6f9de38d87fc 100644
--- a/sfx2/source/dialog/tplcitem.cxx
+++ b/sfx2/source/dialog/tplcitem.cxx
@@ -145,7 +145,9 @@ void SfxTemplateControllerItem::StateChanged( sal_uInt16 nSID, SfxItemState eSta
         {
             const SfxUInt16Item *pStateItem = dynamic_cast< const SfxUInt16Item* >(pItem);
             if (pStateItem)
-                rTemplateDlg.SetFamily( pStateItem->GetValue() );
+            {
+                rTemplateDlg.SetFamily(static_cast<SfxStyleFamily>(pStateItem->GetValue()));
+            }
             break;
         }
     }
diff --git a/sfx2/source/inc/templdgi.hxx b/sfx2/source/inc/templdgi.hxx
index b8a08d3524ed..27aa178d56d7 100644
--- a/sfx2/source/inc/templdgi.hxx
+++ b/sfx2/source/inc/templdgi.hxx
@@ -273,7 +273,7 @@ protected:
     void Notify( SfxBroadcaster& rBC, const SfxHint& rHint ) override;
 
     void FamilySelect( sal_uInt16 nId );
-    void SetFamily( sal_uInt16 nId );
+    void SetFamily(SfxStyleFamily nFamily);
     void ActionSelect( sal_uInt16 nId );
 
     sal_Int32 LoadFactoryStyleFilter( SfxObjectShell* i_pObjSh );
diff --git a/sw/source/uibase/app/docst.cxx b/sw/source/uibase/app/docst.cxx
index 635a0daca4bc..f7bbf80fdcf4 100644
--- a/sw/source/uibase/app/docst.cxx
+++ b/sw/source/uibase/app/docst.cxx
@@ -113,7 +113,7 @@ void  SwDocShell::StateStyleSheet(SfxItemSet& rSet, SwWrtShell* pSh)
         SfxUInt16Item* pFamilyItem = dynamic_cast<SfxUInt16Item*>(pItem.get());
         if (pFamilyItem)
         {
-            nActualFamily = SfxTemplate::NIdToSfxFamilyId(pFamilyItem->GetValue());
+            nActualFamily = static_cast<SfxStyleFamily>(pFamilyItem->GetValue());
         }
     }