tdf#70318 tdf#90260 writerfilter: pasted RTF documents may contain no \par

This is a squash of 3 commits, as the first doesn't build without the
second one.

First commit:

sw core is not yet adapted, will be done in the next commit.

(cherry picked from commit e702c78843e387d83fd9c8fbd1597cbe27e3e656)

Second commit:

Author: Mike Kaganski <mikekaganski@hotmail.com>
tdf#70318: don't forget to clean up second fake paragraph

RTF insert is made into an empty paragraph. To do that, two splits
are made before the insert, but only one is reverted afterwards.
This patch removes the second.

Also fixes a memory leak from unreleased heap object

The corresponding unit test is corrected. It was incorrect
because \par doesn't begin new paragraph; it only ends paragraph.
If a RTF is ended with \par } then no newline is added to its end.
The old unit test only worked because of the bug fixed by this
patch. Correct way of inserting new paragraph in the end of a RTF
is \par \par}

(cherry picked from commit 0ddd9f9ff45f61013ea18763eca4c68aedce6caa)

Third commit:

tdf#90260 testcase

(cherry picked from commit 8931abc0b9fded1ee78eca6bf28e8d2438a76add)

Conflicts:
	writerfilter/source/filter/RtfFilter.cxx
	writerfilter/source/rtftok/rtfdocumentfactory.cxx
	writerfilter/source/rtftok/rtfdocumentimpl.cxx
	sw/source/filter/rtf/swparrtf.cxx

Change-Id: If8da12427e0cdaced4c1c1776b9f0b8cbde5c57c
63d50a940d7960beb35f7d774c833ed8499acbef
06a5ff604e6782863c4a2d6e002c9d67d42912fb
Reviewed-on: https://gerrit.libreoffice.org/15963
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>

12 files changed, 71 insertions, 15 deletions

diff --git a/sw/qa/extras/rtfimport/data/copypaste-footnote-paste.rtf b/sw/qa/extras/rtfimport/data/copypaste-footnote-paste.rtf
index 1f4a75e44629..a7c6bb244e55 100644
--- a/sw/qa/extras/rtfimport/data/copypaste-footnote-paste.rtf
+++ b/sw/qa/extras/rtfimport/data/copypaste-footnote-paste.rtf
@@ -1,3 +1,2 @@
 {\rtf1
-bbb
-\par }
+bbb}
diff --git a/sw/qa/extras/rtfimport/data/tdf90260-nopar.rtf b/sw/qa/extras/rtfimport/data/tdf90260-nopar.rtf
new file mode 100644
index 000000000000..c54eedd28878
--- /dev/null
+++ b/sw/qa/extras/rtfimport/data/tdf90260-nopar.rtf
@@ -0,0 +1,2 @@
+{\rtf1\ansi\ansicpg1252\deff0\deflang1035{\fonttbl{\f0\fnil\fcharset0 Calibri;}}

+\uc1\pard\sa200\sl276\slmult1\lang11\f0\fs22 simple}

diff --git a/sw/qa/extras/rtfimport/data/tdf90260-par.rtf b/sw/qa/extras/rtfimport/data/tdf90260-par.rtf
new file mode 100644
index 000000000000..e2115b634dfe
--- /dev/null
+++ b/sw/qa/extras/rtfimport/data/tdf90260-par.rtf
@@ -0,0 +1,2 @@
+{\rtf1\ansi\ansicpg1252\deff0\deflang1035{\fonttbl{\f0\fnil\fcharset0 Calibri;}}

+\uc1\pard\sa200\sl276\slmult1\lang11\f0\fs22 simple\par}

diff --git a/sw/qa/extras/rtfimport/rtfimport.cxx b/sw/qa/extras/rtfimport/rtfimport.cxx
index 535687fb07b8..0a57d66337fe 100644
--- a/sw/qa/extras/rtfimport/rtfimport.cxx
+++ b/sw/qa/extras/rtfimport/rtfimport.cxx
@@ -2272,6 +2272,24 @@ DECLARE_RTFIMPORT_TEST(testTdf91074, "tdf91074.rtf")
     CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(COL_LIGHTRED), getProperty<table::BorderLine2>(xShape, "TopBorder").Color);
 }
 
