wayland: Check serial instead of focus for changing selections

Clients can have valid reasons to change the selection when the
same user action that also caused the selection request
to lose keyboard focus. This is notbaly the case for menus
created from a Plasma panel which itself will not take focus
but when clicking on action it only triggers after the menu
is closed.
This also matches what weston and sway do.
BUG:490803

autotests/integration/helper/copy.cpp

@@ -21,7 +21,7 @@ public:
 
 protected:
     void paintEvent(QPaintEvent *event) override;
-    void focusInEvent(QFocusEvent *event) override;
+    void keyPressEvent(QKeyEvent *) override;
 };
 
 Window::Window()
@@ -37,13 +37,9 @@ void Window::paintEvent(QPaintEvent *event)
     p.fillRect(0, 0, width(), height(), Qt::red);
 }
 
-void Window::focusInEvent(QFocusEvent *event)
+void Window::keyPressEvent(QKeyEvent *event)
 {
-    QRasterWindow::focusInEvent(event);
-    // TODO: make it work without singleshot
-    QTimer::singleShot(100, [] {
-        qApp->clipboard()->setText(QStringLiteral("test"));
-    });
+    qApp->clipboard()->setText(QStringLiteral("test"));
 }
 
 int main(int argc, char *argv[])

autotests/integration/xwayland_selections_test.cpp

@@ -10,6 +10,7 @@
 #include "kwin_wayland_test.h"
 
 #include "core/output.h"
+#include "wayland/display.h"
 #include "wayland/seat.h"
 #include "wayland_server.h"
 #include "window.h"
@@ -20,6 +21,8 @@
 #include <QProcessEnvironment>
 #include <QSignalSpy>
 
+#include <linux/input-event-codes.h>
+
 using namespace KWin;
 
 static const QString s_socketName = QStringLiteral("wayland_test_kwin_xwayland_selections-0");
@@ -111,6 +114,10 @@ void XwaylandSelectionsTest::testSync()
         workspace()->activateWindow(copyWindow);
     }
     QCOMPARE(workspace()->activeWindow(), copyWindow);
+    // Wait until the window has a surface so we can pass input to it
+    QTRY_VERIFY(copyWindow->surface());
+    Test::keyboardKeyPressed(KEY_C, waylandServer()->display()->nextSerial());
+    Test::keyboardKeyReleased(KEY_C, waylandServer()->display()->nextSerial());
     clipboardChangedSpy.wait();
 
     // start the paste process

autotests/wayland/client/test_datadevice.cpp

@@ -173,7 +173,7 @@ void TestDataDevice::testCreate()
 
     // this will probably fail, we need to make a selection client side
     QVERIFY(!m_seatInterface->selection());
-    m_seatInterface->setSelection(deviceInterface->selection());
+    m_seatInterface->setSelection(deviceInterface->selection(), m_display->nextSerial());
     QCOMPARE(m_seatInterface->selection(), deviceInterface->selection());
 
     // and destroy
@@ -402,13 +402,13 @@ void TestDataDevice::testSetSelection()
     QCOMPARE(dataOffer->offeredMimeTypes().last().name(), QStringLiteral("text/html"));
 
     // now clear the selection
-    dataDevice->clearSelection(1);
+    dataDevice->clearSelection(2);
     QVERIFY(selectionChangedSpy.wait());
     QCOMPARE(selectionChangedSpy.count(), 2);
     QVERIFY(!deviceInterface->selection());
 
     // set another selection
-    dataDevice->setSelection(2, dataSource.get());
+    dataDevice->setSelection(3, dataSource.get());
     QVERIFY(selectionChangedSpy.wait());
     // now unbind the dataDevice
     QSignalSpy unboundSpy(deviceInterface, &QObject::destroyed);
@@ -509,14 +509,14 @@ void TestDataDevice::testReplaceSource()
     QVERIFY(dataSource2->isValid());
     dataSource2->offer(QStringLiteral("text/plain"));
     QSignalSpy sourceCancelled2Spy(dataSource2.get(), &KWayland::Client::DataSource::cancelled);
