vcl/inc/qt5/QtTransferable.hxx | 20 +++++++++++++------- vcl/qt5/QtTransferable.cxx | 37 +++++++++++++++---------------------- 2 files changed, 28 insertions(+), 29 deletions(-)
New commits: commit 1db5b87fe69c2375f1d66974dafcd563303c76db Author: Michael Weghorn <m.wegh...@posteo.de> AuthorDate: Tue Feb 13 13:23:17 2024 +0100 Commit: Michael Weghorn <m.wegh...@posteo.de> CommitDate: Tue Feb 13 17:19:19 2024 +0100 tdf#156562 qt: Sync with system clipboard content if necessary If the `QtClipboardTransferable`'s mime data gets out of sync with the system clipboard's one, no longer cease operation and stop reporting any transfer data, but sync with the clipboard data, i.e. update the mime data with those currently in the system clipboard. For the "Paste Special" dialog with qt6 on Wayland (see tdf#156562), an external clipboard update gets triggered between the point in time when the std::shared_ptr<const TransferableDataHelper> aDataHelper in the `SID_PASTE_SPECIAL` case in `SwBaseShell::ExecClpbrd` gets assigned and when the callback set via `pDlg->StartExecuteAsync` gets called, even if there was no user interaction with the system clipboard at all. This however meant that the `aDataHelper` used in the callback would no longer return any transfer/mime data, so pasting wouldn't work. Handle that case by updating the `QtClipboardTransferable` mime data with the current system clipboard's data, but keep emitting a warning at least. As a consequence, opening the "Paste Special" dialog, then copying something else to the clipboard, then confirming the dialog will copy the newly copied data rather than what was in the clipboard when the dialog was initially started. That's the same for gtk3 already, but on Windows, the original clipboard content would still be pasted. (Retrieving the clipboard content anew in the callback using `TransferableDataHelper::CreateFromSystemClipboard` instead of passing the original `aDataHelper` into the callback would have a similar effect. However, on other platforms, reusing the previously copied data from the clipboard when the actual system clipboard was changed in between may be what's desired.) The observed extra/unexpected clipboard change event may be related/similar to what the following commit was addressing for the case of LO itself being the clipboard owner: commit 71471a36b125f6bdc915d5dbcae92ebcaa7ff5a4 Date: Tue Apr 6 01:41:08 2021 +0200 tdf#140404 Qt ignore "unchanged" clipboard events LO gets a Qt signal on all clipboard changes. For X11 you get one signal when you set the clipboard. Anything else normally signals lost of clipboard ownership. But on Wayland LO somehow gets a second notification without any actual change. AFAIK it's not triggered by any LO actions and isOwner still indicates, that LO has the ownership. This breaks the single notification assumption, the code was relying on. (...) Backtrace showing how the clipboard update gets triggered (with mode `QClipboardMode::Clipboard`, not just `QClipboardMode::Selection`; qtbase dev as of 0d0810e2dcc8a9ee28935af5daadc2ef36ed25a2): 1 QtClipboard::handleChanged QtClipboard.cxx 156 0x7f6284c7a7a8 2 QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<QClipboard::Mode>, void, void (QtClipboard:: *)(QClipboard::Mode)>::call(void (QtClipboard:: *)(QClipboard::Mode), QtClipboard *, void * *)::{lambda()#1}::operator()() const qobjectdefs_impl.h 153 0x7f6284c8450f 3 QtPrivate::FunctorCallBase::call_internal<void, QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<QClipboard::Mode>, void, void (QtClipboard:: *)(QClipboard::Mode)>::call(void (QtClipboard:: *)(QClipboard::Mode), QtClipboard *, void * *)::{lambda()#1}>(void * *, QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<QClipboard::Mode>, void, void (QtClipboard:: *)(QClipboard::Mode)>::call(void (QtClipboard:: *)(QClipboard::Mode), QtClipboard *, void * *)::{lambda()#1}&&) qobjectdefs_impl.h 72 0x7f6284c8500b 4 QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<QClipboard::Mode>, void, void (QtClipboard:: *)(QClipboard::Mode)>::call qobjectdefs_impl.h 152 0x7f6284c8457f 5 QtPrivate::FunctionPointer<void (QtClipboard:: *)(QClipboard::Mode)>::call<QtPrivate::List<QClipboard::Mode>, void> qobjectdefs_impl.h 200 0x7f6284c833ee 6 QtPrivate::QCallableObject<void (QtClipboard:: *)(QClipboard::Mode), QtPrivate::List<QClipboard::Mode>, void>::impl qobjectdefs_impl.h 571 0x7f6284c81f81 7 QtPrivate::QSlotObjectBase::call qobjectdefs_impl.h 487 0x7f62841b863f 8 doActivate<false> qobject.cpp 4116 0x7f628425772e 9 QMetaObject::activate qobject.cpp 4176 0x7f628424cdef 10 QClipboard::changed moc_qclipboard.cpp 182 0x7f62831e9fcc 11 QClipboard::emitChanged qclipboard.cpp 558 0x7f62831e9bc1 12 QPlatformClipboard::emitChanged qplatformclipboard.cpp 89 0x7f628324ed69 13 QtWaylandClient::QWaylandDataDevice::data_device_selection qwaylanddatadevice.cpp 295 0x7f6281b8eaf1 14 QtWayland::wl_data_device::handle_selection qwayland-wayland.cpp 985 0x7f6281b7389a 15 ?? 0x7f629681a40e 16 ?? 0x7f629681971d 17 ffi_call 0x7f6296819ef3 18 ?? 0x7f629955c921 19 ?? 0x7f6299558c09 20 wl_display_dispatch_queue_pending 0x7f629955a5ac 21 QtWaylandClient::EventThread::dispatchQueuePending qwaylanddisplay.cpp 227 0x7f6281afd5f4 22 QtWaylandClient::EventThread::readAndDispatchEvents qwaylanddisplay.cpp 109 0x7f6281afd124 23 QtWaylandClient::QWaylandDisplay::flushRequests qwaylanddisplay.cpp 508 0x7f6281af5b98 24 QtPrivate::FunctorCall<QtPrivate::IndexesList<>, QtPrivate::List<>, void, void (QtWaylandClient::QWaylandDisplay:: *)()>::call(void (QtWaylandClient::QWaylandDisplay:: *)(), QtWaylandClient::QWaylandDisplay *, void * *)::{lambda()#1}::operator()() const qobjectdefs_impl.h 153 0x7f6281b13ebb 25 QtPrivate::FunctorCallBase::call_internal<void, QtPrivate::FunctorCall<QtPrivate::IndexesList<>, QtPrivate::List<>, void, void (QtWaylandClient::QWaylandDisplay:: *)()>::call(void (QtWaylandClient::QWaylandDisplay:: *)(), QtWaylandClient::QWaylandDisplay *, void * *)::{lambda()#1}>(void * *, QtPrivate::FunctorCall<QtPrivate::IndexesList<>, QtPrivate::List<>, void, void (QtWaylandClient::QWaylandDisplay:: *)()>::call(void (QtWaylandClient::QWaylandDisplay:: *)(), QtWaylandClient::QWaylandDisplay *, void * *)::{lambda()#1}&&) qobjectdefs_impl.h 72 0x7f6281b151f0 26 QtPrivate::FunctorCall<QtPrivate::IndexesList<>, QtPrivate::List<>, void, void (QtWaylandClient::QWaylandDisplay:: *)()>::call(void (QtWaylandClient::QWaylandDisplay:: *)(), QtWaylandClient::QWaylandDisplay *, void * *) qobjectdefs_impl.h 152 0x7f6281b13f1c 27 QtPrivate::FunctionPointer<void (QtWaylandClient::QWaylandDisplay:: *)()>::call<QtPrivate::List<>, void>(void (QtWaylandClient::QWaylandDisplay:: *)(), QtWaylandClient::QWaylandDisplay *, void * *) qobjectdefs_impl.h 200 0x7f6281b11250 28 QtPrivate::QCallableObject<void (QtWaylandClient::QWaylandDisplay:: *)(), QtPrivate::List<>, void>::impl(int, QtPrivate::QSlotObjectBase *, QObject *, void * *, bool *) qobjectdefs_impl.h 571 0x7f6281b0d5e3 29 QtPrivate::QSlotObjectBase::call qobjectdefs_impl.h 487 0x7f62841b863f 30 QMetaCallEvent::placeMetaCall qobject.cpp 650 0x7f6284243bf9 31 QObject::event qobject.cpp 1447 0x7f6284245095 32 QApplicationPrivate::notify_helper qapplication.cpp 3298 0x7f62823a696c 33 QApplication::notify qapplication.cpp 3249 0x7f62823a677d 34 QCoreApplication::notifyInternal2 qcoreapplication.cpp 1138 0x7f62841b1162 35 QCoreApplication::sendEvent qcoreapplication.cpp 1581 0x7f62841b1d0b 36 QCoreApplicationPrivate::sendPostedEvents qcoreapplication.cpp 1936 0x7f62841b33c4 37 QCoreApplication::sendPostedEvents qcoreapplication.cpp 1770 0x7f62841b255e 38 postEventSourceDispatch qeventdispatcher_glib.cpp 244 0x7f62846384ac 39 ?? 0x7f628b9111f4 40 ?? 0x7f628b914317 41 g_main_context_iteration 0x7f628b914930 42 QEventDispatcherGlib::processEvents qeventdispatcher_glib.cpp 394 0x7f6284638d41 43 QPAEventDispatcherGlib::processEvents qeventdispatcher_glib.cpp 87 0x7f628395c418 44 QtInstance::ImplYield QtInstance.cxx 453 0x7f6284cd9a40 45 QtInstance::DoYield QtInstance.cxx 464 0x7f6284cd9b69 46 ImplYield svapp.cxx 390 0x7f628f739c32 47 Application::Yield svapp.cxx 474 0x7f628f73a9d0 48 Application::Execute svapp.cxx 368 0x7f628f739925 49 desktop::Desktop::Main app.cxx 1614 0x7f6299037216 50 ImplSVMain svmain.cxx 229 0x7f628f758df4 51 SVMain svmain.cxx 261 0x7f628f759109 52 soffice_main sofficemain.cxx 94 0x7f62990a4367 53 sal_main main.c 51 0x56508cf899d4 54 main main.c 49 0x56508cf899ba Change-Id: I1720d8acd72dca4ef6bc32935b920b2c1ff4d8f1 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163304 Tested-by: Jenkins Reviewed-by: Michael Weghorn <m.wegh...@posteo.de> diff --git a/vcl/inc/qt5/QtTransferable.hxx b/vcl/inc/qt5/QtTransferable.hxx index 11d673b98a12..38cb4d92005b 100644 --- a/vcl/inc/qt5/QtTransferable.hxx +++ b/vcl/inc/qt5/QtTransferable.hxx @@ -36,6 +36,14 @@ class QtTransferable : public cppu::WeakImplHelper<css::datatransfer::XTransfera const QMimeData* m_pMimeData; bool m_bProvideUTF16FromOtherEncoding; +protected: + /** Sets new mime data. + * Since data flavors supported by this class depend on the mime data, + * results from previous calls to the public methods of this + * class are no longer valid after setting new mime data using this method. + */ + void setMimeData(const QMimeData* pMimeData) { m_pMimeData = pMimeData; } + public: QtTransferable(const QMimeData* pMimeData); const QMimeData* mimeData() const { return m_pMimeData; } @@ -52,17 +60,17 @@ public: * the QClipboard's object thread, which is the QApplication's thread, so all of * the access has to go through RunInMainThread(). * - * If we detect a QMimeData change, we simply drop reporting any content. In theory - * we can recover in the case where there hadn't been any calls of the XTransferable - * interface, but currently we don't. But we ensure to never report mixed content, - * so we'll just cease operation on QMimeData change. + * If we detect a QMimeData change, the mime data is updated with the new one from + * the system clipboard. Note however that this means that results of any previous + * calls of the XTransferable interface will be out of sync with the newly set mime + * data, so this scenario should generally be avoided. **/ class QtClipboardTransferable final : public QtTransferable { // to detect in-flight QMimeData changes const QClipboard::Mode m_aMode; - bool hasInFlightChanged() const; + void ensureConsistencyWithSystemClipboard(); public: explicit QtClipboardTransferable(const QClipboard::Mode aMode, const QMimeData* pMimeData); diff --git a/vcl/qt5/QtTransferable.cxx b/vcl/qt5/QtTransferable.cxx index 675e5ef75cdc..41c2c58392ef 100644 --- a/vcl/qt5/QtTransferable.cxx +++ b/vcl/qt5/QtTransferable.cxx @@ -164,11 +164,15 @@ QtClipboardTransferable::QtClipboardTransferable(const QClipboard::Mode aMode, { } -bool QtClipboardTransferable::hasInFlightChanged() const +void QtClipboardTransferable::ensureConsistencyWithSystemClipboard() { - const bool bChanged(mimeData() != QApplication::clipboard()->mimeData(m_aMode)); - SAL_WARN_IF(bChanged, "vcl.qt", "In flight clipboard change detected - broken clipboard read!"); - return bChanged; + const QMimeData* pCurrentClipboardData = QApplication::clipboard()->mimeData(m_aMode); + if (mimeData() != pCurrentClipboardData) + { + SAL_WARN("vcl.qt", "In flight clipboard change detected - updating mime data with current " + "clipboard contents."); + setMimeData(pCurrentClipboardData); + } } css::uno::Any SAL_CALL @@ -178,8 +182,8 @@ QtClipboardTransferable::getTransferData(const css::datatransfer::DataFlavor& rF auto* pSalInst(GetQtInstance()); SolarMutexGuard g; pSalInst->RunInMainThread([&, this]() { - if (!hasInFlightChanged()) - aAny = QtTransferable::getTransferData(rFlavor); + ensureConsistencyWithSystemClipboard(); + aAny = QtTransferable::getTransferData(rFlavor); }); return aAny; } @@ -191,8 +195,8 @@ css::uno::Sequence<css::datatransfer::DataFlavor> auto* pSalInst(GetQtInstance()); SolarMutexGuard g; pSalInst->RunInMainThread([&, this]() { - if (!hasInFlightChanged()) - aSeq = QtTransferable::getTransferDataFlavors(); + ensureConsistencyWithSystemClipboard(); + aSeq = QtTransferable::getTransferDataFlavors(); }); return aSeq; } @@ -204,8 +208,8 @@ QtClipboardTransferable::isDataFlavorSupported(const css::datatransfer::DataFlav auto* pSalInst(GetQtInstance()); SolarMutexGuard g; pSalInst->RunInMainThread([&, this]() { - if (!hasInFlightChanged()) - bIsSupported = QtTransferable::isDataFlavorSupported(rFlavor); + ensureConsistencyWithSystemClipboard(); + bIsSupported = QtTransferable::isDataFlavorSupported(rFlavor); }); return bIsSupported; } commit ee03b3a7e85c7bb293e161cc6ea7fe7bb3bc9240 Author: Michael Weghorn <m.wegh...@posteo.de> AuthorDate: Fri Feb 9 10:55:16 2024 +0100 Commit: Michael Weghorn <m.wegh...@posteo.de> CommitDate: Tue Feb 13 17:19:12 2024 +0100 tdf#156562 qt: Don't cache supported mime types/data flavors Drop the optimization to remember the supported data flavors in `QtTransferable::m_aMimeTypeSeq` and evaluate them each time instead. This is in preparation of allowing to change the mime data in an upcoming commit. Another alternative would be to (re-)calculate `QtTransferable::m_aMimeTypeSeq` only at the point in time that the mime data are set. Keep that in mind for further consideration in case this should ever become relevant from a performance perspective. Change-Id: Ic1792f6c2a19bf4c8f642a6288e9f3413fe7c893 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163303 Tested-by: Jenkins Reviewed-by: Michael Weghorn <m.wegh...@posteo.de> diff --git a/vcl/inc/qt5/QtTransferable.hxx b/vcl/inc/qt5/QtTransferable.hxx index 5f1533dd5968..11d673b98a12 100644 --- a/vcl/inc/qt5/QtTransferable.hxx +++ b/vcl/inc/qt5/QtTransferable.hxx @@ -34,9 +34,7 @@ class QtTransferable : public cppu::WeakImplHelper<css::datatransfer::XTransfera QtTransferable(const QtTransferable&) = delete; const QMimeData* m_pMimeData; - osl::Mutex m_aMutex; bool m_bProvideUTF16FromOtherEncoding; - css::uno::Sequence<css::datatransfer::DataFlavor> m_aMimeTypeSeq; public: QtTransferable(const QMimeData* pMimeData); diff --git a/vcl/qt5/QtTransferable.cxx b/vcl/qt5/QtTransferable.cxx index d9e0beaa71d3..675e5ef75cdc 100644 --- a/vcl/qt5/QtTransferable.cxx +++ b/vcl/qt5/QtTransferable.cxx @@ -50,16 +50,6 @@ QtTransferable::QtTransferable(const QMimeData* pMimeData) css::uno::Sequence<css::datatransfer::DataFlavor> SAL_CALL QtTransferable::getTransferDataFlavors() { - // it's just filled once, ever, so just try to get it without locking first - if (m_aMimeTypeSeq.hasElements()) - return m_aMimeTypeSeq; - - // better safe then sorry; preventing broken usage - // DnD should not be shared and Clipboard access runs in the GUI thread - osl::MutexGuard aGuard(m_aMutex); - if (m_aMimeTypeSeq.hasElements()) - return m_aMimeTypeSeq; - QStringList aFormatList(m_pMimeData->formats()); // we might add the UTF-16 mime text variant later const int nMimeTypeSeqSize = aFormatList.size() + 1; @@ -113,8 +103,7 @@ css::uno::Sequence<css::datatransfer::DataFlavor> SAL_CALL QtTransferable::getTr aMimeTypeSeq.realloc(nMimeTypeCount); - m_aMimeTypeSeq = aMimeTypeSeq; - return m_aMimeTypeSeq; + return aMimeTypeSeq; } sal_Bool SAL_CALL