[Libreoffice-commits] core.git: Branch 'libreoffice-6-4' - sw/qa writerfilter/source

Justin Luth (via logerrit) logerrit at kemper.freedesktop.org
Wed Jan 8 16:42:27 UTC 2020


 sw/qa/extras/ooxmlexport/data/tdf123262_textFootnoteSeparators.docx |binary
 sw/qa/extras/ooxmlexport/ooxmlexport5.cxx                           |   18 ++++++
 writerfilter/source/dmapper/DomainMapper.cxx                        |   28 +++++++---
 writerfilter/source/dmapper/DomainMapper_Impl.cxx                   |   14 -----
 writerfilter/source/dmapper/DomainMapper_Impl.hxx                   |   18 ++++--
 5 files changed, 52 insertions(+), 26 deletions(-)

New commits:
commit 1bbff471cdcfcd4a9ceb6ff6fed7006fbc3f6e1c
Author:     Justin Luth <justin.luth at collabora.com>
AuthorDate: Fri Nov 29 22:10:27 2019 +0300
Commit:     Michael Stahl <michael.stahl at cib.de>
CommitDate: Wed Jan 8 17:41:52 2020 +0100

    tdf#123262 writerfilter: completely ignore footnote separators
    
    ... except for processing enough to observe the separator exists.
    
    For each footnote reference, the entire footnote.xml file is
    parsed every time. The text in the "separator" footnote was
    being added to every footnote.  The normal case where this is just
    a single paragraph was already handled, but this patch generalizes
    everything to handle cases of actual text or multiple paragraphs.
    
    Not every footnote has a type, so we can't depend on that to turn
    ignoringText ON/OFF. Every footnote has an ID, but theoretically
    the ID could be processed before or after the type, and it has
    no idea which type it is. Finally, the skipped text has no idea
    how many times/paragaphs it needs to skip. So a three-way
    control was needed to handle on/used/off. As a safeguard, finishing
    the footnote.xml parse (PopFootOrEndnote) ensures that
    ignoring won't be left on in the unlikely case that
    the separator is the last footnote.
    
    Change-Id: Ia30ca8d3a36417a4691e3b2e1c978720be017030
    Reviewed-on: https://gerrit.libreoffice.org/82172
    Tested-by: Jenkins
    Reviewed-by: Justin Luth <justin_luth at sil.org>
    Reviewed-by: Miklos Vajna <vmiklos at collabora.com>
    (cherry picked from commit acb9d901009d026cb48e6a8b94e6200f05110504)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/85734
    Reviewed-by: Michael Stahl <michael.stahl at cib.de>

diff --git a/sw/qa/extras/ooxmlexport/data/tdf123262_textFootnoteSeparators.docx b/sw/qa/extras/ooxmlexport/data/tdf123262_textFootnoteSeparators.docx
new file mode 100644
index 000000000000..ceccb767e27e
Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf123262_textFootnoteSeparators.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport5.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport5.cxx
index 14ba1cde9683..2560cf89a506 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport5.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport5.cxx
@@ -523,6 +523,24 @@ DECLARE_OOXMLEXPORT_TEST(testFDO79062, "fdo79062.docx")
     CPPUNIT_ASSERT_EQUAL_MESSAGE( "Paragraph starts with W(87), not tab(9)", u'W', sFootnotePara[0] );
 }
 
