fdo#57708 fix fake page break problem during RTF import

The core of this change is: so far the continous section break at the
end of the document was sent as a normal section break. This was
introduced in commit 892d33c8d5033b4f8f7889bf91d257f55adf0e1f, probably
as a workaround (sadly it's not documented and I no longer remember).
Don't do this, since it causes additional page breaks during import.
Instead, fix properly whatever was broken after getting rid of this
workaround.

(cherry picked from commits b836bf389d1150c9cafbb0aefa641af2316e536c and
3f13a8255be93cd3b697f42d691b35418eb87114)

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

Change-Id: I28c372d539c150fe21ff9db31209f9935a5e9063
Reviewed-on: https://gerrit.libreoffice.org/1214
Tested-by: Caolán McNamara <caolanm@redhat.com>
Reviewed-by: Caolán McNamara <caolanm@redhat.com>

3 files changed, 51 insertions, 35 deletions

diff --git a/writerfilter/source/dmapper/PropertyMap.cxx b/writerfilter/source/dmapper/PropertyMap.cxx
index 60de3eb4b3a1..78dfe0518c23 100644
--- a/writerfilter/source/dmapper/PropertyMap.cxx
+++ b/writerfilter/source/dmapper/PropertyMap.cxx
@@ -800,13 +800,13 @@ void SectionPropertyMap::PrepareHeaderFooterProperties( bool bFirstPage )
 uno::Reference<beans::XPropertySet> lcl_GetRangeProperties(bool bIsFirstSection, DomainMapper_Impl& rDM_Impl, uno::Reference<text::XTextRange> xStartingRange)
 {
     uno::Reference< beans::XPropertySet > xRangeProperties;
-    if (bIsFirstSection)
+    if (bIsFirstSection && rDM_Impl.GetBodyText().is())
     {
         uno::Reference<container::XEnumerationAccess> xEnumAccess(rDM_Impl.GetBodyText(), uno::UNO_QUERY_THROW);
         uno::Reference<container::XEnumeration> xEnum = xEnumAccess->createEnumeration();
         xRangeProperties = uno::Reference<beans::XPropertySet>(xEnum->nextElement(), uno::UNO_QUERY_THROW);
     }
-    else
+    else if (xStartingRange.is())
         xRangeProperties = uno::Reference<beans::XPropertySet>(xStartingRange, uno::UNO_QUERY_THROW);
     return xRangeProperties;
 }
@@ -851,7 +851,8 @@ void SectionPropertyMap::CloseSectionGroup( DomainMapper_Impl& rDM_Impl )
         if( m_nColumnCount > 0 && xSection.is())
             ApplyColumnProperties( xSection );
         uno::Reference<beans::XPropertySet> xRangeProperties(lcl_GetRangeProperties(m_bIsFirstSection, rDM_Impl, m_xStartingRange));
-        xRangeProperties->setPropertyValue(rPropNameSupplier.GetName(PROP_PAGE_DESC_NAME), uno::makeAny(m_bTitlePage ? m_sFirstPageStyleName : m_sFollowPageStyleName));
+        if (xRangeProperties.is())
+            xRangeProperties->setPropertyValue(rPropNameSupplier.GetName(PROP_PAGE_DESC_NAME), uno::makeAny(m_bTitlePage ? m_sFirstPageStyleName : m_sFollowPageStyleName));
     }
     // If the section is of type "New column" (0x01), then simply insert a column break.
     // But only if there actually are columns on the page, otherwise a column break
@@ -1010,8 +1011,9 @@ void SectionPropertyMap::CloseSectionGroup( DomainMapper_Impl& rDM_Impl )
                 uno::Reference<beans::XPropertySet> xRangeProperties(lcl_GetRangeProperties(m_bIsFirstSection, rDM_Impl, m_xStartingRange));
             /* break type
             0 - No break 1 - New Colunn 2 - New page 3 - Even page 4 - odd page */
-                xRangeProperties->setPropertyValue(rPropNameSupplier.GetName( PROP_PAGE_DESC_NAME ),
-                    uno::makeAny( m_bTitlePage ? m_sFirstPageStyleName : m_sFollowPageStyleName ));
+                if (xRangeProperties.is())
+                    xRangeProperties->setPropertyValue(rPropNameSupplier.GetName( PROP_PAGE_DESC_NAME ),
+                            uno::makeAny( m_bTitlePage ? m_sFirstPageStyleName : m_sFollowPageStyleName ));
                 // handle page breaks with odd/even page numbering
                 style::PageStyleLayout nPageStyleLayout(style::PageStyleLayout_ALL);
                 if (m_nBreakType == 3)
