tdf#153304: Add undo entries and set modified in SwXFrame::setProperty*

Similar to SwDoc::SetFlyFrameAttr; the latter should only set modified
state on actual changes.

Change-Id: I66982e68fa9132fda132d811f7a48b4461645df4
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/146484
Tested-by: Mike Kaganski <mike.kaganski@collabora.com>
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
(cherry picked from commit 333183d9a72d1e2b7ae65145092efec5e357ad14)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/146515
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>

6 files changed, 86 insertions, 18 deletions

diff --git a/sw/qa/core/undo/data/image-as-character.odt b/sw/qa/core/undo/data/image-as-character.odt
new file mode 100644
index 000000000000..0b1aa6a21fb2
--- /dev/null
+++ b/sw/qa/core/undo/data/image-as-character.odt
diff --git a/sw/qa/core/undo/undo.cxx b/sw/qa/core/undo/undo.cxx
index 583458d30127..154226d2129f 100644
--- a/sw/qa/core/undo/undo.cxx
+++ b/sw/qa/core/undo/undo.cxx
@@ -106,6 +106,52 @@ CPPUNIT_TEST_FIXTURE(SwCoreUndoTest, testTableCopyRedline)
     pWrtShell->Undo();
 }
 
+CPPUNIT_TEST_FIXTURE(SwCoreUndoTest, testImagePropsCreateUndoAndModifyDoc)
+{
+    createSwDoc("image-as-character.odt");
+    SwXTextDocument* pTextDoc = dynamic_cast<SwXTextDocument*>(mxComponent.get());
+    SwDocShell* pDocShell = pTextDoc->GetDocShell();
+    SwWrtShell* pWrtShell = pDocShell->GetWrtShell();
+    css::uno::Reference<css::beans::XPropertySet> xImage(
+        pTextDoc->getGraphicObjects()->getByName("Image1"), css::uno::UNO_QUERY_THROW);
+
+    CPPUNIT_ASSERT(pTextDoc->isSetModifiedEnabled());
+    CPPUNIT_ASSERT(!pTextDoc->isModified());
+    CPPUNIT_ASSERT(!pWrtShell->GetLastUndoInfo(nullptr, nullptr, nullptr));
+
+    // Check that modifications of the geometry mark document dirty, and create an undo
+
+    xImage->setPropertyValue("RelativeWidth", css::uno::Any(sal_Int16(80)));
+
+    // Without the fix, this would fail
+    CPPUNIT_ASSERT(pTextDoc->isModified());
+    CPPUNIT_ASSERT(pWrtShell->GetLastUndoInfo(nullptr, nullptr, nullptr));
+
+    pWrtShell->Undo();
+    CPPUNIT_ASSERT(!pTextDoc->isModified());
+    CPPUNIT_ASSERT(!pWrtShell->GetLastUndoInfo(nullptr, nullptr, nullptr));
+
+    // Check that modifications of anchor mark document dirty, and create an undo
+
+    xImage->setPropertyValue("AnchorType",
+                             css::uno::Any(css::text::TextContentAnchorType_AT_PARAGRAPH));
+
+    CPPUNIT_ASSERT(pTextDoc->isModified());
+    CPPUNIT_ASSERT(pWrtShell->GetLastUndoInfo(nullptr, nullptr, nullptr));
+
+    pWrtShell->Undo();
+    CPPUNIT_ASSERT(!pTextDoc->isModified());
+    CPPUNIT_ASSERT(!pWrtShell->GetLastUndoInfo(nullptr, nullptr, nullptr));
+
+    // Check that setting the same values do not make it dirty and do not add undo
+
+    xImage->setPropertyValue("RelativeWidth", xImage->getPropertyValue("RelativeWidth"));
+    xImage->setPropertyValue("AnchorType", xImage->getPropertyValue("AnchorType"));
+
+    CPPUNIT_ASSERT(!pTextDoc->isModified());
+    CPPUNIT_ASSERT(!pWrtShell->GetLastUndoInfo(nullptr, nullptr, nullptr));
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/doc/docfly.cxx b/sw/source/core/doc/docfly.cxx
index 7bf5c31681ac..5ddc713ff4af 100644
--- a/sw/source/core/doc/docfly.cxx
+++ b/sw/source/core/doc/docfly.cxx
@@ -555,23 +555,10 @@ bool SwDoc::SetFlyFrameAttr( SwFrameFormat& rFlyFormat, SfxItemSet& rSet )
     if( !rSet.Count() )
         return false;
 
-    std::unique_ptr<SwUndoFormatAttrHelper> pSaveUndo;
-
-    if (GetIDocumentUndoRedo().DoesUndo())
-    {
-        GetIDocumentUndoRedo().ClearRedo(); // AppendUndo far below, so leave it
-        pSaveUndo.reset( new SwUndoFormatAttrHelper( rFlyFormat ) );
-    }
+    SwDocModifyAndUndoGuard guard(rFlyFormat);
 
     bool const bRet = lcl_SetFlyFrameAttr(*this, &SwDoc::SetFlyFrameAnchor, rFlyFormat, rSet);
 
-    if (pSaveUndo && pSaveUndo->GetUndo() )
-    {
-        GetIDocumentUndoRedo().AppendUndo( pSaveUndo->ReleaseUndo() );
-    }
-
-    getIDocumentState().SetModified();
-
     //SwTextBoxHelper::syncFlyFrameAttr(rFlyFormat, rSet);
 
     return bRet;
diff --git a/sw/source/core/inc/UndoAttribute.hxx b/sw/source/core/inc/UndoAttribute.hxx
index c3cf4925d95a..62470a11aa53 100644
--- a/sw/source/core/inc/UndoAttribute.hxx
+++ b/sw/source/core/inc/UndoAttribute.hxx
@@ -176,6 +176,16 @@ public:
     std::unique_ptr<SwUndoFormatAttr> ReleaseUndo() { return std::move(m_pUndo); }
 };
 
