diff options
-rw-r--r-- | sw/qa/extras/ooxmlexport/data/section_break_after_table.docx | bin | 0 -> 12583 bytes | |||
-rw-r--r-- | sw/qa/extras/ooxmlexport/ooxmlexport20.cxx | 19 | ||||
-rw-r--r-- | sw/qa/extras/ooxmlexport/ooxmlexport8.cxx | 2 | ||||
-rw-r--r-- | writerfilter/source/dmapper/DomainMapper.cxx | 13 | ||||
-rw-r--r-- | writerfilter/source/dmapper/DomainMapper_Impl.cxx | 137 | ||||
-rw-r--r-- | writerfilter/source/dmapper/DomainMapper_Impl.hxx | 3 |
6 files changed, 122 insertions, 52 deletions
diff --git a/sw/qa/extras/ooxmlexport/data/section_break_after_table.docx b/sw/qa/extras/ooxmlexport/data/section_break_after_table.docx Binary files differnew file mode 100644 index 000000000000..9f8e4c687bbb --- /dev/null +++ b/sw/qa/extras/ooxmlexport/data/section_break_after_table.docx 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 ); |