sw/qa/extras/odfimport/data/i61225.sxw |binary sw/qa/extras/odfimport/odfimport.cxx | 8 + sw/qa/extras/uiwriter/data/i95698.odt |binary sw/qa/extras/uiwriter/data/lines-in-section-in-table.odt |binary sw/qa/extras/uiwriter/data/rhbz739252-3.odt |binary sw/qa/extras/uiwriter/uiwriter.cxx | 76 ++++++++++++++- sw/source/core/layout/findfrm.cxx | 13 ++ sw/source/core/layout/flowfrm.cxx | 5 sw/source/core/layout/sectfrm.cxx | 66 +++++++++++-- sw/source/core/layout/tabfrm.cxx | 12 ++ 10 files changed, 164 insertions(+), 16 deletions(-)
New commits: commit 5948b5b11b7428787d435a2788facccfefe5363a Author: Miklos Vajna <[email protected]> AuthorDate: Wed Aug 16 17:19:05 2017 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Mon Aug 13 15:16:23 2018 +0200 i#95698 sw: fix crash on splitting in-table section containing a nested table Found by crashtesting, ooo95698-1.odt crashed sw layout on load. The intended use-case is splitting section frames inside a table frame, so can just blacklist the non-interesting table-in-section-in-table case that causes the problem here. Reviewed-on: https://gerrit.libreoffice.org/41224 Reviewed-by: Miklos Vajna <[email protected]> Tested-by: Jenkins <[email protected]> (cherry picked from commit 802477ae75b39194442d9c01a1342d068c7b9300) Conflicts: sw/qa/extras/uiwriter/uiwriter.cxx Change-Id: Ic47cd8c46cc71f7eaa36b03ec2c4a5df8ca8051c (cherry picked from commit 0d644aeba1580865173840f6b41e6f2f0ac5ccf0) diff --git a/sw/qa/extras/uiwriter/data/i95698.odt b/sw/qa/extras/uiwriter/data/i95698.odt new file mode 100644 index 000000000000..9fe3ec207648 Binary files /dev/null and b/sw/qa/extras/uiwriter/data/i95698.odt differ diff --git a/sw/qa/extras/uiwriter/uiwriter.cxx b/sw/qa/extras/uiwriter/uiwriter.cxx index d67eed1af969..1493a80b5884 100644 --- a/sw/qa/extras/uiwriter/uiwriter.cxx +++ b/sw/qa/extras/uiwriter/uiwriter.cxx @@ -256,6 +256,7 @@ public: void testTdf108524(); void testTableInSection(); void testTableInNestedSection(); + void testTableInSectionInTable(); void testLinesInSectionInTable(); void testLinesMoveBackwardsInSectionInTable(); @@ -402,6 +403,7 @@ public: CPPUNIT_TEST(testTableInSection); CPPUNIT_TEST(testTableInNestedSection); CPPUNIT_TEST(testLinesInSectionInTable); + CPPUNIT_TEST(testTableInSectionInTable); CPPUNIT_TEST(testLinesMoveBackwardsInSectionInTable); CPPUNIT_TEST_SUITE_END(); @@ -5138,6 +5140,14 @@ void SwUiWriterTest::testTableInNestedSection() assertXPath(pXmlDoc, "//page[2]//section/tab", 1); } +void SwUiWriterTest::testTableInSectionInTable() +{ + // The document has a table, containing a section, containing a nested + // table. + // This crashed the layout. + createDoc("i95698.odt"); +} + CPPUNIT_TEST_SUITE_REGISTRATION(SwUiWriterTest); CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/sw/source/core/layout/tabfrm.cxx b/sw/source/core/layout/tabfrm.cxx index 0775d381843f..c30165ffa56d 100644 --- a/sw/source/core/layout/tabfrm.cxx +++ b/sw/source/core/layout/tabfrm.cxx @@ -925,9 +925,21 @@ static bool lcl_FindSectionsInRow( const SwRowFrame& rRow ) if (const SwFrame* pSectionLower = pTmpFrame->GetLower()) { if (!pSectionLower->IsColumnFrame()) + { // Section has a single column only, try to // split that. bRet = false; + + for (const SwFrame* pFrame = pSectionLower; pFrame; pFrame = pFrame->GetNext()) + { + if (pFrame->IsTabFrame()) + { + // Section contains a table, no split in that case. + bRet = true; + break; + } + } + } } } } commit 825d117002f6313bc6c8d823b5bcea266c8df9a9 Author: Miklos Vajna <[email protected]> AuthorDate: Fri Jul 14 16:05:10 2017 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Mon Aug 13 15:16:10 2018 +0200 CppunitTest_sw_uiwriter: disable testLinesMoveBackwards... on macOS No idea off the top of my head what is the problem here, seeing Linux and Windows is happy; clang on Linux as well. Change-Id: I56c79b37a5648d9afd02d8e161ea4a279cc89744 (cherry picked from commit 39dd0121f5994dee56f95bc57fae3323bf849a20) (cherry picked from commit d67b100f44f7c320af713355dc6022451347447d) diff --git a/sw/qa/extras/uiwriter/uiwriter.cxx b/sw/qa/extras/uiwriter/uiwriter.cxx index 542e5720fa8f..d67eed1af969 100644 --- a/sw/qa/extras/uiwriter/uiwriter.cxx +++ b/sw/qa/extras/uiwriter/uiwriter.cxx @@ -5087,6 +5087,7 @@ void SwUiWriterTest::testLinesInSectionInTable() void SwUiWriterTest::testLinesMoveBackwardsInSectionInTable() { +#ifndef MACOSX // Assert that paragraph "4" is on page 1 and "5" is on page 2. SwDoc* pDoc = createDoc("lines-in-section-in-table.odt"); xmlDocPtr pXmlDoc = parseLayoutDump(); @@ -5110,6 +5111,7 @@ void SwUiWriterTest::testLinesMoveBackwardsInSectionInTable() sal_uInt32 nPage1LastNode = getXPath(pXmlDoc, "/root/page[1]/body/tab/row/cell[1]/section/txt[last()]", "txtNodeIndex").toUInt32(); // This was "3", paragraph "4" was deleted, but "5" was not moved backwards from page 2. CPPUNIT_ASSERT_EQUAL(OUString("5"), pDoc->GetNodes()[nPage1LastNode]->GetTextNode()->GetText()); +#endif } void SwUiWriterTest::testTableInSection() commit e99eb63aaa115d3e75f648101b766f74f7e00527 Author: Miklos Vajna <[email protected]> AuthorDate: Fri Jul 14 14:20:36 2017 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Mon Aug 13 15:15:54 2018 +0200 tdf#108524 sw: handle sections inside tables in SwFrame::GetPrevSctLeaf() This addresses the sub-problem described in comment 12 of the bug, i.e. text frames are now moved to the first page from the second one when text frames are deleted on the first page. Change-Id: Ic0ede45381fb84b13d1ac02e4d1f39d817650616 Reviewed-on: https://gerrit.libreoffice.org/39946 Reviewed-by: Miklos Vajna <[email protected]> Tested-by: Jenkins <[email protected]> (cherry picked from commit 850bf99e7d1abcf2e0cce731b6715f87420d0ee6) (cherry picked from commit af65d584b2c62752bfb37e39048ee76dffad525b) diff --git a/sw/qa/extras/uiwriter/uiwriter.cxx b/sw/qa/extras/uiwriter/uiwriter.cxx index 2c3368d578dd..542e5720fa8f 100644 --- a/sw/qa/extras/uiwriter/uiwriter.cxx +++ b/sw/qa/extras/uiwriter/uiwriter.cxx @@ -257,6 +257,7 @@ public: void testTableInSection(); void testTableInNestedSection(); void testLinesInSectionInTable(); + void testLinesMoveBackwardsInSectionInTable(); CPPUNIT_TEST_SUITE(SwUiWriterTest); CPPUNIT_TEST(testReplaceForward); @@ -401,6 +402,7 @@ public: CPPUNIT_TEST(testTableInSection); CPPUNIT_TEST(testTableInNestedSection); CPPUNIT_TEST(testLinesInSectionInTable); + CPPUNIT_TEST(testLinesMoveBackwardsInSectionInTable); CPPUNIT_TEST_SUITE_END(); private: @@ -5083,6 +5085,33 @@ void SwUiWriterTest::testLinesInSectionInTable() assertXPath(pXmlDoc, "/root/page[2]/body/tab/row/cell/section", 1); } +void SwUiWriterTest::testLinesMoveBackwardsInSectionInTable() +{ + // Assert that paragraph "4" is on page 1 and "5" is on page 2. + SwDoc* pDoc = createDoc("lines-in-section-in-table.odt"); + xmlDocPtr pXmlDoc = parseLayoutDump(); + assertXPath(pXmlDoc, "/root/page", 2); + sal_uInt32 nPara4Node = getXPath(pXmlDoc, "/root/page[1]/body/tab/row/cell[1]/section/txt[last()]", "txtNodeIndex").toUInt32(); + CPPUNIT_ASSERT_EQUAL(OUString("4"), pDoc->GetNodes()[nPara4Node]->GetTextNode()->GetText()); + sal_uInt32 nPara5Node = getXPath(pXmlDoc, "/root/page[2]/body/tab/row/cell[1]/section/txt[1]", "txtNodeIndex").toUInt32(); + CPPUNIT_ASSERT_EQUAL(OUString("5"), pDoc->GetNodes()[nPara5Node]->GetTextNode()->GetText()); + + // Remove paragraph "4". + SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell(); + while (pWrtShell->GetCursor()->GetNode().GetIndex() < nPara4Node) + pWrtShell->Down(/*bSelect=*/false); + pWrtShell->EndPara(); + pWrtShell->Up(/*bSelect=*/true); + pWrtShell->DelLeft(); + + // Assert that paragraph "5" is now moved back to page 1 and is the last paragraph there. + discardDumpedLayout(); + pXmlDoc = parseLayoutDump(); + sal_uInt32 nPage1LastNode = getXPath(pXmlDoc, "/root/page[1]/body/tab/row/cell[1]/section/txt[last()]", "txtNodeIndex").toUInt32(); + // This was "3", paragraph "4" was deleted, but "5" was not moved backwards from page 2. + CPPUNIT_ASSERT_EQUAL(OUString("5"), pDoc->GetNodes()[nPage1LastNode]->GetTextNode()->GetText()); +} + void SwUiWriterTest::testTableInSection() { // The document has a section, containing a table that spans over 2 pages. diff --git a/sw/source/core/layout/sectfrm.cxx b/sw/source/core/layout/sectfrm.cxx index 3754f55b1997..f36820c5316c 100644 --- a/sw/source/core/layout/sectfrm.cxx +++ b/sw/source/core/layout/sectfrm.cxx @@ -1726,11 +1726,24 @@ SwLayoutFrame *SwFrame::GetPrevSctLeaf() if( bJump ) // Did we skip a blank page? SwFlowFrame::SetMoveBwdJump( true ); + SwSectionFrame *pSect = FindSctFrame(); + if (!pCol && pSect && IsInTab() && !IsInTableInSection(this)) + { + // We don't have a previous section yet, and we're in a + // section-in-table. + if (SwFlowFrame* pPrecede = pSect->GetPrecede()) + { + // Our section has a precede, work with that. + if (pPrecede->GetFrame().IsLayoutFrame()) + pCol = static_cast<SwLayoutFrame*>(&pPrecede->GetFrame()); + } + } + // Within sections in tables or section in headers/footers there can // be only one column change be made, one of the above shortcuts should // have applied, also when the section has a pPrev. // Now we even consider an empty column... - OSL_ENSURE( FindSctFrame(), "GetNextSctLeaf: Missing SectionFrame" ); + OSL_ENSURE( pSect, "GetNextSctLeaf: Missing SectionFrame" ); if( ( IsInTab() && !IsTabFrame() ) || FindFooterOrHeader() ) return pCol; @@ -1738,7 +1751,6 @@ SwLayoutFrame *SwFrame::GetPrevSctLeaf() // Precondition, which needs to be hold, is that the <this> frame can be // inside a table, but then the found section frame <pSect> is also inside // this table. - SwSectionFrame *pSect = FindSctFrame(); // #i95698# // A table cell containing directly a section does not break - see lcl_FindSectionsInRow(..) commit 71b30d43a2ea763bfae500595ff76c4a04e13dbc Author: Miklos Vajna <[email protected]> AuthorDate: Thu Jul 13 15:54:23 2017 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Mon Aug 13 15:15:38 2018 +0200 tdf#108524 sw: allow frames in follow sect-in-tables in SwFlowFrame::MoveBwd() The intention is to filter out text frames directly inside tables; since tables in general reflow by moving all of the content to the first page, then moving not fitting content to the next pages. Section frames are different, there we explicitly move content backwards, similarly to page body frames. Teach SwFlowFrame::MoveFwd() that a text frame inside a section-in-table is the section situation, not the table situation, since what matters here is the direct parent. To be on the safe side allow this for follow section frames only. This is necessary, but not enough to address the sub-problem described in comment 12 of the bug. At least SwFrame::GetPrevSctLeaf() is invoked to consider a precede section frame, though. Change-Id: Ic88602cffefbbc81ecc90e3880be2a098f60fb04 (cherry picked from commit 4908d6d6742e2f0700ea4ccd9d636d91ea281046) diff --git a/sw/source/core/layout/flowfrm.cxx b/sw/source/core/layout/flowfrm.cxx index 514e4a43eae5..76051e71aa46 100644 --- a/sw/source/core/layout/flowfrm.cxx +++ b/sw/source/core/layout/flowfrm.cxx @@ -2026,7 +2026,10 @@ bool SwFlowFrame::MoveBwd( bool &rbReformat ) { return false; } - if ( pUpperFrame->IsColumnFrame() && pUpperFrame->IsInSct() ) + // If the text frame is a follow-section-in-table, that can move + // backward as well. + bool bIsFollowSection = pUpperFrame->IsSctFrame() && static_cast<const SwSectionFrame*>(pUpperFrame)->GetPrecede(); + if ( ( pUpperFrame->IsColumnFrame() && pUpperFrame->IsInSct() ) || bIsFollowSection ) { break; } commit 1a7333ee9165a4583e11846abdb6df398e1c62cf Author: Miklos Vajna <[email protected]> AuthorDate: Wed Jul 12 17:53:40 2017 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Mon Aug 13 15:07:48 2018 +0200 tdf#108524 sw: allow frames in growable follow sects in SwFrame::IsMoveable() In general the move of frames inside growable sections is not allowed, so that the behavior is deterministic: either the section grows or content is moved to a follow section frame. But in case of split sections it is necessary to allow the move of text frames inside growable section frames, otherwise it is impossible to move text frames from the last follow of a section frame to the previous one. This is necessary, but not enough to address the sub-problem described in comment 12 of the bug. At least now SwFlowFrame::MoveBwd() is invoked to consider moving the frame backwards, though. Change-Id: I0b79e9db72a4e335701491dd6f7745058901e176 Reviewed-on: https://gerrit.libreoffice.org/39873 Reviewed-by: Miklos Vajna <[email protected]> Tested-by: Jenkins <[email protected]> (cherry picked from commit 5b9edf5d5779b9ce578084250a6c87808ddc5fa8) diff --git a/sw/source/core/layout/findfrm.cxx b/sw/source/core/layout/findfrm.cxx index 4a3d3722edd8..b4e61b7a19f4 100644 --- a/sw/source/core/layout/findfrm.cxx +++ b/sw/source/core/layout/findfrm.cxx @@ -1263,8 +1263,12 @@ static bool lcl_IsInSectionDirectly( const SwFrame *pUp ) if( pUp->IsColumnFrame() ) bSeenColumn = true; else if( pUp->IsSctFrame() ) + { + auto pSection = static_cast<const SwSectionFrame*>(pUp); // Allow move of frame in case our only column is not growable. - return bSeenColumn || !static_cast<const SwSectionFrame*>(pUp)->Growable(); + // Also allow if there is a previous section frame (to move back). + return bSeenColumn || !pSection->Growable() || pSection->GetPrecede(); + } else if( pUp->IsTabFrame() ) return false; pUp = pUp->GetUpper(); commit 1d716b5205b72ddce5b2d9f7be5c95d1baad6660 Author: Miklos Vajna <[email protected]> AuthorDate: Fri Jul 7 14:04:28 2017 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Mon Aug 13 15:06:55 2018 +0200 rhbz#739252 sw: fix crash on split tables inside nested sections Commit b5e0a143308e976b4165ff6181f4dccc3db0bd31 (tdf#108524 sw: attempt to split section frames inside table cells, take two, 2017-07-03) checked for tables in SwFrame::GetNextSctLeaf() when it considered looking up the next "follow" cell frame. But this is too general, in practice it is only necessary to look for follow cell frames in case the frame in question is in a table, but not in a table-in-section. This at the same time avoids a crash with tables inside nested sections, as it happens in the bugdoc. Reviewed-on: https://gerrit.libreoffice.org/39692 Reviewed-by: Miklos Vajna <[email protected]> Tested-by: Jenkins <[email protected]> (cherry picked from commit 652556ec3e9218655a67b4c4de4e26fbe81855de) [ This fixes the layout loop introduced 3 commits ago, so re-enable that test. ] Change-Id: If648cb477be5492c7158f89934435ca7021a6a63 diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx index f01ba1d82951..1097eddd5707 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx @@ -978,13 +978,11 @@ DECLARE_OOXMLEXPORT_TEST(testTdf99090_pgbrkAfterTable, "tdf99090_pgbrkAfterTable assertXPath(pXmlDoc, "/w:document/w:body/w:p/w:r/w:br", 1); } -#if 0 DECLARE_OOXMLEXPORT_TEST(testTdf96750_landscapeFollow, "tdf96750_landscapeFollow.docx") { uno::Reference<beans::XPropertySet> xStyle(getStyles("PageStyles")->getByName("Standard"), uno::UNO_QUERY); CPPUNIT_ASSERT_EQUAL(true, getProperty<bool>(xStyle, "IsLandscape")); } -#endif DECLARE_OOXMLEXPORT_TEST(testTdf86926_A3, "tdf86926_A3.docx") { diff --git a/sw/qa/extras/uiwriter/data/rhbz739252-3.odt b/sw/qa/extras/uiwriter/data/rhbz739252-3.odt new file mode 100644 index 000000000000..e457c035b662 Binary files /dev/null and b/sw/qa/extras/uiwriter/data/rhbz739252-3.odt differ diff --git a/sw/qa/extras/uiwriter/uiwriter.cxx b/sw/qa/extras/uiwriter/uiwriter.cxx index f9a13657fea2..2c3368d578dd 100644 --- a/sw/qa/extras/uiwriter/uiwriter.cxx +++ b/sw/qa/extras/uiwriter/uiwriter.cxx @@ -255,6 +255,7 @@ public: void testParagraphOfTextRange(); void testTdf108524(); void testTableInSection(); + void testTableInNestedSection(); void testLinesInSectionInTable(); CPPUNIT_TEST_SUITE(SwUiWriterTest); @@ -398,6 +399,7 @@ public: CPPUNIT_TEST(testParagraphOfTextRange); CPPUNIT_TEST(testTdf108524); CPPUNIT_TEST(testTableInSection); + CPPUNIT_TEST(testTableInNestedSection); CPPUNIT_TEST(testLinesInSectionInTable); CPPUNIT_TEST_SUITE_END(); @@ -5094,6 +5096,17 @@ void SwUiWriterTest::testTableInSection() assertXPath(pXmlDoc, "/root/page[2]/body/section/tab/row/cell", 2); } +void SwUiWriterTest::testTableInNestedSection() +{ + // The document has a nested section, containing a table that spans over 2 pages. + // This crashed the layout. + createDoc("rhbz739252-3.odt"); + xmlDocPtr pXmlDoc = parseLayoutDump(); + // Make sure the table is inside a section and spans over 2 pages. + assertXPath(pXmlDoc, "//page[1]//section/tab", 1); + assertXPath(pXmlDoc, "//page[2]//section/tab", 1); +} + 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 d48d6da09d63..3754f55b1997 100644 --- a/sw/source/core/layout/sectfrm.cxx +++ b/sw/source/core/layout/sectfrm.cxx @@ -1515,9 +1515,10 @@ SwLayoutFrame *SwFrame::GetNextSctLeaf( MakePageType eMakePage ) SwLayoutFrame *pLayLeaf; SwLayoutFrame* pCellLeaf = nullptr; - if (IsInTab()) + if (IsInTab() && !IsInTableInSection(this)) { - // We are in a table, see if there is a follow cell frame created already. + // We are in a table (which is itself not in a section), see if there + // is a follow cell frame created already. pCellLeaf = GetNextCellLeaf(); if (!pCellLeaf) { commit ae8f5057150a13b6fba482e9abe9a6e133f80056 Author: Miklos Vajna <[email protected]> AuthorDate: Thu Jul 6 14:07:45 2017 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Mon Aug 13 15:04:58 2018 +0200 i#61225 sw: fix layout loop with growable single-column sections Commit 6ade80cf142664e78954c7544534e9436ceb90c7 (tdf#108524 sw: allow move of frame inside section without columns, 2017-06-16) relaxed lcl_IsInSectionDirectly() used in SwFrame::IsMoveable() to allow move for all section frame contents if it has a single column. That looked safe, as the multiple column case was already allowed. There is one situation where this still causes a problem: when the section has a single column and the section frame is growable -- as in that case we should grow the section frame, not move the contents. So go back to unconditionally allowing multi-column section contents and allow single-column section contents only in case the section frame is now growable. With this, ooo61225-1.sxw from the crashtesting corpus can be opened again without a layout loop. Reviewed-on: https://gerrit.libreoffice.org/39653 Reviewed-by: Miklos Vajna <[email protected]> Tested-by: Jenkins <[email protected]> (cherry picked from commit 972fbddf80510f7daaf2128dbfda01c0e7535020) Conflicts: sw/qa/extras/odfimport/odfimport.cxx Change-Id: Ib2d3702a33da8e62b9bbf468d558ae16db8aa94b diff --git a/sw/qa/extras/odfimport/data/i61225.sxw b/sw/qa/extras/odfimport/data/i61225.sxw new file mode 100644 index 000000000000..4f43541995c3 Binary files /dev/null and b/sw/qa/extras/odfimport/data/i61225.sxw differ diff --git a/sw/qa/extras/odfimport/odfimport.cxx b/sw/qa/extras/odfimport/odfimport.cxx index 6eda80f3febf..5b501b3003a2 100644 --- a/sw/qa/extras/odfimport/odfimport.cxx +++ b/sw/qa/extras/odfimport/odfimport.cxx @@ -867,5 +867,13 @@ DECLARE_ODFIMPORT_TEST(testTdf115079, "tdf115079.odt") // This document caused segfault when layouting } +DECLARE_ODFIMPORT_TEST(testI61225, "i61225.sxw") +{ + // Part of ooo61225-1.sxw from crashtesting. + + // This never returned. + calcLayout(); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/source/core/layout/findfrm.cxx b/sw/source/core/layout/findfrm.cxx index 74942256bef4..4a3d3722edd8 100644 --- a/sw/source/core/layout/findfrm.cxx +++ b/sw/source/core/layout/findfrm.cxx @@ -1256,10 +1256,15 @@ void SwFrame::InvalidateNextPrtArea() /// but not if it sits in a table which itself sits in a section. static bool lcl_IsInSectionDirectly( const SwFrame *pUp ) { + bool bSeenColumn = false; + while( pUp ) { - if( pUp->IsSctFrame() ) - return true; + if( pUp->IsColumnFrame() ) + bSeenColumn = true; + else if( pUp->IsSctFrame() ) + // Allow move of frame in case our only column is not growable. + return bSeenColumn || !static_cast<const SwSectionFrame*>(pUp)->Growable(); else if( pUp->IsTabFrame() ) return false; pUp = pUp->GetUpper(); commit 3ff51ef04f49b9acba17b1a80cbfeb81d88e3218 Author: Miklos Vajna <[email protected]> AuthorDate: Thu Jul 6 11:16:54 2017 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Mon Aug 13 15:02:25 2018 +0200 tdf#108524 sw: split section frames inside table cells, non-split text frames Commit f991b842addddeada6dc45c4054deeca5aa7f17b (tdf#108524 sw: attempt to split section frames inside table cells, 2017-06-19) added initial support for multi-page sections inside a table cell, but turns out this only worked in case at the split point there was a long enough paragraph, so it was split into two (a "master" text frame and a "follow" one), and then the follow was moved to the next page by SwContentFrame::MakeAll(), with the MoveFwd() call in the "If a Follow sits next to its Master and doesn't fit, we know it can be moved right now." block. However, if the section contains lots of one-liner text frames, then all of them are masters, so the above code doesn't move them to the next page; so the section frame is still not split. Fix the problem by allowing the move of frames inside table-in-sections in SwSectionFrame::MoveAllowed(), that way SwTextFrame::AdjustFrame() will not set the text frame as undersized, so at the end SwContentFrame::MakeAll() will call MoveFwd() in the "If a column section can't find any space for its first ContentFrame" block. With this the split of text frames in section-in-table frames is consistent regardless if they are of multiple or single lines. Change-Id: Ief9d62da3fd8a5c707e1f9489a92f7a81e7b38ac Reviewed-on: https://gerrit.libreoffice.org/39623 Reviewed-by: Miklos Vajna <[email protected]> Tested-by: Jenkins <[email protected]> (cherry picked from commit f8a76d218305a56d15b82b9dac4fafa558872780) diff --git a/sw/qa/extras/uiwriter/data/lines-in-section-in-table.odt b/sw/qa/extras/uiwriter/data/lines-in-section-in-table.odt new file mode 100644 index 000000000000..4f0abd6e5e88 Binary files /dev/null and b/sw/qa/extras/uiwriter/data/lines-in-section-in-table.odt differ diff --git a/sw/qa/extras/uiwriter/uiwriter.cxx b/sw/qa/extras/uiwriter/uiwriter.cxx index 77d7894e1454..f9a13657fea2 100644 --- a/sw/qa/extras/uiwriter/uiwriter.cxx +++ b/sw/qa/extras/uiwriter/uiwriter.cxx @@ -255,6 +255,7 @@ public: void testParagraphOfTextRange(); void testTdf108524(); void testTableInSection(); + void testLinesInSectionInTable(); CPPUNIT_TEST_SUITE(SwUiWriterTest); CPPUNIT_TEST(testReplaceForward); @@ -397,6 +398,7 @@ public: CPPUNIT_TEST(testParagraphOfTextRange); CPPUNIT_TEST(testTdf108524); CPPUNIT_TEST(testTableInSection); + CPPUNIT_TEST(testLinesInSectionInTable); CPPUNIT_TEST_SUITE_END(); private: @@ -5063,6 +5065,22 @@ void SwUiWriterTest::testTdf108524() assertXPath(pXmlDoc, "/root/page[2]/body/tab/row/cell/section", 1); } +void SwUiWriterTest::testLinesInSectionInTable() +{ + // This is similar to testTdf108524(), but the page boundary now is not in + // the middle of a multi-line paragraph: the section only contains oneliner + // paragraphs instead. + createDoc("lines-in-section-in-table.odt"); + xmlDocPtr pXmlDoc = parseLayoutDump(); + // In total we expect two cells containing a section. + assertXPath(pXmlDoc, "/root/page/body/tab/row/cell/section", 2); + + assertXPath(pXmlDoc, "/root/page[1]/body/tab/row/cell/section", 1); + // This was 0, section wasn't split, instead it was only on the first page + // and it was cut off. + assertXPath(pXmlDoc, "/root/page[2]/body/tab/row/cell/section", 1); +} + void SwUiWriterTest::testTableInSection() { // The document has a section, containing a table that spans over 2 pages. diff --git a/sw/source/core/layout/sectfrm.cxx b/sw/source/core/layout/sectfrm.cxx index 34d0e696b320..d48d6da09d63 100644 --- a/sw/source/core/layout/sectfrm.cxx +++ b/sw/source/core/layout/sectfrm.cxx @@ -590,7 +590,7 @@ namespace } /// Checks if pFrame is in a table, which itself is in a section. - bool IsInTableInSection(SwFrame* pFrame) + bool IsInTableInSection(const SwFrame* pFrame) { if (!pFrame->IsInTab()) return false; @@ -2160,8 +2160,8 @@ bool SwSectionFrame::MoveAllowed( const SwFrame* pFrame) const return false; // Now it has to be examined whether there is a layout sheet wherein // a section Follow can be created - if( IsInTab() || ( !IsInDocBody() && FindFooterOrHeader() ) ) - return false; // It doesn't work in tables/headers/footers + if( IsInTableInSection(this) || ( !IsInDocBody() && FindFooterOrHeader() ) ) + return false; // It doesn't work in table-in-sections/headers/footers if( IsInFly() ) // In column based or chained frames return nullptr != const_cast<SwFrame*>(static_cast<SwFrame const *>(GetUpper()))->GetNextLeaf( MAKEPAGE_NONE ); return true; commit c8e321877ffdf2c74825f464d329c05953e95391 Author: Miklos Vajna <[email protected]> AuthorDate: Mon Jul 3 13:53:28 2017 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Mon Aug 13 14:57:22 2018 +0200 tdf#108524 sw: attempt to split section frames inside table cells, take two Tables-in-sections were already split across multiple pages, but not secions-in-tables. To be safe still don't allow sections-in-tables-in-sections, so you can combine these in both orders now, but not recursively. To achieve this, relax two "not in table" conditions to just require "not in a table that is already in a section", and define that in case a section-in-table is to be split, the follow section frame should be inserted under the follow of its cell. With this, finally the section frame in the bugdoc is split into two, and the second section frame is moved to the second page as expected. This restores commit f991b842addddeada6dc45c4054deeca5aa7f17b, but this second take makes sure that we do not crash while laying out ooo61225-1.sxw. GetNextSctLeaf() used to assume that SwTableFrame::Split() created the follow cell frames; and when that wasn't the case, it crashed. Still don't attempt to create the cell frame in GetNextSctLeaf(), but handle when the cell follow frame is missing, and return early, as it used to be the case before this commit. Reviewed-on: https://gerrit.libreoffice.org/39471 Reviewed-by: Miklos Vajna <[email protected]> Tested-by: Jenkins <[email protected]> (cherry picked from commit b5e0a143308e976b4165ff6181f4dccc3db0bd31) Conflicts: sw/qa/extras/uiwriter/uiwriter.cxx [ testTdf96750_landscapeFollow is disabled as it passes on master, so it's expected that a later commit will fix it. ] Change-Id: I9dcc76b4c61b39b9d23b140b84420e89cf274cf3 diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx index 1097eddd5707..f01ba1d82951 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx @@ -978,11 +978,13 @@ DECLARE_OOXMLEXPORT_TEST(testTdf99090_pgbrkAfterTable, "tdf99090_pgbrkAfterTable assertXPath(pXmlDoc, "/w:document/w:body/w:p/w:r/w:br", 1); } +#if 0 DECLARE_OOXMLEXPORT_TEST(testTdf96750_landscapeFollow, "tdf96750_landscapeFollow.docx") { uno::Reference<beans::XPropertySet> xStyle(getStyles("PageStyles")->getByName("Standard"), uno::UNO_QUERY); CPPUNIT_ASSERT_EQUAL(true, getProperty<bool>(xStyle, "IsLandscape")); } +#endif DECLARE_OOXMLEXPORT_TEST(testTdf86926_A3, "tdf86926_A3.docx") { diff --git a/sw/qa/extras/uiwriter/uiwriter.cxx b/sw/qa/extras/uiwriter/uiwriter.cxx index d1e2cbd8ef2c..77d7894e1454 100644 --- a/sw/qa/extras/uiwriter/uiwriter.cxx +++ b/sw/qa/extras/uiwriter/uiwriter.cxx @@ -253,9 +253,7 @@ public: void testTdf107976(); void testTdf113790(); void testParagraphOfTextRange(); -#if 0 void testTdf108524(); -#endif void testTableInSection(); CPPUNIT_TEST_SUITE(SwUiWriterTest); @@ -397,9 +395,7 @@ public: CPPUNIT_TEST(testTdf107976); CPPUNIT_TEST(testTdf113790); CPPUNIT_TEST(testParagraphOfTextRange); -#if 0 CPPUNIT_TEST(testTdf108524); -#endif CPPUNIT_TEST(testTableInSection); CPPUNIT_TEST_SUITE_END(); @@ -5054,7 +5050,6 @@ void SwUiWriterTest::testParagraphOfTextRange() CPPUNIT_ASSERT_EQUAL(OUString("In section"), xParagraph->getString()); } -#if 0 void SwUiWriterTest::testTdf108524() { createDoc("tdf108524.odt"); @@ -5067,7 +5062,6 @@ void SwUiWriterTest::testTdf108524() // and it was cut off. assertXPath(pXmlDoc, "/root/page[2]/body/tab/row/cell/section", 1); } -#endif void SwUiWriterTest::testTableInSection() { diff --git a/sw/source/core/layout/sectfrm.cxx b/sw/source/core/layout/sectfrm.cxx index 7dd2e43a976c..34d0e696b320 100644 --- a/sw/source/core/layout/sectfrm.cxx +++ b/sw/source/core/layout/sectfrm.cxx @@ -22,6 +22,7 @@ #include <fmtftn.hxx> #include <fmtclbl.hxx> #include "sectfrm.hxx" +#include "cellfrm.hxx" #include "section.hxx" #include <IDocumentSettingAccess.hxx> #include "rootfrm.hxx" @@ -587,6 +588,16 @@ namespace return pLayFrame->GetNextLayoutLeaf(); return pLayFrame; } + + /// Checks if pFrame is in a table, which itself is in a section. + bool IsInTableInSection(SwFrame* pFrame) + { + if (!pFrame->IsInTab()) + return false; + + // The frame is in a table, see if the table is in a section. + return pFrame->FindTabFrame()->IsInSct(); + } } void SwSectionFrame::MoveContentAndDelete( SwSectionFrame* pDel, bool bSave ) @@ -1439,9 +1450,9 @@ SwLayoutFrame *SwFrame::GetNextSctLeaf( MakePageType eMakePage ) return static_cast<SwLayoutFrame*>(static_cast<SwLayoutFrame*>(GetUpper()->GetNext())->Lower()); if( GetUpper()->IsColBodyFrame() && GetUpper()->GetUpper()->GetNext() ) return static_cast<SwLayoutFrame*>(static_cast<SwLayoutFrame*>(GetUpper()->GetUpper()->GetNext())->Lower()); - // Inside a section, in tables, or sections of headers/footers, there can be only + // Inside a table-in-section, or sections of headers/footers, there can be only // one column shift be made, one of the above shortcuts should have applied! - if( GetUpper()->IsInTab() || FindFooterOrHeader() ) + if( IsInTableInSection(GetUpper()) || FindFooterOrHeader() ) return nullptr; SwSectionFrame *pSect = FindSctFrame(); @@ -1498,7 +1509,23 @@ SwLayoutFrame *SwFrame::GetNextSctLeaf( MakePageType eMakePage ) const bool bBody = IsInDocBody(); const bool bFootnotePage = FindPageFrame()->IsFootnotePage(); + // The "pLayLeaf is in a table" case is rejected by default, so that it + // can't happen that we try to move a table to one of its own cells. + bool bLayLeafTableAllowed = false; SwLayoutFrame *pLayLeaf; + + SwLayoutFrame* pCellLeaf = nullptr; + if (IsInTab()) + { + // We are in a table, see if there is a follow cell frame created already. + pCellLeaf = GetNextCellLeaf(); + if (!pCellLeaf) + { + SAL_WARN("sw.layout", "section is in table, but the table is not split"); + return nullptr; + } + } + // A shortcut for TabFrames such that not all cells need to be visited if( bWrongPage ) pLayLeaf = nullptr; @@ -1507,6 +1534,16 @@ SwLayoutFrame *SwFrame::GetNextSctLeaf( MakePageType eMakePage ) SwContentFrame* pTmpCnt = static_cast<SwTabFrame*>(this)->FindLastContent(); pLayLeaf = pTmpCnt ? pTmpCnt->GetUpper() : nullptr; } + else if (pCellLeaf && !IsInTableInSection(this)) + { + // This frame is in a table-not-in-section, its follow should be + // inserted under the follow of the frame's cell. + pLayLeaf = pCellLeaf; + if (pLayLeaf->FindTabFrame() == FindTabFrame()) + SAL_WARN("sw.layout", "my table frame and my follow's table frame is the same"); + // In this case pLayLeaf pointing to an in-table frame is OK. + bLayLeafTableAllowed = true; + } else { pLayLeaf = GetNextLayoutLeaf(); @@ -1534,10 +1571,10 @@ SwLayoutFrame *SwFrame::GetNextSctLeaf( MakePageType eMakePage ) pLayLeaf = nullptr; continue; } - // Once inBody always inBody, don't step into tables and not into other sections + // Once inBody always inBody, don't step into tables-in-sections and not into other sections if ( (bBody && !pLayLeaf->IsInDocBody()) || (IsInFootnote() != pLayLeaf->IsInFootnote() ) || - pLayLeaf->IsInTab() || + (pLayLeaf->IsInTab() && !bLayLeafTableAllowed) || ( pLayLeaf->IsInSct() && ( !pSect->HasFollow() || pSect->GetFollow() != pLayLeaf->FindSctFrame() ) ) ) { _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
