sw/qa/extras/rtfexport/rtfexport8.cxx | 14 ++++++++------
writerfilter/source/rtftok/rtfdispatchsymbol.cxx | 9 ++++-----
writerfilter/source/rtftok/rtfdocumentimpl.cxx | 18 ++++++++++++++++--
writerfilter/source/rtftok/rtfdocumentimpl.hxx | 2 ++
4 files changed, 30 insertions(+), 13 deletions(-)
New commits:
commit 57abad5cf990111fd7de011809d4421dc0550193
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Tue Jan 30 17:46:12 2024 +0100
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Wed Jan 31 18:34:44 2024 +0100
tdf#158586 writerfilter: RTF import: fix \page \sect \skbnone
The problem is that \page is actually completely ignored in the bugdoc
testTdf158586_1.
If you delete the \sbknone then there is a page break but it's caused by
\sect; the \page is still ignored.
It is ignored because, first, the \page results in a deferred break in
DomainMapper, then for \sect, the synthetic \par is dispatched and that
moves the break from deferred to the top paragraph properties context,
then sectBreak() calls endParagraphGroup() which just removes the top
paragraph properties context.
Remove the dispatchSymbol(RTFKeyword::PAR) for \sect, instead set a flag
so that RTFDocumentImpl::sectBreak() does it.
Add a new flag m_bParAtEndOfSection so that RTFDocumentImpl::parBreak()
can suppress the startParagraphGroup(), so the deferred break remains
present.
This also fixes testTdf158586_lostFrame.
(regression from commit 15b886f460919ea3dce425a621dc017c2992a96b)
Change-Id: I82a00899a9448069832a0b2f98a96df00da75518
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162770
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
diff --git a/sw/qa/extras/rtfexport/rtfexport8.cxx
b/sw/qa/extras/rtfexport/rtfexport8.cxx
index 67d48a27dcf6..b1c694f7daa4 100644
--- a/sw/qa/extras/rtfexport/rtfexport8.cxx
+++ b/sw/qa/extras/rtfexport/rtfexport8.cxx
@@ -68,12 +68,14 @@ DECLARE_RTFEXPORT_TEST(testTdf158586_0B,
"tdf158586_pageBreak0B.rtf")
DECLARE_RTFEXPORT_TEST(testTdf158586_1, "tdf158586_pageBreak1.rtf")
{
// None of the specified text frame settings initiates a real text frame -
page break not lost
- // CPPUNIT_ASSERT_EQUAL(2, getPages());
- // CPPUNIT_ASSERT_EQUAL(2, getParagraphs());
+ CPPUNIT_ASSERT_EQUAL(2, getPages());
+ CPPUNIT_ASSERT_EQUAL(2, getParagraphs());
// There should be no empty carriage return at the start of the second page
- // const auto& pLayout = parseLayoutDump();
- // assertXPathContent(pLayout, "//page[2]/body/txt"_ostr, "Second page");
+ const auto& pLayout = parseLayoutDump();
+ // on import there is a section on page 2; on reimport there is no section
+ // (probably not an important difference?)
+ assertXPathContent(pLayout, "/root/page[2]/body//txt"_ostr, "Second page");
}
DECLARE_RTFEXPORT_TEST(testTdf158586_lostFrame, "tdf158586_lostFrame.rtf")
@@ -82,9 +84,9 @@ DECLARE_RTFEXPORT_TEST(testTdf158586_lostFrame,
"tdf158586_lostFrame.rtf")
const auto& pLayout = parseLayoutDump();
assertXPath(pLayout, "//anchored"_ostr, 1);
assertXPathContent(pLayout, "//page[1]/body//txt"_ostr, "1st page");
- // assertXPathContent(pLayout, "//page[2]/body//txt"_ostr, "2nd page");
+ assertXPathContent(pLayout, "//page[2]/body//txt"_ostr, "2nd page");
- // CPPUNIT_ASSERT_EQUAL(2, getPages());
+ CPPUNIT_ASSERT_EQUAL(2, getPages());
}
DECLARE_RTFEXPORT_TEST(testTdf158826_extraCR, "tdf158826_extraCR.rtf")
diff --git a/writerfilter/source/rtftok/rtfdispatchsymbol.cxx
b/writerfilter/source/rtftok/rtfdispatchsymbol.cxx
index aa1360f6dc55..9347174b6da2 100644
--- a/writerfilter/source/rtftok/rtfdispatchsymbol.cxx
+++ b/writerfilter/source/rtftok/rtfdispatchsymbol.cxx
@@ -131,9 +131,6 @@ RTFError RTFDocumentImpl::dispatchSymbol(RTFKeyword
nKeyword)
break;
case RTFKeyword::SECT:
{
- if (m_bNeedCr)
- dispatchSymbol(RTFKeyword::PAR);
-
m_bHadSect = true;
if (m_bIgnoreNextContSectBreak)
{
@@ -143,6 +140,10 @@ RTFError RTFDocumentImpl::dispatchSymbol(RTFKeyword
nKeyword)
}
else
{
+ if (m_bNeedCr)
+ { // tdf#158586 don't dispatch \par here, it eats deferred
page breaks
+ setNeedPar(true);
+ }
sectBreak();
if (m_nResetBreakOnSectBreak != RTFKeyword::invalid)
{
@@ -405,8 +406,6 @@ RTFError RTFDocumentImpl::dispatchSymbol(RTFKeyword
nKeyword)
}
sal_uInt8 const sBreak[] = { 0xc };
Mapper().text(sBreak, 1);
- // testFdo81892 don't do another \par break directly; because
of
- // GetSplitPgBreakAndParaMark() it does finishParagraph *twice*
m_bNeedCr = true;
}
}
diff --git a/writerfilter/source/rtftok/rtfdocumentimpl.cxx
b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
index 8a820fa477da..2da578e181c4 100644
--- a/writerfilter/source/rtftok/rtfdocumentimpl.cxx
+++ b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
@@ -672,7 +672,10 @@ void RTFDocumentImpl::parBreak()
m_bHadPicture = false;
// start new one
- Mapper().startParagraphGroup();
+ if (!m_bParAtEndOfSection)
+ {
+ Mapper().startParagraphGroup();
+ }
}
void RTFDocumentImpl::sectBreak(bool bFinal)
@@ -686,14 +689,26 @@ void RTFDocumentImpl::sectBreak(bool bFinal)
// unless this is the end of the doc, we had nothing since the last
section break and this is not a continuous one.
// 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)
+ {
+ m_bParAtEndOfSection = true;
dispatchSymbol(RTFKeyword::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)
{
dispatchFlag(RTFKeyword::PARD);
+ m_bParAtEndOfSection = true;
dispatchSymbol(RTFKeyword::PAR);
m_bNeedSect = bNeedSect;
}
+ // testTdf148515, if RTF ends with ow, endParagraphGroup() must be called!
+ if (!m_bParAtEndOfSection || m_aStates.top().getCurrentBuffer())
+ {
+ Mapper().endParagraphGroup(); // < top para context dies with page
break
+ }
+ m_bParAtEndOfSection = false;
+ // paragraph properties are *done* now - only section properties following
+
while (!m_nHeaderFooterPositions.empty())
{
std::pair<Id, std::size_t> aPair = m_nHeaderFooterPositions.front();
@@ -726,7 +741,6 @@ void RTFDocumentImpl::sectBreak(bool bFinal)
// The trick is that we send properties of the previous section right now,
which will be exactly what dmapper expects.
Mapper().props(pProperties);
- Mapper().endParagraphGroup();
// End Section
if (!m_pSuperstream)
diff --git a/writerfilter/source/rtftok/rtfdocumentimpl.hxx
b/writerfilter/source/rtftok/rtfdocumentimpl.hxx
index eb50e3c7e088..f05f7d321cdd 100644
--- a/writerfilter/source/rtftok/rtfdocumentimpl.hxx
+++ b/writerfilter/source/rtftok/rtfdocumentimpl.hxx
@@ -852,6 +852,8 @@ private:
bool m_bNeedPar;
/// If set, an empty paragraph will be added at the end of the document.
bool m_bNeedFinalPar;
+ /// a synthetic \par was dispatched at the end of the current section
+ bool m_bParAtEndOfSection = false;
/// The list table and list override table combined.
RTFSprms m_aListTableSprms;
/// Maps between listoverridetable and listtable indexes.
