From 42ce2a49fac689476b1a0b94e3e4d91f842782cb Mon Sep 17 00:00:00 2001
From: Carlo Teubner <carlo@cteubner.net>
Date: Sun, 23 Jun 2024 21:16:03 +0100
Subject: [PATCH] Fix copy-to-clipboard shortcut on macOS

It turns out that the previous implementation, based on installing an event filter in every QAction instance, does not work on macOS, likely due to a Qt bug.

Attempt to work around this by using a different implementation of the same idea, by reacting to ShortcutOverride events in the MainWindow object.

Fixes #10929.
---
 src/gui/MainWindow.cpp | 59 ++++++++++++++++++++----------------------
 src/gui/MainWindow.h   | 11 +-------
 2 files changed, 29 insertions(+), 41 deletions(-)

diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp
index e80a9e7ad..9e38dcda9 100644
--- a/src/gui/MainWindow.cpp
+++ b/src/gui/MainWindow.cpp
@@ -78,28 +78,6 @@
 #include "mainwindowadaptor.h"
 #endif
 
-// This filter gets installed on all the QAction objects within the MainWindow.
-bool ActionEventFilter::eventFilter(QObject* watched, QEvent* event)
-{
-    auto databaseWidget = getMainWindow()->m_ui->tabWidget->currentDatabaseWidget();
-    if (databaseWidget && event->type() == QEvent::Shortcut) {
-        // We check if we got a Shortcut event that uses the same key sequence as the
-        // OS default copy-to-clipboard shortcut.
-        static const auto stdCopyShortcuts = QKeySequence::keyBindings(QKeySequence::Copy);
-        if (stdCopyShortcuts.contains(static_cast<QShortcutEvent*>(event)->key())) {
-            // If so, we ask the database widget to check if any of its sub-widgets has text
-            // selected, and to copy it to the clipboard if that is the case. We do this
-            // because that is what the user likely expects to happen, yet Qt does not
-            // behave like that on all platforms.
-            if (databaseWidget->copyFocusedTextSelection()) {
-                // In that case, we return true to stop further processing of this event.
-                return true;
-            }
-        }
-    }
-    return QObject::eventFilter(watched, event);
-}
-
 const QString MainWindow::BaseWindowTitle = "KeePassXC";
 
 MainWindow* g_MainWindow = nullptr;
@@ -1391,6 +1369,33 @@ void MainWindow::databaseTabChanged(int tabIndex)
     updateEntryCountLabel();
 }
 
+bool MainWindow::event(QEvent* event)
+{
+    if (event->type() == QEvent::ShortcutOverride) {
+        const auto keyevent = static_cast<QKeyEvent*>(event);
+        // Did we get a ShortcutOverride event with the same key sequence as the OS default
+        // copy-to-clipboard shortcut?
+        if (keyevent->matches(QKeySequence::Copy)) {
+            // If so, we ask the database widget to check if any of its sub-widgets has
+            // text selected, and to copy it to the clipboard if that is the case.
+            // We do this because that is what the user likely expects to happen, yet Qt does not
+            // behave like that (at least on some platforms).
+            auto dbWidget = m_ui->tabWidget->currentDatabaseWidget();
+            if (dbWidget && dbWidget->copyFocusedTextSelection()) {
+                // Note: instead of actively copying the selected text to the clipboard
+                // above, simply accepting the event would have a similar effect (Qt
+                // would deliver it as a key press to the current widget, which would
+                // trigger the built-in copy-to-clipboard behaviour). However, that
+                // would not come with our special (configurable) behaviour of
+                // clearing the clipboard after a certain time period.
+                keyevent->accept();
+                return true;
+            }
+        }
+    }
+    return QMainWindow::event(event);
+}
+
 void MainWindow::showEvent(QShowEvent* event)
 {
     Q_UNUSED(event)
@@ -1476,7 +1481,7 @@ void MainWindow::keyPressEvent(QKeyEvent* event)
         }
     }
 
-    QWidget::keyPressEvent(event);
+    QMainWindow::keyPressEvent(event);
 }
 
 bool MainWindow::focusNextPrevChild(bool next)
@@ -2197,14 +2202,6 @@ void MainWindow::initActionCollection()
     ac->setDefaultShortcut(m_ui->actionEntryRemoveFromAgent, Qt::META + Qt::SHIFT + Qt::Key_H);
 #endif
 
-    // Install an event filter on every action. It improves handling of keyboard
-    // shortcuts that match the system copy-to-clipboard key sequence; by default
-    // this applies to actionEntryCopyPassword, but this could differ based on
-    // shortcuts the user has configured, or may configure later.
-    for (auto action : ac->actions()) {
-        action->installEventFilter(&m_actionEventFilter);
-    }
-
     QTimer::singleShot(1, ac, &ActionCollection::restoreShortcuts);
 }
 
diff --git a/src/gui/MainWindow.h b/src/gui/MainWindow.h
index fcc90f4b0..59ff6042a 100644
--- a/src/gui/MainWindow.h
+++ b/src/gui/MainWindow.h
@@ -39,14 +39,6 @@ class InactivityTimer;
 class SearchWidget;
 class MainWindowEventFilter;
 
-class ActionEventFilter : public QObject
-{
-    Q_OBJECT
-
-protected:
-    bool eventFilter(QObject* obj, QEvent* event) override;
-};
-
 class MainWindow : public QMainWindow
 {
     Q_OBJECT
@@ -105,6 +97,7 @@ public slots:
     void restartApp(const QString& message);
 
 protected:
+    bool event(QEvent* event) override;
     void showEvent(QShowEvent* event) override;
     void hideEvent(QHideEvent* event) override;
     void closeEvent(QCloseEvent* event) override;
@@ -179,7 +172,6 @@ private:
 
     const QScopedPointer<Ui::MainWindow> m_ui;
     SignalMultiplexer m_actionMultiplexer;
-    ActionEventFilter m_actionEventFilter;
     QPointer<QAction> m_clearHistoryAction;
     QPointer<QAction> m_searchWidgetAction;
     QPointer<QMenu> m_entryContextMenu;
@@ -211,7 +203,6 @@ private:
     QTimer m_trayIconTriggerTimer;
     QSystemTrayIcon::ActivationReason m_trayIconTriggerReason;
 
-    friend class ActionEventFilter;
     friend class MainWindowEventFilter;
 };