2.7.9 copies password instead of marked text from notes field into clipboard

[foss-]

Overview

Since installing 2.7.9 seeing behavior I find unexpected as follows:

Steps to Reproduce

1. select entry that has some text in Notes field
2. select some of that text and press CMD + C

Expected Behavior

The marked text should be copied to clipboard.

Actual Behavior

The password of the selected entry is copied to clipboard.

Context

The behavior has changed since 2.7.9 and there is a commit that may be relevant to this: https://github.com/keepassxreboot/keepassxc/pull/10853

KeePassXC 2.7.9 macOS 14.5

Apparently the change was made with good intentions, but the current behavior is very unexpected. The reasoning is very simple: if content is selected and cmd+c is pressed the selection should be copied to clipboard. I would consider anything deviating from that simple logic as unexpected. cmd + c is copy. And when I right click on the marked text and select copy it is actually copied to clipboard. cmd+c should follow this.

Copying the password when an entry is selected and no text is marked is fine, but as soon as any other detail - e.g. username or note - is selected, cmd + c should copy that and not the password.

[c4rlo]

I'm unable to reproduce this with 2.7.9 on Linux (sway / Wayland). When pressing Ctrl+C (Linux equivalent of Cmd+C) with Notes text selected, it always copies the selected Notes text to the clipboard. I tested this just now both with the entry selected (copying Notes text from the preview), and with the entry editor opened (copying text from the Notes entry field).

So this might be a macOS-specific issue...?

[baekelite]

I also observe this behavior on macOS

[c4rlo]

I don't have the ability to build/test on macOS. If someone can reproduce this bug (on macOS or otherwise), please could they apply the following patch, reproduce the issue, and paste the output here:

--- a/src/gui/MainWindow.cpp
+++ b/src/gui/MainWindow.cpp
@@ -83,6 +83,26 @@
 // This filter gets installed on all the QAction objects within the MainWindow.
 bool ActionEventFilter::eventFilter(QObject* watched, QEvent* event)
 {
+    if (watched->objectName() == "actionEntryCopyPassword") {
+        if (event->type() == QEvent::Shortcut) {
+            static const auto stdCopyShortcuts = QKeySequence::keyBindings(QKeySequence::Copy);
+            QString stdCopyStr;
+            for (const auto& k : stdCopyShortcuts) {
+                if (!stdCopyStr.isEmpty()) {
+                    stdCopyStr += ", ";
+                }
+                stdCopyStr += k.toString(QKeySequence::NativeText);
+            }
+            const auto eventKey = static_cast<QShortcutEvent*>(event)->key();
+            qDebug("actionEntryCopyPassword Shortcut event: key=%s, copyKeys=[%s], match=%d",
+                   qPrintable(eventKey.toString(QKeySequence::NativeText)),
+                   qPrintable(stdCopyStr),
+                   stdCopyShortcuts.contains(eventKey));
+        } else {
+            qDebug("actionEntryCopyPassword type %d event", event->type());
+        }
+    }
+
     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
@@ -95,7 +115,10 @@ bool ActionEventFilter::eventFilter(QObject* watched, QEvent* event)
             // behave like that on all platforms.
             if (databaseWidget->copyFocusedTextSelection()) {
                 // In that case, we return true to stop further processing of this event.
+                qDebug("copied text selection instead of regular action processing");
                 return true;
+            } else {
+                qDebug("no text was selected hence regular processing");
             }
         }
     }

For reference, here is what I get when I unsuccessfully attempt to reproduce this on Linux:

actionEntryCopyPassword Shortcut event: key=Ctrl+C, copyKeys=[Ctrl+C, Ctrl+Ins, F16], match=1
copied text selection instead of regular action processing

(In addition to some "type 113" events which are irrelevant.)

Note that the output goes to stdout/stderr, so start keepassxc from a terminal/console to see it.

[droidmonkey]

I cannot replicate this on Windows.

[baekelite]

I can build with c4rlo's above change and provide the output, sometime this weekend.

[droidmonkey]

@c4rlo the problem is that QEvent::Shortcut isn't consistently advertised on macOS.

Edit: I got it to trigger the Shortcut event about once every 10 times.

[c4rlo]

@c4rlo the problem is that QEvent::Shortcut isn't consistently advertised on macOS.

Edit: I got it to trigger the Shortcut event about once every 10 times.

Is there another event type that we do get reliably on macOS? For example, ShortcutOverride, KeyPress or KeyRelease?

[c4rlo]

Digging a bit through the Qt sources, I wonder if this is related:

https://github.com/qt/qtbase/blob/86c62c8f6088ec148512457cb7e964661ba643b0/src/gui/kernel/qguiapplication.cpp#L2394-L2399

#if !defined(Q_OS_MACOS)
    // FIXME: Include OS X in this code path by passing the key event through
    // QPlatformInputContext::filterEvent().
    if (e->keyType == QEvent::KeyPress && window) {
        if (QWindowSystemInterface::handleShortcutEvent(window, e->timestamp, e->key, e->modifiers,
            e->nativeScanCode, e->nativeVirtualKey, e->nativeModifiers, e->unicode, e->repeat, e->repeatCount)) {

Introduced in https://github.com/qt/qtbase/commit/ee9621b9dc6cab96df627aa7b926e6256ea2102a. Commit message says "This commit restores the handling to QtGui for non-OS X platforms, and we should fix OS X by adding callbacks through the IME for the special case of OS X." Maybe that was never done?

[droidmonkey]

Hah, probably not...

The only real fix to this now on macOS is to intercept key presses at MainWindow level and detect when a copy shortcut key press is detected.

[c4rlo]

10966 is my proposed fix. Since I don't have a macOS dev environment, it would be great it someone else could check if that fixes the issue.