[Libreoffice-commits] core.git: vcl/inc vcl/qt5

[Jan-Marek Glogowski (via logerrit)]

 vcl/inc/qt5/Qt5Clipboard.hxx |    1 +
 vcl/qt5/Qt5Clipboard.cxx     |   17 +++++++++++------
 2 files changed, 12 insertions(+), 6 deletions(-)

New commits:
commit c386f07ce195c2167f1b56d23cfd95292634e2de
Author:     Jan-Marek Glogowski <jan-marek.glogowski at extern.cib.de>
AuthorDate: Tue Oct 1 20:02:25 2019 +0200
Commit:     Katarina Behrens <Katarina.Behrens at cib.de>
CommitDate: Wed Oct 2 11:10:40 2019 +0200

    tdf#112368 Qt5 handle owned, non-LO clipboard content
    
    LO can actually create clipboard content, which is not backed by
    an XTransferable, for example when copying / selecting the text
    of the file name in the QFileDialog. So the ownership check in
    Qt5Clipboard::handleChanged is wrong and we just have to prevent
    freeing the content in handleChanged while changing the handle
    from Qt5Clipboard::setContents.
    
    This patch simply sets a boolean member, while the QClipboard is
    changed by LO, to handle this case.
    
    Change-Id: Icc41c32c1f9807e7adff7a9ae16a6c6cacc83f1b
    Reviewed-on: https://gerrit.libreoffice.org/79992
    Reviewed-by: Michael Weghorn <m.weghorn at posteo.de>
    Reviewed-by: Katarina Behrens <Katarina.Behrens at cib.de>
    Tested-by: Jenkins

diff --git a/vcl/inc/qt5/Qt5Clipboard.hxx b/vcl/inc/qt5/Qt5Clipboard.hxx
index 93ad36a0e672..56d109b2e528 100644
--- a/vcl/inc/qt5/Qt5Clipboard.hxx
+++ b/vcl/inc/qt5/Qt5Clipboard.hxx
@@ -38,6 +38,7 @@ class Qt5Clipboard final
     osl::Mutex m_aMutex;
     const OUString m_aClipboardName;
     const QClipboard::Mode m_aClipboardMode;
+    bool m_bInSetContents;
 
     // if not empty, this holds the setContents provided XTransferable or a Qt5ClipboardTransferable
     css::uno::Reference<css::datatransfer::XTransferable> m_aContents;
diff --git a/vcl/qt5/Qt5Clipboard.cxx b/vcl/qt5/Qt5Clipboard.cxx
index db018c82150f..4b31864bf2bd 100644
--- a/vcl/qt5/Qt5Clipboard.cxx
+++ b/vcl/qt5/Qt5Clipboard.cxx
@@ -29,6 +29,7 @@ Qt5Clipboard::Qt5Clipboard(const OUString& aModeString, const QClipboard::Mode a
                                     XServiceInfo>(m_aMutex)
     , m_aClipboardName(aModeString)
     , m_aClipboardMode(aMode)
+    , m_bInSetContents(false)
 {
     assert(isSupported(m_aClipboardMode));
     // DirectConnection guarantees the changed slot runs in the same thread as the QClipboard
@@ -73,8 +74,10 @@ css::uno::Reference<css::datatransfer::XTransferable> Qt5Clipboard::getContents(
 {
     osl::MutexGuard aGuard(m_aMutex);
 
-    // if we're the owner, we have the XTransferable from setContents
-    if (isOwner(m_aClipboardMode))
+    // if we're the owner, we might have the XTransferable from setContents. but
+    // maybe a non-LO clipboard change from within LO, like some C'n'P in the
+    // QFileDialog, might have invalidated m_aContents, so we need to check it too.
+    if (isOwner(m_aClipboardMode) && m_aContents.is())
         return m_aContents;
 
     // check if we can still use the shared Qt5ClipboardTransferable
@@ -103,7 +106,9 @@ void Qt5Clipboard::setContents(
     m_aContents = xTrans;
     m_aOwner = xClipboardOwner;
 
-    // these will trigger QClipboard::changed / handleChanged
+    // these QApplication::clipboard() calls will trigger QClipboard::changed / handleChanged.
+    // we need to prevent freeing the contents, so tell handleChanged about us setting it
+    m_bInSetContents = true;
     if (m_aContents.is())
         QApplication::clipboard()->setMimeData(new Qt5MimeData(m_aContents), m_aClipboardMode);
     else
@@ -111,6 +116,7 @@ void Qt5Clipboard::setContents(
         assert(!m_aOwner.is());
         QApplication::clipboard()->clear(m_aClipboardMode);
     }
+    m_bInSetContents = false;
 
     aGuard.clear();
 
@@ -130,8 +136,7 @@ void Qt5Clipboard::handleChanged(QClipboard::Mode aMode)
     css::uno::Reference<css::datatransfer::clipboard::XClipboardOwner> xOldOwner(m_aOwner);
     css::uno::Reference<css::datatransfer::XTransferable> xOldContents(m_aContents);
     // ownership change from LO POV is handled in setContents
-    const bool bLostOwnership = !isOwner(m_aClipboardMode);
-    if (bLostOwnership)
+    if (!m_bInSetContents)
     {
         m_aContents.clear();
         m_aOwner.clear();
@@ -144,7 +149,7 @@ void Qt5Clipboard::handleChanged(QClipboard::Mode aMode)
 
     aGuard.clear();
 
-    if (bLostOwnership && xOldOwner.is())
+    if (!m_bInSetContents && xOldOwner.is())
         xOldOwner->lostOwnership(this, xOldContents);
     for (auto const& listener : aListeners)
         listener->changedContents(aEv);