tdf#158855: Make sure to not add extra paragraph after a table in a section

Simplify the condition for last paragraph removal in lcl_utext, and clean up
the complications piled after commit 2e8aad6d45c53d554ccaf26de998ede708cfc289
(fdo#39056 fdo#75431 Section Properties if section starts with table,
2014-04-24). This makes sure that the trailing extra paragraph gets properly
removed, even when there was a dummy paragraph added for a table in section.

Because of the removed check, import used to avoid the removal of the extra
paragraph immediately following a table (when the table needed to be the last
element in the section); hence, in DomainMapper_Impl::appendTextSectionAfter,
a last section's table was followed by two paragraphs (one that is expected
to stay after the created section, and one before that, that was not removed
after the table). xCursor had ininitally both paragraphs in the selection
after the forward goto* calls; and then, gotoLeft excluded only the last
paragraph from the selection. After the change, in such a case, there is only
one paragraph in the end: the one that should stay after the created section.
But calling gotoLeft then would have to put the end of selection to a text
node inside the table; since the selection that starts before the table can't
end inside the table, the call resulted in the whole table excluded from the
selection. This requires to add a paragraph here, to allow the previous
behavior of cursor; and then the last paragraph in section is destroyed. The
code makes sure that the trailing paragraph kept after the creation of the
section has all the properties applied to the old last paragraph.

testFdo53985 used to check for total 9 paragraphs in the document; while
3 of those were introduced exactly by the bug fixed here; they are absent
in Word. The test is fixed; its logic is unchanged, and it still correctly
tests that a round-trip doesn't increase number of paragraphs.

Change-Id: Ibf6349ba7906dac185c759eb3b58c1849ee5064a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161275
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

3 files changed, 102 insertions, 51 deletions

diff --git a/writerfilter/source/dmapper/DomainMapper.cxx b/writerfilter/source/dmapper/DomainMapper.cxx
index 0c9720071068..c6be4d9f8c98 100644
--- a/writerfilter/source/dmapper/DomainMapper.cxx
+++ b/writerfilter/source/dmapper/DomainMapper.cxx
@@ -3897,10 +3897,7 @@ void DomainMapper::lcl_text(const sal_uInt8 * data_, size_t len)
                 case 0x0c: //page break
                     // page breaks aren't supported in footnotes and endnotes
                     if (!m_pImpl->IsInFootOrEndnote())
-                    {
                         m_pImpl->deferBreak(PAGE_BREAK);
-                        m_pImpl->SetIsDummyParaAddedForTableInSectionPage(false);
-                    }
                     return;
                 case 0x0e: //column break
                     m_pImpl->deferBreak(COLUMN_BREAK);
@@ -4403,7 +4400,6 @@ void DomainMapper::lcl_utext(const sal_Unicode *const data_, size_t len)
                             && !bIsColumnBreak
                             && !m_pImpl->GetIsLastSectionGroup() // testContSectionPageBreak
                             && !m_pImpl->GetParaHadField()
-                            && (!m_pImpl->GetIsDummyParaAddedForTableInSectionPage())
                             && !m_pImpl->GetIsPreviousParagraphFramed()
                             && !m_pImpl->HasTopAnchoredObjects()
                             && !m_pImpl->IsParaWithInlineObject());
@@ -4424,15 +4420,6 @@ void DomainMapper::lcl_utext(const sal_Unicode *const data_, size_t len)
             if (bRemove)
                 m_pImpl->RemoveLastParagraph();
 
-            // When the table is closed and the section's initial dummy paragraph has been processed
-            // then any following sectPr paragraph in the section must be eligible for removal.
-            if (!bRemove && m_pImpl->GetIsDummyParaAddedForTableInSectionPage() && !IsInTable()
-                && !m_pImpl->GetFootnoteContext() && !m_pImpl->IsInComments() && !IsInHeaderFooter()
-                && !IsInShape())
-            {
-               m_pImpl->SetIsDummyParaAddedForTableInSectionPage(false);
-            }
-
             m_pImpl->SetParaSectpr(false);
         }
         else
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index bc6a3dfdf5a1..e247294a4e52 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -29,6 +29,7 @@
 #include "TagLogger.hxx"
 #include <com/sun/star/uno/XComponentContext.hpp>
 #include <com/sun/star/graphic/XGraphic.hpp>
+#include <com/sun/star/beans/PropertyAttribute.hpp>
 #include <com/sun/star/beans/XPropertyState.hpp>
 #include <com/sun/star/container/XNamed.hpp>
 #include <com/sun/star/document/PrinterIndependentLayout.hpp>