-    dataDevice->setSelection(1, dataSource2.get());
+    dataDevice->setSelection(2, dataSource2.get());
     QCOMPARE(selectionOfferedSpy.count(), 1);
     QVERIFY(sourceCancelledSpy.wait());
     QCOMPARE(selectionOfferedSpy.count(), 2);
     QVERIFY(sourceCancelled2Spy.isEmpty());
 
     // replace the data source with itself, ensure that it did not get cancelled
-    dataDevice->setSelection(1, dataSource2.get());
+    dataDevice->setSelection(3, dataSource2.get());
     QVERIFY(!sourceCancelled2Spy.wait(500));
     QCOMPARE(selectionOfferedSpy.count(), 2);
     QVERIFY(sourceCancelled2Spy.isEmpty());
@@ -527,7 +527,7 @@ void TestDataDevice::testReplaceSource()
     std::unique_ptr<KWayland::Client::DataSource> dataSource3(m_dataDeviceManager->createDataSource());
     QVERIFY(dataSource3->isValid());
     dataSource3->offer(QStringLiteral("text/plain"));
-    dataDevice2->setSelection(1, dataSource3.get());
+    dataDevice2->setSelection(4, dataSource3.get());
     QVERIFY(sourceCancelled2Spy.wait());
 
     // try to crash by first destroying dataSource3 and setting a new DataSource
@@ -535,7 +535,7 @@ void TestDataDevice::testReplaceSource()
     QVERIFY(dataSource4->isValid());
     dataSource4->offer(QStringLiteral("text/plain"));
     dataSource3.reset();
-    dataDevice2->setSelection(1, dataSource4.get());
+    dataDevice2->setSelection(5, dataSource4.get());
     QVERIFY(selectionOfferedSpy.wait());
 
     auto dataOffer = selectionOfferedSpy.last()[0].value<KWayland::Client::DataOffer *>();

autotests/wayland/client/test_selection.cpp

@@ -234,7 +234,7 @@ void SelectionTest::testClearOnEnter()
     m_client2.dataDevice->setSelection(keyboardEnteredClient2Spy.first().first().value<quint32>(), dataSource2.get());
     QVERIFY(selectionOfferedClient2Spy.wait());
     // and clear
-    m_client2.dataDevice->clearSelection(keyboardEnteredClient2Spy.first().first().value<quint32>());
+    m_client2.dataDevice->clearSelection(keyboardEnteredClient2Spy.first().first().value<quint32>() + 1);
     QVERIFY(selectionClearedClient2Spy.wait());
 
     // now pass focus to first surface

autotests/wayland/client/test_wayland_seat.cpp

@@ -1571,7 +1571,7 @@ void TestWaylandSeat::testSelection()
     std::unique_ptr<KWayland::Client::DataSource> ds(ddm->createDataSource());
     QVERIFY(ds->isValid());
     ds->offer(QStringLiteral("text/plain"));
-    dd1->setSelection(0, ds.get());
+    dd1->setSelection(m_display->nextSerial(), ds.get());
     QVERIFY(selectionSpy.wait());
     QCOMPARE(selectionSpy.count(), 1);
     auto ddi = m_seatInterface->selection();
@@ -1581,7 +1581,7 @@ void TestWaylandSeat::testSelection()
     QCOMPARE(df->offeredMimeTypes().first().name(), QStringLiteral("text/plain"));
 
     // try to clear
-    dd1->setSelection(0);
+    dd1->setSelection(m_display->nextSerial());
     QVERIFY(selectionClearedSpy.wait());
     QCOMPARE(selectionClearedSpy.count(), 1);
     QCOMPARE(selectionSpy.count(), 1);
@@ -1594,30 +1594,30 @@ void TestWaylandSeat::testSelection()
     QCoreApplication::processEvents();
 
     // try to set Selection
-    dd1->setSelection(0, ds.get());
+    dd1->setSelection(m_display->nextSerial(), ds.get());
     wl_display_flush(m_connection->display());
     QCoreApplication::processEvents();
     QCoreApplication::processEvents();
     QCOMPARE(selectionSpy.count(), 1);
 
     // let's unset the selection on the seat
-    m_seatInterface->setSelection(nullptr);
+    m_seatInterface->setSelection(nullptr, m_display->nextSerial());
     // and pass focus back on our surface
     m_seatInterface->setFocusedKeyboardSurface(serverSurface);
     // we don't have a selection, so it should not send a selection
     QVERIFY(sync());
     QCOMPARE(selectionSpy.count(), 1);
     // now let's set it manually
