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 c5c38b905f0ce39ec58e68b9cb952da0d2e04d1c Author: Miklos Vajna <[email protected]> AuthorDate: Tue May 2 08:15:33 2023 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Thu May 4 13:50:22 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 (cherry picked from commit 1c99616f86f7d5b83b91edc225fc95fec227d710) Change-Id: I30b9ae423914b0b2cb1ebc7c1dce7efcd4cc3cf7 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/151312 Tested-by: Miklos Vajna <[email protected]> Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx index 737bd594ffa4..d19dbd68b40c 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx @@ -468,17 +468,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 596067d61306..dc021fb437f6 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 7715435e3a46..827c1d4612d8 100644 --- a/writerfilter/source/dmapper/DomainMapper.cxx +++ b/writerfilter/source/dmapper/DomainMapper.cxx @@ -3159,6 +3159,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; @@ -4740,6 +4751,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 004f34971b6a..bf3c78dcebee 100644 --- a/writerfilter/source/dmapper/DomainMapperTableManager.cxx +++ b/writerfilter/source/dmapper/DomainMapperTableManager.cxx @@ -334,6 +334,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; } }; }