+class SwDocModifyAndUndoGuard final
+{
+    SwDoc* doc;
+    std::unique_ptr<SwUndoFormatAttrHelper> helper;
+
+public:
+    SwDocModifyAndUndoGuard(SwFormat& format);
+    ~SwDocModifyAndUndoGuard();
+};
+
 class SwUndoMoveLeftMargin final : public SwUndo, private SwUndRng
 {
     const std::unique_ptr<SwHistory> m_pHistory;
diff --git a/sw/source/core/undo/unattr.cxx b/sw/source/core/undo/unattr.cxx
index 19929658e342..bd02f019fbd2 100644
--- a/sw/source/core/undo/unattr.cxx
+++ b/sw/source/core/undo/unattr.cxx
@@ -37,6 +37,8 @@
 #include <doc.hxx>
 #include <IDocumentLayoutAccess.hxx>
 #include <IDocumentRedlineAccess.hxx>
+#include <IDocumentState.hxx>
+#include <IDocumentUndoRedo.hxx>
 #include <IShellCursorSupplier.hxx>
 #include <docary.hxx>
 #include <swcrsr.hxx>
@@ -92,6 +94,24 @@ void SwUndoFormatAttrHelper::SwClientNotify(const SwModify&, const SfxHint& rHin
     }
 }
 
+SwDocModifyAndUndoGuard::SwDocModifyAndUndoGuard(SwFormat& format)
+    : doc(format.GetName().isEmpty() ? nullptr : format.GetDoc())
+    , helper(doc ? new SwUndoFormatAttrHelper(format) : nullptr)
+{
+}
+
+SwDocModifyAndUndoGuard::~SwDocModifyAndUndoGuard()
+{
+    if (helper && helper->GetUndo())
+    {
+        // helper tracks changes, even when DoesUndo is false, to detect modified state
+        if (doc->GetIDocumentUndoRedo().DoesUndo())
+            doc->GetIDocumentUndoRedo().AppendUndo(helper->ReleaseUndo());
+
+        doc->getIDocumentState().SetModified();
+    }
+}
+
 SwUndoFormatAttr::SwUndoFormatAttr( SfxItemSet&& rOldSet,
                               SwFormat& rChgFormat,
                               bool bSaveDrawPt )
