Improve HIDE_NON_NUMERICAL compatibility with Word

The previous implementation of REFFLDFLAG_STYLE_HIDE_NON_NUMERICAL had
some major incompatibilites with Word. In particular, it stripped
letters even if they were included in the "numbering" system.

This commit fixes a lot of the flaws in the previous implementation, so
it's now a lot closer to Word.

Change-Id: Ifaa67fbc2d53b0d4fb85e7305b2dbdf78cf0a1ad
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/158451
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>

7 files changed, 152 insertions, 72 deletions

diff --git a/sw/inc/numrule.hxx b/sw/inc/numrule.hxx
index f642e21e746c..b21cc5259656 100644
--- a/sw/inc/numrule.hxx
+++ b/sw/inc/numrule.hxx
@@ -169,11 +169,13 @@ public:
     OUString MakeNumString( const SwNumberTree::tNumberVector & rNumVector,
                           const bool bInclStrings = true,
                           const unsigned int _nRestrictToThisLevel = MAXLEVEL,
+                          const bool bHideNonNumerical = false,
                           Extremities* pExtremities = nullptr,
                           LanguageType nLang = LANGUAGE_SYSTEM) const;
     OUString MakeRefNumString( const SwNodeNum& rNodeNum,
                              const bool bInclSuperiorNumLabels,
-                             const int nRestrictInclToThisLevel ) const;
+                             const int nRestrictInclToThisLevel,
+                             const bool bHideNonNumerical ) const;
     OUString MakeParagraphStyleListString() const;
 
     /** @return list of associated text nodes */
diff --git a/sw/inc/reffld.hxx b/sw/inc/reffld.hxx
index 3c1e3c63b5e6..b65e8c209633 100644
--- a/sw/inc/reffld.hxx
+++ b/sw/inc/reffld.hxx
@@ -117,11 +117,6 @@ private:
 
     virtual OUString    ExpandImpl(SwRootFrame const* pLayout) const override;
     virtual std::unique_ptr<SwField> Copy() const override;
-
-    /// Strip out text that is not either a number or a delimiter. Used in STYLEREF for when you
-    /// have chapters labelled "Chapter X.Y" and want to just keep the "X.Y". Distinct from
-    /// GetExpandedTextOfReferencedTextNode so you can run it after any other processing
-    void StylerefStripNonnumerical(OUString& rText) const;
 public:
     SwGetRefField( SwGetRefFieldType*, OUString aSetRef, OUString aReferenceLanguage,
                     sal_uInt16 nSubType, sal_uInt16 nSeqNo, sal_uInt16 nFlags, sal_uLong nFormat );
diff --git a/sw/source/core/doc/number.cxx b/sw/source/core/doc/number.cxx
index 3ab36c63c160..d2cb98924e0f 100644
--- a/sw/source/core/doc/number.cxx
+++ b/sw/source/core/doc/number.cxx
@@ -648,9 +648,51 @@ OUString SwNumRule::MakeNumString( const SwNodeNum& rNum, bool bInclStrings ) co
     return OUString();
 }
 
