writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler.cxx      |   25 
++++++++++
 writerfilter/qa/cppunittests/dmapper/data/floattable-break-before.docx |binary
 writerfilter/source/dmapper/DomainMapperTableHandler.cxx               |   20 
+++++++-
 3 files changed, 44 insertions(+), 1 deletion(-)

New commits:
commit 4b6b9411e4ac912817dd804782ad2054bc0d1660
Author:     Miklos Vajna <vmik...@collabora.com>
AuthorDate: Thu Apr 27 08:15:51 2023 +0200
Commit:     Miklos Vajna <vmik...@collabora.com>
CommitDate: Thu Apr 27 12:21:30 2023 +0200

    sw floattable, crashtesting: fix PDF export of fdo72790-1.docx, part 4
    
    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).
    
    Part 1 already fixed the crash and parts 2-3 already improved the layout
    partially, towards avoiding a layout loop. The top problem now seems to
    be that page breaks before floating tables are ignored, which leads to a
    layout situation that loops. This problem was hidden before, since page
    breaks were not ignored.
    
    Fix the problem at DOCX import time: if there is a "break before" on the
    table, then transfer that to the anchor paragraph, which gives the
    correct layout, and also side-steps the above described layout loop.
    
    We should probably never call SwTextFrame::JoinFrame() when creating the
    initial layout for a document, that part is still unfixed, but that
    looks like a pre-existing problem.
    
    Change-Id: I1e2ecdbf0a3d4e2477cd4768a9b4a35a155e815b
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/151082
    Reviewed-by: Miklos Vajna <vmik...@collabora.com>
    Tested-by: Jenkins

diff --git a/writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler.cxx 
b/writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler.cxx
index 7f1a1db064d8..0650e48fe607 100644
--- a/writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler.cxx
+++ b/writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler.cxx
@@ -14,6 +14,8 @@
 #include <com/sun/star/text/XTextTable.hpp>
 #include <com/sun/star/text/XTextTablesSupplier.hpp>
 #include <com/sun/star/drawing/XDrawPageSupplier.hpp>
+#include <com/sun/star/text/XTextDocument.hpp>
+#include <com/sun/star/style/BreakType.hpp>
 
 using namespace ::com::sun::star;
 
@@ -57,6 +59,29 @@ CPPUNIT_TEST_FIXTURE(Test, testNestedFloatingTable)
     // was partly positioned outside the table cell, leading to overlapping 
text.
     CPPUNIT_ASSERT(bIsFollowingTextFlow);
 }
+
+CPPUNIT_TEST_FIXTURE(Test, testFloatingTableBreakBefore)
+{
+    // Given a 3 pages document: page break, then a multi-page floating table 
on pages 2 and 3:
+    // When laying out that document:
+    loadFromURL(u"floattable-break-before.docx");
+
+    // Then make sure the page break property is on the anchor of the floating 
table, otherwise it
+    // has no effect:
+    uno::Reference<text::XTextDocument> xTextDocument(mxComponent, 
uno::UNO_QUERY);
+    uno::Reference<container::XEnumerationAccess> 
xText(xTextDocument->getText(), uno::UNO_QUERY);
+    uno::Reference<container::XEnumeration> xParagraphs = 
xText->createEnumeration();
+    xParagraphs->nextElement();
+    xParagraphs->nextElement();
+    uno::Reference<beans::XPropertySet> xParagraph(xParagraphs->nextElement(), 
uno::UNO_QUERY);
+    style::BreakType eBreakType{};
+    xParagraph->getPropertyValue("BreakType") >>= eBreakType;
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: 4 (style::BreakType_PAGE_BEFORE)
+    // - Actual  : 0 (style::BreakType_NONE)
+    // i.e. the page break was lost.
+    CPPUNIT_ASSERT_EQUAL(style::BreakType_PAGE_BEFORE, eBreakType);
+}
 }
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git 
a/writerfilter/qa/cppunittests/dmapper/data/floattable-break-before.docx 
b/writerfilter/qa/cppunittests/dmapper/data/floattable-break-before.docx
new file mode 100644
index 000000000000..7fcfed4a637d
Binary files /dev/null and 
b/writerfilter/qa/cppunittests/dmapper/data/floattable-break-before.docx differ
diff --git a/writerfilter/source/dmapper/DomainMapperTableHandler.cxx 
b/writerfilter/source/dmapper/DomainMapperTableHandler.cxx
index eaddae615817..ac2f00a7d5a5 100644
--- a/writerfilter/source/dmapper/DomainMapperTableHandler.cxx
+++ b/writerfilter/source/dmapper/DomainMapperTableHandler.cxx
@@ -50,6 +50,7 @@
 #include <comphelper/sequence.hxx>
 #include <comphelper/propertyvalue.hxx>
 #include <com/sun/star/lang/IndexOutOfBoundsException.hpp>
