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
