tdf#111522 svx: fix crash with view1 doing text edit and view2 doing sdr undo

If one view has an active text edit, then current code doesn't handle
manipulating the undo text outside the text edit, so avoid problems by
simply not adding an undo action to the undo stack for shape creation or
move.

Other actions might want to do the same in the future: check for
CanDoSdrUndo() before calling SdrUndoFactory member functions.

[ Found no existing test suite similar to CppunitTest_sw_uiwriter, so
added one. ]

(cherry picked from commit 3a874f1c80c37e8b35666e1d73161ff762eb7e4c)

Change-Id: I3a364bf4fe6f9b0c13aa07283681b1045368cf7a
Reviewed-on: https://gerrit.libreoffice.org/73649
Tested-by: Jenkins
Reviewed-by: Xisco Faulí <xiscofauli@libreoffice.org>

7 files changed, 209 insertions, 2 deletions

diff --git a/include/svx/svdedtv.hxx b/include/svx/svdedtv.hxx
index 6fc2e1aab75f..17fc9af93c76 100644
--- a/include/svx/svdedtv.hxx
+++ b/include/svx/svdedtv.hxx
@@ -186,6 +186,12 @@ public:
     void SetUndoComment(const OUString& rComment, const OUString& rObjDescr) { mpModel->SetUndoComment(rComment,rObjDescr); }
     bool IsUndoEnabled() const;
 
+    /**
+     * Checks if this or other views have an active text edit, in which case object undos are not
+     * created.
+     */
+    bool CanDoSdrUndo() const;
+
     std::vector< std::unique_ptr<SdrUndoAction> > CreateConnectorUndo( SdrObject& rO );
     void AddUndoActions( std::vector< std::unique_ptr<SdrUndoAction> > );
 
diff --git a/sd/CppunitTest_sd_uiimpress.mk b/sd/CppunitTest_sd_uiimpress.mk
new file mode 100644
index 000000000000..7881d15d2ef8
--- /dev/null
+++ b/sd/CppunitTest_sd_uiimpress.mk
@@ -0,0 +1,84 @@
+# -*- Mode: makefile-gmake; tab-width: 4; indent-tabs-mode: t -*-
+#*************************************************************************
+#
+# This file is part of the LibreOffice project.
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+#*************************************************************************
+
+$(eval $(call gb_CppunitTest_CppunitTest,sd_uiimpress))
+
+$(eval $(call gb_CppunitTest_use_externals,sd_uiimpress,\
+	boost_headers \
+	libxml2 \
+))
+
+$(eval $(call gb_CppunitTest_add_exception_objects,sd_uiimpress, \
+    sd/qa/unit/uiimpress \
+))
+
+$(eval $(call gb_CppunitTest_use_libraries,sd_uiimpress, \
+    $(call gb_Helper_optional,AVMEDIA,avmedia) \
+    basegfx \
+    canvastools \
+    comphelper \
+    cppcanvas \
+    cppu \
+    cppuhelper \
+    drawinglayer \
+    editeng \
+    for \
+    forui \
+    i18nlangtag \
+    i18nutil \
+    msfilter \
+    oox \
+    sal \
+    salhelper \
+    sax \
+    sb \
+    sd \
+    sfx \
+    sot \
+    svl \
+    svt \
+    svx \
+    svxcore \
+    test \
+    tl \
+    tk \
+    ucbhelper \
+    unotest \
+    utl \
+    vcl \
+    xo \
+))
+
+$(eval $(call gb_CppunitTest_set_include,sd_uiimpress,\
+    -I$(SRCDIR)/sd/inc \
+    -I$(SRCDIR)/sd/source/ui/inc \
+    -I$(SRCDIR)/sd/source/ui/slidesorter/inc \
+    $$(INCLUDE) \
+))
+
+$(eval $(call gb_CppunitTest_use_sdk_api,sd_uiimpress))
+
+$(eval $(call gb_CppunitTest_use_externals,sd_uiimpress,\
+    boost_headers \
+))
+
+$(eval $(call gb_CppunitTest_use_ure,sd_uiimpress))
+$(eval $(call gb_CppunitTest_use_vcl,sd_uiimpress))
+
+$(eval $(call gb_CppunitTest_use_rdb,sd_uiimpress,services))
+
+$(eval $(call gb_CppunitTest_use_custom_headers,sd_uiimpress,\
+	officecfg/registry \
+))
+
+$(eval $(call gb_CppunitTest_use_configuration,sd_uiimpress))
+
+# vim: set noet sw=4 ts=4:
diff --git a/sd/Module_sd.mk b/sd/Module_sd.mk
index e1ead370b2e3..b0f63ddc1ed1 100644
--- a/sd/Module_sd.mk
+++ b/sd/Module_sd.mk
@@ -36,6 +36,7 @@ $(eval $(call gb_Module_add_slowcheck_targets,sd,\
     CppunitTest_sd_export_tests \
     CppunitTest_sd_filters_test \
     CppunitTest_sd_misc_tests \
+    CppunitTest_sd_uiimpress \
     CppunitTest_sd_html_export_tests \
     CppunitTest_sd_activex_controls_tests \
 ))