@@ -141,7 +161,8 @@ void SwUndoFormatAttr::Init( const SwFormat & rFormat )
                                ->FindTableNode()->GetIndex();
             }
         } else if (dynamic_cast<const SwSectionFormat*>(&rFormat)) {
-            m_nNodeIndex = rFormat.GetContent().GetContentIdx()->GetIndex();
+            if (auto pContentIndex = rFormat.GetContent().GetContentIdx())
+                m_nNodeIndex = pContentIndex->GetIndex();
         } else if(auto pBoxFormat = dynamic_cast<const SwTableBoxFormat*>(&rFormat))
         {
             auto pTableBox = pBoxFormat->GetTableBox();
diff --git a/sw/source/core/unocore/unoframe.cxx b/sw/source/core/unocore/unoframe.cxx
index d81ed0ff6b63..381109f4e97e 100644
--- a/sw/source/core/unocore/unoframe.cxx
+++ b/sw/source/core/unocore/unoframe.cxx
@@ -46,6 +46,7 @@
 #include <IDocumentDrawModelAccess.hxx>
 #include <IDocumentLayoutAccess.hxx>
 #include <IDocumentStylePoolAccess.hxx>
+#include <UndoAttribute.hxx>
 #include <docsh.hxx>
 #include <editsh.hxx>
 #include <ndindex.hxx>
@@ -1405,6 +1406,7 @@ void SwXFrame::setPropertyValue(const OUString& rPropertyName, const ::uno::Any&
     {
         if (pFormat)
         {
+            SwDocModifyAndUndoGuard guard(*pFormat);
             SvxFrameDirectionItem aItem(SvxFrameDirection::Environment, RES_FRAMEDIR);
             aItem.PutValue(_rValue, 0);
             GetFrameFormat()->SetFormatAttr(aItem);
@@ -1934,6 +1936,7 @@ void SwXFrame::setPropertyValue(const OUString& rPropertyName, const ::uno::Any&
             }
             else
             {
+                SwDocModifyAndUndoGuard guard(*pFormat);
                 pFormat->SetFormatAttr(aSet);
             }
         }
@@ -2534,6 +2537,7 @@ void SwXFrame::setPropertyToDefault( const OUString& rPropertyName )
             aSet.ClearItem(XATTR_FILLBMP_STRETCH);
             aSet.ClearItem(XATTR_FILLBMP_TILE);
 
+            SwDocModifyAndUndoGuard guard(*pFormat);
             pFormat->SetFormatAttr(aSet);
         }
         else if( pEntry->nWID &&
@@ -2570,14 +2574,14 @@ void SwXFrame::setPropertyToDefault( const OUString& rPropertyName )
                 GetOrCreateSdrObject(rFlyFormat);
                 rFlyFormat.GetDoc()->SetFlyFrameDescription(rFlyFormat, OUString());
             }
-            else
+            else if (rPropertyName != UNO_NAME_ANCHOR_TYPE)
             {
                 SwDoc* pDoc = pFormat->GetDoc();
                 SfxItemSetFixed<RES_FRMATR_BEGIN, RES_FRMATR_END - 1> aSet( pDoc->GetAttrPool() );
                 aSet.SetParent(&pFormat->GetAttrSet());
                 aSet.ClearItem(pEntry->nWID);
-                if(rPropertyName != UNO_NAME_ANCHOR_TYPE)
-                    pFormat->SetFormatAttr(aSet);
+                SwDocModifyAndUndoGuard guard(*pFormat);
+                pFormat->SetFormatAttr(aSet);
             }
         }
         else