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. ]

Change-Id: I3a364bf4fe6f9b0c13aa07283681b1045368cf7a
Reviewed-on: https://gerrit.libreoffice.org/73558Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

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> > );
 

sd/CppunitTest_sd_uiimpress.mk

+# -*- 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:

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 \
 ))

sd/qa/unit/uiimpress.cxx

+/* -*- 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: */

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: */

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 )
     {