diff --git a/sd/qa/unit/data/tdf111522.odp b/sd/qa/unit/data/tdf111522.odp
new file mode 100644
index 000000000000..fd6ebeb6ed22
--- /dev/null
+++ b/sd/qa/unit/data/tdf111522.odp
diff --git a/sd/qa/unit/uiimpress.cxx b/sd/qa/unit/uiimpress.cxx
new file mode 100644
index 000000000000..910f4a9dd0c3
--- /dev/null
+++ b/sd/qa/unit/uiimpress.cxx
@@ -0,0 +1,100 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <test/bootstrapfixture.hxx>
+#include <unotest/macros_test.hxx>
+
+#include <com/sun/star/frame/Desktop.hpp>
+#include <com/sun/star/uno/Reference.hxx>
+
+#include <comphelper/processfactory.hxx>
+#include <sfx2/dispatch.hxx>
+#include <sfx2/request.hxx>
+#include <sfx2/viewfrm.hxx>
+#include <svl/intitem.hxx>
+#include <svx/svxids.hrc>
+
+#include <DrawDocShell.hxx>
+#include <ViewShell.hxx>
+#include <app.hrc>
+#include <sdpage.hxx>
+#include <unomodel.hxx>
+
+using namespace ::com::sun::star;
+
+/// Impress UI tests.
+class SdUiImpressTest : public test::BootstrapFixture, public unotest::MacrosTest
+{
+protected:
+    uno::Reference<uno::XComponentContext> mxComponentContext;
+    uno::Reference<lang::XComponent> mxComponent;
+
+public:
+    virtual void setUp() override;
+    virtual void tearDown() override;
+};
+
+void SdUiImpressTest::setUp()
+{
+    test::BootstrapFixture::setUp();
+
+    mxComponentContext.set(comphelper::getComponentContext(getMultiServiceFactory()));
+    mxDesktop.set(frame::Desktop::create(mxComponentContext));
+}
+
+void SdUiImpressTest::tearDown()
+{
+    if (mxComponent.is())
+        mxComponent->dispose();
+
+    test::BootstrapFixture::tearDown();
+}
+
+CPPUNIT_TEST_FIXTURE(SdUiImpressTest, testTdf111522)
+{
+    // Load the document and create two new windows.
+    mxComponent = loadFromDesktop(m_directories.getURLFromSrc("sd/qa/unit/data/tdf111522.odp"));
+    auto pImpressDocument = dynamic_cast<SdXImpressDocument*>(mxComponent.get());
+    sd::ViewShell* pViewShell = pImpressDocument->GetDocShell()->GetViewShell();
+    pViewShell->GetViewFrame()->GetDispatcher()->Execute(SID_NEWWINDOW, SfxCallMode::SYNCHRON);
+    sd::ViewShell* pViewShell1 = pImpressDocument->GetDocShell()->GetViewShell();
+    pViewShell->GetViewFrame()->GetDispatcher()->Execute(SID_NEWWINDOW, SfxCallMode::SYNCHRON);
+    sd::ViewShell* pViewShell2 = pImpressDocument->GetDocShell()->GetViewShell();
+    CPPUNIT_ASSERT(pViewShell1 != pViewShell2);
+
+    // Have slide 1 in window 1, slide 2 in window 2.
+    SfxRequest aRequest(pViewShell2->GetViewFrame(), SID_SWITCHPAGE);
+    aRequest.AppendItem(SfxUInt32Item(ID_VAL_WHATPAGE, 1));
+    aRequest.AppendItem(
+        SfxUInt32Item(ID_VAL_WHATKIND, static_cast<sal_uInt32>(PageKind::Standard)));
+    pViewShell2->ExecuteSlot(aRequest, /*bAsync=*/false);
+
+    // Start text edit in window 1.
+    SdPage* pPage1 = pViewShell1->GetActualPage();
+    SdrObject* pShape1 = pPage1->GetObj(0);
+    SdrView* pView1 = pViewShell1->GetView();
+    pView1->MarkObj(pShape1, pView1->GetSdrPageView());
+    pView1->SdrBeginTextEdit(pShape1);
+    CPPUNIT_ASSERT(pView1->IsTextEdit());
+
+    // Move the shape in window 2 & undo.
+    SdPage* pPage2 = pViewShell2->GetActualPage();
+    CPPUNIT_ASSERT(pPage1 != pPage2);
+    SdrObject* pShape2 = pPage2->GetObj(0);
+    SdrView* pView2 = pViewShell2->GetView();
+    pView2->MarkObj(pShape2, pView2->GetSdrPageView());
+    pView2->MoveMarkedObj(Size(1, 1), /*bCopy=*/false);
+    // Without the accompanying fix in place, this test would have failed with an assertion failure
+    // in SdrObjEditView::SdrEndTextEdit() as mpOldTextEditUndoManager was not nullptr.
+    pViewShell2->GetViewFrame()->GetDispatcher()->Execute(SID_UNDO, SfxCallMode::SYNCHRON);
+}
+
+CPPUNIT_PLUGIN_IMPLEMENT();
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/svx/source/svdraw/svdedtv.cxx b/svx/source/svdraw/svdedtv.cxx
index 2197c3fbc42e..e42e90fcedeb 100644
--- a/svx/source/svdraw/svdedtv.cxx
+++ b/svx/source/svdraw/svdedtv.cxx
@@ -977,7 +977,7 @@ bool SdrEditView::InsertObjectAtView(SdrObject* pObj, SdrPageView& rPV, SdrInser
             rPV.GetObjList()->InsertObject(pObj, SAL_MAX_SIZE);
         }
     }
