[Libreoffice-commits] core.git: writerfilter/source

Justin Luth (via logerrit) logerrit at kemper.freedesktop.org
Sat Jul 10 05:25:28 UTC 2021


 writerfilter/source/dmapper/DomainMapper_Impl.cxx |   31 +++++++++-------------
 writerfilter/source/dmapper/NumberingManager.cxx  |    5 +++
 2 files changed, 18 insertions(+), 18 deletions(-)

New commits:
commit 11211f2ff0a4d894f889874d46c6ba8be4d3dc39
Author:     Justin Luth <justin_luth at sil.org>
AuthorDate: Fri Apr 30 13:42:58 2021 +0200
Commit:     Justin Luth <justin_luth at sil.org>
CommitDate: Sat Jul 10 07:24:54 2021 +0200

    tdf#141964 writerfilter CN: why ignore paragraph's listId?
    
    This seems bizarre. nListId was only set when the style
    defined it? What about the directly applied rule?
    Surely that has more relevance in all the
    subsequent code that used the value in nListId,
    as borne out in all of the code that re-calculated nlistId2.
    
    Change-Id: I188e812388c706cf0e785fdede29a6d21be12867
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/115260
    Tested-by: Justin Luth <justin_luth at sil.org>
    Reviewed-by: Justin Luth <justin_luth at sil.org>

diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 40602ee0d40d..c3d6e4efbe6d 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -1533,13 +1533,14 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
     const StyleSheetEntryPtr pEntry = GetStyleSheetTable()->FindStyleSheetByConvertedStyleName( GetCurrentParaStyleName() );
     OSL_ENSURE( pEntry, "no style sheet found" );
     const StyleSheetPropertyMap* pStyleSheetProperties = dynamic_cast<const StyleSheetPropertyMap*>(pEntry ? pEntry->pProperties.get() : nullptr);
+    sal_Int32 nListId = pParaContext ? pParaContext->GetListId() : -1;
     bool isNumberingViaStyle(false);
