From 1cc5c6587e72581c83f7289a95e906a3a1be38b6 Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Mon, 18 Oct 2021 16:22:45 +0200 Subject: sw: avoid popup window for the EmbeddedObject property of an UNO OLE object The old behavior was that in case loading the native data of the embedded object failed, then we presented a popup to the user, but we still returned a valid com.sun.star.comp.embed.OCommonEmbeddedObject. Given that the return valid is desired even in the error case, don't turn the error popup into an UNO exception, rather redirect the popup to SAL_WARN(). This is meant to be consistent with other UNO API functions that pass down an "is API" boolean to internal code to control if interactive behavior is allowed or not. One extreme would be to do this only for the EmbeddedObject property of frames, the other one would be to do this for more object types. Go with a middle ground for now, so all properties of Writer frame objects use this, but leave other object types unchanged. (cherry picked from commit 86e07d951bc4fd16648fe10befb40dd953fcc208) Conflicts: sw/qa/core/unocore/unocore.cxx Change-Id: I655fcb3780e96cecc7ed3f01ded52948d013172e --- include/vcl/errinf.hxx | 7 +++++ sw/qa/core/unocore/data/broken-embedded-object.odt | Bin 0 -> 14360 bytes sw/qa/core/unocore/unocore.cxx | 31 +++++++++++++++++++++ sw/source/core/unocore/unoframe.cxx | 20 +++++++++++++ vcl/source/window/errinf.cxx | 15 +++++++++- 5 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 sw/qa/core/unocore/data/broken-embedded-object.odt diff --git a/include/vcl/errinf.hxx b/include/vcl/errinf.hxx index 4a0edc75b259..705e6d97cde4 100644 --- a/include/vcl/errinf.hxx +++ b/include/vcl/errinf.hxx @@ -59,11 +59,18 @@ public: static void RegisterDisplay(BasicDisplayErrorFunc*); static void RegisterDisplay(WindowDisplayErrorFunc*); + + static void SetLock(bool bLock); + static bool GetLock(); + static void Reset(); private: DisplayFnPtr pDsp; bool bIsWindowDsp; + + bool m_bLock; + sal_uInt16 nNextError; std::vector errorHandlers; diff --git a/sw/qa/core/unocore/data/broken-embedded-object.odt b/sw/qa/core/unocore/data/broken-embedded-object.odt new file mode 100644 index 000000000000..94a0809235e9 Binary files /dev/null and b/sw/qa/core/unocore/data/broken-embedded-object.odt differ diff --git a/sw/qa/core/unocore/unocore.cxx b/sw/qa/core/unocore/unocore.cxx index a4a39489da60..a0cd7a253cba 100644 --- a/sw/qa/core/unocore/unocore.cxx +++ b/sw/qa/core/unocore/unocore.cxx @@ -31,6 +31,7 @@ #include #include #include +#include #include #include @@ -108,6 +109,36 @@ CPPUNIT_TEST_FIXTURE(SwCoreUnocoreTest, flyAtParaAnchor) xText->insertTextContent(xAnchor, xFieldmark, false); } +/// Fails the test if an error popup would be presented. +static void BasicDisplayErrorHandler(const OUString& /*rErr*/, const OUString& /*rAction*/) +{ + CPPUNIT_ASSERT(false); +} + +CPPUNIT_TEST_FIXTURE(SwCoreUnocoreTest, testBrokenEmbeddedObject) +{ + // Given a document with a broken embedded object (the XML markup is not well-formed): + load(DATA_DIRECTORY, "broken-embedded-object.odt"); + uno::Reference xSupplier(mxComponent, uno::UNO_QUERY); + uno::Reference xObjects(xSupplier->getEmbeddedObjects(), + uno::UNO_QUERY); + uno::Reference xObject(xObjects->getByIndex(0), uno::UNO_QUERY); + uno::Reference xEmbeddedObject; + // Get the property first, which initializes Draw, which would overwrite our error handler. + xObject->getPropertyValue("EmbeddedObject") >>= xEmbeddedObject; + ErrorRegistry::RegisterDisplay(&BasicDisplayErrorHandler); + + // When trying to load that embedded object: + xObject->getPropertyValue("EmbeddedObject") >>= xEmbeddedObject; + + // Then make sure we get a non-empty reference and an error popup it not shown: + CPPUNIT_ASSERT(xEmbeddedObject.is()); + // Without the accompanying fix in place, we got this reference, but first an error popup was + // shown to the user. + CPPUNIT_ASSERT( + xEmbeddedObject->supportsService("com.sun.star.comp.embed.OCommonEmbeddedObject")); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/source/core/unocore/unoframe.cxx b/sw/source/core/unocore/unoframe.cxx index a04ac6908b92..58801e2fb971 100644 --- a/sw/source/core/unocore/unoframe.cxx +++ b/sw/source/core/unocore/unoframe.cxx @@ -112,6 +112,7 @@ #include #include #include +#include #include #include @@ -1958,9 +1959,28 @@ void SwXFrame::setPropertyValue(const OUString& rPropertyName, const ::uno::Any& throw uno::RuntimeException(); } +namespace +{ +/// Redirect error popups to developer warnings for the duration of the UNO API call. +class DisplayLockGuard +{ + bool m_bLock; + +public: + DisplayLockGuard() + { + m_bLock = ErrorRegistry::GetLock(); + ErrorRegistry::SetLock(true); + } + + ~DisplayLockGuard() { ErrorRegistry::SetLock(m_bLock); } +}; +} + uno::Any SwXFrame::getPropertyValue(const OUString& rPropertyName) { SolarMutexGuard aGuard; + DisplayLockGuard aDisplayGuard; uno::Any aAny; SwFrameFormat* pFormat = GetFrameFormat(); const SfxItemPropertySimpleEntry* pEntry = m_pPropSet->getPropertyMap().getByName(rPropertyName); diff --git a/vcl/source/window/errinf.cxx b/vcl/source/window/errinf.cxx index 8e08dc361acd..80c9c36ee897 100644 --- a/vcl/source/window/errinf.cxx +++ b/vcl/source/window/errinf.cxx @@ -54,6 +54,7 @@ bool ErrorStringFactory::CreateString(const ErrorInfo* pInfo, OUString& rStr) ErrorRegistry::ErrorRegistry() : pDsp(nullptr) , bIsWindowDsp(false) + , m_bLock(false) , nNextError(0) { for(DynamicErrorInfo*& rp : ppDynErrInfo) @@ -74,6 +75,18 @@ void ErrorRegistry::RegisterDisplay(WindowDisplayErrorFunc *aDsp) rData.pDsp = reinterpret_cast< DisplayFnPtr >(aDsp); } +void ErrorRegistry::SetLock(bool bLock) +{ + ErrorRegistry &rData = TheErrorRegistry::get(); + rData.m_bLock = bLock; +} + +bool ErrorRegistry::GetLock() +{ + ErrorRegistry &rData = TheErrorRegistry::get(); + return rData.m_bLock; +} + void ErrorRegistry::Reset() { ErrorRegistry &rData = TheErrorRegistry::get(); @@ -160,7 +173,7 @@ DialogMask ErrorHandler::HandleError(ErrCode nErrCodeId, weld::Window *pParent, OUString aErr; if (ErrorStringFactory::CreateString(pInfo.get(), aErr)) { - if(!rData.pDsp) + if (!rData.pDsp || rData.m_bLock) { SAL_WARN( "vcl", "Action: " << aAction << "Error: " << aErr); } -- cgit