+DECLARE_RTFIMPORT_TEST(testTdf90260Nopar, "hello.rtf")
+{
+    uno::Reference<text::XTextDocument> xTextDocument(mxComponent, uno::UNO_QUERY);
+    uno::Reference<text::XTextRange> xText(xTextDocument->getText(), uno::UNO_QUERY);
+    uno::Reference<text::XTextRange> xEnd = xText->getEnd();
+    paste("tdf90260-nopar.rtf", xEnd);
+    CPPUNIT_ASSERT_EQUAL(1, getParagraphs());
+}
+
+DECLARE_RTFIMPORT_TEST(testTdf90260Par, "hello.rtf")
+{
+    uno::Reference<text::XTextDocument> xTextDocument(mxComponent, uno::UNO_QUERY);
+    uno::Reference<text::XTextRange> xText(xTextDocument->getText(), uno::UNO_QUERY);
+    uno::Reference<text::XTextRange> xEnd = xText->getEnd();
+    paste("tdf90260-par.rtf", xEnd);
+    CPPUNIT_ASSERT_EQUAL(2, getParagraphs());
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/filter/rtf/swparrtf.cxx b/sw/source/filter/rtf/swparrtf.cxx
index b144f0b86f2d..51ff6268278b 100644
--- a/sw/source/filter/rtf/swparrtf.cxx
+++ b/sw/source/filter/rtf/swparrtf.cxx
@@ -56,7 +56,7 @@ sal_uLong SwRTFReader::Read( SwDoc &rDoc, const OUString& /*rBaseURL*/, SwPaM& r
     // the end position.
     const uno::Reference<text::XTextRange> xInsertPosition =
         SwXTextRange::CreateXTextRange(rDoc, *rPam.GetPoint(), 0);
-    SwNodeIndex *pSttNdIdx = new SwNodeIndex(rDoc.GetNodes());
+    std::shared_ptr<SwNodeIndex> pSttNdIdx(new SwNodeIndex(rDoc.GetNodes()));
     const SwPosition* pPos = rPam.GetPoint();
 
     // Step 2: Split once and remember the node that has been split.
@@ -65,6 +65,8 @@ sal_uLong SwRTFReader::Read( SwDoc &rDoc, const OUString& /*rBaseURL*/, SwPaM& r
 
     // Step 3: Split again.
     rDoc.getIDocumentContentOperations().SplitNode( *pPos, false );
+    std::shared_ptr<SwNodeIndex> pSttNdIdx2(new SwNodeIndex(rDoc.GetNodes()));
+    *pSttNdIdx2 = pPos->nNode.GetIndex();
 
     // Step 4: Insert all content into the new node
     rPam.Move( fnMoveBackward );
@@ -135,6 +137,27 @@ sal_uLong SwRTFReader::Read( SwDoc &rDoc, const OUString& /*rBaseURL*/, SwPaM& r
         }
     }
 
+    if (pSttNdIdx2->GetIndex())
+    {
+        // If we are in insert mode, join the split node that is after
+        // the new content with the last new node. Or in other words:
+        // Revert the second split node.
+        SwTxtNode* pTxtNode = pSttNdIdx2->GetNode().GetTxtNode();
+        SwNodeIndex aPrevIdx(*pSttNdIdx2);
+        if (pTxtNode && pTxtNode->CanJoinPrev(&aPrevIdx) && pSttNdIdx2->GetIndex() - 1 == aPrevIdx.GetIndex())
+        {
+            // If the last new node isn't empty, convert  the node's text
+            // attributes into hints. Otherwise, set the new node's
+            // paragraph style at the next (empty) node.
+            SwTxtNode* pDelNd = aPrevIdx.GetNode().GetTxtNode();
+            if (pTxtNode->GetTxt().getLength())
+                pDelNd->FmtToTxtAttr(pTxtNode);
+            else
+                pTxtNode->ChgFmtColl(pDelNd->GetTxtColl());
+            pTxtNode->JoinPrev();
+        }
+    }
+
     return ret;
 }
 
diff --git a/writerfilter/inc/rtftok/RTFDocument.hxx b/writerfilter/inc/rtftok/RTFDocument.hxx
index 7f5391cdf080..470760ac22b1 100644
--- a/writerfilter/inc/rtftok/RTFDocument.hxx
+++ b/writerfilter/inc/rtftok/RTFDocument.hxx
@@ -43,7 +43,8 @@ public:
                    css::uno::Reference<css::io::XInputStream> const& xInputStream,
                    css::uno::Reference<css::lang::XComponent> const& xDstDoc,
                    css::uno::Reference<css::frame::XFrame> const& xFrame,
-                   css::uno::Reference<css::task::XStatusIndicator> const& xStatusIndicator);
+                   css::uno::Reference<css::task::XStatusIndicator> const& xStatusIndicator,
+                   bool bIsNewDoc);
 };
 } // namespace rtftok
 } // namespace writerfilter
