sw/qa/extras/ooxmlexport/data/tdf141966_chapterNumbering.docx |binary
 sw/qa/extras/ooxmlexport/ooxmlexport16.cxx                    |   13 +
 writerfilter/source/dmapper/DomainMapper_Impl.cxx             |    7 
 writerfilter/source/dmapper/NumberingManager.cxx              |   90 ++++++----
 writerfilter/source/dmapper/NumberingManager.hxx              |   13 -
 5 files changed, 79 insertions(+), 44 deletions(-)

New commits:
commit 3e09e0784ad7669d3e0a7655f5e604a2387b1b5d
Author:     Justin Luth <justin_l...@sil.org>
AuthorDate: Thu May 13 13:50:56 2021 +0200
Commit:     Miklos Vajna <vmik...@collabora.com>
CommitDate: Tue Jul 6 08:52:55 2021 +0200

    tdf#141966 writerfilter CN: fix chapter number identification
    
    This totally up-ends the current way of doing things,
    but it was so totally broken, and confused us for a very long time.
    Chapter Numbering has nothing to do with "Heading X" styles!!!!!!!!
    Thus, we can clean up a lot of nonsense clauses.
    
    The existing code makes it sound like the constraints of
    "outline numbering" are an MS format thing, but actually it is
    just an implementation detail of how LO contrains styles
    and listLevels.
    
    Yes, it is still nice to be able to round-trip the
    Chapter Numbering settings, so we won't ignore it completely,
    but really, all the complexity comes from trying to cram
    the normal flexibility of inheritable, style-assignable
    listLevels into a single special-purpose numbering rule.
    
    One thing that has always killed us is that direct formatting
    could not share in this unique style. But it needs to in DOCX.
    So, the key here is to NAME-ASSOCIATE the "Outline" style,
    so that ALL references to that numId end up pointing to the
    same rule, even if it is the super-special oddball one.
    
    P.S. Chapter Numbering should have NO IMPACT on the
    import process. It should only affect UI interaction
    when a user assigns a paragraph style: whether LO can
    apply a numbering listLevel or not.
    
    THIS IS EXTREMELY DANGEROUS TERRITORY! WAIT FOR 7.3!
    
    existing unit tests that mapped to a numId other than 1:
    ooxmlexport3: tdf95495 - numId 4
    ooxmlexport4: tdf81345 - numId 3
    ooxmlexport5: tdf123262 - numId 48
    ooxmlexport8: fdo74745 - numId 6
    ooxmlexport10: tdf89165 - numId 12
    ooxmlexport14: tdf133605 - numId 2
    ooxmlexport16: tdf132752 - numId 2
    
    and one example which didn't take the first available choice:
    ooxmlexport13: tdf95848 - preferred numId 3 over numId 2
    
    Change-Id: I561cd1594f198ad402893c77c1c983f206f53c57
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/115614
    Tested-by: Jenkins
    Reviewed-by: Justin Luth <justin_l...@sil.org>
    Reviewed-by: Miklos Vajna <vmik...@collabora.com>

diff --git a/sw/qa/extras/ooxmlexport/data/tdf141966_chapterNumbering.docx 
b/sw/qa/extras/ooxmlexport/data/tdf141966_chapterNumbering.docx
new file mode 100644
index 000000000000..3957fadf43c9
Binary files /dev/null and 
b/sw/qa/extras/ooxmlexport/data/tdf141966_chapterNumbering.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport16.cxx 
b/sw/qa/extras/ooxmlexport/ooxmlexport16.cxx
index b670c72333ff..8ec840b3b479 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport16.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport16.cxx
@@ -31,10 +31,12 @@
 #include <com/sun/star/view/XViewCursor.hpp>
 #include <comphelper/configuration.hxx>
 #include <comphelper/propertysequence.hxx>
+#include <comphelper/sequenceashashmap.hxx>
 #include <editeng/escapementitem.hxx>
 #include <unotools/fltrcfg.hxx>
 #include <textboxhelper.hxx>
 #include <unoprnms.hxx>
+#include <unotxdoc.hxx> //XChapterNumberingSupplier
 
 constexpr OUStringLiteral DATA_DIRECTORY = u"/sw/qa/extras/ooxmlexport/data/";
 
@@ -137,6 +139,17 @@ 
DECLARE_OOXMLEXPORT_TEST(testTdf141231_arabicHebrewNumbering, "tdf141231_arabicH
     CPPUNIT_ASSERT_EQUAL(style::NumberingType::CHARS_ARABIC_ABJAD, nActual);
 }
 