-    if( IsUndoEnabled() )
+    if( IsUndoEnabled() && CanDoSdrUndo())
         AddUndo(GetModel()->GetSdrUndoFactory().CreateUndoNewObject(*pObj));
 
     if (!(nOptions & SdrInsertFlags::DONTMARK)) {
@@ -1033,4 +1033,20 @@ bool SdrEditView::IsUndoEnabled() const
     return mpModel->IsUndoEnabled();
 }
 
+bool SdrEditView::CanDoSdrUndo() const
+{
+    size_t nViews = mpModel->GetListenerCount();
+    for (size_t nView = 0; nView < nViews; ++nView)
+    {
+        SdrEditView* pView = dynamic_cast<SdrEditView*>(mpModel->GetListener(nView));
+        if (!pView)
+            continue;
+
+        if (pView->IsTextEdit())
+            return false;
+    }
+
+    return true;
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/svx/source/svdraw/svdedtv1.cxx b/svx/source/svdraw/svdedtv1.cxx
index 960dd2c58269..aa8e5d6df761 100644
--- a/svx/source/svdraw/svdedtv1.cxx
+++ b/svx/source/svdraw/svdedtv1.cxx
@@ -172,7 +172,7 @@ void SdrEditView::AddUndoActions( std::vector< std::unique_ptr<SdrUndoAction> >
 
 void SdrEditView::MoveMarkedObj(const Size& rSiz, bool bCopy)
 {
-    const bool bUndo = IsUndoEnabled();
+    const bool bUndo = IsUndoEnabled() && CanDoSdrUndo();
 
     if( bUndo )
     {