diff --git a/writerfilter/source/rtftok/rtfdocumentimpl.cxx b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
index cb54d38208ab..3ebb8da59f0c 100644
--- a/writerfilter/source/rtftok/rtfdocumentimpl.cxx
+++ b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
@@ -278,7 +278,9 @@ RTFDocumentImpl::RTFDocumentImpl(uno::Reference<uno::XComponentContext> const& x
     m_bIsInFrame(false),
     m_aUnicodeBuffer(),
     m_aHexBuffer(),
-    m_bDeferredContSectBreak(false)
+    m_bIgnoreNextContSectBreak(false),
+    m_bNeedSect(true),
+    m_bWasInFrame(false)
 {
     OSL_ASSERT(xInputStream.is());
     m_pInStream.reset(utl::UcbStreamHelper::CreateStream(xInputStream, sal_True));
@@ -395,6 +397,11 @@ void RTFDocumentImpl::setNeedPar(bool bNeedPar)
     m_bNeedPar = bNeedPar;
 }
 
+void RTFDocumentImpl::setNeedSect(bool bNeedSect)
+{
+    m_bNeedSect = bNeedSect;
+}
+
 void RTFDocumentImpl::checkNeedPap()
 {
     if (m_bNeedPap)
@@ -479,6 +486,8 @@ void RTFDocumentImpl::parBreak()
 
 void RTFDocumentImpl::sectBreak(bool bFinal = false)
 {
+    SAL_INFO("writerfilter", OSL_THIS_FUNC << ": final? " << bFinal << ", needed? " << m_bNeedSect);
+    bool bNeedSect = m_bNeedSect;
     // If there is no paragraph in this section, then insert a dummy one, as required by Writer
     if (m_bNeedPar)
         dispatchSymbol(RTF_PAR);
@@ -495,10 +504,15 @@ void RTFDocumentImpl::sectBreak(bool bFinal = false)
         resolveSubstream(aPair.second, aPair.first);
     }
 
-    RTFValue::Pointer_t pBreak = m_aStates.top().aSectionSprms.find(NS_sprm::LN_SBkc);
-    // In case the last section is a continous one, we don't need to output a section break.
-    if (bFinal && pBreak.get() && !pBreak->getInt())
-        m_aStates.top().aSectionSprms.erase(NS_sprm::LN_SBkc);
+    // Normally a section break at the end of the doc is necessary. Unless the
+    // last control word in the document is a section break itself.
+    if (!bNeedSect)
+    {
+        RTFValue::Pointer_t pBreak = m_aStates.top().aSectionSprms.find(NS_sprm::LN_SBkc);
+        // In case the last section is a continous one, we don't need to output a section break.
+        if (bFinal && pBreak.get() && !pBreak->getInt())
+            m_aStates.top().aSectionSprms.erase(NS_sprm::LN_SBkc);
+    }
 
     // Section properties are a paragraph sprm.
     RTFValue::Pointer_t pValue(new RTFValue(m_aStates.top().aSectionAttributes, m_aStates.top().aSectionSprms));
@@ -519,6 +533,7 @@ void RTFDocumentImpl::sectBreak(bool bFinal = false)
         Mapper().startParagraphGroup();
     }
     m_bNeedPar = true;
+    m_bNeedSect = false;
 }
 
 void RTFDocumentImpl::seek(sal_uInt32 nPos)