-    m_seatInterface->setSelection(ddi);
+    m_seatInterface->setSelection(ddi, m_display->nextSerial());
     QCOMPARE(m_seatInterface->selection(), ddi);
     QVERIFY(selectionSpy.wait());
     QCOMPARE(selectionSpy.count(), 2);
     // setting the same again should not change
-    m_seatInterface->setSelection(ddi);
+    m_seatInterface->setSelection(ddi, m_display->nextSerial());
     QVERIFY(sync());
     QCOMPARE(selectionSpy.count(), 2);
     // now clear it manually
-    m_seatInterface->setSelection(nullptr);
+    m_seatInterface->setSelection(nullptr, m_display->nextSerial());
     QVERIFY(selectionClearedSpy.wait());
     QCOMPARE(selectionSpy.count(), 2);
 
@@ -1627,10 +1627,10 @@ void TestWaylandSeat::testSelection()
     std::unique_ptr<KWayland::Client::DataSource> ds2(ddm->createDataSource());
     QVERIFY(ds2->isValid());
     ds2->offer(QStringLiteral("text/plain"));
-    dd2->setSelection(0, ds2.get());
+    dd2->setSelection(m_display->nextSerial(), ds2.get());
     QVERIFY(selectionSpy.wait());
     QSignalSpy cancelledSpy(ds2.get(), &KWayland::Client::DataSource::cancelled);
-    m_seatInterface->setSelection(ddi);
+    m_seatInterface->setSelection(ddi, m_display->nextSerial());
     QVERIFY(cancelledSpy.wait());
 }
 
@@ -1680,7 +1680,7 @@ void TestWaylandSeat::testDataDeviceForKeyboardSurface()
     QVERIFY(ddiCreatedSpy.wait());
     auto ddi = ddiCreatedSpy.first().first().value<DataDeviceInterface *>();
     QVERIFY(ddi);
-    m_seatInterface->setSelection(ddi->selection());
+    m_seatInterface->setSelection(ddi->selection(), m_display->nextSerial());
 
     // switch to other client
     // create a surface and pass it keyboard focus

autotests/wayland/server/test_datacontrol_interface.cpp

@@ -257,7 +257,7 @@ void DataControlInterfaceTest::testCopyToControl()
     QSignalSpy selectionSpy(dataControlDevice.get(), &DataControlDevice::selection);
 
     std::unique_ptr<TestDataSource> testSelection(new TestDataSource);
-    m_seat->setSelection(testSelection.get());
+    m_seat->setSelection(testSelection.get(), m_display->nextSerial());
 
     // selection will be sent after we've been sent a new offer object and the mimes have been sent to that object
     selectionSpy.wait();
@@ -283,7 +283,7 @@ void DataControlInterfaceTest::testCopyToControlPrimarySelection()
     QSignalSpy selectionSpy(dataControlDevice.get(), &DataControlDevice::primary_selection);
 
     std::unique_ptr<TestDataSource> testSelection(new TestDataSource);
-    m_seat->setPrimarySelection(testSelection.get());
+    m_seat->setPrimarySelection(testSelection.get(), m_display->nextSerial());
 
     // selection will be sent after we've been sent a new offer object and the mimes have been sent to that object
     selectionSpy.wait();
@@ -353,7 +353,7 @@ void DataControlInterfaceTest::testKlipperCase()
 
     // Client A has a data source
     std::unique_ptr<TestDataSource> testSelection(new TestDataSource);
-    m_seat->setSelection(testSelection.get());
+    m_seat->setSelection(testSelection.get(), m_display->nextSerial());
 
     // klipper gets it
     selectionSpy.wait();
@@ -366,7 +366,7 @@ void DataControlInterfaceTest::testKlipperCase()
 
     // Client A sets something else
     std::unique_ptr<TestDataSource> testSelection2(new TestDataSource);
-    m_seat->setSelection(testSelection2.get());
+    m_seat->setSelection(testSelection2.get(), m_display->nextSerial());
 
     // Meanwhile klipper updates with the old content
     std::unique_ptr<DataControlSource> source(new DataControlSource);

