sw/qa/core/frmedt/frmedt.cxx | 12 ++++++++++++
sw/source/core/frmedt/fefly1.cxx | 22 ++++++++++++++++++++--
2 files changed, 32 insertions(+), 2 deletions(-)
New commits:
commit 5601c0845caeb65db20dfceb927725906dce3f5c
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Mon Nov 20 08:35:57 2023 +0100
Commit: Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Wed Nov 22 10:35:56 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.
(cherry picked from commit 45a4ed02281a7a8ca52fccf626c792e417c8ef1c)
Conflicts:
sw/qa/core/frmedt/frmedt.cxx
Change-Id: I95ce0abbab79a293ecb209579c105f96d284a1c9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159775
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
Tested-by: Caolán McNamara <caolan.mcnam...@collabora.com>
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
diff --git a/sw/qa/core/frmedt/frmedt.cxx b/sw/qa/core/frmedt/frmedt.cxx
index 9d6a60fce8eb..3ce234685777 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
@@ -203,6 +204,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);
SwFrameFormats& rFlyFormats = *pDoc->GetSpzFrameFormats();
CPPUNIT_ASSERT(rFlyFormats.empty());
// Insert a table:
@@ -225,6 +228,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);
@@ -235,6 +239,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 0d8efa725c2e..79486415c403 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.
