writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl.cxx |
24 +++
writerfilter/qa/cppunittests/dmapper/data/floating-table-section-break.docx
|binary
writerfilter/source/dmapper/DomainMapper_Impl.cxx |
75 ++++++++++
3 files changed, 99 insertions(+)
New commits:
commit 2a380dba73d57f825128fbada91c7a9fe79e8a06
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Wed May 31 08:19:24 2023 +0200
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Wed May 31 09:04:22 2023 +0200
tdf#150769 sw floattable: fix lost PageDescName if section starts with a
table
The bugdoc has 2 tables in it, separated by a section break (next page).
This page break was missing in Writer, so the tables overlapped.
The page break was lost because the 2nd section started with a table,
where we insert a dummy initial paragraph (to side-step the problem that
you can't have a selection starting in a table and finishing outside
that table), and once it's disposed, its properties are lost.
Fix the problem by copying the PageDescName property from the dummy
paragraph to the next paragraph before dispose. Note that we need this
combination of creating a range (using gotoNextParagraph()) and then
enumerate over the range, because gotoNextParagraph() would skip a
leading non-floating table, which would lead to an additional, not
wanted page break, as pointed out by CppunitTest_sw_ooxmlexport15's
testTdf134649_pageBreak.
This went wrong in ce5f82dbaf1c22f45a08c60eb213bc9bc821c1d1 (DOCX
import: floating table with negative top margin has to be a fly frame,
2022-01-21), and it turns out to be related to normal floating tables,
unrelated to multi-page floating tables.
Change-Id: I83245c78c63ec8f3d6015ce3e72ab232220a9fdb
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/152411
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
Tested-by: Jenkins
diff --git a/writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl.cxx
b/writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl.cxx
index 359c051fb0df..858faf709fd4 100644
--- a/writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl.cxx
@@ -19,6 +19,8 @@
#include <com/sun/star/lang/XServiceInfo.hpp>
#include <com/sun/star/style/XStyleFamiliesSupplier.hpp>
#include <com/sun/star/document/XDocumentInsertable.hpp>
+#include <com/sun/star/text/XTextViewCursorSupplier.hpp>
+#include <com/sun/star/text/XPageCursor.hpp>
#include <vcl/scheduler.hxx>
@@ -324,6 +326,28 @@ CPPUNIT_TEST_FIXTURE(Test,
testContentControlDataBindingColor)
// i.e. the char color was red, not the default / automatic.
CPPUNIT_ASSERT_EQUAL(COL_AUTO, nColor);
}
+
+CPPUNIT_TEST_FIXTURE(Test, testFloatingTableSectionBreak)
+{
+ // Given a document with 2 floating tables and 2 pages, section break
(next page) between the
+ // two:
+ loadFromURL(u"floating-table-section-break.docx");
+
+ // When going to the last page:
+ uno::Reference<frame::XModel> xModel(mxComponent, uno::UNO_QUERY);
+ uno::Reference<text::XTextViewCursorSupplier> xTextViewCursorSupplier(
+ xModel->getCurrentController(), uno::UNO_QUERY);
+ uno::Reference<text::XPageCursor>
xCursor(xTextViewCursorSupplier->getViewCursor(),
+ uno::UNO_QUERY);
+ xCursor->jumpToLastPage();
+
+ // Then make sure that we're on page 2:
+ // Without the accompanying fix in place, this test would have failed with:
+ // - Expected: 2
+ // - Actual : 1
+ // i.e. the document was of 1 page, the section break was lost.
+ CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int16>(2), xCursor->getPage());
+}
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git
a/writerfilter/qa/cppunittests/dmapper/data/floating-table-section-break.docx
b/writerfilter/qa/cppunittests/dmapper/data/floating-table-section-break.docx
new file mode 100644
index 000000000000..e5c0cb19b342
Binary files /dev/null and
b/writerfilter/qa/cppunittests/dmapper/data/floating-table-section-break.docx
differ
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 1eb5753110c8..b4405517eab0 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -576,6 +576,79 @@ void DomainMapper_Impl::SetDocumentSettingsProperty( const
OUString& rPropName,
}
}
}
+
+namespace
+{
+void CopyPageDescNameToNextParagraph(const uno::Reference<lang::XComponent>&
xParagraph,
+ const uno::Reference<text::XTextCursor>&
xCursor)
+{
+ // First check if xParagraph has a non-empty page style name to copy from.
+ uno::Reference<beans::XPropertySet> xParagraphProps(xParagraph,
uno::UNO_QUERY);
+ if (!xParagraphProps.is())
+ {
+ return;
+ }
+
+ uno::Any aPageDescName = xParagraphProps->getPropertyValue("PageDescName");
+ OUString sPageDescName;
+ aPageDescName >>= sPageDescName;
+ if (sPageDescName.isEmpty())
+ {
+ return;
+ }
+
+ // If so, search for the next paragraph.
+ uno::Reference<text::XParagraphCursor> xParaCursor(xCursor,
uno::UNO_QUERY);
+ if (!xParaCursor.is())
+ {
+ return;
+ }
+
+ // Create a range till the next paragraph and then enumerate on the range.
+ if (!xParaCursor->gotoNextParagraph(/*bExpand=*/true))
+ {
+ return;
+ }
+
+ uno::Reference<container::XEnumerationAccess>
xEnumerationAccess(xParaCursor, uno::UNO_QUERY);
+ if (!xEnumerationAccess.is())
+ {
+ return;
+ }
+
+ uno::Reference<container::XEnumeration> xEnumeration =
xEnumerationAccess->createEnumeration();
+ if (!xEnumeration.is())
+ {
+ return;
+ }
+
+ xEnumeration->nextElement();
+ if (!xEnumeration->hasMoreElements())
+ {
+ return;
+ }
+
+ // We found a next item in the enumeration: it's usually a paragraph, but
may be a table as
+ // well.
+ uno::Reference<beans::XPropertySet>
xNextParagraph(xEnumeration->nextElement(), uno::UNO_QUERY);
+ if (!xNextParagraph.is())
+ {
+ return;
+ }
+
+ // See if there is page style set already: if so, don't touch it.
+ OUString sNextPageDescName;
+ xNextParagraph->getPropertyValue("PageDescName") >>= sNextPageDescName;
+ if (!sNextPageDescName.isEmpty())
+ {
+ return;
+ }
+
+ // Finally copy it over, so it's not lost.
+ xNextParagraph->setPropertyValue("PageDescName", aPageDescName);
+}
+}
+
void DomainMapper_Impl::RemoveDummyParaForTableInSection()
{
SetIsDummyParaAddedForTableInSection(false);
@@ -603,6 +676,8 @@ void DomainMapper_Impl::RemoveDummyParaForTableInSection()
{
uno::Reference<container::XEnumeration> xEnumeration =
xEnumerationAccess->createEnumeration();
uno::Reference<lang::XComponent>
xParagraph(xEnumeration->nextElement(), uno::UNO_QUERY);
+ // Make sure no page breaks are lost.
+ CopyPageDescNameToNextParagraph(xParagraph, xCursor);
xParagraph->dispose();
}
}