@@ -1092,7 +1107,7 @@ void RTFDocumentImpl::replayBuffer(RTFBuffer_t& rBuffer)
 int RTFDocumentImpl::dispatchDestination(RTFKeyword nKeyword)
 {
     checkUnicode();
-    checkDeferredContSectBreak();
+    setNeedSect();
     RTFSkipDestination aSkip(*this);
     switch (nKeyword)
     {
@@ -1422,7 +1437,7 @@ int RTFDocumentImpl::dispatchSymbol(RTFKeyword nKeyword)
 {
     if (nKeyword != RTF_HEXCHAR)
         checkUnicode();
-    checkDeferredContSectBreak();
+    setNeedSect();
     RTFSkipDestination aSkip(*this);
     sal_uInt8 cCh = 0;
 
@@ -1480,13 +1495,8 @@ int RTFDocumentImpl::dispatchSymbol(RTFKeyword nKeyword)
             break;
         case RTF_SECT:
             {
-                RTFValue::Pointer_t pBreak = m_aStates.top().aSectionSprms.find(NS_sprm::LN_SBkc);
-                if (pBreak.get() && !pBreak->getInt())
-                {
-                    // This is a continous section break, don't send it yet.
-                    // It's possible that we'll have nothing after this token, and then we should ignore it.
-                    m_bDeferredContSectBreak = true;
-                }
+                if (m_bIgnoreNextContSectBreak)
+                    m_bIgnoreNextContSectBreak = false;
                 else
                     sectBreak();
             }
@@ -1630,9 +1640,17 @@ int RTFDocumentImpl::dispatchSymbol(RTFKeyword nKeyword)
                 RTFValue::Pointer_t pBreak = m_aStates.top().aSectionSprms.find(NS_sprm::LN_SBkc);
                 if (pBreak.get() && !pBreak->getInt())
                 {
+                    if (m_bWasInFrame)
+                    {
+                        dispatchSymbol(RTF_PAR);
+                        m_bWasInFrame = false;
+                    }
                     dispatchFlag(RTF_SBKPAGE);
                     sectBreak();
                     dispatchFlag(RTF_SBKNONE);
+                    if (m_bNeedPar)
+                        dispatchSymbol(RTF_PAR);
+                    m_bIgnoreNextContSectBreak = true;
                 }
                 else
                 {
@@ -1663,7 +1681,7 @@ int RTFDocumentImpl::dispatchSymbol(RTFKeyword nKeyword)
 int RTFDocumentImpl::dispatchFlag(RTFKeyword nKeyword)
 {
     checkUnicode();
-    checkDeferredContSectBreak();
+    setNeedSect();
     RTFSkipDestination aSkip(*this);
     int nParam = -1;
     int nSprm = -1;
@@ -2282,7 +2300,7 @@ int RTFDocumentImpl::dispatchFlag(RTFKeyword nKeyword)
 int RTFDocumentImpl::dispatchValue(RTFKeyword nKeyword, int nParam)
 {
     checkUnicode(nKeyword != RTF_U, true);
-    checkDeferredContSectBreak();
+    setNeedSect();
     RTFSkipDestination aSkip(*this);
     int nSprm = 0;
     RTFValue::Pointer_t pIntValue(new RTFValue(nParam));
@@ -3002,7 +3020,7 @@ int RTFDocumentImpl::dispatchValue(RTFKeyword nKeyword, int nParam)
 int RTFDocumentImpl::dispatchToggle(RTFKeyword nKeyword, bool bParam, int nParam)
 {
     checkUnicode();
-    checkDeferredContSectBreak();
+    setNeedSect();
     RTFSkipDestination aSkip(*this);
     int nSprm = -1;
     RTFValue::Pointer_t pBoolValue(new RTFValue(!bParam || nParam != 0));
@@ -3191,6 +3209,7 @@ int RTFDocumentImpl::popState()
     bool bFaltEnd = false;
     bool bPopFrame = false;
     RTFParserState aState(m_aStates.top());
+    m_bWasInFrame = aState.aFrame.inFrame();
 
     if (m_aStates.top().nDestinationState == DESTINATION_FONTTABLE)
     {
@@ -3538,7 +3557,6 @@ int RTFDocumentImpl::popState()
     {
         if (m_bNeedCr)
             dispatchSymbol(RTF_PAR);
-        m_bDeferredContSectBreak = false;
         sectBreak(true);
     }
 
@@ -3758,15 +3776,6 @@ void RTFDocumentImpl::checkUnicode(bool bUnicode, bool bHex)
     }
 }
 
-void RTFDocumentImpl::checkDeferredContSectBreak()
-{
-    if (m_bDeferredContSectBreak)
-    {
-        m_bDeferredContSectBreak = false;
-        sectBreak();
-    }
-}
-
 RTFParserState::RTFParserState(RTFDocumentImpl *pDocumentImpl)
     : m_pDocumentImpl(pDocumentImpl),
     nInternalState(INTERNAL_NORMAL),
diff --git a/writerfilter/source/rtftok/rtfdocumentimpl.hxx b/writerfilter/source/rtftok/rtfdocumentimpl.hxx
index 6c0608fb7ca7..d36a5ec37d34 100644
--- a/writerfilter/source/rtftok/rtfdocumentimpl.hxx
+++ b/writerfilter/source/rtftok/rtfdocumentimpl.hxx
@@ -421,8 +421,8 @@ namespace writerfilter {
                 void replayBuffer(RTFBuffer_t& rBuffer);
                 /// If we have some unicode or hex characters to send.
                 void checkUnicode(bool bUnicode = true, bool bHex = true);
-                /// If we have a pending continous section break.
-                void checkDeferredContSectBreak();
+                /// If we need a final section break at the end of the document.
+                void setNeedSect(bool bNeedSect = true);
 
                 uno::Reference<uno::XComponentContext> const& m_xContext;
                 uno::Reference<io::XInputStream> const& m_xInputStream;
@@ -514,7 +514,12 @@ namespace writerfilter {
                 rtl::OUStringBuffer m_aUnicodeBuffer;
                 /// Same for hex characters.
                 rtl::OStringBuffer m_aHexBuffer;
-                bool m_bDeferredContSectBreak;
+                /// If the next continous section break should be ignored.
+                bool m_bIgnoreNextContSectBreak;
+                /// If a section break is needed before the end of the doc (false right after a section break).
+                bool m_bNeedSect;
+                /// If aFrame.inFrame() was true in the previous state.
+                bool m_bWasInFrame;
         };
     } // namespace rtftok
 } // namespace writerfilter