@@ -380,7 +381,6 @@ DomainMapper_Impl::DomainMapper_Impl(
         m_bIsFirstParaInSection( true ),
         m_bIsFirstParaInSectionAfterRedline( true ),
         m_bDummyParaAddedForTableInSection( false ),
-        m_bDummyParaAddedForTableInSectionPage( false ),
         m_bTextFrameInserted(false),
         m_bIsPreviousParagraphFramed( false ),
         m_bIsLastParaInSection( false ),
@@ -979,12 +979,6 @@ void DomainMapper_Impl::SetIsFirstParagraphInShape(bool bIsFirst)
 void DomainMapper_Impl::SetIsDummyParaAddedForTableInSection( bool bIsAdded )
 {
     m_bDummyParaAddedForTableInSection = bIsAdded;
-    m_bDummyParaAddedForTableInSectionPage = bIsAdded;
-}
-
-void DomainMapper_Impl::SetIsDummyParaAddedForTableInSectionPage( bool bIsAdded )
-{
-    m_bDummyParaAddedForTableInSectionPage = bIsAdded;
 }
 
 
@@ -3518,6 +3512,86 @@ void DomainMapper_Impl::adjustLastPara(sal_Int8 nAlign)
     pLastPara->Insert(PROP_PARA_ADJUST, uno::Any(nAlign), true);
 }
 
