sw/qa/extras/ooxmlexport/data/section_break_after_table.docx |binary
sw/qa/extras/ooxmlexport/ooxmlexport20.cxx | 19 +
sw/qa/extras/ooxmlexport/ooxmlexport8.cxx | 2
writerfilter/source/dmapper/DomainMapper.cxx | 13 -
writerfilter/source/dmapper/DomainMapper_Impl.cxx | 137 ++++++++---
writerfilter/source/dmapper/DomainMapper_Impl.hxx | 3
6 files changed, 122 insertions(+), 52 deletions(-)
New commits:
commit f09420fa189be5165b0311083ba127073500a121
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Mon Dec 25 15:32:41 2023 +0600
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Mon Dec 25 13:23:36 2023 +0100
tdf#158855: Make sure to not add extra paragraph after a table in a section
Simplify the condition for last paragraph removal in lcl_utext, and clean up
the complications piled after commit
2e8aad6d45c53d554ccaf26de998ede708cfc289
(fdo#39056 fdo#75431 Section Properties if section starts with table,
2014-04-24). This makes sure that the trailing extra paragraph gets properly
removed, even when there was a dummy paragraph added for a table in section.
Because of the removed check, import used to avoid the removal of the extra
paragraph immediately following a table (when the table needed to be the
last
element in the section); hence, in
DomainMapper_Impl::appendTextSectionAfter,
a last section's table was followed by two paragraphs (one that is expected
to stay after the created section, and one before that, that was not removed
after the table). xCursor had ininitally both paragraphs in the selection
after the forward goto* calls; and then, gotoLeft excluded only the last
paragraph from the selection. After the change, in such a case, there is
only
one paragraph in the end: the one that should stay after the created
section.
But calling gotoLeft then would have to put the end of selection to a text
node inside the table; since the selection that starts before the table
can't
end inside the table, the call resulted in the whole table excluded from the
selection. This requires to add a paragraph here, to allow the previous
behavior of cursor; and then the last paragraph in section is destroyed. The
code makes sure that the trailing paragraph kept after the creation of the
section has all the properties applied to the old last paragraph.
testFdo53985 used to check for total 9 paragraphs in the document; while
3 of those were introduced exactly by the bug fixed here; they are absent
in Word. The test is fixed; its logic is unchanged, and it still correctly
tests that a round-trip doesn't increase number of paragraphs.
Change-Id: Ibf6349ba7906dac185c759eb3b58c1849ee5064a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161275
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
diff --git a/sw/qa/extras/ooxmlexport/data/section_break_after_table.docx
b/sw/qa/extras/ooxmlexport/data/section_break_after_table.docx
new file mode 100644
index 000000000000..9f8e4c687bbb
Binary files /dev/null and
b/sw/qa/extras/ooxmlexport/data/section_break_after_table.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport20.cxx
b/sw/qa/extras/ooxmlexport/ooxmlexport20.cxx
index 0cf1c6c4f709..1dc12ff2085a 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport20.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport20.cxx
@@ -1042,6 +1042,25 @@ CPPUNIT_TEST_FIXTURE(Test, testtdf158044)
assertXPath(pXmlDoc,
"/w:document/w:body/w:p[8]/w:r[4]/w:rPr[1]/w:shadow[1]"_ostr);
}
+CPPUNIT_TEST_FIXTURE(Test, testTdf158855)
+{
+ // Given a table immediately followed by a section break
+ load(createFileURL(u"section_break_after_table.docx"));
+
+ // Check that the import doesn't produce an extra empty paragraph before a
page break
+ CPPUNIT_ASSERT_EQUAL(2, getPages()); // was 3
+ CPPUNIT_ASSERT_EQUAL(2, getParagraphs()); // was 3
+ uno::Reference<text::XTextTable>(getParagraphOrTable(1),
uno::UNO_QUERY_THROW);
+ getParagraph(2, u"Next page"_ustr); // was empty, with the 3rd being "Next
page"
+
+ saveAndReload(mpFilter);
+
+ CPPUNIT_ASSERT_EQUAL(2, getPages());
+ CPPUNIT_ASSERT_EQUAL(2, getParagraphs());
+ uno::Reference<text::XTextTable>(getParagraphOrTable(1),
uno::UNO_QUERY_THROW);
+ getParagraph(2, u"Next page"_ustr);
+}
+
} // end of anonymous namespace
CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx
b/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx
index 158b8f61d873..10fd87ebec2c 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx
@@ -756,7 +756,7 @@ DECLARE_OOXMLEXPORT_TEST(testFdo53985, "fdo53985.docx")
CPPUNIT_ASSERT_EQUAL_MESSAGE("Section3 is protected", false,
getProperty<bool>(xSect, "IsProtected"));
// This was increasing by 3 every round-trip - an extra paragraph after
each table in sections
- CPPUNIT_ASSERT_EQUAL(9, getParagraphs());
+ CPPUNIT_ASSERT_EQUAL(6, getParagraphs());
}
DECLARE_OOXMLEXPORT_TEST(testFdo59638, "fdo59638.docx")
diff --git a/writerfilter/source/dmapper/DomainMapper.cxx
b/writerfilter/source/dmapper/DomainMapper.cxx
index 0c9720071068..c6be4d9f8c98 100644
--- a/writerfilter/source/dmapper/DomainMapper.cxx
+++ b/writerfilter/source/dmapper/DomainMapper.cxx
@@ -3897,10 +3897,7 @@ void DomainMapper::lcl_text(const sal_uInt8 * data_,
size_t len)
case 0x0c: //page break
// page breaks aren't supported in footnotes and endnotes
if (!m_pImpl->IsInFootOrEndnote())
- {
m_pImpl->deferBreak(PAGE_BREAK);
-
m_pImpl->SetIsDummyParaAddedForTableInSectionPage(false);
- }
return;
case 0x0e: //column break
m_pImpl->deferBreak(COLUMN_BREAK);
@@ -4403,7 +4400,6 @@ void DomainMapper::lcl_utext(const sal_Unicode *const
data_, size_t len)
&& !bIsColumnBreak
&& !m_pImpl->GetIsLastSectionGroup() //
testContSectionPageBreak
&& !m_pImpl->GetParaHadField()
- &&
(!m_pImpl->GetIsDummyParaAddedForTableInSectionPage())
&& !m_pImpl->GetIsPreviousParagraphFramed()
&& !m_pImpl->HasTopAnchoredObjects()
&& !m_pImpl->IsParaWithInlineObject());
@@ -4424,15 +4420,6 @@ void DomainMapper::lcl_utext(const sal_Unicode *const
data_, size_t len)
if (bRemove)
m_pImpl->RemoveLastParagraph();
- // When the table is closed and the section's initial dummy
paragraph has been processed
- // then any following sectPr paragraph in the section must be
eligible for removal.
- if (!bRemove &&
m_pImpl->GetIsDummyParaAddedForTableInSectionPage() && !IsInTable()
- && !m_pImpl->GetFootnoteContext() && !m_pImpl->IsInComments()
&& !IsInHeaderFooter()
- && !IsInShape())
- {
- m_pImpl->SetIsDummyParaAddedForTableInSectionPage(false);
- }
-
m_pImpl->SetParaSectpr(false);
}
else
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index bc6a3dfdf5a1..e247294a4e52 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -29,6 +29,7 @@
#include "TagLogger.hxx"
#include <com/sun/star/uno/XComponentContext.hpp>
#include <com/sun/star/graphic/XGraphic.hpp>
+#include <com/sun/star/beans/PropertyAttribute.hpp>
#include <com/sun/star/beans/XPropertyState.hpp>
#include <com/sun/star/container/XNamed.hpp>
#include <com/sun/star/document/PrinterIndependentLayout.hpp>
@@ -380,7 +381,6 @@ DomainMapper_Impl::DomainMapper_Impl(
m_bIsFirstParaInSection( true ),
m_bIsFirstParaInSectionAfterRedline( true ),
m_bDummyParaAddedForTableInSection( false ),
- m_bDummyParaAddedForTableInSectionPage( false ),
m_bTextFrameInserted(false),
m_bIsPreviousParagraphFramed( false ),
m_bIsLastParaInSection( false ),
@@ -979,12 +979,6 @@ void DomainMapper_Impl::SetIsFirstParagraphInShape(bool
bIsFirst)
void DomainMapper_Impl::SetIsDummyParaAddedForTableInSection( bool bIsAdded )
{
m_bDummyParaAddedForTableInSection = bIsAdded;
- m_bDummyParaAddedForTableInSectionPage = bIsAdded;
-}
-
-void DomainMapper_Impl::SetIsDummyParaAddedForTableInSectionPage( bool
bIsAdded )
-{
- m_bDummyParaAddedForTableInSectionPage = bIsAdded;
}
@@ -3518,6 +3512,86 @@ void DomainMapper_Impl::adjustLastPara(sal_Int8 nAlign)
pLastPara->Insert(PROP_PARA_ADJUST, uno::Any(nAlign), true);
}
+static void checkAndAddPropVal(const OUString& prop, const css::uno::Any& val,
+ std::vector<OUString>& props,
std::vector<css::uno::Any>& values)
+{
+ // Avoid well-known reasons for exceptions when setting property values
+ if (!val.hasValue())
+ return;
+ if (prop == "CharAutoStyleName" || prop == "ParaAutoStyleName")
+ return;
+ if (prop == "CharStyleName" || prop == "DropCapCharStyleName")
+ if (OUString val_string; (val >>= val_string) && val_string.isEmpty())
+ return;
+
+ props.push_back(prop);
+ values.push_back(val);
+}
+
+static void copyAllProps(const css::uno::Reference<css::uno::XInterface>& from,
+ const css::uno::Reference<css::uno::XInterface>& to)
+{
+ css::uno::Reference<css::beans::XPropertySet> xFromProps(from,
css::uno::UNO_QUERY_THROW);
+ css::uno::Reference<css::beans::XPropertySetInfo>
xFromInfo(xFromProps->getPropertySetInfo(),
+
css::uno::UNO_SET_THROW);
+ css::uno::Sequence<css::beans::Property>
rawProps(xFromInfo->getProperties());
+ std::vector<OUString> props;
+ props.reserve(rawProps.getLength());
+ for (const auto& prop : rawProps)
+ if ((prop.Attributes & css::beans::PropertyAttribute::READONLY) == 0)
+ props.push_back(prop.Name);
+ std::vector<css::uno::Any> values;
+ values.reserve(props.size());
+ if (css::uno::Reference<css::beans::XMultiPropertySet> xFromMulti{
xFromProps,
+
css::uno::UNO_QUERY })
+ {
+ const auto propsSeq = comphelper::containerToSequence(props);
+ const auto valuesSeq = xFromMulti->getPropertyValues(propsSeq);
+ assert(propsSeq.getLength() == valuesSeq.getLength());
+ props.clear();
+ for (size_t i = 0; i < propsSeq.size(); ++i)
+ checkAndAddPropVal(propsSeq[i], valuesSeq[i], props, values);
+ }
+ else
+ {
+ std::vector<OUString> filtered_props;
+ filtered_props.reserve(props.size());
+ for (const auto& prop : props)
+ checkAndAddPropVal(prop, xFromProps->getPropertyValue(prop),
filtered_props, values);
+ filtered_props.swap(props);
+ }
+ assert(props.size() == values.size());
+
+ css::uno::Reference<css::beans::XPropertySet> xToProps(to,
css::uno::UNO_QUERY_THROW);
+ if (css::uno::Reference<css::beans::XMultiPropertySet> xToMulti{ xToProps,
+
css::uno::UNO_QUERY })
+ {
+ try
+ {
+ xToMulti->setPropertyValues(comphelper::containerToSequence(props),
+
comphelper::containerToSequence(values));
+ return;
+ }
+ catch (css::uno::Exception&)
+ {
+ DBG_UNHANDLED_EXCEPTION("writerfilter.dmapper");
+ }
+ // Fallback to property-by-property iteration
+ }
+
+ for (size_t i = 0; i < props.size(); ++i)
+ {
+ try
+ {
+ xToProps->setPropertyValue(props[i], values[i]);
+ }
+ catch (css::uno::Exception&)
+ {
+ DBG_UNHANDLED_EXCEPTION("writerfilter.dmapper");
+ }
+ }
+}
+
uno::Reference< beans::XPropertySet >
DomainMapper_Impl::appendTextSectionAfter(
uno::Reference< text::XTextRange > const &
xBefore )
{
@@ -3537,40 +3611,33 @@ uno::Reference< beans::XPropertySet >
DomainMapper_Impl::appendTextSectionAfter(
xCursor->gotoRange( m_aTextAppendStack.top().xInsertPosition,
true );
else
xCursor->gotoEnd( true );
- //the paragraph after this new section is already inserted
- xCursor->goLeft(1, true);
- css::uno::Reference<css::text::XTextRange> xTextRange(xCursor,
css::uno::UNO_QUERY_THROW);
+ // The paragraph after this new section is already inserted. The
previous node may be a
+ // table; then trying to go left would skip the whole table. Split
the trailing
+ // paragraph; let the section span over the first of the two
resulting paragraphs;
+ // destroy the last section's paragraph afterwards.
+ css::uno::Reference<css::text::XTextRange> xEndPara =
xCursor->getEnd();
+ xTextAppend->insertControlCharacter(
+ xEndPara, css::text::ControlCharacter::PARAGRAPH_BREAK, false);
+ css::uno::Reference<css::text::XTextRange> xNewPara =
xCursor->getEnd();
+ xCursor->gotoPreviousParagraph(true);
+ xEndPara = xCursor->getEnd();
+ // xEndPara may already have properties (like page break); make
sure to apply them
+ // to the newly appended paragraph, which will be kept in the end.
+ copyAllProps(xEndPara, xNewPara);
- if (css::uno::Reference<css::text::XDocumentIndexesSupplier>
xIndexSupplier{
- GetTextDocument(), css::uno::UNO_QUERY })
+ uno::Reference< text::XTextContent > xSection(
m_xTextFactory->createInstance("com.sun.star.text.TextSection"),
uno::UNO_QUERY_THROW );
+ xSection->attach(xCursor);
+
+ // Remove the extra paragraph (last inside the section)
+ if (uno::Reference<container::XEnumerationAccess> xEA{ xEndPara,
uno::UNO_QUERY })
{
- css::uno::Reference<css::text::XTextRangeCompare> xCompare(
- xTextAppend, css::uno::UNO_QUERY);
- const auto xIndexAccess = xIndexSupplier->getDocumentIndexes();
- for (sal_Int32 i = xIndexAccess->getCount(); i > 0; --i)
+ if (uno::Reference<lang::XComponent> xParagraph{
+ xEA->createEnumeration()->nextElement(),
uno::UNO_QUERY })
{
- if (css::uno::Reference<css::text::XDocumentIndex> xIndex{
- xIndexAccess->getByIndex(i - 1),
css::uno::UNO_QUERY })
- {
- const auto xIndexTextRange = xIndex->getAnchor();
- if (xCompare->compareRegionStarts(xTextRange,
xIndexTextRange) == 0
- && xCompare->compareRegionEnds(xTextRange,
xIndexTextRange) == 0)
- {
- // The boundaries coincide with an index: trying
to attach a section
- // to the range will insert the section inside the
index. goRight will
- // extend the range outside of the index, so that
created section will
- // be around it. Alternatively we could return
index section itself
- // instead : xRet.set(xIndex, uno::UNO_QUERY) - to
set its properties,
- // like columns/fill.
- xCursor->goRight(1, true);
- break;
- }
- }
+ xParagraph->dispose();
}
}
- uno::Reference< text::XTextContent > xSection(
m_xTextFactory->createInstance("com.sun.star.text.TextSection"),
uno::UNO_QUERY_THROW );
- xSection->attach( xTextRange );
xRet.set(xSection, uno::UNO_QUERY );
}
catch(const uno::Exception&)
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
index f555dd14c048..dc8dbcee3880 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
@@ -589,7 +589,6 @@ private:
bool m_bIsFirstParaInSectionAfterRedline;
bool m_bIsFirstParaInShape = false;
bool m_bDummyParaAddedForTableInSection;
- bool m_bDummyParaAddedForTableInSectionPage;
bool m_bTextFrameInserted;
bool m_bIsPreviousParagraphFramed;
bool m_bIsLastParaInSection;
@@ -720,8 +719,6 @@ public:
bool GetIsFirstParagraphInShape() const { return m_bIsFirstParaInShape; }
void SetIsDummyParaAddedForTableInSection( bool bIsAdded );
bool GetIsDummyParaAddedForTableInSection() const { return
m_bDummyParaAddedForTableInSection;}
- void SetIsDummyParaAddedForTableInSectionPage(bool bIsAdded);
- bool GetIsDummyParaAddedForTableInSectionPage() const { return
m_bDummyParaAddedForTableInSectionPage; }
/// Track if a textframe has been inserted into this section
void SetIsTextFrameInserted( bool bIsInserted );