diff --git a/writerfilter/source/dmapper/DomainMapper.cxx b/writerfilter/source/dmapper/DomainMapper.cxx
index e1c595454f33..612b208a4031 100644
--- a/writerfilter/source/dmapper/DomainMapper.cxx
+++ b/writerfilter/source/dmapper/DomainMapper.cxx
@@ -2638,7 +2638,8 @@ void DomainMapper::lcl_endSectionGroup()
         m_pImpl->ExecuteFrameConversion();
         // First paragraph in a footnote doesn't count: that would create
         // additional paragraphs before and after the real footnote content.
-        if(m_pImpl->GetIsFirstParagraphInSection() && !m_pImpl->IsInFootOrEndnote())
+        // Also, when pasting, it's fine to not have any paragraph inside the document at all.
+        if (m_pImpl->GetIsFirstParagraphInSection() && !m_pImpl->IsInFootOrEndnote() && m_pImpl->IsNewDoc())
         {
             // This section has no paragraph at all (e.g. they are all actually in a frame).
             // If this section has a page break, there would be nothing to apply to the page
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 040e228b8cda..0f404af394d8 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -241,7 +241,9 @@ DomainMapper_Impl::DomainMapper_Impl(
 DomainMapper_Impl::~DomainMapper_Impl()
 {
     ChainTextFrames();
-    RemoveLastParagraph( );
+    // Don't remove last paragraph when pasting, sw expects that empty paragraph.
+    if (m_bIsNewDoc)
+        RemoveLastParagraph();
     getTableManager( ).endLevel();
     popTableManager( );
 }
diff --git a/writerfilter/source/filter/RtfFilter.cxx b/writerfilter/source/filter/RtfFilter.cxx
index 62438180085e..c5d088f9dce0 100644
--- a/writerfilter/source/filter/RtfFilter.cxx
+++ b/writerfilter/source/filter/RtfFilter.cxx
@@ -115,7 +115,7 @@ sal_Bool RtfFilter::filter(const uno::Sequence< beans::PropertyValue >& aDescrip
         writerfilter::Stream::Pointer_t pStream(
             new writerfilter::dmapper::DomainMapper(m_xContext, xInputStream, m_xDstDoc, bRepairStorage, writerfilter::dmapper::DOCUMENT_RTF, xInsertTextRange, bIsNewDoc));
         writerfilter::rtftok::RTFDocument::Pointer_t const pDocument(
-            writerfilter::rtftok::RTFDocumentFactory::createDocument(m_xContext, xInputStream, m_xDstDoc, xFrame, xStatusIndicator));
+            writerfilter::rtftok::RTFDocumentFactory::createDocument(m_xContext, xInputStream, m_xDstDoc, xFrame, xStatusIndicator, bIsNewDoc));
         pDocument->resolve(*pStream);
         bResult = true;
 #ifdef DEBUG_WRITERFILTER
diff --git a/writerfilter/source/rtftok/rtfdocumentfactory.cxx b/writerfilter/source/rtftok/rtfdocumentfactory.cxx
index 63cf33f42aab..113dc31c51ce 100644
--- a/writerfilter/source/rtftok/rtfdocumentfactory.cxx
+++ b/writerfilter/source/rtftok/rtfdocumentfactory.cxx
@@ -18,9 +18,10 @@ RTFDocument::Pointer_t RTFDocumentFactory::createDocument(css::uno::Reference< c
         css::uno::Reference< css::io::XInputStream > const& xInputStream,
         css::uno::Reference< css::lang::XComponent > const& xDstDoc,
         css::uno::Reference< css::frame::XFrame > const& xFrame,
-        css::uno::Reference< css::task::XStatusIndicator > const& xStatusIndicator)
+        css::uno::Reference< css::task::XStatusIndicator > const& xStatusIndicator,
+        bool bIsNewDoc)
 {
-    return RTFDocument::Pointer_t(new RTFDocumentImpl(xContext, xInputStream, xDstDoc, xFrame, xStatusIndicator));
+    return RTFDocument::Pointer_t(new RTFDocumentImpl(xContext, xInputStream, xDstDoc, xFrame, xStatusIndicator, bIsNewDoc));
 }
 
 } // namespace rtftok
diff --git a/writerfilter/source/rtftok/rtfdocumentimpl.cxx b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
index b039ede90d55..0941d8505dcb 100644
--- a/writerfilter/source/rtftok/rtfdocumentimpl.cxx
+++ b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
@@ -212,7 +212,8 @@ RTFDocumentImpl::RTFDocumentImpl(uno::Reference<uno::XComponentContext> const& x
                                  uno::Reference<io::XInputStream> const& xInputStream,
                                  uno::Reference<lang::XComponent> const& xDstDoc,
                                  uno::Reference<frame::XFrame> const& xFrame,
-                                 uno::Reference<task::XStatusIndicator> const& xStatusIndicator)
+                                 uno::Reference<task::XStatusIndicator> const& xStatusIndicator,
+                                 bool bIsNewDoc)
     : m_xContext(xContext),
       m_xInputStream(xInputStream),
       m_xDstDoc(xDstDoc),