src/wayland/datadevice.cpp

@@ -140,7 +140,7 @@ void DataDeviceInterfacePrivate::data_device_set_selection(Resource *resource, w
         selection->cancel();
     }
     selection = dataSource;
-    Q_EMIT q->selectionChanged(selection);
+    Q_EMIT q->selectionChanged(selection, serial);
 }
 
 void DataDeviceInterfacePrivate::data_device_release(QtWaylandServer::wl_data_device::Resource *resource)

src/wayland/datadevice.h

@@ -109,7 +109,7 @@ public:
 Q_SIGNALS:
     void aboutToBeDestroyed();
     void dragStarted(AbstractDataSource *source, SurfaceInterface *originSurface, quint32 serial, DragAndDropIcon *dragIcon);
-    void selectionChanged(KWin::DataSourceInterface *);
+    void selectionChanged(KWin::DataSourceInterface *, quint32 serial);
 
 private:
     friend class DataDeviceManagerInterfacePrivate;

src/wayland/primaryselectiondevice_v1.cpp

@@ -62,7 +62,7 @@ void PrimarySelectionDeviceV1InterfacePrivate::zwp_primary_selection_device_v1_s
     }
     selection = dataSource;
     if (selection) {
-        Q_EMIT q->selectionChanged(selection);
+        Q_EMIT q->selectionChanged(selection, serial);
     }
 }
 

src/wayland/primaryselectiondevice_v1.h

@@ -45,7 +45,7 @@ public:
     wl_client *client() const;
 
 Q_SIGNALS:
-    void selectionChanged(KWin::PrimarySelectionSourceV1Interface *);
+    void selectionChanged(KWin::PrimarySelectionSourceV1Interface *, quint32 serial);
 
 private:
     friend class PrimarySelectionDeviceManagerV1InterfacePrivate;

src/wayland/seat.cpp

@@ -150,8 +150,8 @@ void SeatInterfacePrivate::registerDataDevice(DataDeviceInterface *dataDevice)
         globalKeyboard.focus.selections.removeOne(dataDevice);
     };
     QObject::connect(dataDevice, &QObject::destroyed, q, dataDeviceCleanup);