+namespace {
+/// Strip out text that is not a delimiter. Used in STYLEREF for when you
+/// have chapters labelled "Chapter X.Y" and want to just keep the "X.Y"
+/// Only used on the prefix/infix/suffix, so the numbers are not modified
+void StripNonDelimiter(OUString& rText)
+{
+    std::vector<sal_Unicode> charactersToKeep;
+
+    for (int i = 0; i < rText.getLength(); i++) {
+        auto character = rText[i];
+
+         // tdf#86790# for Word compatibility: I haven't found any better way to determine whether a
+         // character is a delimiter than testing in Word and listing them out. Furthermore, I haven't
+         // found a list so I can't be certain this is the complete set- if there's a compatibility issue
+         // with this in the future, here's the first place to look...
+        if (
+            character == '.'
+            || character == ','
+            || character == ':'
+            || character == ';'
+            || character == '-'
+            || character == '('
+            || character == ')'
+            || character == '['
+            || character == ']'
+            || character == '{'
+            || character == '}'
+            || character == '/'
+            || character == '\\'
+            || character == '|'
+        )
+            charactersToKeep.push_back(character);
+    }
+
+    if (charactersToKeep.size())
+        rText = OUString(charactersToKeep.data(), charactersToKeep.size());
+    else
+        rText = OUString();
+}
+}
+
 OUString SwNumRule::MakeNumString( const SwNumberTree::tNumberVector & rNumVector,
                                  const bool bInclStrings,
                                  const unsigned int _nRestrictToThisLevel,
+                                 const bool bHideNonNumerical,
                                  SwNumRule::Extremities* pExtremities,
                                  LanguageType nLang ) const
 {
@@ -672,12 +714,27 @@ OUString SwNumRule::MakeNumString( const SwNumberTree::tNumberVector & rNumVecto
 
     if (rMyNFormat.GetNumberingType() == SVX_NUM_NUMBER_NONE)
     {
-        if (!rMyNFormat.HasListFormat())
-            return bInclStrings ? rMyNFormat.GetPrefix() + rMyNFormat.GetSuffix() : OUString();
+        if (!rMyNFormat.HasListFormat()) {
+            OUString sRet = bInclStrings ? rMyNFormat.GetPrefix() + rMyNFormat.GetSuffix() : OUString();
+            StripNonDelimiter(sRet);
+            return sRet;
+        }
 
         // If numbering is disabled for this level we should emit just prefix/suffix
         // Remove everything between first %1% and last %n% (including markers)
-        OUString sLevelFormat = rMyNFormat.GetListFormat(bInclStrings);
+        OUString sLevelFormat = rMyNFormat.GetListFormat(bInclStrings && !bHideNonNumerical);
+
+        if (bInclStrings && bHideNonNumerical) {
+            // If hiding non numerical text, we need to strip the prefix and suffix properly, so let's add them manually
+            OUString sPrefix = rMyNFormat.GetPrefix();
+            OUString sSuffix = rMyNFormat.GetSuffix();
+
+            StripNonDelimiter(sPrefix);
+            StripNonDelimiter(sSuffix);
+
+            sLevelFormat = sPrefix + sLevelFormat + sSuffix;
+        }
+
         sal_Int32 nFirstPosition = sLevelFormat.indexOf("%");
         sal_Int32 nLastPosition = sLevelFormat.lastIndexOf("%");
         if (nFirstPosition >= 0 && nLastPosition >= nFirstPosition)
@@ -689,7 +746,17 @@ OUString SwNumRule::MakeNumString( const SwNumberTree::tNumberVector & rNumVecto
 
     if (rMyNFormat.HasListFormat())
     {
-        OUString sLevelFormat = rMyNFormat.GetListFormat(bInclStrings);
+        OUString sLevelFormat = rMyNFormat.GetListFormat(bInclStrings && !bHideNonNumerical);
+
+        if (bInclStrings && bHideNonNumerical) {
+            OUString sPrefix = rMyNFormat.GetPrefix();
+            OUString sSuffix = rMyNFormat.GetSuffix();
+
+            StripNonDelimiter(sPrefix);
+            StripNonDelimiter(sSuffix);
+
+            sLevelFormat = sPrefix + sLevelFormat + sSuffix;
+        }
 
         // In this case we are ignoring GetIncludeUpperLevels: we put all
         // level numbers requested by level format
@@ -697,28 +764,46 @@ OUString SwNumRule::MakeNumString( const SwNumberTree::tNumberVector & rNumVecto
         {
             OUString sReplacement;
             const SwNumFormat& rNFormat = Get(i);
+
+            OUString sFind("%" + OUString::number(i + 1) + "%");
+            sal_Int32 nPosition = sLevelFormat.indexOf(sFind);
+
             if (rNFormat.GetNumberingType() == SVX_NUM_NUMBER_NONE)
             {
                 // Numbering disabled - replacement is empty
                 // And we should skip all level string content until next level marker:
                 // so %1%.%2%.%3% with second level as NONE will result 1.1, not 1..1
-                OUString sFind("%" + OUString::number(i + 1) + "%");
-                sal_Int32 nPositionToken = sLevelFormat.indexOf(sFind);
-                sal_Int32 nPositionNextToken = sLevelFormat.indexOf('%', nPositionToken + sFind.getLength());
-                if (nPositionToken >= 0 && nPositionNextToken >= nPositionToken)
+                sal_Int32 nPositionNext = sLevelFormat.indexOf('%', nPosition + sFind.getLength());
+                if (nPosition >= 0 && nPositionNext >= nPosition)
                 {
-                    sLevelFormat = sLevelFormat.replaceAt(nPositionToken, nPositionNextToken - nPositionToken, u"");
+                    sLevelFormat = sLevelFormat.replaceAt(nPosition, nPositionNext - nPosition, u"");
                 }
+                continue;
             }
             else if (rNumVector[i])
                 sReplacement = Get(i).GetNumStr(rNumVector[i], aLocale, rMyNFormat.GetIsLegal());
             else
                 sReplacement = "0";        // all 0 level are a 0
 
-            OUString sFind("%" + OUString::number(i + 1) + "%");
-            sal_Int32 nPosition = sLevelFormat.indexOf(sFind);
             if (nPosition >= 0)
+            {
+                if (bHideNonNumerical)
+                {
+                    sal_Int32 nPositionNext = sLevelFormat.indexOf('%', nPosition + sFind.getLength());
+
+                    if (nPositionNext >= nPosition) {
+                        sal_Int32 nReplaceStart = nPosition + sFind.getLength();
+                        sal_Int32 nReplaceCount = nPositionNext - nReplaceStart;
+
+                        OUString sSeparator = sLevelFormat.copy(nReplaceStart, nReplaceCount);
+                        StripNonDelimiter(sSeparator);
+
+                        sLevelFormat = sLevelFormat.replaceAt(nReplaceStart, nReplaceCount, sSeparator);
+                    }
+                }
+
                 sLevelFormat = sLevelFormat.replaceAt(nPosition, sFind.getLength(), sReplacement);
+            }
         }
 
         aStr = sLevelFormat;
@@ -769,8 +854,13 @@ OUString SwNumRule::MakeNumString( const SwNumberTree::tNumberVector & rNumVecto
             SVX_NUM_CHAR_SPECIAL != rMyNFormat.GetNumberingType() &&
             SVX_NUM_BITMAP != rMyNFormat.GetNumberingType())
         {
-            const OUString& sPrefix = rMyNFormat.GetPrefix();
-            const OUString& sSuffix = rMyNFormat.GetSuffix();
+            OUString sPrefix = rMyNFormat.GetPrefix();
+            OUString sSuffix = rMyNFormat.GetSuffix();
+
+            if (bHideNonNumerical) {
+                StripNonDelimiter(sPrefix);
+                StripNonDelimiter(sSuffix);
+            }
 
             aStr.insert(0, sPrefix);
             aStr.append(sSuffix);
@@ -787,7 +877,8 @@ OUString SwNumRule::MakeNumString( const SwNumberTree::tNumberVector & rNumVecto
 
 OUString SwNumRule::MakeRefNumString( const SwNodeNum& rNodeNum,
                                     const bool bInclSuperiorNumLabels,
-                                    const int nRestrictInclToThisLevel ) const
+                                    const int nRestrictInclToThisLevel,
+                                    const bool bHideNonNumerical ) const
 {
     OUString aRefNumStr;
 
@@ -822,7 +913,7 @@ OUString SwNumRule::MakeRefNumString( const SwNodeNum& rNodeNum,
                 Extremities aExtremities;
                 OUString aPrevStr = MakeNumString( pWorkingNodeNum->GetNumberVector(),
                                                  true, MAXLEVEL,
-                                                 &aExtremities);
+                                                 bHideNonNumerical, &aExtremities);
                 sal_Int32 nStrip = 0;
                 while ( nStrip < aExtremities.nPrefixChars )
                 {
diff --git a/sw/source/core/fields/reffld.cxx b/sw/source/core/fields/reffld.cxx
index ee7791a68753..c92d4b6d37bb 100644
--- a/sw/source/core/fields/reffld.cxx
+++ b/sw/source/core/fields/reffld.cxx
@@ -75,7 +75,9 @@ using namespace ::com::sun::star::lang;
 static std::pair<OUString, bool> MakeRefNumStr(SwRootFrame const* pLayout,
       const SwTextNode& rTextNodeOfField,
       const SwTextNode& rTextNodeOfReferencedItem,
-      sal_uInt32 nRefNumFormat);
+      sal_uInt16 nSubType,
+      sal_uInt32 nRefNumFormat,
+      sal_uInt16 nFlags);
 
 static void lcl_GetLayTree( const SwFrame* pFrame, std::vector<const SwFrame*>& rArr )
 {
@@ -425,30 +427,6 @@ static OUString lcl_formatStringByCombiningCharacter(std::u16string_view sText,
     return sRet.makeStringAndClear();
 }
 
-void SwGetRefField::StylerefStripNonnumerical(OUString& rText) const
-{
-    // for STYLEREF, hide text that is neither a delimiter nor a number if that flag is set
-    if ( m_nSubType != REF_STYLE || (GetFlags() & REFFLDFLAG_STYLE_HIDE_NON_NUMERICAL) != REFFLDFLAG_STYLE_HIDE_NON_NUMERICAL )
-        return;
-
-    std::vector<sal_Unicode> charactersToKeep;
-
-    for (int i = 0; i < rText.getLength(); i++) {
-        auto character = rText[i];
-
-        if (
-            (character >= '(' && character <= '@') || // includes 0-9 and most of the punctuation we want
-            (character >= '[' && character <= '_')  // includes the rest of the punctuation we want
-        )
-            charactersToKeep.push_back(character);
-    }
-
-    if (charactersToKeep.size())
-        rText = OUString(charactersToKeep.data(), charactersToKeep.size());
-    else
-        rText = OUString();
-}
-
 // #i85090#
 OUString SwGetRefField::GetExpandedTextOfReferencedTextNode(
         SwRootFrame const& rLayout, SwTextNode* pTextNode, SwFrame* pFrame) const
@@ -468,8 +446,6 @@ OUString SwGetRefField::GetExpandedTextOfReferencedTextNode(
        sRet = lcl_formatStringByCombiningCharacter( sRet, cStrikethrough );
     }
 
-    StylerefStripNonnumerical(sRet);
-
     return sRet;
 }
 
@@ -798,7 +774,7 @@ void SwGetRefField::UpdateField(const SwTextField* pFieldTextAttr, SwFrame* pFra
             if ( pFieldTextAttr && pFieldTextAttr->GetpTextNode() )
             {
                 auto result =
-                    MakeRefNumStr(pLayout, pFieldTextAttr->GetTextNode(), *pTextNd, GetFormat());
+                    MakeRefNumStr(pLayout, pFieldTextAttr->GetTextNode(), *pTextNd, m_nSubType, GetFormat(), GetFlags());
                 rText = result.first;
                 // for differentiation of Roman numbers and letters in Hungarian article handling
                 bool bClosingParenthesis = result.second;
@@ -808,22 +784,25 @@ void SwGetRefField::UpdateField(const SwTextField* pFieldTextAttr, SwFrame* pFra
                 }
             }
         }
+
         break;
 
     default:
         OSL_FAIL("<SwGetRefField::UpdateField(..)> - unknown format type");
     }
-
-    StylerefStripNonnumerical(rText);
 }
 
+
 // #i81002#
 static std::pair<OUString, bool> MakeRefNumStr(
         SwRootFrame const*const pLayout,
         const SwTextNode& i_rTextNodeOfField,
         const SwTextNode& i_rTextNodeOfReferencedItem,
-        const sal_uInt32 nRefNumFormat)
+        const sal_uInt16 nSubType,
+        const sal_uInt32 nRefNumFormat,
+        const sal_uInt16 nFlags)
 {
+    bool bHideNonNumerical = (nSubType == REF_STYLE) && ((nFlags & REFFLDFLAG_STYLE_HIDE_NON_NUMERICAL) == REFFLDFLAG_STYLE_HIDE_NON_NUMERICAL);
     SwTextNode const& rTextNodeOfField(pLayout
             ?   *sw::GetParaPropsNode(*pLayout, i_rTextNodeOfField)
             :   i_rTextNodeOfField);
@@ -897,7 +876,8 @@ static std::pair<OUString, bool> MakeRefNumStr(
                 rTextNodeOfReferencedItem.GetNumRule()->MakeRefNumString(
                     *(rTextNodeOfReferencedItem.GetNum(pLayout)),
                     bInclSuperiorNumLabels,
-                    nRestrictInclToThisLevel ),
+                    nRestrictInclToThisLevel,
+                    bHideNonNumerical ),
                 rTextNodeOfReferencedItem.GetNumRule()->MakeNumString(
                     *(rTextNodeOfReferencedItem.GetNum(pLayout)),
                     true).endsWith(")") );
diff --git a/sw/source/core/txtnode/ndtxt.cxx b/sw/source/core/txtnode/ndtxt.cxx
index bd87f2d2bcbe..af9b5f72fc92 100644
--- a/sw/source/core/txtnode/ndtxt.cxx
+++ b/sw/source/core/txtnode/ndtxt.cxx
@@ -3267,6 +3267,7 @@ OUString SwTextNode::GetNumString( const bool _bInclPrefixAndSuffixStrings,
             return pRule->MakeNumString( GetNum(pLayout, eRedline)->GetNumberVector(),
                                      _bInclPrefixAndSuffixStrings,
                                      _nRestrictToThisLevel,
+                                     false,
                                      nullptr,
                                      GetLang(0));
         }
diff --git a/sw/source/ui/fldui/fldref.cxx b/sw/source/ui/fldui/fldref.cxx
index feab4e976385..a08715c8ca3c 100644
--- a/sw/source/ui/fldui/fldref.cxx
+++ b/sw/source/ui/fldui/fldref.cxx
@@ -46,6 +46,25 @@ static sal_uInt16 nFieldDlgFormatSel = 0;
 #define USER_DATA_VERSION_1 "1"
 #define USER_DATA_VERSION USER_DATA_VERSION_1
 
+namespace {
+
+enum FMT_REF_IDX
+{
+    FMT_REF_PAGE_IDX                = 0,
+    FMT_REF_CHAPTER_IDX             = 1,
+    FMT_REF_TEXT_IDX                = 2,
+    FMT_REF_UPDOWN_IDX              = 3,
+    FMT_REF_PAGE_PGDSC_IDX          = 4,
+    FMT_REF_ONLYNUMBER_IDX          = 5,
+    FMT_REF_ONLYCAPTION_IDX         = 6,
+    FMT_REF_ONLYSEQNO_IDX           = 7,
+    FMT_REF_NUMBER_IDX              = 8,
+    FMT_REF_NUMBER_NO_CONTEXT_IDX   = 9,
+    FMT_REF_NUMBER_FULL_CONTEXT_IDX = 10
+};
+
+}
+
 SwFieldRefPage::SwFieldRefPage(weld::Container* pPage, weld::DialogController* pController, const SfxItemSet *const pCoreSet )
     : SwFieldPage(pPage, pController, "modules/swriter/ui/fldrefpage.ui", "FieldRefPage", pCoreSet)
     , mpSavedSelectedTextNode(nullptr)
@@ -98,6 +117,7 @@ SwFieldRefPage::SwFieldRefPage(weld::Container* pPage, weld::DialogController* p
     m_xSelectionLB->connect_changed(LINK(this, SwFieldRefPage, SubTypeListBoxHdl));
     m_xSelectionLB->connect_row_activated(LINK(this, SwFieldRefPage, TreeViewInsertHdl));
     m_xFormatLB->connect_row_activated(LINK(this, SwFieldRefPage, TreeViewInsertHdl));
+    m_xFormatLB->connect_changed(LINK(this, SwFieldRefPage, FormatHdl));
 
     // #i83479#
     m_xSelectionToolTipLB->connect_changed( LINK(this, SwFieldRefPage, SubTypeTreeListBoxHdl) );
@@ -455,10 +475,19 @@ IMPL_LINK_NOARG(SwFieldRefPage, SubTypeListBoxHdl, weld::TreeView&, void)
     SubTypeHdl();
 }
 
+IMPL_LINK_NOARG(SwFieldRefPage, FormatHdl, weld::TreeView&, void)
+{
+    SubTypeHdl();
+}
+
 void SwFieldRefPage::SubTypeHdl()
 {
     sal_uInt16 nTypeId = m_xTypeLB->get_id(GetTypeSel()).toUInt32();
 
+    sal_uInt16 nFormat = m_xFormatLB->get_selected_id().toUInt32();
+    m_xStylerefHideNonNumericalCB->set_visible(nFormat == FMT_REF_NUMBER_IDX
+                                               || nFormat == FMT_REF_NUMBER_NO_CONTEXT_IDX
+                                               || nFormat == FMT_REF_NUMBER_FULL_CONTEXT_IDX);
     m_xStylerefFlags->set_visible(nTypeId == REFFLDFLAG_STYLE);
 
     switch(nTypeId)
@@ -800,25 +829,6 @@ bool SwFieldRefPage::MatchSubstring( const OUString& rListString, const OUString
     return aListString.indexOf(aSubstr) >= 0;
 }
 
-namespace {
-
-enum FMT_REF_IDX
-{
-    FMT_REF_PAGE_IDX                = 0,
-    FMT_REF_CHAPTER_IDX             = 1,
-    FMT_REF_TEXT_IDX                = 2,
-    FMT_REF_UPDOWN_IDX              = 3,
-    FMT_REF_PAGE_PGDSC_IDX          = 4,
-    FMT_REF_ONLYNUMBER_IDX          = 5,
-    FMT_REF_ONLYCAPTION_IDX         = 6,
-    FMT_REF_ONLYSEQNO_IDX           = 7,
-    FMT_REF_NUMBER_IDX              = 8,
-    FMT_REF_NUMBER_NO_CONTEXT_IDX   = 9,
-    FMT_REF_NUMBER_FULL_CONTEXT_IDX = 10
-};
-
-}
-
 const TranslateId FMT_REF_ARY[] =
 {
     FMT_REF_PAGE,
diff --git a/sw/source/ui/fldui/fldref.hxx b/sw/source/ui/fldui/fldref.hxx
index 24727e26169a..472d26a3c4a0 100644
--- a/sw/source/ui/fldui/fldref.hxx
+++ b/sw/source/ui/fldui/fldref.hxx
@@ -66,6 +66,7 @@ class SwFieldRefPage : public SwFieldPage
     DECL_LINK(SubTypeTreeListBoxHdl, weld::TreeView&, void);
     DECL_LINK(ModifyHdl, weld::Entry&, void);
     DECL_LINK(ModifyHdl_Impl, weld::Entry&, void);
+    DECL_LINK(FormatHdl, weld::TreeView&, void);
 
     void SubTypeHdl();