+DECLARE_OOXMLEXPORT_TEST(testTdf141966_chapterNumbering, 
"tdf141966_chapterNumbering.docx")
+{
+    uno::Reference<text::XChapterNumberingSupplier> 
xNumberingSupplier(mxComponent, uno::UNO_QUERY);
+    uno::Reference<container::XIndexAccess> xNumberingRules = 
xNumberingSupplier->getChapterNumberingRules();
+    comphelper::SequenceAsHashMap hashMap(xNumberingRules->getByIndex(0));
+
+    CPPUNIT_ASSERT(hashMap["HeadingStyleName"].get<OUString>().match("CN1"));
+
+    uno::Reference<beans::XPropertySet> xPara(getParagraph(7, "Direct 
formatting with \"Outline\" numbering."), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(OUString("2nd"), getProperty<OUString>(xPara, 
"ListLabelString"));
+}
 
 DECLARE_OOXMLEXPORT_TEST(testTdf132752, "tdf132752.docx")
 {
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx 
b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 881c512358d8..30928027007c 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -1516,13 +1516,10 @@ void DomainMapper_Impl::finishParagraph( const 
PropertyMapPtr& pPropertyMap, con
                 isNumberingViaStyle = true;
                 // Since LO7.0/tdf#131321 fixed the loss of numbering in 
styles, this OUGHT to be obsolete,
                 // but now other new/critical LO7.0 code expects it, and 
perhaps some corner cases still need it as well.
-                // So we skip it only for default outline styles, which are 
recognized by NumberingManager.
-                if (!GetCurrentParaStyleName().startsWith("Heading ") || 
nListLevel >= pList->GetDefaultParentLevels())
-                    pParaContext->Insert( PROP_NUMBERING_STYLE_NAME, 
uno::makeAny(pList->GetStyleName()), true );
+                pParaContext->Insert(PROP_NUMBERING_STYLE_NAME, 
uno::makeAny(pList->GetStyleName()), true);
             }
-            else if ( !pList->isOutlineNumbering(nListLevel) )
+            else
             {
-                // After ignoring anything related to the special Outline 
levels,
                 // we have direct numbering, as well as paragraph-style 
numbering.
                 // Apply the style if it uses the same list as the direct 
numbering,
                 // otherwise the directly-applied-to-paragraph status will be 
lost,
diff --git a/writerfilter/source/dmapper/NumberingManager.cxx 
b/writerfilter/source/dmapper/NumberingManager.cxx
index 764aace41808..b8a511f83a6c 100644
--- a/writerfilter/source/dmapper/NumberingManager.cxx
+++ b/writerfilter/source/dmapper/NumberingManager.cxx
@@ -133,6 +133,11 @@ void ListLevel::SetValue( Id nId, sal_Int32 nValue )
 
 void ListLevel::SetCustomNumberFormat(const OUString& rValue) { 
m_aCustomNumberFormat = rValue; }
 
+sal_Int16 ListLevel::GetNumberingType(sal_Int16 nDefault) const
+{
+    return ConversionHelper::ConvertNumberingType(m_nNFC, nDefault);
+}
+
 bool ListLevel::HasValues() const
 {
     return m_bHasValues;
@@ -143,14 +148,6 @@ void ListLevel::SetParaStyle( const tools::SvRef< 
StyleSheetEntry >& pStyle )
     if (!pStyle)
         return;
     m_pParaStyle = pStyle;
-    // AFAICT .docx spec does not identify which numberings or paragraph
-    // styles are actually the ones to be used for outlines (chapter 
numbering),
-    // it only kind of says somewhere that they should be named Heading1 to 
Heading9.
-    const OUString styleId= pStyle->sConvertedStyleName;
-    m_outline = ( styleId.getLength() == RTL_CONSTASCII_LENGTH( "Heading 1" )
-        && styleId.match( "Heading ", 0 )
-        && styleId[ RTL_CONSTASCII_LENGTH( "Heading " ) ] >= '1'
-        && styleId[ RTL_CONSTASCII_LENGTH( "Heading " ) ] <= '9' );
 }
 
 uno::Sequence<beans::PropertyValue> ListLevel::GetProperties(bool bDefaults)
@@ -407,7 +404,6 @@ const OUString& AbstractListDef::MapListId(OUString const& 
rId)
 
 ListDef::ListDef( ) : AbstractListDef( )
 {
-    m_nDefaultParentLevels = WW_OUTLINE_MAX + 1;
 }
 
 ListDef::~ListDef( )
@@ -484,8 +480,40 @@ static uno::Reference< container::XNameContainer > 
lcl_getUnoNumberingStyles(
     return xStyles;
 }
 
+/// Rank the list in terms of suitability for becoming the Outline numbering 
rule in LO.
+sal_uInt16 ListDef::GetChapterNumberingWeight() const
+{
+    sal_Int16 nWeight = 0;
+    const sal_Int8 nAbstLevels = m_pAbstractDef ? m_pAbstractDef->Size() : 0;
+    for (sal_Int8 nLevel = 0; nLevel < nAbstLevels; ++nLevel)
+    {
+        const ListLevel::Pointer pAbsLevel = m_pAbstractDef->GetLevel(nLevel);
+        const StyleSheetEntryPtr pParaStyle = pAbsLevel->GetParaStyle();
+        if (!pParaStyle)
+            continue;
+        const StyleSheetPropertyMap& rProps =
+            
*static_cast<StyleSheetPropertyMap*>(pParaStyle->pProperties.get());
+        // In LO, the level's paraStyle outlineLevel always matches this 
listLevel.
+        // An undefined listLevel is treated as the first level.
+        sal_Int8 nListLevel = std::clamp<sal_Int8>(rProps.GetListLevel(), 0, 
9);
+        if (nListLevel != nLevel || rProps.GetOutlineLevel() != nLevel)
+            return 0;
+        else if (pAbsLevel->GetNumberingType(style::NumberingType::NUMBER_NONE)
+                 != style::NumberingType::NUMBER_NONE)
+        {
+            // Arbitrarily chosen weighting factors - trying to round-trip LO 
choices if possible.
+            // LibreOffice always saves Outline rule (usually containing 
heading styles) as numId 1.
+            sal_uInt16 nWeightingFactor = GetId() == 1 ? 8 : 1;
+            if (pParaStyle->sStyleIdentifierD.startsWith("Heading") )
+                ++nWeightingFactor;
+            nWeight += nWeightingFactor;
+        }
+    }
+    return nWeight;
+}
+
 void ListDef::CreateNumberingRules( DomainMapper& rDMapper,
-        uno::Reference<lang::XMultiServiceFactory> const& xFactory)
+        uno::Reference<lang::XMultiServiceFactory> const& xFactory, sal_Int16 
nOutline)
 {
     // Get the UNO Numbering styles
     uno::Reference< container::XNameContainer > xStyles = 
lcl_getUnoNumberingStyles( xFactory );
@@ -501,11 +529,12 @@ void ListDef::CreateNumberingRules( DomainMapper& 
rDMapper,
             xFactory->createInstance("com.sun.star.style.NumberingStyle"),
             uno::UNO_QUERY_THROW );
 
-        OUString sStyleName = GetStyleName(GetId(), xStyles);
-
-        xStyles->insertByName( sStyleName, makeAny( xStyle ) );
+        if (GetId() == nOutline)
+            m_StyleName = "Outline"; //SwNumRule.GetOutlineRuleName()
+        else
+            xStyles->insertByName(GetStyleName(GetId(), xStyles), 
makeAny(xStyle));
 
-        uno::Any oStyle = xStyles->getByName( sStyleName );
+        uno::Any oStyle = xStyles->getByName(GetStyleName());
         xStyle.set( oStyle, uno::UNO_QUERY_THROW );
 
         // Get the default OOo Numbering style rules
@@ -572,7 +601,7 @@ void ListDef::CreateNumberingRules( DomainMapper& rDMapper,
             m_xNumRules->replaceByIndex(nLevel, 
uno::makeAny(comphelper::containerToSequence(aLvlProps)));
 
             // Handle the outline level here
-            if (pAbsLevel && pAbsLevel->isOutlineNumbering())
+            if (GetId() == nOutline && pAbsLevel && pAbsLevel->GetParaStyle())
             {
                 uno::Reference< text::XChapterNumberingSupplier > xOutlines (
                     xFactory, uno::UNO_QUERY_THROW );
@@ -585,19 +614,6 @@ void ListDef::CreateNumberingRules( DomainMapper& rDMapper,
                 xOutlineRules->replaceByIndex(nLevel, 
uno::makeAny(comphelper::containerToSequence(aLvlProps)));
             }
 
-            if (pAbsLevel)
-            {
-                // first level without default outline paragraph style
-                const tools::SvRef< StyleSheetEntry >& aParaStyle = 
pAbsLevel->GetParaStyle();
-                if ( WW_OUTLINE_MAX + 1 == m_nDefaultParentLevels && ( 
!aParaStyle ||
-                    aParaStyle->sConvertedStyleName.getLength() != 
RTL_CONSTASCII_LENGTH( "Heading 1" ) ||
-                    !aParaStyle->sConvertedStyleName.startsWith("Heading ") ||
-                    aParaStyle->sConvertedStyleName[ RTL_CONSTASCII_LENGTH( 
"Heading " ) ] - u'1' != nLevel ) )
-                {
-                    m_nDefaultParentLevels = nLevel;
-                }
-            }
-
             nLevel++;
         }
 
@@ -1162,10 +1178,26 @@ ListDef::Pointer ListsManager::GetList( sal_Int32 nId )
 
 void ListsManager::CreateNumberingRules( )
 {
+    // Try to determine which numId would best work as LO's Chapter Numbering 
Outline rule.
+    sal_Int16 nChosenAsChapterNumberingId = -1;
+    sal_uInt16 nHighestWeight = 0;
+    for (const auto& rList : m_aLists)
+    {
+        sal_uInt16 nWeight = rList->GetChapterNumberingWeight();
+        if (nWeight > nHighestWeight)
+        {
+            nHighestWeight = nWeight;
+            nChosenAsChapterNumberingId = rList->GetId();
+            //Optimization: if the weight cannot be beaten anymore, then quit 
early
+            if (nHighestWeight > 17)
+                break;
+        }
+    }
+
     // Loop over the definitions
     for ( const auto& rList : m_aLists )
     {
-        rList->CreateNumberingRules( m_rDMapper, m_xFactory );
+        rList->CreateNumberingRules(m_rDMapper, m_xFactory, 
nChosenAsChapterNumberingId);
     }
     m_rDMapper.GetStyleSheetTable()->ApplyNumberingStyleNameToParaStyles();
 }
diff --git a/writerfilter/source/dmapper/NumberingManager.hxx 
b/writerfilter/source/dmapper/NumberingManager.hxx
index 2a1e0204e02d..252f26149fea 100644
--- a/writerfilter/source/dmapper/NumberingManager.hxx
+++ b/writerfilter/source/dmapper/NumberingManager.hxx
@@ -51,7 +51,6 @@ class ListLevel : public PropertyMap
     css::uno::Reference<css::awt::XBitmap> m_xGraphicBitmap;
     std::optional<sal_Int32>               m_nTabstop;
     tools::SvRef< StyleSheetEntry >          m_pParaStyle;
-    bool                                          m_outline;
     bool m_bHasValues = false;
 
 public:
@@ -63,7 +62,6 @@ public:
         ,m_nStartOverride(-1)
         ,m_nNFC(-1)
         ,m_nXChFollow(SvxNumberFormat::LISTTAB)
-        ,m_outline(false)
         {}
 
     // Setters for the import
@@ -77,9 +75,9 @@ public:
     void SetParaStyle( const tools::SvRef< StyleSheetEntry >& pStyle );
 
     // Getters
+    sal_Int16 GetNumberingType(sal_Int16 nDefault) const;
     const OUString& GetBulletChar( ) const { return m_sBulletChar; };
     const tools::SvRef< StyleSheetEntry >& GetParaStyle( ) const { return 
m_pParaStyle; };
-    bool isOutlineNumbering() const { return m_outline; }
     sal_Int32 GetStartOverride() const { return m_nStartOverride; };
     /// Determines if SetValue() was called at least once.
     bool HasValues() const;
@@ -163,7 +161,6 @@ public:
     const OUString&       GetStyleLink() const { return m_sStyleLink; };
 
     const OUString& MapListId(OUString const& rId);
-    bool isOutlineNumbering( sal_uInt16 nLvl ) { return GetLevel(nLvl) && 
GetLevel(nLvl)->isOutlineNumbering(); }
 };
 
 class ListDef : public AbstractListDef
@@ -178,9 +175,6 @@ private:
     /// mapped list style name
     OUString m_StyleName;
 
-    /// not custom outline parent levels
-    sal_Int16 m_nDefaultParentLevels;
-
 public:
     typedef tools::SvRef< ListDef > Pointer;
 
@@ -195,11 +189,10 @@ public:
     const OUString & GetStyleName() const { return m_StyleName; };
     const OUString & GetStyleName(sal_Int32 nId, 
css::uno::Reference<css::container::XNameContainer> const& xStyles);
 
-    sal_Int16 GetDefaultParentLevels() const { return m_nDefaultParentLevels; 
};
-
     css::uno::Sequence< css::uno::Sequence<css::beans::PropertyValue> > 
GetMergedPropertyValues();
 
-    void CreateNumberingRules(DomainMapper& rDMapper, 
css::uno::Reference<css::lang::XMultiServiceFactory> const& xFactory);
+    sal_uInt16 GetChapterNumberingWeight() const;
+    void CreateNumberingRules(DomainMapper& rDMapper, 
css::uno::Reference<css::lang::XMultiServiceFactory> const& xFactory, sal_Int16 
nOutline);
 
     const css::uno::Reference<css::container::XIndexReplace>& 
GetNumberingRules() const { return m_xNumRules; }
 
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to