+#include <com/sun/star/style/BreakType.hpp>
 #include <boost/lexical_cast.hpp>
 #include <officecfg/Office/Writer.hxx>
 
@@ -1541,6 +1542,15 @@ void DomainMapperTableHandler::endTable(unsigned int 
nestedTableLevel, bool bTab
             // A non-zero left margin would move the table out of the frame, 
move the frame itself instead.
             xTableProperties->setPropertyValue("LeftMargin", 
uno::Any(sal_Int32(0)));
 
+            style::BreakType eBreakType{};
+            xTableProperties->getPropertyValue("BreakType") >>= eBreakType;
+            if (eBreakType != style::BreakType_NONE)
+            {
+                // A break before the table was requested. Reset that break 
here, since the table
+                // will be at the start of the fly frame, not in the body 
frame.
+                xTableProperties->setPropertyValue("BreakType", 
uno::Any(style::BreakType_NONE));
+            }
+
             if (nestedTableLevel >= 2)
             {
                 // Floating tables inside a table always stay inside the cell.
@@ -1572,6 +1582,7 @@ void DomainMapperTableHandler::endTable(unsigned int 
nestedTableLevel, bool bTab
             // Multi-page floating tables works if an outer/toplevel table is 
floating, but not
             // when an inner table would float.
             bool bToplevelSplitFly = nestedTableLevel <= 1;
+            uno::Reference<beans::XPropertySet> xFrameAnchor;
             if (xTextAppendAndConvert.is() && (!bTableStartsAtCellStart || 
bToplevelSplitFly))
             {
                 std::deque<css::uno::Any> aFramedRedlines = 
m_rDMapper_Impl.m_aStoredRedlines[StoredRedlines::FRAME];
@@ -1580,10 +1591,17 @@ void DomainMapperTableHandler::endTable(unsigned int 
nestedTableLevel, bool bTab
                 std::vector<OUString> redTable;
                 BeforeConvertToTextFrame(aFramedRedlines, redPos, redLen, 
redCell, redTable);
 
-                xTextAppendAndConvert->convertToTextFrame(xStart, xEnd, 
comphelper::containerToSequence(aFrameProperties));
+                uno::Reference<text::XTextContent> xContent = 
xTextAppendAndConvert->convertToTextFrame(xStart, xEnd, 
comphelper::containerToSequence(aFrameProperties));
+                xFrameAnchor.set(xContent->getAnchor(), uno::UNO_QUERY);
 
                 AfterConvertToTextFrame(m_rDMapper_Impl, aFramedRedlines, 
redPos, redLen, redCell, redTable);
             }
+
+            if (xFrameAnchor.is() && eBreakType != style::BreakType_NONE)
+            {
+                // A break before the table was requested. Restore that on the 
anchor.
+                xFrameAnchor->setPropertyValue("BreakType", 
uno::Any(eBreakType));
+            }
         }
     }
 

Reply via email to