fdo#65632 RTF import: send NS_ooxml::LN_trackchange only once for one range

The problem was that in case we had
{\deleted foo\b bar}
then both "foo" and "bar" triggered a trackchange, but "}" only ended
one, resulting in overlapping redline ranges. This was cought by core,
but caused a performance problem.

For the first bugdoc, before:
real    3m57.803s
after:
real    0m3.072s

(cherry picked from commits 7c0a1557406ffffbb8145f8035ce86d31e927667,
ed187fcbd457d01be6ac382d61b493039a5af7d5 and
54518a209d0ffe00f8e391472da92e398c474392 and
9f7676033585ab3bf352d5dc2ef43a3a9d8d5c46)

Conflicts:
	writerfilter/source/rtftok/rtfdocumentimpl.cxx
	writerfilter/source/rtftok/rtfdocumentimpl.hxx

Change-Id: Ibf6f2db30109f0b9a2571d2e4fb3cc76294f68d1
Reviewed-on: https://gerrit.libreoffice.org/4964
Reviewed-by: Fridrich Strba <fridrich@documentfoundation.org>
Tested-by: Fridrich Strba <fridrich@documentfoundation.org>

8 files changed, 56 insertions, 19 deletions

diff --git a/sw/qa/extras/ooxmlimport/data/fdo65632.docx b/sw/qa/extras/ooxmlimport/data/fdo65632.docx
new file mode 100755
index 000000000000..8c336c20fdfc
--- /dev/null
+++ b/sw/qa/extras/ooxmlimport/data/fdo65632.docx
diff --git a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
index 26ecdf393a11..1db782971ce1 100644
--- a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
+++ b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
@@ -124,6 +124,7 @@ public:
     void testN820504();
     void testFdo43641();
     void testFdo46361();
+    void testFdo65632();
 
     CPPUNIT_TEST_SUITE(Test);
 #if !defined(MACOSX) && !defined(WNT)
@@ -213,6 +214,7 @@ void Test::run()
         {"n820504.docx", &Test::testN820504},
         {"fdo43641.docx", &Test::testFdo43641},
         {"fdo46361.docx", &Test::testFdo46361},
+        {"fdo65632.docx", &Test::testFdo65632},
     };
     header();
     for (unsigned int i = 0; i < SAL_N_ELEMENTS(aMethods); ++i)
@@ -1511,6 +1513,17 @@ void Test::testFdo46361()
     CPPUNIT_ASSERT_EQUAL(OUString("text\ntext\n"), uno::Reference<text::XTextRange>(xGroupShape->getByIndex(2), uno::UNO_QUERY)->getString());
 }
 
