sw/qa/extras/ooxmlexport/ooxmlexport10.cxx | 10 ++--- sw/qa/extras/ooxmlimport/ooxmlimport.cxx | 2 - writerfilter/qa/cppunittests/dmapper/DomainMapper.cxx | 19 +++++++++ writerfilter/qa/cppunittests/dmapper/data/floattable-then-table.docx |binary writerfilter/source/dmapper/DomainMapper.cxx | 20 ++++++++++ writerfilter/source/dmapper/DomainMapperTableManager.cxx | 3 + writerfilter/source/dmapper/DomainMapperTableManager.hxx | 5 ++ 7 files changed, 53 insertions(+), 6 deletions(-)
New commits: commit 1c99616f86f7d5b83b91edc225fc95fec227d710 Author: Miklos Vajna <[email protected]> AuthorDate: Tue May 2 08:15:33 2023 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Tue May 2 16:35:52 2023 +0200 sw floattable, crashtesting: fix PDF export of forum-mso-en3-26783.docx Converting the bugdoc to PDF crashed Writer layout since commit ce3308a926f036b87515b8cd97d2b197063dc77a (tdf#61594 sw floattable: import floating tables as split flys by default, 2023-04-12). Inspecting what the layout does, the trouble was that the bugdoc has a floating table, immediately followed by a non-floating table. This breaks the invariant that floating tables are anchored in the next, text node. In practice we anchored the floating table in the first cell of the next table, which is not the same as a floating table followed by a normal table. (For comparison, the DOC import simply lost the floating table in this case.) Fix the problem by making sure there is a text node after a floating table, in case it's directly followed by a table (no text node in-between). Changed tests: - testFloatingTablesAnchor: this still asserts that the flys are all anchored in the body text - testTdf95755: the problematic table is still not as wide as it used to be in the bad case Change-Id: I30b9ae423914b0b2cb1ebc7c1dce7efcd4cc3cf7 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/151251 Reviewed-by: Miklos Vajna <[email protected]> Tested-by: Jenkins diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx index 5a886f6907ee..5ad22de67e62 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx @@ -432,17 +432,17 @@ DECLARE_OOXMLEXPORT_TEST(testFdo69656, "Table_cell_auto_width_fdo69656.docx") DECLARE_OOXMLEXPORT_TEST(testFloatingTablesAnchor, "floating-tables-anchor.docx") { + uno::Reference<text::XTextDocument> xTextDocument(mxComponent, uno::UNO_QUERY); + uno::Reference<text::XText> xBodyText = xTextDocument->getText(); // Problem was one of the two text frames was anchored to the other text frame - // Both frames should be anchored to the paragraph with the text "Anchor point" + // Both frames should be anchored to the body text uno::Reference<text::XTextContent> xTextContent(getShape(1), uno::UNO_QUERY); uno::Reference<text::XTextRange> xRange = xTextContent->getAnchor(); - uno::Reference<text::XText> xText = xRange->getText(); - CPPUNIT_ASSERT_EQUAL(OUString("Anchor point"), xText->getString()); + CPPUNIT_ASSERT_EQUAL(xBodyText, xRange->getText()); xTextContent.set(getShape(2), uno::UNO_QUERY); xRange = xTextContent->getAnchor(); - xText = xRange->getText(); - CPPUNIT_ASSERT_EQUAL(OUString("Anchor point"), xText->getString()); + CPPUNIT_ASSERT_EQUAL(xBodyText, xRange->getText()); // tdf#149292 pre-emptive test - ensure "First Page" page style CPPUNIT_ASSERT_EQUAL(OUString("First Page"), getProperty<OUString>(getParagraph(1), "PageDescName")); diff --git a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx index 73ac05f6311e..95e526f9c3a6 100644 --- a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx +++ b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx @@ -1416,7 +1416,7 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf95755) uno::Any aValue = xTableProperties->getPropertyValue("Width"); sal_Int32 nWidth; aValue >>= nWidth; - CPPUNIT_ASSERT_EQUAL(sal_Int32(10659), nWidth); + CPPUNIT_ASSERT_EQUAL(sal_Int32(10592), nWidth); } CPPUNIT_TEST_FIXTURE(Test, testTdf60351) diff --git a/writerfilter/qa/cppunittests/dmapper/DomainMapper.cxx b/writerfilter/qa/cppunittests/dmapper/DomainMapper.cxx index 714e46267f57..97f856044d6c 100644 --- a/writerfilter/qa/cppunittests/dmapper/DomainMapper.cxx +++ b/writerfilter/qa/cppunittests/dmapper/DomainMapper.cxx @@ -12,6 +12,7 @@ #include <com/sun/star/text/XTextDocument.hpp> #include <com/sun/star/beans/XPropertySet.hpp> #include <com/sun/star/beans/PropertyValues.hpp> +#include <com/sun/star/drawing/XDrawPageSupplier.hpp> #include <tools/UnitConversion.hxx> @@ -100,6 +101,24 @@ CPPUNIT_TEST_FIXTURE(Test, testSdtDropdownNoDisplayText) // i.e. the list item was lost on import. CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1), aListItems.getLength()); } + +CPPUNIT_TEST_FIXTURE(Test, testFloattableThenTable) +{ + // Given a document with an in-section floating table, followed by a table: + // When laying out that document: + loadFromURL(u"floattable-then-table.docx"); + + // Then make sure that instead of crashing, the floating table is anchored inside the body text + // (and not a cell): + uno::Reference<text::XTextDocument> xTextDocument(mxComponent, uno::UNO_QUERY); + uno::Reference<text::XText> xBodyText = xTextDocument->getText(); + uno::Reference<drawing::XDrawPageSupplier> xDrawPageSupplier(mxComponent, uno::UNO_QUERY); + uno::Reference<drawing::XDrawPage> xDrawPage = xDrawPageSupplier->getDrawPage(); + uno::Reference<text::XTextContent> xShape(xDrawPage->getByIndex(0), uno::UNO_QUERY); + uno::Reference<text::XTextRange> xAnchor = xShape->getAnchor(); + // Make sure the anchor text is the body text, not some cell. + CPPUNIT_ASSERT_EQUAL(xBodyText, xAnchor->getText()); +} } /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/writerfilter/qa/cppunittests/dmapper/data/floattable-then-table.docx b/writerfilter/qa/cppunittests/dmapper/data/floattable-then-table.docx new file mode 100644 index 000000000000..134473c46012 Binary files /dev/null and b/writerfilter/qa/cppunittests/dmapper/data/floattable-then-table.docx differ diff --git a/writerfilter/source/dmapper/DomainMapper.cxx b/writerfilter/source/dmapper/DomainMapper.cxx index b0030e781ab7..9da64ce6921a 100644 --- a/writerfilter/source/dmapper/DomainMapper.cxx +++ b/writerfilter/source/dmapper/DomainMapper.cxx @@ -3160,6 +3160,17 @@ void DomainMapper::sprmWithProps( Sprm& rSprm, const PropertyMapPtr& rContext ) break; case NS_ooxml::LN_tblStart: { + if (m_pImpl->hasTableManager()) + { + if (m_pImpl->getTableManager().IsFloating()) + { + // We're starting a new table, but the previous table was floating. Insert a dummy + // paragraph to ensure that the floating table is not anchored inside the next + // table. + finishParagraph(); + } + } + if (m_pImpl->hasTableManager()) { bool bTableStartsAtCellStart = m_pImpl->m_nTableDepth > 0 && m_pImpl->m_nTableCellDepth > m_pImpl->m_nLastTableCellParagraphDepth + 1; @@ -4757,6 +4768,15 @@ void DomainMapper::finishParagraph(const bool bRemove, const bool bNoNumbering) if (m_pImpl->m_pSdtHelper->getControlType() == SdtControlType::datePicker) m_pImpl->m_pSdtHelper->createDateContentControl(); m_pImpl->finishParagraph(m_pImpl->GetTopContextOfType(CONTEXT_PARAGRAPH), bRemove, bNoNumbering); + if (m_pImpl->m_nTableDepth == 0) + { + if (m_pImpl->hasTableManager()) + { + // Non-table content, possibly after a table. Forget that such a previous table was + // floating. + m_pImpl->getTableManager().SetFloating(false); + } + } } void DomainMapper::commentProps(const OUString& sId, const CommentProperties& rProps) diff --git a/writerfilter/source/dmapper/DomainMapperTableManager.cxx b/writerfilter/source/dmapper/DomainMapperTableManager.cxx index 2c802d3ce2a9..9f467f7ae409 100644 --- a/writerfilter/source/dmapper/DomainMapperTableManager.cxx +++ b/writerfilter/source/dmapper/DomainMapperTableManager.cxx @@ -333,6 +333,9 @@ bool DomainMapperTableManager::sprm(Sprm & rSprm) break; case NS_ooxml::LN_CT_TblPrBase_tblpPr: { + // Remember that this will be a floating table. + m_bIsFloating = true; + writerfilter::Reference<Properties>::Pointer_t pProperties = rSprm.getProps(); // Ignore <w:tblpPr> in shape text, those tables should be always non-floating ones. if (!m_bIsInShape && pProperties) diff --git a/writerfilter/source/dmapper/DomainMapperTableManager.hxx b/writerfilter/source/dmapper/DomainMapperTableManager.hxx index a2e492936b96..955ccfafc963 100644 --- a/writerfilter/source/dmapper/DomainMapperTableManager.hxx +++ b/writerfilter/source/dmapper/DomainMapperTableManager.hxx @@ -42,6 +42,8 @@ class DomainMapperTableManager : public TableManager sal_Int32 m_nTableWidth; //might be set directly or has to be calculated from the column positions /// Are we in a shape (text append stack is not empty) or in the body document? bool m_bIsInShape; + /// Is the current table floating? + bool m_bIsFloating = false; std::vector< OUString > m_aTableStyleNames; /// Moved table (in moveRangeFromStart...moveRangeFromEnd or moveRangeToStart...moveRangeToEnd) std::vector< OUString > m_aMoved; @@ -165,6 +167,9 @@ public: return OUString(); } + bool IsFloating() const { return m_bIsFloating; } + + void SetFloating(bool bFloating) { m_bIsFloating = bFloating; } }; }