@@ -272,7 +273,8 @@ RTFDocumentImpl::RTFDocumentImpl(uno::Reference<uno::XComponentContext> const& x
       m_bHadPicture(false),
       m_bHadSect(false),
       m_nCellxMax(0),
-      m_nListPictureId(0)
+      m_nListPictureId(0),
+      m_bIsNewDoc(bIsNewDoc)
 {
     OSL_ASSERT(xInputStream.is());
     m_pInStream.reset(utl::UcbStreamHelper::CreateStream(xInputStream, true));
@@ -343,7 +345,7 @@ void RTFDocumentImpl::resolveSubstream(sal_Size nPos, Id nId, OUString& rIgnoreF
 {
     sal_Size nCurrent = Strm().Tell();
     // Seek to header position, parse, then seek back.
-    RTFDocumentImpl::Pointer_t pImpl(new RTFDocumentImpl(m_xContext, m_xInputStream, m_xDstDoc, m_xFrame, m_xStatusIndicator));
+    RTFDocumentImpl::Pointer_t pImpl(new RTFDocumentImpl(m_xContext, m_xInputStream, m_xDstDoc, m_xFrame, m_xStatusIndicator, m_bIsNewDoc));
     pImpl->setSuperstream(this);
     pImpl->setStreamType(nId);
     pImpl->setIgnoreFirst(rIgnoreFirst);
@@ -572,7 +574,8 @@ void RTFDocumentImpl::sectBreak(bool bFinal = false)
     bool bContinuous = pBreak.get() && pBreak->getInt() == static_cast<sal_Int32>(NS_ooxml::LN_Value_ST_SectionMark_continuous);
     // If there is no paragraph in this section, then insert a dummy one, as required by Writer,
     // unless this is the end of the doc, we had nothing since the last section break and this is not a continuous one.
-    if (m_bNeedPar && !(bFinal && !m_bNeedSect && !bContinuous) && !isSubstream())
+    // Also, when pasting, it's fine to not have any paragraph inside the document at all.
+    if (m_bNeedPar && !(bFinal && !m_bNeedSect && !bContinuous) && !isSubstream() && m_bIsNewDoc)
         dispatchSymbol(RTF_PAR);
     // It's allowed to not have a non-table paragraph at the end of an RTF doc, add it now if required.
     if (m_bNeedFinalPar && bFinal)
@@ -5831,7 +5834,7 @@ int RTFDocumentImpl::popState()
     {
         // \par means an empty paragraph at the end of footnotes/endnotes, but
         // not in case of other substreams, like headers.
-        if (m_bNeedCr && !(m_nStreamType == NS_ooxml::LN_footnote || m_nStreamType == NS_ooxml::LN_endnote))
+        if (m_bNeedCr && !(m_nStreamType == NS_ooxml::LN_footnote || m_nStreamType == NS_ooxml::LN_endnote) && m_bIsNewDoc)
             dispatchSymbol(RTF_PAR);
         if (m_bNeedSect) // may be set by dispatchSymbol above!
             sectBreak(true);
diff --git a/writerfilter/source/rtftok/rtfdocumentimpl.hxx b/writerfilter/source/rtftok/rtfdocumentimpl.hxx
index 85af01fdb90d..6500eaa2728b 100644
--- a/writerfilter/source/rtftok/rtfdocumentimpl.hxx
+++ b/writerfilter/source/rtftok/rtfdocumentimpl.hxx
@@ -329,7 +329,8 @@ public:
                     css::uno::Reference<css::io::XInputStream> const& xInputStream,
                     css::uno::Reference<css::lang::XComponent> const& xDstDoc,
                     css::uno::Reference<css::frame::XFrame> const& xFrame,
-                    css::uno::Reference<css::task::XStatusIndicator> const& xStatusIndicator);
+                    css::uno::Reference<css::task::XStatusIndicator> const& xStatusIndicator,
+                    bool bIsNewDoc);
     virtual ~RTFDocumentImpl();
 
     // RTFDocument
@@ -587,6 +588,9 @@ private:
     int m_nCellxMax;
     /// ID of the next \listlevel picture.
     int m_nListPictureId;
+
+    /// New document means not pasting into an existing one.
+    bool m_bIsNewDoc;
 };
 } // namespace rtftok
 } // namespace writerfilter