undo: group crop operations to revert in one step

- in Writer when crop was done on an image, to revert it
  the user needed click many times Undo

- group undo operations for crop (size, position, etc)

Signed-off-by: Szymon Kłos <szymon.klos@collabora.com>
Change-Id: I29d80e0b1abec67f2b48419e681908bfbcd648b8
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/13363
Tested-by: Jenkins CPCI <releng@collaboraoffice.com>
Reviewed-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.com>
(cherry picked from commit 3e57770dc8780abc60f16111cbf6f8b0c0aadb89)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/211534
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
Tested-by: Jenkins

sw/CppunitTest_sw_core_undo.mk

@@ -21,6 +21,7 @@ $(eval $(call gb_CppunitTest_use_libraries,sw_core_undo, \
     comphelper \
     cppu \
     cppuhelper \
+    editeng \
     sal \
     sfx \
     subsequenttest \

sw/qa/core/undo/undo.cxx

@@ -37,6 +37,17 @@
 #include <com/sun/star/text/XTextTablesSupplier.hpp>
 #include <com/sun/star/text/XTextViewCursorSupplier.hpp>
 
+#include <com/sun/star/text/GraphicCrop.hpp>
+#include <com/sun/star/view/XSelectionSupplier.hpp>
+
+#include <comphelper/propertysequence.hxx>
+
+#include <svx/svdhdl.hxx>
+#include <svx/svdview.hxx>
+
+#include <view.hxx>
+#include <fmtfsize.hxx>
+
 /// Covers sw/source/core/undo/ fixes.
 class SwCoreUndoTest : public SwModelTestBase
 {
@@ -332,6 +343,68 @@ CPPUNIT_TEST_FIXTURE(SwCoreUndoTest, testImagePropsCreateUndoAndModifyDoc)
     CPPUNIT_ASSERT(!pWrtShell->GetLastUndoInfo(nullptr, nullptr, nullptr));
 }
 
+CPPUNIT_TEST_FIXTURE(SwCoreUndoTest, testCropImageSingleUndo)
+{
+    // Cropping an image by dragging a handle is a single undo step.
+    createSwDoc("image-as-character.odt");
+    SwXTextDocument* pTextDoc = getSwTextDoc();
+    uno::Any aImage = pTextDoc->getGraphicObjects()->getByName(u"Image1"_ustr);
+    uno::Reference<css::beans::XPropertySet> xImage(aImage, uno::UNO_QUERY_THROW);
+
+    auto getCrop = [&xImage] {
+        css::text::GraphicCrop aCrop;
+        xImage->getPropertyValue(u"GraphicCrop"_ustr) >>= aCrop;
+        return aCrop;
+    };
+
+    const auto& rFormats = *getSwDoc()->GetSpzFrameFormats();
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rFormats.size());
+    const Size aOldSize = rFormats[0]->GetFrameSize().GetSize();
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0), getCrop().Left);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0), getCrop().Top);
+
+    // Select the image and enter crop mode.
+    uno::Reference<css::view::XSelectionSupplier> xSelectionSupplier(
+        pTextDoc->getCurrentController(), uno::UNO_QUERY_THROW);
+    xSelectionSupplier->select(aImage);
+    getSwDocShell()->GetView()->StopShellTimer();
+    dispatchCommand(mxComponent, u".uno:Crop"_ustr, {});
+
+    SwWrtShell* pWrtShell = getSwDocShell()->GetWrtShell();
+    const SdrHdlList& rHdlList = pWrtShell->GetDrawView()->GetHdlList();
+    const SdrHdl* pTopLeft = rHdlList.GetHdl(0);
+    const SdrHdl* pBottomRight = rHdlList.GetHdl(7);
+    CPPUNIT_ASSERT(pTopLeft);
+    CPPUNIT_ASSERT(pBottomRight);
+    CPPUNIT_ASSERT_EQUAL(SdrHdlKind::UpperLeft, pTopLeft->GetKind());
+    CPPUNIT_ASSERT_EQUAL(SdrHdlKind::LowerRight, pBottomRight->GetKind());
+
+    // Drag the upper left handle a quarter of the way towards the lower right one.
+    const Point aNewPos(
+        pTopLeft->GetPos().X() + (pBottomRight->GetPos().X() - pTopLeft->GetPos().X()) / 4,
+        pTopLeft->GetPos().Y() + (pBottomRight->GetPos().Y() - pTopLeft->GetPos().Y()) / 4);
+    uno::Sequence<css::beans::PropertyValue> aArgs(comphelper::InitPropertySequence({
+        { u"HandleNum"_ustr, uno::Any(sal_Int32(0)) },
+        { u"NewPosX"_ustr, uno::Any(static_cast<sal_Int32>(aNewPos.X())) },
+        { u"NewPosY"_ustr, uno::Any(static_cast<sal_Int32>(aNewPos.Y())) },
+    }));
+    dispatchCommand(mxComponent, u".uno:MoveShapeHandle"_ustr, aArgs);
+
+    CPPUNIT_ASSERT_GREATER(sal_Int32(0), getCrop().Left);
+    CPPUNIT_ASSERT_GREATER(sal_Int32(0), getCrop().Top);
+    CPPUNIT_ASSERT_LESS(aOldSize.Width(), rFormats[0]->GetFrameSize().GetWidth());
+
+    dispatchCommand(mxComponent, u".uno:Undo"_ustr, {});
+
+    // Without the fix, the crop was left for a second undo, so this failed with:
+    // - Expected: 0
+    // - Actual  : 1270
+    // i.e. the frame had its original size back while the graphic stayed cropped.
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0), getCrop().Left);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0), getCrop().Top);
+    CPPUNIT_ASSERT_EQUAL(aOldSize, rFormats[0]->GetFrameSize().GetSize());
+}
+
 CPPUNIT_TEST_FIXTURE(SwCoreUndoTest, testAnchorTypeChangePosition)
 {
     // Given a document with a textbox (draw + fly format pair) + an inner image:

sw/source/core/draw/dflyobj.cxx

@@ -913,7 +913,8 @@ void SwVirtFlyDrawObj::NbcCrop(const basegfx::B2DPoint& rRef, double fxFact, dou
 
     // Apply values
     pSh->StartAllAction();
-    // pSh->StartUndo(SwUndoId::START);
+    // Group: crop changes the crop item, the frame size and possibly the position.
+    pSh->StartUndo(SwUndoId::START);
 
     // Set new crop values in twips
     aCrop.SetLeft  (o3tl::toTwips(nLeftCrop, o3tl::Length::mm100));
@@ -978,7 +979,7 @@ void SwVirtFlyDrawObj::NbcCrop(const basegfx::B2DPoint& rRef, double fxFact, dou
         NbcMove(aDeltaMove);
     }
 
-    // pSh->EndUndo(SwUndoId::END);
+    pSh->EndUndo(SwUndoId::END);
     pSh->EndAllAction();
 }