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(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(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(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 #include +#include #include #include #include @@ -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& props, std::vector& 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& from, + const css::uno::Reference& to) +{ + css::uno::Reference xFromProps(from, css::uno::UNO_QUERY_THROW); + css::uno::Reference xFromInfo(xFromProps->getPropertySetInfo(), + css::uno::UNO_SET_THROW); + css::uno::Sequence rawProps(xFromInfo->getProperties()); + std::vector 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 values; + values.reserve(props.size()); + if (css::uno::Reference 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 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 xToProps(to, css::uno::UNO_QUERY_THROW); + if (css::uno::Reference 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 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 xEndPara = xCursor->getEnd(); + xTextAppend->insertControlCharacter( + xEndPara, css::text::ControlCharacter::PARAGRAPH_BREAK, false); + css::uno::Reference 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 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 xEA{ xEndPara, uno::UNO_QUERY }) { - css::uno::Reference xCompare( - xTextAppend, css::uno::UNO_QUERY); - const auto xIndexAccess = xIndexSupplier->getDocumentIndexes(); - for (sal_Int32 i = xIndexAccess->getCount(); i > 0; --i) + if (uno::Reference xParagraph{ + xEA->createEnumeration()->nextElement(), uno::UNO_QUERY }) { - if (css::uno::Reference 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 );