+DECLARE_OOXMLEXPORT_TEST(testTdf123262_textFootnoteSeparators, "tdf123262_textFootnoteSeparators.docx")
+{
+    //Everything easily fits on one page
+    CPPUNIT_ASSERT_EQUAL_MESSAGE( "Number of Pages", 1, getPages() );
+
+    uno::Reference<text::XFootnotesSupplier> xFootnotesSupplier(mxComponent, uno::UNO_QUERY);
+    uno::Reference<container::XIndexAccess> xFootnotes = xFootnotesSupplier->getFootnotes();
+    uno::Reference<text::XText> xFootnoteText(xFootnotes->getByIndex(0), uno::UNO_QUERY);
+
+    // The text in the separator footnote should not be added to the footnotes
+    OUString sText = " Microsoft Office.";
+    CPPUNIT_ASSERT_EQUAL(sText, xFootnoteText->getString());
+
+    // Ensure that paragraph markers are not lost.
+    xFootnoteText.set(xFootnotes->getByIndex(1), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL_MESSAGE( "Number of paragraphs in second footnote", 2, getParagraphs(xFootnoteText) );
+}
+
 DECLARE_OOXMLEXPORT_TEST(testfdo79668,"fdo79668.docx")
 {
     // fdo#79668: Document was Crashing on DebugUtil build while Saving
diff --git a/writerfilter/source/dmapper/DomainMapper.cxx b/writerfilter/source/dmapper/DomainMapper.cxx
index 5417b6a05e0b..87b7fdbcbecc 100644
--- a/writerfilter/source/dmapper/DomainMapper.cxx
+++ b/writerfilter/source/dmapper/DomainMapper.cxx
@@ -1078,9 +1078,20 @@ void DomainMapper::lcl_attribute(Id nName, Value & val)
             }
         break;
         case NS_ooxml::LN_CT_FtnEdn_type:
-            // This is the "separator" footnote, ignore its linebreak.
+            // This is the "separator" footnote, ignore its linebreaks/text.
             if (static_cast<sal_uInt32>(nIntValue) == NS_ooxml::LN_Value_doc_ST_FtnEdn_separator)
-                m_pImpl->SeenFootOrEndnoteSeparator();
+                m_pImpl->SetSkipFootnoteState( SkipFootnoteSeparator::ON );
+            else
+                m_pImpl->SetSkipFootnoteState( SkipFootnoteSeparator::OFF );
+        break;
+        case NS_ooxml::LN_CT_FtnEdn_id:
+        {
+            SkipFootnoteSeparator eSkip = m_pImpl->GetSkipFootnoteState();
+            if ( eSkip == SkipFootnoteSeparator::ON )
+                m_pImpl->SetSkipFootnoteState( SkipFootnoteSeparator::SKIPPING );
+            else if ( eSkip == SkipFootnoteSeparator::SKIPPING )
+                m_pImpl->SetSkipFootnoteState( SkipFootnoteSeparator::OFF );
+        }
         break;
         case NS_ooxml::LN_CT_DataBinding_prefixMappings:
             m_pImpl->appendGrabBag(m_pImpl->m_aInteropGrabBag, "ooxml:CT_DataBinding_prefixMappings", sStringValue);
@@ -3398,18 +3409,19 @@ void DomainMapper::lcl_utext(const sal_uInt8 * data_, size_t len)
     if (!m_pImpl->hasTableManager())
         return;
 
+    SkipFootnoteSeparator eSkip = m_pImpl->GetSkipFootnoteState();
+    if ( eSkip == SkipFootnoteSeparator::ON || eSkip == SkipFootnoteSeparator::SKIPPING )
+    {
+        m_pImpl->SetSkipFootnoteState( SkipFootnoteSeparator::SKIPPING );
+        return;
+    }
+
     try
     {
         m_pImpl->getTableManager().utext(data_, len);
 
         if (bNewLine)
         {
-            if (m_pImpl->m_bIgnoreNextPara)
-            {
-                m_pImpl->m_bIgnoreNextPara = false;
-                return;
-            }
-
             const bool bSingleParagraph = m_pImpl->GetIsFirstParagraphInSection() && m_pImpl->GetIsLastParagraphInSection();
             const bool bSingleParagraphAfterRedline = m_pImpl->GetIsFirstParagraphInSection(true) && m_pImpl->GetIsLastParagraphInSection();
             PropertyMapPtr pContext = m_pImpl->GetTopContextOfType(CONTEXT_PARAGRAPH);
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 82c5b08c9e0f..ec337152525f 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -268,7 +268,7 @@ DomainMapper_Impl::DomainMapper_Impl(
         m_bInFootOrEndnote(false),
         m_bHasFootnoteStyle(false),
         m_bCheckFootnoteStyle(false),
-        m_bSeenFootOrEndnoteSeparator(false),
+        m_eSkipFootnoteState(SkipFootnoteSeparator::OFF),
         m_bLineNumberingSet( false ),
         m_bIsInFootnoteProperties( false ),
         m_bIsParaMarkerChange( false ),
@@ -299,7 +299,6 @@ DomainMapper_Impl::DomainMapper_Impl(
         m_nLastTableCellParagraphDepth(0),
         m_bHasFtn(false),
         m_bHasFtnSep(false),
-        m_bIgnoreNextPara(false),
         m_bCheckFirstFootnoteTab(false),
         m_bIgnoreNextTab(false),
         m_bIsSplitPara(false),
@@ -2470,21 +2469,12 @@ void DomainMapper_Impl::PopFootOrEndnote()
         return;
     }
     m_aRedlines.pop();
-    m_bSeenFootOrEndnoteSeparator = false;
+    m_eSkipFootnoteState = SkipFootnoteSeparator::OFF;
     m_bInFootOrEndnote = false;
     m_pFootnoteContext = nullptr;
     m_bFirstParagraphInCell = m_bSaveFirstParagraphInCell;
 }
 
-void DomainMapper_Impl::SeenFootOrEndnoteSeparator()
-{
-    if (!m_bSeenFootOrEndnoteSeparator)
-    {
-        m_bSeenFootOrEndnoteSeparator = true;
-        m_bIgnoreNextPara = true;
-    }
-}
-
 void DomainMapper_Impl::PopAnnotation()
 {
     RemoveLastParagraph();
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.hxx b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
index 18b019e7c402..c8de67b69674 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
@@ -114,6 +114,13 @@ enum BreakType
     COLUMN_BREAK
 };
 
+enum SkipFootnoteSeparator
+{
+    OFF,
+    ON,
+    SKIPPING
+};
+
 /**
  * Storage for state that is relevant outside a header/footer, but not inside it.
  *
@@ -512,8 +519,8 @@ private:
     PropertyMapPtr m_pFootnoteContext;
     bool m_bHasFootnoteStyle;
     bool m_bCheckFootnoteStyle;
-    /// Did we get a <w:separator/> for this footnote already?
-    bool                            m_bSeenFootOrEndnoteSeparator;
+    /// Skip paragraphs from the <w:separator/> footnote
+    SkipFootnoteSeparator           m_eSkipFootnoteState;
 
     bool                            m_bLineNumberingSet;
     bool                            m_bIsInFootnoteProperties;
@@ -781,8 +788,9 @@ public:
     void SetCheckFootnoteStyle(bool bVal) { m_bCheckFootnoteStyle = bVal; }
 
     const PropertyMapPtr& GetFootnoteContext() const { return m_pFootnoteContext; }
-    /// Got a <w:separator/>.
-    void SeenFootOrEndnoteSeparator();
+
+    SkipFootnoteSeparator GetSkipFootnoteState() const { return m_eSkipFootnoteState; }
+    void SetSkipFootnoteState(SkipFootnoteSeparator eId) { m_eSkipFootnoteState =  eId; }
 
     void PushAnnotation();
     void PopAnnotation();
@@ -992,8 +1000,6 @@ public:
     /// If the current section has a footnote separator.
     bool m_bHasFtnSep;
 
-    /// If the next newline should be ignored, used by the special footnote separator paragraph.
-    bool m_bIgnoreNextPara;
     /// If the next tab should be ignored, used for footnotes.
     bool m_bCheckFirstFootnoteTab;
     bool m_bIgnoreNextTab;


More information about the Libreoffice-commits mailing list