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/73594
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>

7 files changed, 209 insertions, 2 deletions

diff --git a/include/svx/svdedtv.hxx b/include/svx/svdedtv.hxx
index af60deaac0f6..0305b7fb144c 100644
--- a/include/svx/svdedtv.hxx
+++ b/include/svx/svdedtv.hxx
@@ -185,6 +185,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 bd2796ae818e..a7edda76cbe4 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 51678b78b0d0..5883eab62640 100644
--- a/svx/source/svdraw/svdedtv.cxx
+++ b/svx/source/svdraw/svdedtv.cxx
@@ -972,7 +972,7 @@ bool SdrEditView::InsertObjectAtView(SdrObject* pObj, SdrPageView& rPV, SdrInser
     if (!pObj->IsInserted()) {
         rPV.GetObjList()->InsertObject(pObj, SAL_MAX_SIZE);
     }
-    if( IsUndoEnabled() )
+    if( IsUndoEnabled() && CanDoSdrUndo())
         AddUndo(GetModel()->GetSdrUndoFactory().CreateUndoNewObject(*pObj));
 
     if (!(nOptions & SdrInsertFlags::DONTMARK)) {
@@ -1028,4 +1028,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 f781b64b36f7..66bc4c15ae69 100644
--- a/svx/source/svdraw/svdedtv1.cxx
+++ b/svx/source/svdraw/svdedtv1.cxx
@@ -175,7 +175,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 )
     {