sw/qa/extras/rtfexport/rtfexport8.cxx | 29 +++++++++++++++--------
writerfilter/source/rtftok/rtfdispatchsymbol.cxx | 11 +++-----
writerfilter/source/rtftok/rtfdocumentimpl.cxx | 18 ++++++++++++--
writerfilter/source/rtftok/rtfdocumentimpl.hxx | 2 +
4 files changed, 42 insertions(+), 18 deletions(-)
New commits:
commit d5e2e66f8eb7cb89863ee0096f0569d2358fa1b7
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Tue Jan 30 20:03:45 2024 +0100
Commit: Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Tue Feb 6 20:33:34 2024 +0100
tdf#158586 writerfilter: RTF import: handle \sect in frame as \par
This fixes the test testTdf158586_0 and testTdf158586_0B to look like in
Word; the case appears a bit esoteric, hopefully Word won't actually
create such documents? But Word will round-trip such bugdoc to a DOCX
where the first w:p contains all of w:framePr and w:sectPr and w:br...
Change-Id: I6ec09478a774e1e9c785e9482618c1afc388df0e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162778
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
(cherry picked from commit dbe78489e98d565b72a703524308523135ffdd67)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162822
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
diff --git a/sw/qa/extras/rtfexport/rtfexport8.cxx
b/sw/qa/extras/rtfexport/rtfexport8.cxx
index b1c694f7daa4..ebc2c3d2bf3f 100644
--- a/sw/qa/extras/rtfexport/rtfexport8.cxx
+++ b/sw/qa/extras/rtfexport/rtfexport8.cxx
@@ -52,17 +52,24 @@ DECLARE_RTFEXPORT_TEST(testTdf158586_0,
"tdf158586_pageBreak0.rtf")
{
// The specified page break must be lost because it is in a text frame
CPPUNIT_ASSERT_EQUAL(1, getPages());
- // CPPUNIT_ASSERT_EQUAL(1, getParagraphs());
+ CPPUNIT_ASSERT_EQUAL(1, getParagraphs());
- // There should be no empty carriage return at the start of the second page
- // const auto& pLayout = parseLayoutDump();
- // assertXPathContent(pLayout, "//page[1]/body/txt"_ostr, "First page");}
+ // There should be no empty paragraph at the start
+ const auto& pLayout = parseLayoutDump();
+ assertXPath(pLayout, "//anchored"_ostr, 1);
+ assertXPathContent(pLayout, "/root/page[1]/body//txt"_ostr, "First page");
}
DECLARE_RTFEXPORT_TEST(testTdf158586_0B, "tdf158586_pageBreak0B.rtf")
{
// The specified page break must be lost because it is in a text frame
CPPUNIT_ASSERT_EQUAL(1, getPages());
+ CPPUNIT_ASSERT_EQUAL(1, getParagraphs());
+
+ // There should be no empty paragraph at the start
+ const auto& pLayout = parseLayoutDump();
+ assertXPath(pLayout, "//anchored"_ostr, 1);
+ assertXPathContent(pLayout, "/root/page[1]/body//txt"_ostr, "First page");
}
DECLARE_RTFEXPORT_TEST(testTdf158586_1, "tdf158586_pageBreak1.rtf")
diff --git a/writerfilter/source/rtftok/rtfdispatchsymbol.cxx
b/writerfilter/source/rtftok/rtfdispatchsymbol.cxx
index 9347174b6da2..6c1c94b944d9 100644
--- a/writerfilter/source/rtftok/rtfdispatchsymbol.cxx
+++ b/writerfilter/source/rtftok/rtfdispatchsymbol.cxx
@@ -132,7 +132,7 @@ RTFError RTFDocumentImpl::dispatchSymbol(RTFKeyword
nKeyword)
case RTFKeyword::SECT:
{
m_bHadSect = true;
- if (m_bIgnoreNextContSectBreak)
+ if (m_bIgnoreNextContSectBreak ||
m_aStates.top().getFrame().hasProperties())
{
// testContSectionPageBreak: need \par now
dispatchSymbol(RTFKeyword::PAR);
commit 6dde3695c27b28165b5aa27de5a6e309b860e02f
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Tue Jan 30 17:46:12 2024 +0100
Commit: Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Tue Feb 6 20:33:25 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>
(cherry picked from commit 57abad5cf990111fd7de011809d4421dc0550193)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162821
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
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 cc85a48ae27b..7b6254ee0d0c 100644
--- a/writerfilter/source/rtftok/rtfdocumentimpl.cxx
+++ b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
@@ -671,7 +671,10 @@ void RTFDocumentImpl::parBreak()
m_bHadPicture = false;
// start new one
- Mapper().startParagraphGroup();
+ if (!m_bParAtEndOfSection)
+ {
+ Mapper().startParagraphGroup();
+ }
}
void RTFDocumentImpl::sectBreak(bool bFinal)
@@ -685,14 +688,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();
@@ -725,7 +740,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.
