sw/qa/uibase/wrtsh/wrtsh.cxx | 67 ++++++++++++++++++++++++++++++++++++++ sw/source/core/inc/txtfrm.hxx | 2 + sw/source/core/text/itratr.cxx | 5 ++ sw/source/uibase/wrtsh/delete.cxx | 23 +++++++++++++ 4 files changed, 97 insertions(+)
New commits: commit 5b0256f30ee154edb28b999bc4faba2453fc32b8 Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Tue Sep 3 08:17:23 2024 +0200 Commit: Miklos Vajna <vmik...@collabora.com> CommitDate: Tue Sep 3 09:53:17 2024 +0200 tdf#162507 sw floattable: fix not wanted join of two different split anchors Open the bugdoc, go to the end of the "Cannot press Delete here" paragraph, press delete, layout hangs. The trouble seems to be that this creates a doc model where multiple multi-page floating tables are anchored to the same paragraph, which is not something the layout supports. We explicitly avoid this at import time from DOCX since commit 01ad8ec4bb5425446e95dbada81de435646824b4 (sw floattable: fix lost tables around a floating table from DOCX, 2023-06-05), but there was no mechanism to prevent the UI creating the same unsupported doc model. Fix the problem by extending SwWrtShell::DelRight() to prevent joining 2 paragraphs in the unwanted case, interesting Word doesn't allow deleting at the end of that paragraph, either. Regression from commit 693ad3aadbf84afa750af73c330fe7e09b38a2e7 (tdf#120262 sw floattable, legacy: go outside body only for page frame vert pos, 2023-07-31), which restricted the amount of cases where some weird legacy behavior was performed, so good to keep that side as-is. Change-Id: I4d8a6702d8ac1689690fa464014c99fcd5ef7bd2 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/172780 Reviewed-by: Miklos Vajna <vmik...@collabora.com> Tested-by: Jenkins diff --git a/sw/qa/uibase/wrtsh/wrtsh.cxx b/sw/qa/uibase/wrtsh/wrtsh.cxx index 9c19c4474b9d..467068b69afc 100644 --- a/sw/qa/uibase/wrtsh/wrtsh.cxx +++ b/sw/qa/uibase/wrtsh/wrtsh.cxx @@ -28,6 +28,9 @@ #include <textcontentcontrol.hxx> #include <fmtanchr.hxx> #include <view.hxx> +#include <itabenum.hxx> +#include <frmmgr.hxx> +#include <formatflysplit.hxx> namespace { @@ -451,6 +454,70 @@ CPPUNIT_TEST_FIXTURE(Test, testInsertComboBoxContentControl) std::shared_ptr<SwContentControl> pContentControl = rFormatContentControl.GetContentControl(); CPPUNIT_ASSERT(pContentControl->GetComboBox()); } + +void InsertSplitFly(SwWrtShell* pWrtShell) +{ + SwPosition aInsertPos = *pWrtShell->GetCursor()->GetPoint(); + // Insert a table: + SwInsertTableOptions aTableOptions(SwInsertTableFlags::DefaultBorder, 0); + pWrtShell->InsertTable(aTableOptions, /*nRows=*/2, /*nCols=*/1); + pWrtShell->MoveTable(GotoPrevTable, fnTableStart); + pWrtShell->GoPrevCell(); + pWrtShell->Insert(u"A1"_ustr); + // Select cell: + pWrtShell->SelAll(); + // Select table: + pWrtShell->SelAll(); + // Wrap the table in a text frame: + SwFlyFrameAttrMgr aMgr(true, pWrtShell, Frmmgr_Type::TEXT, nullptr); + pWrtShell->StartAllAction(); + aMgr.InsertFlyFrame(RndStdIds::FLY_AT_PARA, aMgr.GetPos(), aMgr.GetSize()); + pWrtShell->EndAllAction(); + // Set fly properties: + pWrtShell->StartAllAction(); + SwFrameFormat* pFly = pWrtShell->GetFlyFrameFormat(); + SwAttrSet aSet(pFly->GetAttrSet()); + aSet.Put(SwFormatFlySplit(true)); + SwFormatAnchor aAnchor(RndStdIds::FLY_AT_PARA); + aAnchor.SetAnchor(&aInsertPos); + aSet.Put(aAnchor); + SwDoc* pDoc = pWrtShell->GetDoc(); + pDoc->SetAttr(aSet, *pFly); + pWrtShell->EndAllAction(); + pWrtShell->EnterStdMode(); +} + +CPPUNIT_TEST_FIXTURE(Test, testSplitFlysAnchorJoin) +{ + // Given a document with two paragraphs, each serving as an anchor of a split fly: + createSwDoc(); + SwWrtShell* pWrtShell = getSwDocShell()->GetWrtShell(); + pWrtShell->Insert(u"first para"_ustr); + pWrtShell->SplitNode(); + pWrtShell->Insert(u"second para"_ustr); + pWrtShell->SttEndDoc(/*bStt=*/true); + InsertSplitFly(pWrtShell); + pWrtShell->SttEndDoc(/*bStt=*/false); + pWrtShell->SttPara(); + InsertSplitFly(pWrtShell); + + // When trying to delete at the end of the first para: + pWrtShell->SttEndDoc(/*bStt=*/true); + pWrtShell->EndPara(); + pWrtShell->DelRight(); + + // Then make sure the join doesn't happen till a text node can only be an anchor for one split + // fly: + pWrtShell->SttEndDoc(/*bStt=*/true); + SwCursor* pCursor = pWrtShell->GetCursor(); + // Without the accompanying fix in place, this test would have failed with: + // - Expected: first para + // - Actual : first parasecond para + // i.e. we did join the 2 anchors and for complex enough documents the layout never finished. + CPPUNIT_ASSERT_EQUAL(u"first para"_ustr, pCursor->GetPointNode().GetTextNode()->GetText()); + pWrtShell->SttEndDoc(/*bStt=*/false); + CPPUNIT_ASSERT_EQUAL(u"second para"_ustr, pCursor->GetPointNode().GetTextNode()->GetText()); +} } CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/sw/source/core/inc/txtfrm.hxx b/sw/source/core/inc/txtfrm.hxx index 186b014153c8..9cf16ac2bf10 100644 --- a/sw/source/core/inc/txtfrm.hxx +++ b/sw/source/core/inc/txtfrm.hxx @@ -807,6 +807,8 @@ public: /// with a negative vertical offset. Such frames may need explicit splitting. bool IsEmptyWithSplitFly() const; + bool HasSplitFlyDrawObjs() const; + static SwView* GetView(); void dumpAsXml(xmlTextWriterPtr writer = nullptr) const override; diff --git a/sw/source/core/text/itratr.cxx b/sw/source/core/text/itratr.cxx index 1c31831dd27f..b002e80610aa 100644 --- a/sw/source/core/text/itratr.cxx +++ b/sw/source/core/text/itratr.cxx @@ -1481,6 +1481,11 @@ std::vector<SwFlyAtContentFrame*> SwTextFrame::GetSplitFlyDrawObjs() const return aObjs; } +bool SwTextFrame::HasSplitFlyDrawObjs() const +{ + return !GetSplitFlyDrawObjs().empty(); +} + SwFlyAtContentFrame* SwTextFrame::HasNonLastSplitFlyDrawObj() const { const SwTextFrame* pFollow = GetFollow(); diff --git a/sw/source/uibase/wrtsh/delete.cxx b/sw/source/uibase/wrtsh/delete.cxx index 4d0f857a45a5..2cb0e2ed6d72 100644 --- a/sw/source/uibase/wrtsh/delete.cxx +++ b/sw/source/uibase/wrtsh/delete.cxx @@ -36,6 +36,8 @@ #include <osl/diagnose.h> #include <doc.hxx> #include <IDocumentRedlineAccess.hxx> +#include <IDocumentLayoutAccess.hxx> +#include <txtfrm.hxx> inline void SwWrtShell::OpenMark() { @@ -336,6 +338,7 @@ bool SwWrtShell::DelRight(bool const isReplaceHeuristic) // #108049# Save the startnode of the current cell const SwStartNode* pSNdOld = pWasInTableNd ? GetCursor()->GetPointNode().FindTableBoxStartNode() : nullptr; + SwTextNode* pOldTextNode = GetCursor()->GetPointNode().GetTextNode(); bool bCheckDelFull = SelectionType::Text & nSelection && SwCursorShell::IsSttPara(); bool bDelFull = false; bool bDoNothing = false; @@ -365,6 +368,7 @@ bool SwWrtShell::DelRight(bool const isReplaceHeuristic) } // restore cursor + SwTextNode* pNewTextNode = GetCursor()->GetPointNode().GetTextNode(); SwCursorShell::Pop(SwCursorShell::PopMode::DeleteCurrent); if (bDelFull) @@ -372,6 +376,25 @@ bool SwWrtShell::DelRight(bool const isReplaceHeuristic) DelFullPara(); UpdateAttr(); } + + if (pOldTextNode && pNewTextNode && pNewTextNode != pOldTextNode) + { + SwRootFrame* pLayout = mxDoc->getIDocumentLayoutAccess().GetCurrentLayout(); + if (pLayout) + { + auto pOldFrame = static_cast<SwTextFrame*>(pOldTextNode->getLayoutFrame(pLayout)); + auto pNewFrame = static_cast<SwTextFrame*>(pNewTextNode->getLayoutFrame(pLayout)); + if (pOldFrame && pNewFrame && pOldFrame->HasSplitFlyDrawObjs() && pNewFrame->HasSplitFlyDrawObjs()) + { + // We have a selection where both the old and the new position is an anchor + // for a potentially split fly. Don't allow join of the nodes in this case, + // since the layout supports multiple anchors for one split fly, but it + // doesn't support the usage of the same anchor for multiple split flys. + bDoNothing = true; + } + } + } + if (bDelFull || bDoNothing) break; }