+static void checkAndAddPropVal(const OUString& prop, const css::uno::Any& val,
+                               std::vector<OUString>& props, std::vector<css::uno::Any>& values)
+{
+    // Avoid well-known reasons for exceptions when setting property values
+    if (!val.hasValue())
+        return;
+    if (prop == "CharAutoStyleName" || prop == "ParaAutoStyleName")
+        return;
+    if (prop == "CharStyleName" || prop == "DropCapCharStyleName")
+        if (OUString val_string; (val >>= val_string) && val_string.isEmpty())
+            return;
+
+    props.push_back(prop);
+    values.push_back(val);
+}
+
+static void copyAllProps(const css::uno::Reference<css::uno::XInterface>& from,
+                         const css::uno::Reference<css::uno::XInterface>& to)
+{
+    css::uno::Reference<css::beans::XPropertySet> xFromProps(from, css::uno::UNO_QUERY_THROW);
+    css::uno::Reference<css::beans::XPropertySetInfo> xFromInfo(xFromProps->getPropertySetInfo(),
+                                                                css::uno::UNO_SET_THROW);
+    css::uno::Sequence<css::beans::Property> rawProps(xFromInfo->getProperties());
+    std::vector<OUString> props;
+    props.reserve(rawProps.getLength());
+    for (const auto& prop : rawProps)
+        if ((prop.Attributes & css::beans::PropertyAttribute::READONLY) == 0)
+            props.push_back(prop.Name);
+    std::vector<css::uno::Any> values;
+    values.reserve(props.size());
+    if (css::uno::Reference<css::beans::XMultiPropertySet> xFromMulti{ xFromProps,
+                                                                       css::uno::UNO_QUERY })
+    {
+        const auto propsSeq = comphelper::containerToSequence(props);
+        const auto valuesSeq = xFromMulti->getPropertyValues(propsSeq);
+        assert(propsSeq.getLength() == valuesSeq.getLength());
+        props.clear();
+        for (size_t i = 0; i < propsSeq.size(); ++i)
+            checkAndAddPropVal(propsSeq[i], valuesSeq[i], props, values);
+    }
+    else
+    {
+        std::vector<OUString> filtered_props;
+        filtered_props.reserve(props.size());
+        for (const auto& prop : props)
+            checkAndAddPropVal(prop, xFromProps->getPropertyValue(prop), filtered_props, values);
+        filtered_props.swap(props);
+    }
+    assert(props.size() == values.size());
+
+    css::uno::Reference<css::beans::XPropertySet> xToProps(to, css::uno::UNO_QUERY_THROW);
+    if (css::uno::Reference<css::beans::XMultiPropertySet> xToMulti{ xToProps,
+                                                                     css::uno::UNO_QUERY })
+    {
+        try
+        {
+            xToMulti->setPropertyValues(comphelper::containerToSequence(props),
+                                        comphelper::containerToSequence(values));
+            return;
+        }
+        catch (css::uno::Exception&)
+        {
+            DBG_UNHANDLED_EXCEPTION("writerfilter.dmapper");
+        }
+        // Fallback to property-by-property iteration
+    }
+
+    for (size_t i = 0; i < props.size(); ++i)
+    {
+        try
+        {
+            xToProps->setPropertyValue(props[i], values[i]);
+        }
+        catch (css::uno::Exception&)
+        {
+            DBG_UNHANDLED_EXCEPTION("writerfilter.dmapper");
+        }
+    }
+}
+
 uno::Reference< beans::XPropertySet > DomainMapper_Impl::appendTextSectionAfter(
                                     uno::Reference< text::XTextRange > const & xBefore )
 {
@@ -3537,40 +3611,33 @@ uno::Reference< beans::XPropertySet > DomainMapper_Impl::appendTextSectionAfter(
                 xCursor->gotoRange( m_aTextAppendStack.top().xInsertPosition, true );
             else
                 xCursor->gotoEnd( true );
-            //the paragraph after this new section is already inserted
-            xCursor->goLeft(1, true);
-            css::uno::Reference<css::text::XTextRange> xTextRange(xCursor, css::uno::UNO_QUERY_THROW);
+            // The paragraph after this new section is already inserted. The previous node may be a
+            // table; then trying to go left would skip the whole table. Split the trailing
+            // paragraph; let the section span over the first of the two resulting paragraphs;
+            // destroy the last section's paragraph afterwards.
+            css::uno::Reference<css::text::XTextRange> xEndPara = xCursor->getEnd();
+            xTextAppend->insertControlCharacter(
+                xEndPara, css::text::ControlCharacter::PARAGRAPH_BREAK, false);
+            css::uno::Reference<css::text::XTextRange> xNewPara = xCursor->getEnd();
+            xCursor->gotoPreviousParagraph(true);
+            xEndPara = xCursor->getEnd();
+            // xEndPara may already have properties (like page break); make sure to apply them
+            // to the newly appended paragraph, which will be kept in the end.
+            copyAllProps(xEndPara, xNewPara);
 
-            if (css::uno::Reference<css::text::XDocumentIndexesSupplier> xIndexSupplier{
-                    GetTextDocument(), css::uno::UNO_QUERY })
+            uno::Reference< text::XTextContent > xSection( m_xTextFactory->createInstance("com.sun.star.text.TextSection"), uno::UNO_QUERY_THROW );
+            xSection->attach(xCursor);
+
+            // Remove the extra paragraph (last inside the section)
+            if (uno::Reference<container::XEnumerationAccess> xEA{ xEndPara, uno::UNO_QUERY })
             {
-                css::uno::Reference<css::text::XTextRangeCompare> xCompare(
-                    xTextAppend, css::uno::UNO_QUERY);
-                const auto xIndexAccess = xIndexSupplier->getDocumentIndexes();
-                for (sal_Int32 i = xIndexAccess->getCount(); i > 0; --i)
+                if (uno::Reference<lang::XComponent> xParagraph{
+                        xEA->createEnumeration()->nextElement(), uno::UNO_QUERY })
                 {
-                    if (css::uno::Reference<css::text::XDocumentIndex> xIndex{
-                            xIndexAccess->getByIndex(i - 1), css::uno::UNO_QUERY })
-                    {
-                        const auto xIndexTextRange = xIndex->getAnchor();
-                        if (xCompare->compareRegionStarts(xTextRange, xIndexTextRange) == 0
-                            && xCompare->compareRegionEnds(xTextRange, xIndexTextRange) == 0)
-                        {
-                            // The boundaries coincide with an index: trying to attach a section
-                            // to the range will insert the section inside the index. goRight will
-                            // extend the range outside of the index, so that created section will
-                            // be around it. Alternatively we could return index section itself
-                            // instead : xRet.set(xIndex, uno::UNO_QUERY) - to set its properties,
-                            // like columns/fill.
-                            xCursor->goRight(1, true);
-                            break;
-                        }
-                    }
+                    xParagraph->dispose();
                 }
             }
 
-            uno::Reference< text::XTextContent > xSection( m_xTextFactory->createInstance("com.sun.star.text.TextSection"), uno::UNO_QUERY_THROW );
-            xSection->attach( xTextRange );
             xRet.set(xSection, uno::UNO_QUERY );
         }
         catch(const uno::Exception&)
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.hxx b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
index f555dd14c048..dc8dbcee3880 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
@@ -589,7 +589,6 @@ private:
     bool                            m_bIsFirstParaInSectionAfterRedline;
     bool                            m_bIsFirstParaInShape = false;
     bool                            m_bDummyParaAddedForTableInSection;
-    bool                            m_bDummyParaAddedForTableInSectionPage;
     bool                            m_bTextFrameInserted;
     bool                            m_bIsPreviousParagraphFramed;
     bool                            m_bIsLastParaInSection;
@@ -720,8 +719,6 @@ public:
     bool GetIsFirstParagraphInShape() const { return m_bIsFirstParaInShape; }
     void SetIsDummyParaAddedForTableInSection( bool bIsAdded );
     bool GetIsDummyParaAddedForTableInSection() const { return m_bDummyParaAddedForTableInSection;}
-    void SetIsDummyParaAddedForTableInSectionPage(bool bIsAdded);
-    bool GetIsDummyParaAddedForTableInSectionPage() const { return m_bDummyParaAddedForTableInSectionPage; }
 
     /// Track if a textframe has been inserted into this section
     void SetIsTextFrameInserted( bool bIsInserted );