-    bool isNumberingViaRule = pParaContext && pParaContext->GetListId() > -1;
-    sal_Int32 nListId = -1;
+    bool isNumberingViaRule = nListId > -1;
     if ( !bRemove && pStyleSheetProperties && pParaContext )
     {
         bool bNumberingFromBaseStyle = false;
-        nListId = lcl_getListId(pEntry, GetStyleSheetTable(), bNumberingFromBaseStyle);
+        if (!isNumberingViaRule)
+            nListId = lcl_getListId(pEntry, GetStyleSheetTable(), bNumberingFromBaseStyle);
 
         //apply numbering level/style to paragraph if it was set at the style, but only if the paragraph itself
         //does not specify the numbering
@@ -1552,10 +1553,10 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
             pParaContext->Insert( PROP_NUMBERING_LEVEL, uno::makeAny(nListLevel), false );
 
         auto const pList(GetListTable()->GetList(nListId));
-        if (pList && nListId >= 0 && !pParaContext->isSet(PROP_NUMBERING_STYLE_NAME))
+        if (pList && nListId > 0 && !pParaContext->isSet(PROP_NUMBERING_STYLE_NAME))
         {
             // ListLevel 9 means Body Level/no numbering.  numId 0 means no numbering.
-            if (bNoNumbering || nListLevel == 9 || (!isNumberingViaRule && !nListId))
+            if (bNoNumbering || nListLevel == 9)
                 pParaContext->Insert(PROP_NUMBERING_STYLE_NAME, uno::makeAny(OUString()), true);
             else if ( !isNumberingViaRule )
             {
@@ -1570,7 +1571,8 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
                 // Apply the style if it uses the same list as the direct numbering,
                 // otherwise the directly-applied-to-paragraph status will be lost,
                 // and the priority of the numbering-style-indents will be lowered. tdf#133000
-                if ( nListId == pParaContext->GetListId() )
+                bool bDummy;
+                if (nListId == lcl_getListId(pEntry, GetStyleSheetTable(), bDummy))
                     pParaContext->Insert( PROP_NUMBERING_STYLE_NAME, uno::makeAny(pList->GetStyleName()), true );
             }
         }
@@ -1621,10 +1623,8 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
             {
                 pParaContext->Insert(PROP_PARA_RIGHT_MARGIN, aRightMargin, /*bOverwrite=*/false);
 
-                sal_Int32 nListId2(pParaContext->GetListId());
-
-                const sal_Int32 nFirstLineIndent = getNumberingProperty(nListId2, nListLevel, "FirstLineIndent");
-                const sal_Int32 nParaLeftMargin  = getNumberingProperty(nListId2, nListLevel, "IndentAt");
+                const sal_Int32 nFirstLineIndent = getNumberingProperty(nListId, nListLevel, "FirstLineIndent");
+                const sal_Int32 nParaLeftMargin  = getNumberingProperty(nListId, nListLevel, "IndentAt");
                 if (nFirstLineIndent != 0)
                     pParaContext->Insert(PROP_PARA_FIRST_LINE_INDENT, uno::makeAny(nFirstLineIndent), /*bOverwrite=*/false);
                 if (nParaLeftMargin != 0)
@@ -1948,12 +1948,7 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
                         (isNumberingViaStyle || isNumberingViaRule))
                     {
                         assert(pParaContext);
-                        // Use lcl_getListId(), so we find the list ID in parent styles as well.
-                        bool bNumberingFromBaseStyle = false;
-                        sal_Int32 const nListId2( isNumberingViaStyle
-                            ? lcl_getListId(pEntry, GetStyleSheetTable(), bNumberingFromBaseStyle)
-                            : pParaContext->GetListId());
-                        if (ListDef::Pointer const& pList = m_pListTable->GetList(nListId2))
+                        if (ListDef::Pointer const& pList = m_pListTable->GetList(nListId))
                         {   // styles could refer to non-existing lists...
                             AbstractListDef::Pointer const& pAbsList =
                                     pList->GetAbstractDefinition();
@@ -1975,7 +1970,7 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
                             if (pList->GetCurrentLevel())
                             {
                                 sal_Int16 nOverrideLevel = pList->GetCurrentLevel()->GetStartOverride();
-                                if (nOverrideLevel != -1 && m_aListOverrideApplied.find(nListId2) == m_aListOverrideApplied.end())
+                                if (nOverrideLevel != -1 && m_aListOverrideApplied.find(nListId) == m_aListOverrideApplied.end())
                                 {
                                     // Apply override: we have override instruction for this level
                                     // And this was not done for this list before: we can do this only once on first occurrence
@@ -1985,7 +1980,7 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
                                     // and we need to register level overrides separately.
                                     m_xPreviousParagraph->setPropertyValue("ParaIsNumberingRestart", uno::makeAny(true));
                                     m_xPreviousParagraph->setPropertyValue("NumberingStartValue", uno::makeAny(nOverrideLevel));
-                                    m_aListOverrideApplied.insert(nListId2);
+                                    m_aListOverrideApplied.insert(nListId);
                                 }
                             }
                         }
diff --git a/writerfilter/source/dmapper/NumberingManager.cxx b/writerfilter/source/dmapper/NumberingManager.cxx
index 62118630e2d4..8f810b435365 100644
--- a/writerfilter/source/dmapper/NumberingManager.cxx
+++ b/writerfilter/source/dmapper/NumberingManager.cxx
@@ -1165,6 +1165,8 @@ AbstractListDef::Pointer ListsManager::GetAbstractList( sal_Int32 nId )
 ListDef::Pointer ListsManager::GetList( sal_Int32 nId )
 {
     ListDef::Pointer pList;
+    if (nId == -1)
+        return pList;
 
     int nLen = m_aLists.size( );
     int i = 0;
@@ -1175,6 +1177,9 @@ ListDef::Pointer ListsManager::GetList( sal_Int32 nId )
         i++;
     }
 
+    // nId 0 is only valid for abstractNum, not numId (which has an abstract definition)
+    assert(nId || !pList || !pList->GetAbstractDefinition());
+
     return pList;
 }
 


More information about the Libreoffice-commits mailing list