sw/qa/core/layout/data/floattable-nested.odt |binary
sw/qa/core/layout/data/table-in-footnote.docx |binary
sw/qa/core/layout/flycnt.cxx | 55
++++++++++
sw/source/core/layout/fly.cxx | 6 +
sw/source/core/layout/flycnt.cxx | 21 +++
sw/source/core/layout/tabfrm.cxx | 19 ++-
sw/source/core/objectpositioning/tocntntanchoredobjectposition.cxx | 4
7 files changed, 99 insertions(+), 6 deletions(-)
New commits:
commit 317305fb31447ff8400f81354c869d862715539a
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Wed Sep 6 08:30:10 2023 +0200
Commit: Xisco Fauli <xiscofa...@libreoffice.org>
CommitDate: Thu Sep 7 11:29:00 2023 +0200
sw floattable, nesting: fix layout crash
The manually created ODT bugdoc has a nested floating table. The outer
table is just 1 cell, the inner one has 2 rows: it's meant to span over
2 pages. This bugdoc currently crashes the layout.
In practice what happens is that the inner fly would split + move the
follow to the next page, so we hit a case where the inner fly is marked
to be "in table", but it won't have a table parent anymore, so the
layout crashes.
As a first step, fix the crash:
1) SwFrame::GetNextFlyLeaf() should only split the inner fly and its
anchor (not move it), the move will happen with the split of the
outer fly.
2) Fix SwToContentAnchoredObjectPosition::CalcPosition() to move the
inner, follow fly down (it should not overlap with its master). This
was not visible previously, as we manually moved the follow fly to the
next page, but now we need a vertical cut position that is between the
inner master and follow flys.
3) lcl_ArrangeLowers(), called by SwCellFrame::Format() now updates the
position of follow flys, so exactly two master flys are on page 1 and
two follow flys are on page 2.
This is related to tdf#55160, but it's not yet focusing on DOCX, where
nested floating tables are currently disabled at a filter level. Also,
this fixes the crash, but the position of the inner follow table still
needs fixing.
Change-Id: Ife42b64cda53946a8262ddabfd000803620c4ff1
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/156587
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
Tested-by: Jenkins
Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/156639
diff --git a/sw/qa/core/layout/data/floattable-nested.odt
b/sw/qa/core/layout/data/floattable-nested.odt
new file mode 100644
index 000000000000..3d21314f5871
Binary files /dev/null and b/sw/qa/core/layout/data/floattable-nested.odt differ
diff --git a/sw/qa/core/layout/flycnt.cxx b/sw/qa/core/layout/flycnt.cxx
index 539d379d97c2..0839e6a34b36 100644
--- a/sw/qa/core/layout/flycnt.cxx
+++ b/sw/qa/core/layout/flycnt.cxx
@@ -1024,6 +1024,47 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyFromAsCharAnchor)
// frame+table inside a footnote.
dispatchCommand(mxComponent, ".uno:SetAnchorToPara", {});
}
+
+CPPUNIT_TEST_FIXTURE(Test, testSplitFlyNested)
+{
+ // Given a document with a nested, multi-page floating table:
+ // When calculating the layout:
+ createSwDoc("floattable-nested.odt");
+ calcLayout();
+
+ // Then make sure we don't crash:
+ // Without the accompanying fix in place, this test would have crashed.
+ // Check that we have exactly 4 fly frames, all of them on the expected
pages: master outer,
+ // follow outer, master inner and follow inner.
+ SwDoc* pDoc = getSwDoc();
+ SwRootFrame* pLayout = pDoc->getIDocumentLayoutAccess().GetCurrentLayout();
+ auto pPage1 = pLayout->Lower()->DynCastPageFrame();
+ CPPUNIT_ASSERT(pPage1);
+ CPPUNIT_ASSERT(pPage1->GetSortedObjs());
+ SwSortedObjs& rPage1Objs = *pPage1->GetSortedObjs();
+ CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), rPage1Objs.size());
+ auto pPage1Fly1 =
rPage1Objs[0]->DynCastFlyFrame()->DynCastFlyAtContentFrame();
+ CPPUNIT_ASSERT(pPage1Fly1);
+ CPPUNIT_ASSERT(pPage1Fly1->GetAnchorFrameContainingAnchPos()->IsInFly());
+ CPPUNIT_ASSERT(pPage1Fly1->GetFollow());
+ auto pPage1Fly2 =
rPage1Objs[1]->DynCastFlyFrame()->DynCastFlyAtContentFrame();
+ CPPUNIT_ASSERT(pPage1Fly2);
+ CPPUNIT_ASSERT(!pPage1Fly2->GetAnchorFrameContainingAnchPos()->IsInFly());
+ CPPUNIT_ASSERT(pPage1Fly2->GetFollow());
+ auto pPage2 = pPage1->GetNext()->DynCastPageFrame();
+ CPPUNIT_ASSERT(pPage2);
+ CPPUNIT_ASSERT(pPage2->GetSortedObjs());
+ SwSortedObjs& rPage2Objs = *pPage2->GetSortedObjs();
+ CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), rPage2Objs.size());
+ auto pPage2Fly1 =
rPage2Objs[0]->DynCastFlyFrame()->DynCastFlyAtContentFrame();
+ CPPUNIT_ASSERT(pPage2Fly1);
+ CPPUNIT_ASSERT(pPage2Fly1->GetAnchorFrameContainingAnchPos()->IsInFly());
+ CPPUNIT_ASSERT(pPage2Fly1->GetPrecede());
+ auto pPage2Fly2 =
rPage2Objs[1]->DynCastFlyFrame()->DynCastFlyAtContentFrame();
+ CPPUNIT_ASSERT(pPage2Fly2);
+ CPPUNIT_ASSERT(!pPage2Fly2->GetAnchorFrameContainingAnchPos()->IsInFly());
+ CPPUNIT_ASSERT(pPage2Fly2->GetPrecede());
+}
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/layout/flycnt.cxx b/sw/source/core/layout/flycnt.cxx
index 113eaafbce9d..efdc91365c26 100644
--- a/sw/source/core/layout/flycnt.cxx
+++ b/sw/source/core/layout/flycnt.cxx
@@ -1591,6 +1591,7 @@ SwLayoutFrame *SwFrame::GetNextFlyLeaf( MakePageType
eMakePage )
}
SwLayoutFrame* pOldLayLeaf = nullptr;
+ bool bNesting = false;
while (true)
{
if (pLayLeaf)
@@ -1609,6 +1610,18 @@ SwLayoutFrame *SwFrame::GetNextFlyLeaf( MakePageType
eMakePage )
bSameBody = true;
}
}
+
+ if (bLeftFly && pFlyAnchor && pFlyAnchor->IsInFly()
+ && FindFlyFrame() == pLayLeaf->FindFlyFrame())
+ {
+ // This is an inner fly, then the follow anchor will be just
next to us.
+ SwLayoutFrame* pFlyAnchorUpper = pFlyAnchor->GetUpper();
+ pOldLayLeaf = pLayLeaf;
+ pLayLeaf = pFlyAnchorUpper;
+ bLeftFly = false;
+ bNesting = true;
+ }
+
if (bLeftBody || bLeftFly || bSameBody)
{
// The above conditions are not held, reject.
@@ -1650,8 +1663,12 @@ SwLayoutFrame *SwFrame::GetNextFlyLeaf( MakePageType
eMakePage )
// Split the anchor at char 0: all the content goes to the follow
of the anchor.
pFlyAnchor->SplitFrame(TextFrameIndex(0));
auto pNext = static_cast<SwTextFrame*>(pFlyAnchor->GetNext());
- // Move the new anchor frame, before the first child of pLayLeaf.
- pNext->MoveSubTree(pLayLeaf, pLayLeaf->Lower());
+ // The nesting case just splits the inner fly; the outer fly will
split and move.
+ if (!bNesting)
+ {
+ // Move the new anchor frame, before the first child of
pLayLeaf.
+ pNext->MoveSubTree(pLayLeaf, pLayLeaf->Lower());
+ }
// Now create the follow of the fly and anchor it in the master of
the anchor.
pNew = new SwFlyAtContentFrame(*pFly);
diff --git a/sw/source/core/layout/tabfrm.cxx b/sw/source/core/layout/tabfrm.cxx
index 02e3dbdc187d..e592467751b7 100644
--- a/sw/source/core/layout/tabfrm.cxx
+++ b/sw/source/core/layout/tabfrm.cxx
@@ -5252,11 +5252,24 @@ static bool lcl_ArrangeLowers( SwLayoutFrame *pLay,
tools::Long lYStart, bool bI
lcl_ArrangeLowers( static_cast<SwLayoutFrame*>(pFrame),
aRectFnSet.GetTop(static_cast<SwLayoutFrame*>(pFrame)->Lower()->getFrameArea())
+ lDiffX, bInva );
- if ( pFrame->GetDrawObjs() )
+ SwSortedObjs* pDrawObjs = pFrame->GetDrawObjs();
+ auto pTextFrame = pFrame->DynCastTextFrame();
+ if (pTextFrame && pTextFrame->IsInFly())
{
- for ( size_t i = 0; i < pFrame->GetDrawObjs()->size(); ++i )
+ // See if this is a follow anchor. If so, we want the flys
anchored in the master
+ // which are also lowers of pFrame.
+ SwTextFrame* pMaster = pTextFrame;
+ while (pMaster->IsFollow())
{
- SwAnchoredObject* pAnchoredObj =
(*pFrame->GetDrawObjs())[i];
+ pMaster = pMaster->FindMaster();
+ }
+ pDrawObjs = pMaster->GetDrawObjs();
+ }
+ if (pDrawObjs)
+ {
+ for (size_t i = 0; i < pDrawObjs->size(); ++i)
+ {
+ SwAnchoredObject* pAnchoredObj = (*pDrawObjs)[i];
// #i26945# - check, if anchored object
// is lower of layout frame by checking, if the anchor
// frame, which contains the anchor position, is a lower
diff --git a/sw/source/core/objectpositioning/tocntntanchoredobjectposition.cxx
b/sw/source/core/objectpositioning/tocntntanchoredobjectposition.cxx
index 73b8b088d1f7..9c1c491b24c9 100644
--- a/sw/source/core/objectpositioning/tocntntanchoredobjectposition.cxx
+++ b/sw/source/core/objectpositioning/tocntntanchoredobjectposition.cxx
@@ -492,11 +492,13 @@ void SwToContentAnchoredObjectPosition::CalcPosition()
// same page as <pOrientFrame> and the vertical position isn't
aligned
// automatic at the anchor character or the top of the line of the
// anchor character, the anchor frame determines the vertical
position.
+ // Split fly follows: always let the anchor char frame determine
the vertical position.
+ // This gives us a vertical cut position between the master and
the follow.
if ( &rAnchorTextFrame == pOrientFrame ||
( rAnchorTextFrame.FindPageFrame() ==
pOrientFrame->FindPageFrame() &&
aVert.GetVertOrient() == text::VertOrientation::NONE &&
aVert.GetRelationOrient() != text::RelOrientation::CHAR &&
- aVert.GetRelationOrient() !=
text::RelOrientation::TEXT_LINE ) )
+ aVert.GetRelationOrient() !=
text::RelOrientation::TEXT_LINE && !bFollowSplitFly ) )
{
pUpperOfOrientFrame = rAnchorTextFrame.GetUpper();
pAnchorFrameForVertPos = &rAnchorTextFrame;
commit ec29ef8aec201b5e4187c08bf740042f0fe704a0
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Thu Aug 24 16:58:14 2023 +0200
Commit: Xisco Fauli <xiscofa...@libreoffice.org>
CommitDate: Thu Sep 7 11:28:53 2023 +0200
sw floattable: don't split if anchored inside a footnote
See
<https://gerrit.libreoffice.org/c/core/+/156025/1#message-94d8cdd4d59a4b991186e5780d41f213105a533d>,
if we import an (inline) table inside a footnote from DOCX, we currently
put that into an as-char anchored fly. In case the user would later
change the anchor type to to-para, then we would crash in
SwFlowFrame::GetPrevFrameForUpperSpaceCalc_().
Avoid all this trouble by simply not allowing split floating tables in
footnotes, seeing that the Word UI is also disabled for this case.
Change-Id: I3cf9461beb291084e1cced5c66ca14e6b6d55126
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/156058
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
Tested-by: Jenkins
Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/156644
diff --git a/sw/qa/core/layout/data/table-in-footnote.docx
b/sw/qa/core/layout/data/table-in-footnote.docx
new file mode 100644
index 000000000000..10e20ffbf44c
Binary files /dev/null and b/sw/qa/core/layout/data/table-in-footnote.docx
differ
diff --git a/sw/qa/core/layout/flycnt.cxx b/sw/qa/core/layout/flycnt.cxx
index ba688728743b..539d379d97c2 100644
--- a/sw/qa/core/layout/flycnt.cxx
+++ b/sw/qa/core/layout/flycnt.cxx
@@ -1010,6 +1010,20 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyIntoTable)
// second part of a floating table into a table on the next page, not
before that table.
calcLayout();
}
+
+CPPUNIT_TEST_FIXTURE(Test, testSplitFlyFromAsCharAnchor)
+{
+ // Given a document with a footnote that has a table (imported in an
as-char anchored frame in
+ // Writer):
+ createSwDoc("table-in-footnote.docx");
+
+ // When changing the anchor type of that frame to to-para:
+ // Then make sure we don't crash:
+ selectShape(1);
+ // Without the accompanying fix in place, this test would have crashed, we
tried to split a
+ // frame+table inside a footnote.
+ dispatchCommand(mxComponent, ".uno:SetAnchorToPara", {});
+}
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/layout/fly.cxx b/sw/source/core/layout/fly.cxx
index a06c2a9e7f70..039aeb549f77 100644
--- a/sw/source/core/layout/fly.cxx
+++ b/sw/source/core/layout/fly.cxx
@@ -687,6 +687,12 @@ bool SwFlyFrame::IsFlySplitAllowed() const
return false;
}
+ if (pFlyAnchor && pFlyAnchor->IsInFootnote())
+ {
+ // No split in footnotes.
+ return false;
+ }
+
const SwFlyFrameFormat* pFormat = GetFormat();
const SwFormatVertOrient& rVertOrient = pFormat->GetVertOrient();
if (rVertOrient.GetVertOrient() == text::VertOrientation::BOTTOM)
