Re: [Libreoffice] [REVIEW] fdo#43965 fix RTF import of page break symbol

Hi Michael,

On Mon, Jan 30, 2012 at 10:07:50PM +0100, Michael Stahl <mstahl@redhat.com> wrote:
> > See
> > http://cgit.freedesktop.org/libreoffice/core/commit/?id=2aa3d43a3746c797391afb1a08c024d512976006
>
> seems that with this change not only the new LN_PFPageBreakBefore from
> \page is reset when a \par occurs, but also the LN_PFPageBreakBefore
> inserted from \pagebb.
> i have no idea if that is good or bad, but your comment says "since it's
> a symbol", which is only true for \page, not \pagebb; is it intentional?

Thanks for the catch, that's indeed not intentional and it is
definitely bad. Here is a followup commit:

http://cgit.freedesktop.org/libreoffice/core/commit/?id=d1f3018b0e6d0889febcf09a6c95a2ec354137c0

I'm attaching a patch combining the two, to be pushed to -3-5.

Miklos

From cf01ca6eb8214fcfa64fa37dfe6592eb0a4920f9 Mon Sep 17 00:00:00 2001
From: Miklos Vajna <vmiklos@frugalware.org>
Date: Mon, 30 Jan 2012 21:29:57 +0100
Subject: [PATCH] fdo#43965 fix RTF import of page break symbol

This was unnoticed so far as both Writer and Word output \pagebb instead
of \page when exporting page breaks.

(cherry-picked from commits 2aa3d43a3746c797391afb1a08c024d512976006 and
d1f3018b0e6d0889febcf09a6c95a2ec354137c0)
---
 writerfilter/source/rtftok/rtfdocumentimpl.cxx |   17 ++++++++++++++++-
 writerfilter/source/rtftok/rtfdocumentimpl.hxx |    2 ++
 2 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/writerfilter/source/rtftok/rtfdocumentimpl.cxx 
b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
index 18643af..e23a25f 100644
--- a/writerfilter/source/rtftok/rtfdocumentimpl.cxx
+++ b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
@@ -294,6 +294,7 @@ RTFDocumentImpl::RTFDocumentImpl(uno::Reference<uno::XComponentContext> const& x
     m_bFormField(false),
     m_bWasInFrame(false),
     m_bIsInFrame(false),
+    m_bHasPage(false),
     m_aUnicodeBuffer()
 {
     OSL_ASSERT(xInputStream.is());
@@ -1320,6 +1321,12 @@ int RTFDocumentImpl::dispatchSymbol(RTFKeyword nKeyword)
                 m_bWasInFrame = inFrame();
                 if (!m_bWasInFrame)
                     m_bNeedPar = false;
+                if (m_bHasPage)
+                {
+                    // this has to be reset even without a pard, since it's a symbol in RTF terms
+                    m_aStates.top().aParagraphSprms.erase(NS_sprm::LN_PFPageBreakBefore);
+                    m_bHasPage = false;
+                }
             }
             break;
         case RTF_SECT:
@@ -1440,6 +1447,14 @@ int RTFDocumentImpl::dispatchSymbol(RTFKeyword nKeyword)
         case RTF_CHFTN:
             // Nothing to do, dmapper assumes this is the default.
             break;
+        case RTF_PAGE:
+            {
+                RTFValue::Pointer_t pValue(new RTFValue(1));
+                dispatchSymbol(RTF_PAR);
+                
m_aStates.top().aParagraphSprms->push_back(make_pair(NS_sprm::LN_PFPageBreakBefore, pValue));
+                m_bHasPage = true;
+            }
+            break;
         default:
 #if OSL_DEBUG_LEVEL > 1
             OSL_TRACE("%s: TODO handle symbol '%s'", OSL_THIS_FUNC, lcl_RtfToString(nKeyword));
@@ -1642,7 +1657,7 @@ int RTFDocumentImpl::dispatchFlag(RTFKeyword nKeyword)
         case RTF_KEEP: if (m_pCurrentBuffer != &m_aTableBuffer) nParam = NS_sprm::LN_PFKeep; break;
         case RTF_KEEPN: if (m_pCurrentBuffer != &m_aTableBuffer) nParam = 
NS_sprm::LN_PFKeepFollow; break;
         case RTF_INTBL: m_pCurrentBuffer = &m_aTableBuffer; nParam = NS_sprm::LN_PFInTable; break;
-        case RTF_PAGEBB: nParam = NS_sprm::LN_PFPageBreakBefore; break;
+        case RTF_PAGEBB: nParam = NS_sprm::LN_PFPageBreakBefore; m_bHasPage = false; break;
         default: break;
     }
     if (nParam >= 0)
diff --git a/writerfilter/source/rtftok/rtfdocumentimpl.hxx 
b/writerfilter/source/rtftok/rtfdocumentimpl.hxx
index 178cdb3..12d414c 100644
--- a/writerfilter/source/rtftok/rtfdocumentimpl.hxx
+++ b/writerfilter/source/rtftok/rtfdocumentimpl.hxx
@@ -431,6 +431,8 @@ namespace writerfilter {
                 bool m_bWasInFrame;
                 /// If a frame start token is already sent to dmapper (nesting them is not OK).
                 bool m_bIsInFrame;
+                /// If we should reset the page break property when we start the next paragraph.
+                bool m_bHasPage;
                 // Unicode characters are collected here so we don't have to send them one by one.
                 rtl::OUStringBuffer m_aUnicodeBuffer;
         };
-- 
1.7.8.1