-    QObject::connect(dataDevice, &DataDeviceInterface::selectionChanged, q, [this, dataDevice] {
-        updateSelection(dataDevice);
+    QObject::connect(dataDevice, &DataDeviceInterface::selectionChanged, q, [this](DataSourceInterface *source, quint32 serial) {
+        updateSelection(source, serial);
     });
     QObject::connect(dataDevice,
                      &DataDeviceInterface::dragStarted,
@@ -208,7 +208,7 @@ void SeatInterfacePrivate::registerDataControlDevice(DataControlDeviceV1Interfac
             dataDevice->sendSelection(currentSelection);
             return;
         }
-        q->setSelection(dataDevice->selection());
+        q->setSelection(dataDevice->selection(), display->nextSerial());
     });
 
     QObject::connect(dataDevice, &DataControlDeviceV1Interface::primarySelectionChanged, q, [this, dataDevice] {
@@ -222,7 +222,7 @@ void SeatInterfacePrivate::registerDataControlDevice(DataControlDeviceV1Interfac
             dataDevice->sendPrimarySelection(currentPrimarySelection);
             return;
         }
-        q->setPrimarySelection(dataDevice->primarySelection());
+        q->setPrimarySelection(dataDevice->primarySelection(), display->nextSerial());
     });
 
     dataDevice->sendSelection(currentSelection);
@@ -239,8 +239,8 @@ void SeatInterfacePrivate::registerPrimarySelectionDevice(PrimarySelectionDevice
         globalKeyboard.focus.primarySelections.removeOne(primarySelectionDevice);
     };
     QObject::connect(primarySelectionDevice, &QObject::destroyed, q, dataDeviceCleanup);
-    QObject::connect(primarySelectionDevice, &PrimarySelectionDeviceV1Interface::selectionChanged, q, [this, primarySelectionDevice] {
-        updatePrimarySelection(primarySelectionDevice);
+    QObject::connect(primarySelectionDevice, &PrimarySelectionDeviceV1Interface::selectionChanged, q, [this](PrimarySelectionSourceV1Interface *source, quint32 serial) {
+        updatePrimarySelection(source, serial);
     });
     // is the new DataDevice for the current keyoard focus?
     if (globalKeyboard.focus.surface) {
@@ -306,30 +306,26 @@ void SeatInterfacePrivate::endDrag()
     Q_EMIT q->dragEnded();
 }
 
-void SeatInterfacePrivate::updateSelection(DataDeviceInterface *dataDevice)
+void SeatInterfacePrivate::updateSelection(DataSourceInterface *dataSource, quint32 serial)
 {
-    DataSourceInterface *selection = dataDevice->selection();
-    // if the update is from the focussed window we should inform the active client
-    if (!(globalKeyboard.focus.surface && (*globalKeyboard.focus.surface->client() == dataDevice->client()))) {
-        if (selection) {
-            selection->cancel();
+    if (currentSelectionSerial - serial < UINT32_MAX / 2) {
+        if (dataSource) {
+            dataSource->cancel();
         }
         return;
     }
-    q->setSelection(selection);
+    q->setSelection(dataSource, serial);
 }
 
-void SeatInterfacePrivate::updatePrimarySelection(PrimarySelectionDeviceV1Interface *primarySelectionDevice)
+void SeatInterfacePrivate::updatePrimarySelection(PrimarySelectionSourceV1Interface *dataSource, quint32 serial)
 {
-    PrimarySelectionSourceV1Interface *selection = primarySelectionDevice->selection();
-    // if the update is from the focussed window we should inform the active client
-    if (!(globalKeyboard.focus.surface && (*globalKeyboard.focus.surface->client() == primarySelectionDevice->client()))) {
-        if (selection) {
-            selection->cancel();
+    if (currentPrimarySelectionSerial - serial < UINT32_MAX / 2) {
+        if (dataSource) {
+            dataSource->cancel();
         }
         return;
     }
-    q->setPrimarySelection(selection);
+    q->setPrimarySelection(dataSource, serial);
 }
 
 void SeatInterfacePrivate::sendCapabilities()
@@ -1286,7 +1282,7 @@ AbstractDataSource *SeatInterface::selection() const
     return d->currentSelection;
 }
 
-void SeatInterface::setSelection(AbstractDataSource *selection)
+void SeatInterface::setSelection(AbstractDataSource *selection, quint32 serial)
 {
     if (d->currentSelection == selection) {
         return;
@@ -1298,13 +1294,14 @@ void SeatInterface::setSelection(AbstractDataSource *selection)
     }
 
     if (selection) {
-        auto cleanup = [this]() {
-            setSelection(nullptr);
+        auto cleanup = [this, serial]() {
+            setSelection(nullptr, serial);
         };
         connect(selection, &AbstractDataSource::aboutToBeDestroyed, this, cleanup);
     }
 
     d->currentSelection = selection;
+    d->currentSelectionSerial = serial;
 
     for (auto focussedSelection : std::as_const(d->globalKeyboard.focus.selections)) {
         focussedSelection->sendSelection(selection);
@@ -1322,7 +1319,7 @@ AbstractDataSource *SeatInterface::primarySelection() const
     return d->currentPrimarySelection;
 }
 
-void SeatInterface::setPrimarySelection(AbstractDataSource *selection)
+void SeatInterface::setPrimarySelection(AbstractDataSource *selection, quint32 serial)
 {
     if (d->currentPrimarySelection == selection) {
         return;
@@ -1333,13 +1330,14 @@ void SeatInterface::setPrimarySelection(AbstractDataSource *selection)
     }
 
     if (selection) {
-        auto cleanup = [this]() {
-            setPrimarySelection(nullptr);
+        auto cleanup = [this, serial]() {
+            setPrimarySelection(nullptr, serial);
         };
         connect(selection, &AbstractDataSource::aboutToBeDestroyed, this, cleanup);
     }
 
     d->currentPrimarySelection = selection;
+    d->currentPrimarySelectionSerial = serial;
 
     for (auto focussedSelection : std::as_const(d->globalKeyboard.focus.primarySelections)) {
         focussedSelection->sendSelection(selection);

src/wayland/seat.h

@@ -645,10 +645,10 @@ public:
      * @see selection
      * @see selectionChanged
      */
-    void setSelection(AbstractDataSource *selection);
+    void setSelection(AbstractDataSource *selection, quint32 serial);
 
     AbstractDataSource *primarySelection() const;
-    void setPrimarySelection(AbstractDataSource *selection);
+    void setPrimarySelection(AbstractDataSource *selection, quint32 serial);
 
     void startDrag(AbstractDataSource *source, SurfaceInterface *sourceSurface, int dragSerial = -1, DragAndDropIcon *dragIcon = nullptr);
 

src/wayland/seat_p.h

@@ -28,6 +28,7 @@ class TextInputV1Interface;
 class TextInputV2Interface;
 class TextInputV3Interface;
 class PrimarySelectionDeviceV1Interface;
+class PrimarySelectionSourceV1Interface;
 class DragAndDropIcon;
 
 class SeatInterfacePrivate : public QtWaylandServer::wl_seat
@@ -68,7 +69,9 @@ public:
 
     // the last thing copied into the clipboard content
     AbstractDataSource *currentSelection = nullptr;
+    quint32 currentSelectionSerial = 0;
     AbstractDataSource *currentPrimarySelection = nullptr;
+    quint32 currentPrimarySelectionSerial = 0;
 
     // Pointer related members
     struct Pointer
@@ -151,8 +154,8 @@ protected:
     void seat_release(Resource *resource) override;
 
 private:
-    void updateSelection(DataDeviceInterface *dataDevice);
-    void updatePrimarySelection(PrimarySelectionDeviceV1Interface *primarySelectionDevice);
+    void updateSelection(DataSourceInterface *dataSource, quint32 serial);
+    void updatePrimarySelection(PrimarySelectionSourceV1Interface *dataSource, quint32);
 };
 
 } // namespace KWin

src/xwayland/clipboard.cpp

@@ -11,6 +11,7 @@
 #include "datasource.h"
 #include "selection_source.h"
 
+#include "wayland/display.h"
 #include "wayland/seat.h"
 #include "wayland_server.h"
 #include "workspace.h"
@@ -160,11 +161,11 @@ void Clipboard::x11OffersChanged(const QStringList &added, const QStringList &re
         connect(newSelection.get(), &XwlDataSource::dataRequested, source, &X11Source::startTransfer);
         // we keep the old selection around because setSelection needs it to be still alive
         std::swap(m_selectionSource, newSelection);
-        waylandServer()->seat()->setSelection(m_selectionSource.get());
+        waylandServer()->seat()->setSelection(m_selectionSource.get(), waylandServer()->display()->nextSerial());
     } else {
         AbstractDataSource *currentSelection = waylandServer()->seat()->selection();
         if (!ownsSelection(currentSelection)) {
-            waylandServer()->seat()->setSelection(nullptr);
+            waylandServer()->seat()->setSelection(nullptr, waylandServer()->display()->nextSerial());
             m_selectionSource.reset();
         }
     }

src/xwayland/primary.cpp

@@ -12,6 +12,7 @@
 #include "datasource.h"
 #include "selection_source.h"
 
+#include "wayland/display.h"
 #include "wayland/seat.h"
 #include "wayland_server.h"
 #include "workspace.h"
@@ -171,11 +172,11 @@ void Primary::x11OffersChanged(const QStringList &added, const QStringList &remo
         connect(newSelection.get(), &XwlDataSource::dataRequested, source, &X11Source::startTransfer);
         // we keep the old selection around because setPrimarySelection needs it to be still alive
         std::swap(m_primarySelectionSource, newSelection);
-        waylandServer()->seat()->setPrimarySelection(m_primarySelectionSource.get());
+        waylandServer()->seat()->setPrimarySelection(m_primarySelectionSource.get(), waylandServer()->display()->nextSerial());
     } else {
         AbstractDataSource *currentSelection = waylandServer()->seat()->primarySelection();
         if (!ownsSelection(currentSelection)) {
-            waylandServer()->seat()->setPrimarySelection(nullptr);
+            waylandServer()->seat()->setPrimarySelection(nullptr, waylandServer()->display()->nextSerial());
             m_primarySelectionSource.reset();
         }
     }