From 5e394dce3482951adc1eefce3014565da0920055 Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Mon, 4 Sep 2017 12:44:15 +0200 Subject: tdf#112160 sw: audit GetNextLayoutLeaf() calls in SwFrame::GetNextSctLeaf() GetNextLayoutLeaf() returns the next container, not the follow one, so calls to that without checking first if we are in a table are always suspicious. This leads at the end to strange content move, as described in the bug. There appears to be only a single place in SwFrame::GetNextSctLeaf() which may be executed for split sections and called GetNextLayoutLeaf() unconditionally. Reviewed-on: https://gerrit.libreoffice.org/41882 Reviewed-by: Miklos Vajna Tested-by: Jenkins (cherry picked from commit ec262cbc56822d8fffccd6e983848df196cf5c44) Conflicts: sw/qa/extras/uiwriter/uiwriter.cxx Change-Id: I759d9ef63660f3d2ffe006c88b18cba7dba99f33 --- sw/qa/extras/uiwriter/data/tdf112160.fodt | 125 ++++++++++++++++++++++++++++++ sw/qa/extras/uiwriter/uiwriter.cxx | 25 ++++++ sw/source/core/layout/sectfrm.cxx | 9 ++- 3 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 sw/qa/extras/uiwriter/data/tdf112160.fodt diff --git a/sw/qa/extras/uiwriter/data/tdf112160.fodt b/sw/qa/extras/uiwriter/data/tdf112160.fodt new file mode 100644 index 000000000000..d02352e4d2a6 --- /dev/null +++ b/sw/qa/extras/uiwriter/data/tdf112160.fodt @@ -0,0 +1,125 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Table1.A1 + + + + + Table1.B1 + + + + + Table1.C1 + + + + + Table1.D1 + + + + + + + Table1.A2 + + + + + Table1.B2 + + + + + Table1.C2 + + + + + Table1.D2 + + + + + + + + diff --git a/sw/qa/extras/uiwriter/uiwriter.cxx b/sw/qa/extras/uiwriter/uiwriter.cxx index c9753a8f712c..3f998534ff59 100644 --- a/sw/qa/extras/uiwriter/uiwriter.cxx +++ b/sw/qa/extras/uiwriter/uiwriter.cxx @@ -205,6 +205,7 @@ public: void testTableInNestedSection(); void testTableInSectionInTable(); void testSectionInTableInTable(); + void testTdf112160(); void testLinesInSectionInTable(); void testLinesMoveBackwardsInSectionInTable(); @@ -316,6 +317,7 @@ public: CPPUNIT_TEST(testLinesInSectionInTable); CPPUNIT_TEST(testTableInSectionInTable); CPPUNIT_TEST(testSectionInTableInTable); + CPPUNIT_TEST(testTdf112160); CPPUNIT_TEST(testLinesMoveBackwardsInSectionInTable); CPPUNIT_TEST_SUITE_END(); @@ -3840,6 +3842,29 @@ void SwUiWriterTest::testSectionInTableInTable() createDoc("tdf112109.fodt"); } +void SwUiWriterTest::testTdf112160() +{ + // Assert that the A2 cell is on page 1. + SwDoc* pDoc = createDoc("tdf112160.fodt"); + xmlDocPtr pXmlDoc = parseLayoutDump(); + sal_uInt32 nA2CellNode = getXPath(pXmlDoc, "/root/page[1]/body/tab/row[2]/cell[1]/section/txt[last()]", "txtNodeIndex").toUInt32(); + CPPUNIT_ASSERT_EQUAL(OUString("Table1.A2"), pDoc->GetNodes()[nA2CellNode]->GetTextNode()->GetText()); + + // Append a new paragraph to the end of the A2 cell. + SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell(); + while (pWrtShell->GetCursor()->GetNode().GetIndex() < nA2CellNode) + pWrtShell->Down(/*bSelect=*/false); + pWrtShell->EndPara(); + pWrtShell->SplitNode(); + + // Assert that after A2 got extended, D2 stays on page 1. + discardDumpedLayout(); + pXmlDoc = parseLayoutDump(); + sal_uInt32 nD2CellNode = getXPath(pXmlDoc, "/root/page[1]/body/tab/row[2]/cell[last()]/section/txt[last()]", "txtNodeIndex").toUInt32(); + // This was Table1.C2, Table1.D2 was moved to the next page, unexpected. + CPPUNIT_ASSERT_EQUAL(OUString("Table1.D2"), pDoc->GetNodes()[nD2CellNode]->GetTextNode()->GetText()); +} + CPPUNIT_TEST_SUITE_REGISTRATION(SwUiWriterTest); CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/sw/source/core/layout/sectfrm.cxx b/sw/source/core/layout/sectfrm.cxx index 8ae220e84272..fa47d70bc63f 100644 --- a/sw/source/core/layout/sectfrm.cxx +++ b/sw/source/core/layout/sectfrm.cxx @@ -1622,7 +1622,14 @@ SwLayoutFrame *SwFrame::GetNextSctLeaf( MakePageType eMakePage ) InsertPage(pOldLayLeaf ? pOldLayLeaf->FindPageFrame() : FindPageFrame(), false ); // and again the whole thing - pLayLeaf = pOldLayLeaf ? pOldLayLeaf : GetNextLayoutLeaf(); + if (pCellLeaf && CanContainSplitSection(this)) + // GetNextLayoutLeaf() would refer to the next cell in the same + // row, avoid that. pCellLeaf points to the correct cell in the + // follow table, and in the next round it'll be used, as we now + // have a next page. + pLayLeaf = pCellLeaf; + else + pLayLeaf = pOldLayLeaf ? pOldLayLeaf : GetNextLayoutLeaf(); continue; } break; -- cgit