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

5 files changed, 72 insertions, 1 deletions

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<ErrorHandler*>  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
--- /dev/null
+++ b/sw/qa/core/unocore/data/broken-embedded-object.odt
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 <comphelper/propertyvalue.hxx>
 #include <toolkit/helper/vclunohelper.hxx>
 #include <vcl/graphicfilter.hxx>
+#include <vcl/errinf.hxx>
 
 #include <wrtsh.hxx>
 #include <ndtxt.hxx>
@@ -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<text::XTextEmbeddedObjectsSupplier> xSupplier(mxComponent, uno::UNO_QUERY);
+    uno::Reference<container::XIndexAccess> xObjects(xSupplier->getEmbeddedObjects(),
+                                                     uno::UNO_QUERY);
+    uno::Reference<beans::XPropertySet> xObject(xObjects->getByIndex(0), uno::UNO_QUERY);
+    uno::Reference<lang::XServiceInfo> 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 <comphelper/servicehelper.hxx>
 #include <cppuhelper/supportsservice.hxx>
 #include <sal/log.hxx>
+#include <vcl/errinf.hxx>
 
 #include <svx/unobrushitemhelper.hxx>
 #include <svx/xbtmpit.hxx>
@@ -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);
         }