sw/qa/core/frmedt/frmedt.cxx | 12 ++++++++++++ sw/source/core/frmedt/fefly1.cxx | 22 ++++++++++++++++++++-- 2 files changed, 32 insertions(+), 2 deletions(-)
New commits: commit 45a4ed02281a7a8ca52fccf626c792e417c8ef1c Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Mon Nov 20 08:35:57 2023 +0100 Commit: Miklos Vajna <vmik...@collabora.com> CommitDate: Mon Nov 20 10:19:22 2023 +0100 sw floattable, delete UI: fix undo/redo Pressing the unfloat context menu item for a frame resulted in 2 undo actions and trying to undo both even crashed. One problem was that no SwNode was left in the TextFrame and then it was deleted; but undo created a TextFrame with an empty SwNode in it. Fix this by inserting an empty text node at the end of the TextFrame and only then moving content from it. The other trouble was that in case the new text node has no matching frame during MoveNodeRange(), then the fly frame will be deleted twice: once during MoveNodeRange() (because it has become empty) and once more during DelLayoutFormat(). This can be avoided by moving AppendTextNode() to its own layout action. Finally also group the (now) 3 undo actions together, because it was a single action on the UI. Change-Id: I95ce0abbab79a293ecb209579c105f96d284a1c9 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159730 Reviewed-by: Miklos Vajna <vmik...@collabora.com> Tested-by: Jenkins diff --git a/sw/qa/core/frmedt/frmedt.cxx b/sw/qa/core/frmedt/frmedt.cxx index 618fe7fb574f..b2a53e60db27 100644 --- a/sw/qa/core/frmedt/frmedt.cxx +++ b/sw/qa/core/frmedt/frmedt.cxx @@ -28,6 +28,7 @@ #include <formatflysplit.hxx> #include <itabenum.hxx> #include <frmmgr.hxx> +#include <UndoManager.hxx> /// Covers sw/source/core/frmedt/ fixes. class SwCoreFrmedtTest : public SwModelTestBase @@ -204,6 +205,8 @@ CPPUNIT_TEST_FIXTURE(SwCoreFrmedtTest, testSplitFlyUnfloat) // Given a document with a floating table: createSwDoc(); SwDoc* pDoc = getSwDocShell()->GetDoc(); + CPPUNIT_ASSERT(pDoc->GetUndoManager().IsUndoEnabled()); + pDoc->GetUndoManager().EnableUndo(false); sw::FrameFormats<sw::SpzFrameFormat*>& rFlyFormats = *pDoc->GetSpzFrameFormats(); CPPUNIT_ASSERT(rFlyFormats.empty()); // Insert a table: @@ -226,6 +229,7 @@ CPPUNIT_TEST_FIXTURE(SwCoreFrmedtTest, testSplitFlyUnfloat) pWrtShell->EndAllAction(); CPPUNIT_ASSERT(!rFlyFormats.empty()); CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), pDoc->GetTableFrameFormatCount(/*bUsed=*/true)); + pDoc->GetUndoManager().EnableUndo(true); // When marking that frame and unfloating it: selectShape(1); @@ -236,6 +240,14 @@ CPPUNIT_TEST_FIXTURE(SwCoreFrmedtTest, testSplitFlyUnfloat) // have failed, the frame was not removed. CPPUNIT_ASSERT(rFlyFormats.empty()); CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), pDoc->GetTableFrameFormatCount(/*bUsed=*/true)); + + // When undoing the conversion to inline: + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), pDoc->GetUndoManager().GetUndoActionCount()); + pDoc->GetUndoManager().Undo(); + + // Then the undo stack had 2 undo actions and undo-all crashed. + CPPUNIT_ASSERT(!rFlyFormats.empty()); + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), pDoc->GetTableFrameFormatCount(/*bUsed=*/true)); } CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/sw/source/core/frmedt/fefly1.cxx b/sw/source/core/frmedt/fefly1.cxx index d9a620122b07..9e2fedcc3827 100644 --- a/sw/source/core/frmedt/fefly1.cxx +++ b/sw/source/core/frmedt/fefly1.cxx @@ -31,6 +31,7 @@ #include <com/sun/star/embed/XEmbeddedObject.hpp> #include <comphelper/types.hxx> #include <osl/diagnose.h> +#include <comphelper/scopeguard.hxx> #include <fmtanchr.hxx> #include <fmtcntnt.hxx> #include <fmtornt.hxx> @@ -275,6 +276,10 @@ void SwFEShell::SelectFlyFrame( SwFlyFrame& rFrame ) void SwFEShell::UnfloatFlyFrame() { + GetIDocumentUndoRedo().StartUndo(SwUndoId::DELLAYFMT, nullptr); + comphelper::ScopeGuard g([this] + { GetIDocumentUndoRedo().EndUndo(SwUndoId::DELLAYFMT, nullptr); }); + SwFlyFrame* pFly = GetSelectedFlyFrame(); if (!pFly) { @@ -295,7 +300,21 @@ void SwFEShell::UnfloatFlyFrame() return; } - SwNodeRange aRange(pFlyStart->GetNode(), SwNodeOffset(1), *pFlyEnd, SwNodeOffset(0)); + // Create an empty paragraph after the table, so the frame's SwNodes section is non-empty after + // MoveNodeRange(). Undo would ensure it's non-empty and then node offsets won't match. + IDocumentContentOperations& rIDCO = GetDoc()->getIDocumentContentOperations(); + { + SwNodeIndex aInsertIndex(*pFlyEnd); + --aInsertIndex; + SwPosition aInsertPos(aInsertIndex); + StartAllAction(); + rIDCO.AppendTextNode(aInsertPos); + // Make sure that a layout frame is created for the node, so the fly frame is not deleted, + // during MoveNodeRange(), either. + EndAllAction(); + } + + SwNodeRange aRange(pFlyStart->GetNode(), SwNodeOffset(1), *pFlyEnd, SwNodeOffset(-1)); const SwFormatAnchor& rAnchor = rFlyFormat.GetAnchor(); SwNode* pAnchor = rAnchor.GetAnchorNode(); if (!pAnchor) @@ -305,7 +324,6 @@ void SwFEShell::UnfloatFlyFrame() // Move the content outside of the text frame. SwNodeIndex aInsertPos(*pAnchor); - IDocumentContentOperations& rIDCO = rFlyFormat.GetDoc()->getIDocumentContentOperations(); rIDCO.MoveNodeRange(aRange, aInsertPos.GetNode(), SwMoveFlags::CREATEUNDOOBJ); // Remove the fly frame frame.