+void Test::testFdo65632()
+{
+    // The problem was that the footnote text had fake redline: only the body
+    // text has redline in fact.
+    uno::Reference<text::XFootnotesSupplier> xFootnotesSupplier(mxComponent, uno::UNO_QUERY);
+    uno::Reference<container::XIndexAccess> xFootnotes(xFootnotesSupplier->getFootnotes(), uno::UNO_QUERY);
+    uno::Reference<text::XText> xText(xFootnotes->getByIndex(0), uno::UNO_QUERY);
+    //uno::Reference<text::XTextRange> xParagraph = getParagraphOfText(1, xText);
+    CPPUNIT_ASSERT_EQUAL(OUString("Text"), getProperty<OUString>(getRun(getParagraphOfText(1, xText), 1), "TextPortionType"));
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(Test);
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/writerfilter/source/dmapper/DomainMapper.cxx b/writerfilter/source/dmapper/DomainMapper.cxx
index 91d5d941cfa2..fb4786e05ecf 100644
--- a/writerfilter/source/dmapper/DomainMapper.cxx
+++ b/writerfilter/source/dmapper/DomainMapper.cxx
@@ -1402,9 +1402,6 @@ void DomainMapper::lcl_attribute(Id nName, Value & val)
         case NS_ooxml::LN_CT_Color_themeShade:
             //unsupported
         break;
-        case NS_ooxml::LN_endtrackchange:
-            m_pImpl->RemoveCurrentRedline( );
-        break;
         case NS_ooxml::LN_CT_DocGrid_linePitch:
         {
             //see SwWW8ImplReader::SetDocumentGrid
@@ -3276,6 +3273,9 @@ void DomainMapper::sprmWithProps( Sprm& rSprm, PropertyMapPtr rContext, SprmType
         m_pImpl->EndParaChange( );
     }
     break;
+    case NS_ooxml::LN_endtrackchange:
+        m_pImpl->RemoveCurrentRedline( );
+    break;
     case NS_ooxml::LN_CT_RPrChange_rPr:
     break;
     case NS_ooxml::LN_object:
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index e2b2ea54e0a3..c43747104a72 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -208,6 +208,8 @@ DomainMapper_Impl::DomainMapper_Impl(
     m_bUsingEnhancedFields = lcl_IsUsingEnhancedFields( m_xComponentContext );
 
     m_pSdtHelper = new SdtHelper(*this);
+
+    m_aRedlines.push(std::vector<RedlineParamsPtr>());
 }
 
 
@@ -1449,6 +1451,9 @@ void DomainMapper_Impl::PushFootOrEndnote( bool bIsFootnote )
 {
     try
     {
+        // Redlines outside the footnote should not affect footnote content
+        m_aRedlines.push(std::vector< RedlineParamsPtr >());
+
         PropertyMapPtr pTopContext = GetTopContext();
         uno::Reference< text::XText > xFootnoteText;
         if (GetTextFactory().is())
@@ -1540,9 +1545,9 @@ void DomainMapper_Impl::CheckParaRedline( uno::Reference< text::XTextRange > xRa
 
 void DomainMapper_Impl::CheckRedline( uno::Reference< text::XTextRange > xRange )
 {
-    vector<RedlineParamsPtr>::iterator pIt = m_aRedlines.begin( );
+    vector<RedlineParamsPtr>::iterator pIt = m_aRedlines.top().begin( );
     vector< RedlineParamsPtr > aCleaned;
-    for (; pIt != m_aRedlines.end( ); ++pIt )
+    for (; pIt != m_aRedlines.top().end( ); ++pIt )
     {
         CreateRedline( xRange, *pIt );
 
@@ -1553,7 +1558,7 @@ void DomainMapper_Impl::CheckRedline( uno::Reference< text::XTextRange > xRange
         }
     }
 
-    m_aRedlines.swap( aCleaned );
+    m_aRedlines.top().swap( aCleaned );
 }
 
 void DomainMapper_Impl::StartParaChange( )
@@ -1596,6 +1601,13 @@ void DomainMapper_Impl::PopFootOrEndnote()
     RemoveLastParagraph();
     if (!m_aTextAppendStack.empty())
         m_aTextAppendStack.pop();
+
+    if (m_aRedlines.size() == 1)
+    {
+        SAL_WARN("writerfilter", "PopFootOrEndnote() is called without PushFootOrEndnote()?");
+        return;
+    }
+    m_aRedlines.pop();
 }
 
 
@@ -3727,7 +3739,7 @@ void DomainMapper_Impl::AddNewRedline(  )
     pNew->m_nToken = ooxml::OOXML_mod;
     if ( !m_bIsParaChange )
     {
-        m_aRedlines.push_back( pNew );
+        m_aRedlines.top().push_back( pNew );
     }
     else
     {
@@ -3738,8 +3750,8 @@ void DomainMapper_Impl::AddNewRedline(  )
 RedlineParamsPtr DomainMapper_Impl::GetTopRedline(  )
 {
     RedlineParamsPtr pResult;
-    if ( !m_bIsParaChange && m_aRedlines.size(  ) > 0 )
-        pResult = m_aRedlines.back(  );
+    if ( !m_bIsParaChange && m_aRedlines.top().size(  ) > 0 )
+        pResult = m_aRedlines.top().back(  );
     else if ( m_bIsParaChange )
         pResult = m_pParaRedline;
     return pResult;
@@ -3802,9 +3814,9 @@ void DomainMapper_Impl::SetCurrentRedlineToken( sal_Int32 nToken )
 
 void DomainMapper_Impl::RemoveCurrentRedline( )
 {
-    if ( m_aRedlines.size( ) > 0 )
+    if ( m_aRedlines.top().size( ) > 0 )
     {
-        m_aRedlines.pop_back( );
+        m_aRedlines.top().pop_back( );
     }
 }
 
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.hxx b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
index f88f49441f01..30cc2c7d901a 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
@@ -355,7 +355,7 @@ private:
     ::com::sun::star::uno::Reference< text::XTextRange >      m_xFrameEndRange;
 
     // Redline stack
-    std::vector< RedlineParamsPtr > m_aRedlines;
+    std::stack< std::vector< RedlineParamsPtr > > m_aRedlines;
     RedlineParamsPtr                m_pParaRedline;
     bool                            m_bIsParaChange;
 
diff --git a/writerfilter/source/ooxml/model.xml b/writerfilter/source/ooxml/model.xml
index b2612e3dfde8..c15f7986a277 100644
--- a/writerfilter/source/ooxml/model.xml
+++ b/writerfilter/source/ooxml/model.xml
@@ -22084,7 +22084,9 @@
       <action name="start" action="tokenproperty"/>
       <action name="start" action="propagateCharacterPropertiesAsSet" sendtokenid="ooxml:trackchange"/>
       <action name="start" action="clearProps"/>
-      <action name="end" action="mark" sendtokenid="ooxml:endtrackchange"/>
+      <action name="end" action="tokenproperty"/>
+      <action name="end" action="propagateCharacterPropertiesAsSet" sendtokenid="ooxml:endtrackchange"/>
+      <action name="end" action="clearProps"/>
     </resource>
     <resource name="EG_RangeMarkupElements" resource="Properties" tag="redlines">
       <element name="bookmarkStart" tokenid="ooxml:EG_RangeMarkupElements_bookmarkStart"/>
diff --git a/writerfilter/source/rtftok/rtfdocumentimpl.cxx b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
index e6d51a5bce04..5e93fc27e6df 100644
--- a/writerfilter/source/rtftok/rtfdocumentimpl.cxx
+++ b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
@@ -469,6 +469,15 @@ void RTFDocumentImpl::runProps()
         RTFValue::Pointer_t pValue(new RTFValue(m_aStates.top().aCharacterAttributes, m_aStates.top().aCharacterSprms));
         m_aStates.top().pCurrentBuffer->push_back(make_pair(BUFFER_PROPS, pValue));
     }
+
+    // Delete the sprm, so the trackchange range will be started only once.
+    // OTOH set a boolean flag, so we'll know we need to end the range later.
+    RTFValue::Pointer_t pTrackchange = m_aStates.top().aCharacterSprms.find(NS_ooxml::LN_trackchange);
+    if (pTrackchange.get())
+    {
+        m_aStates.top().bStartedTrackchange = true;
+        m_aStates.top().aCharacterSprms.erase(NS_ooxml::LN_trackchange);
+    }
 }
 
 void RTFDocumentImpl::runBreak()
@@ -4384,13 +4393,12 @@ int RTFDocumentImpl::popState()
     }
 
     // See if we need to end a track change
-    RTFValue::Pointer_t pTrackchange = aState.aCharacterSprms.find(NS_ooxml::LN_trackchange);
-    if (pTrackchange.get())
+    if (aState.bStartedTrackchange)
     {
-        RTFSprms aTCAttributes;
+        RTFSprms aTCSprms;
         RTFValue::Pointer_t pValue(new RTFValue(0));
-        aTCAttributes.set(NS_ooxml::LN_endtrackchange, pValue);
-        writerfilter::Reference<Properties>::Pointer_t const pProperties(new RTFReferenceProperties(aTCAttributes));
+        aTCSprms.set(NS_ooxml::LN_endtrackchange, pValue);
+        writerfilter::Reference<Properties>::Pointer_t const pProperties(new RTFReferenceProperties(RTFSprms(), aTCSprms));
         Mapper().props(pProperties);
     }
 
@@ -4728,7 +4736,8 @@ RTFParserState::RTFParserState(RTFDocumentImpl *pDocumentImpl)
     nCurrentStyleIndex(-1),
     pCurrentBuffer(0),
     bHasTableStyle(false),
-    bInListpicture(false)
+    bInListpicture(false),
+    bStartedTrackchange(false)
 {
 }
 
diff --git a/writerfilter/source/rtftok/rtfdocumentimpl.hxx b/writerfilter/source/rtftok/rtfdocumentimpl.hxx
index d6216f48ee6d..5bedc6140d24 100644
--- a/writerfilter/source/rtftok/rtfdocumentimpl.hxx
+++ b/writerfilter/source/rtftok/rtfdocumentimpl.hxx
@@ -399,6 +399,7 @@ namespace writerfilter {
 
                 /// If we're inside a \listpicture group.
                 bool bInListpicture;
+                bool bStartedTrackchange; ///< Track change is started, need to end it before popping.
         };
 
         class RTFTokenizer;