Fix global Auto-Type when database locked

* Store the currently active window right when the global keyboard shortcut is triggered
* Eliminate unnecessary window raise/lower and delays on macOS
* Remove duplicate addition of macutils symbols from mac Auto-Type plugin
* Fix tests to fake trigger a global autotype sequence

src/autotype/AutoType.cpp

@@ -47,7 +47,7 @@ AutoType::AutoType(QObject* parent, bool test)
     , m_pluginLoader(new QPluginLoader(this))
     , m_plugin(nullptr)
     , m_executor(nullptr)
-    , m_windowFromGlobal(0)
+    , m_windowForGlobal(0)
 {
     // prevent crash when the plugin has unresolved symbols
     m_pluginLoader->setLoadHints(QLibrary::ResolveAllSymbolsHint);
@@ -90,7 +90,7 @@ void AutoType::loadPlugin(const QString& pluginPath)
         if (m_plugin) {
             if (m_plugin->isAvailable()) {
                 m_executor = m_plugin->createExecutor();
-                connect(pluginInstance, SIGNAL(globalShortcutTriggered()), SIGNAL(globalShortcutTriggered()));
+                connect(pluginInstance, SIGNAL(globalShortcutTriggered()), SLOT(startGlobalAutoType()));
             } else {
                 unloadPlugin();
             }
@@ -222,6 +222,7 @@ void AutoType::executeAutoTypeActions(const Entry* entry, QWidget* hideWindow, c
 
     Tools::wait(qMax(100, config()->get("AutoTypeStartDelay", 500).toInt()));
 
+    // Used only for selected entry auto-type
     if (!window) {
         window = m_plugin->activeWindow();
     }
@@ -240,6 +241,9 @@ void AutoType::executeAutoTypeActions(const Entry* entry, QWidget* hideWindow, c
         QCoreApplication::processEvents(QEventLoop::AllEvents, 10);
     }
 
+    m_windowForGlobal = 0;
+    m_windowTitleForGlobal.clear();
+
     // emit signal only if autotype performed correctly
     emit autotypePerformed();
 
@@ -264,6 +268,13 @@ void AutoType::performAutoType(const Entry* entry, QWidget* hideWindow)
     executeAutoTypeActions(entry, hideWindow, sequences.first());
 }
 
+void AutoType::startGlobalAutoType()
+{
+    m_windowForGlobal = m_plugin->activeWindow();
+    m_windowTitleForGlobal = m_plugin->activeWindowTitle();
+    emit globalAutoTypeTriggered();
+}
+
 /**
  * Global Autotype entry-point function
  * Perform global Auto-Type on the active window
@@ -278,9 +289,7 @@ void AutoType::performGlobalAutoType(const QList<QSharedPointer<Database>>& dbLi
         return;
     }
 
-    QString windowTitle = m_plugin->activeWindowTitle();
+    if (m_windowTitleForGlobal.isEmpty()) {
-
-    if (windowTitle.isEmpty()) {
         m_inGlobalAutoTypeDialog.unlock();
         return;
     }
@@ -290,7 +299,7 @@ void AutoType::performGlobalAutoType(const QList<QSharedPointer<Database>>& dbLi
     for (const auto& db : dbList) {
         const QList<Entry*> dbEntries = db->rootGroup()->entriesRecursive();
         for (Entry* entry : dbEntries) {
-            const QSet<QString> sequences = autoTypeSequences(entry, windowTitle).toSet();
+            const QSet<QString> sequences = autoTypeSequences(entry, m_windowTitleForGlobal).toSet();
             for (const QString& sequence : sequences) {
                 if (!sequence.isEmpty()) {
                     matchList << AutoTypeMatch(entry, sequence);
@@ -304,8 +313,9 @@ void AutoType::performGlobalAutoType(const QList<QSharedPointer<Database>>& dbLi
             auto* msgBox = new QMessageBox();
             msgBox->setAttribute(Qt::WA_DeleteOnClose);
             msgBox->setWindowTitle(tr("Auto-Type - KeePassXC"));
-            msgBox->setText(
+            msgBox->setText(tr("Couldn't find an entry that matches the window title:")
-                tr("Couldn't find an entry that matches the window title:").append("\n\n").append(windowTitle));
+                                .append("\n\n")
+                                .append(m_windowTitleForGlobal));
             msgBox->setIcon(QMessageBox::Information);
             msgBox->setStandardButtons(QMessageBox::Ok);
             msgBox->show();
@@ -316,10 +326,9 @@ void AutoType::performGlobalAutoType(const QList<QSharedPointer<Database>>& dbLi
         m_inGlobalAutoTypeDialog.unlock();
         emit autotypeRejected();
     } else if ((matchList.size() == 1) && !config()->get("security/autotypeask").toBool()) {
-        executeAutoTypeActions(matchList.first().entry, nullptr, matchList.first().sequence);
+        executeAutoTypeActions(matchList.first().entry, nullptr, matchList.first().sequence, m_windowForGlobal);
         m_inGlobalAutoTypeDialog.unlock();
     } else {
-        m_windowFromGlobal = m_plugin->activeWindow();
         auto* selectDialog = new AutoTypeSelectDialog();
 
         // connect slots, both of which must unlock the m_inGlobalAutoTypeDialog mutex
@@ -327,11 +336,12 @@ void AutoType::performGlobalAutoType(const QList<QSharedPointer<Database>>& dbLi
         connect(selectDialog, SIGNAL(rejected()), SLOT(autoTypeRejectedFromGlobal()));
 
         selectDialog->setMatchList(matchList);
-#if defined(Q_OS_MACOS)
+#ifdef Q_OS_MACOS
         m_plugin->raiseOwnWindow();
-        Tools::wait(500);
+        Tools::wait(200);
 #endif
         selectDialog->show();
+        selectDialog->raise();
         // necessary when the main window is minimized
         selectDialog->activateWindow();
     }
@@ -339,8 +349,8 @@ void AutoType::performGlobalAutoType(const QList<QSharedPointer<Database>>& dbLi
 
 void AutoType::performAutoTypeFromGlobal(AutoTypeMatch match)
 {
-    m_plugin->raiseWindow(m_windowFromGlobal);
+    m_plugin->raiseWindow(m_windowForGlobal);
-    executeAutoTypeActions(match.entry, nullptr, match.sequence, m_windowFromGlobal);
+    executeAutoTypeActions(match.entry, nullptr, match.sequence, m_windowForGlobal);
 
     // make sure the mutex is definitely locked before we unlock it
     Q_UNUSED(m_inGlobalAutoTypeDialog.tryLock());
@@ -353,6 +363,8 @@ void AutoType::autoTypeRejectedFromGlobal()
     // so make sure the mutex is locked before we try unlocking it
     Q_UNUSED(m_inGlobalAutoTypeDialog.tryLock());
     m_inGlobalAutoTypeDialog.unlock();
+    m_windowForGlobal = 0;
+    m_windowTitleForGlobal.clear();
 
     emit autotypeRejected();
 }

src/autotype/AutoType.h

@@ -62,18 +62,19 @@ public slots:
     void raiseWindow();
 
 signals:
-    void globalShortcutTriggered();
+    void globalAutoTypeTriggered();
     void autotypePerformed();
     void autotypeRejected();
 
 private slots:
+    void startGlobalAutoType();
     void performAutoTypeFromGlobal(AutoTypeMatch match);
     void autoTypeRejectedFromGlobal();
     void unloadPlugin();
 
 private:
     explicit AutoType(QObject* parent = nullptr, bool test = false);
-    ~AutoType();
+    ~AutoType() override;
     void loadPlugin(const QString& pluginPath);
     void executeAutoTypeActions(const Entry* entry,
                                 QWidget* hideWindow = nullptr,
@@ -94,9 +95,11 @@ private:
     QPluginLoader* m_pluginLoader;
     AutoTypePlatformInterface* m_plugin;
     AutoTypeExecutor* m_executor;
-    WId m_windowFromGlobal;
     static AutoType* m_instance;
 
+    QString m_windowTitleForGlobal;
+    WId m_windowForGlobal;
+
     Q_DISABLE_COPY(AutoType)
 };
 

src/autotype/mac/CMakeLists.txt

@@ -1,10 +1,6 @@
 set(autotype_mac_SOURCES AutoTypeMac.cpp)
 
-set(autotype_mac_mm_SOURCES
+add_library(keepassx-autotype-cocoa MODULE ${autotype_mac_SOURCES})
-        ${CMAKE_SOURCE_DIR}/src/gui/macutils/AppKitImpl.mm
-        ${CMAKE_SOURCE_DIR}/src/gui/macutils/MacUtils.cpp)
-
-add_library(keepassx-autotype-cocoa MODULE ${autotype_mac_SOURCES} ${autotype_mac_mm_SOURCES})
 set_target_properties(keepassx-autotype-cocoa PROPERTIES LINK_FLAGS "-framework Foundation -framework AppKit -framework Carbon")
 target_link_libraries(keepassx-autotype-cocoa ${PROGNAME} Qt5::Core Qt5::Widgets)
 

src/autotype/test/AutoTypeTest.cpp

@@ -68,6 +68,11 @@ AutoTypeExecutor* AutoTypePlatformTest::createExecutor()
     return new AutoTypeExecutorTest(this);
 }
 
+void AutoTypePlatformTest::triggerGlobalAutoType()
+{
+    emit globalShortcutTriggered();
+}
+
 void AutoTypePlatformTest::setActiveWindowTitle(const QString& title)
 {
     m_activeWindowTitle = title;

src/autotype/test/AutoTypeTest.h

@@ -48,6 +48,7 @@ public:
     bool raiseOwnWindow() override;
 #endif
 
+    void triggerGlobalAutoType() override;
     void setActiveWindowTitle(const QString& title) override;
 
     QString actionChars() override;

src/autotype/test/AutoTypeTestInterface.h

@@ -26,6 +26,7 @@ public:
     virtual ~AutoTypeTestInterface()
     {
     }
+    virtual void triggerGlobalAutoType() = 0;
     virtual void setActiveWindowTitle(const QString& title) = 0;
 
     virtual QString actionChars() = 0;

src/gui/DatabaseTabWidget.cpp

@@ -59,7 +59,7 @@ DatabaseTabWidget::DatabaseTabWidget(QWidget* parent)
     connect(this, SIGNAL(currentChanged(int)), SLOT(emitActivateDatabaseChanged()));
     connect(this, SIGNAL(activateDatabaseChanged(DatabaseWidget*)),
             m_dbWidgetStateSync, SLOT(setActive(DatabaseWidget*)));
-    connect(autoType(), SIGNAL(globalShortcutTriggered()), SLOT(performGlobalAutoType()));
+    connect(autoType(), SIGNAL(globalAutoTypeTriggered()), SLOT(performGlobalAutoType()));
     connect(autoType(), SIGNAL(autotypePerformed()), SLOT(relockPendingDatabase()));
     connect(autoType(), SIGNAL(autotypeRejected()), SLOT(relockPendingDatabase()));
     // clang-format on
@@ -562,7 +562,7 @@ void DatabaseTabWidget::unlockDatabaseInDialog(DatabaseWidget* dbWidget,
 #ifdef Q_OS_MACOS
     if (intent == DatabaseOpenDialog::Intent::AutoType || intent == DatabaseOpenDialog::Intent::Browser) {
         macUtils()->raiseOwnWindow();
-        Tools::wait(500);
+        Tools::wait(200);
     }
 #endif
 

tests/TestAutoType.cpp

@@ -157,6 +157,7 @@ void TestAutoType::testGlobalAutoTypeWithNoMatch()
 void TestAutoType::testGlobalAutoTypeWithOneMatch()
 {
     m_test->setActiveWindowTitle("custom window");
+    m_test->triggerGlobalAutoType();
     m_autoType->performGlobalAutoType(m_dbList);
 
     QCOMPARE(m_test->actionChars(), QString("%1association%2").arg(m_entry1->username()).arg(m_entry1->password()));
@@ -167,6 +168,7 @@ void TestAutoType::testGlobalAutoTypeTitleMatch()
     config()->set("AutoTypeEntryTitleMatch", true);
 
     m_test->setActiveWindowTitle("An Entry Title!");
+    m_test->triggerGlobalAutoType();
     m_autoType->performGlobalAutoType(m_dbList);
 
     QCOMPARE(m_test->actionChars(), QString("%1%2").arg(m_entry2->password(), m_test->keyToString(Qt::Key_Enter)));
@@ -177,6 +179,7 @@ void TestAutoType::testGlobalAutoTypeUrlMatch()
     config()->set("AutoTypeEntryTitleMatch", true);
 
     m_test->setActiveWindowTitle("Dummy - http://example.org/ - <My Browser>");
+    m_test->triggerGlobalAutoType();
     m_autoType->performGlobalAutoType(m_dbList);
 
     QCOMPARE(m_test->actionChars(), QString("%1%2").arg(m_entry5->password(), m_test->keyToString(Qt::Key_Enter)));
@@ -187,6 +190,7 @@ void TestAutoType::testGlobalAutoTypeUrlSubdomainMatch()
     config()->set("AutoTypeEntryTitleMatch", true);
 
     m_test->setActiveWindowTitle("Dummy - http://sub.example.org/ - <My Browser>");
+    m_test->triggerGlobalAutoType();
     m_autoType->performGlobalAutoType(m_dbList);
 
     QCOMPARE(m_test->actionChars(), QString("%1%2").arg(m_entry5->password(), m_test->keyToString(Qt::Key_Enter)));
@@ -195,6 +199,7 @@ void TestAutoType::testGlobalAutoTypeUrlSubdomainMatch()
 void TestAutoType::testGlobalAutoTypeTitleMatchDisabled()
 {
     m_test->setActiveWindowTitle("An Entry Title!");
+    m_test->triggerGlobalAutoType();
     MessageBox::setNextAnswer(MessageBox::Ok);
     m_autoType->performGlobalAutoType(m_dbList);
 
@@ -205,58 +210,68 @@ void TestAutoType::testGlobalAutoTypeRegExp()
 {
     // substring matches are ok
     m_test->setActiveWindowTitle("lorem REGEX1 ipsum");
+    m_test->triggerGlobalAutoType();
     m_autoType->performGlobalAutoType(m_dbList);
     QCOMPARE(m_test->actionChars(), QString("regex1"));
     m_test->clearActions();
 
     // should be case-insensitive
     m_test->setActiveWindowTitle("lorem regex1 ipsum");
+    m_test->triggerGlobalAutoType();
     m_autoType->performGlobalAutoType(m_dbList);
     QCOMPARE(m_test->actionChars(), QString("regex1"));
     m_test->clearActions();
 
     // exact match
     m_test->setActiveWindowTitle("REGEX2");
+    m_test->triggerGlobalAutoType();
     m_autoType->performGlobalAutoType(m_dbList);
     QCOMPARE(m_test->actionChars(), QString("regex2"));
     m_test->clearActions();
 
     // a bit more complicated regex
     m_test->setActiveWindowTitle("REGEX3-R2D2");
+    m_test->triggerGlobalAutoType();
     m_autoType->performGlobalAutoType(m_dbList);
     QCOMPARE(m_test->actionChars(), QString("regex3"));
     m_test->clearActions();
 
     // with custom attributes
     m_test->setActiveWindowTitle("CustomAttr1");
+    m_test->triggerGlobalAutoType();
     m_autoType->performGlobalAutoType(m_dbList);
     QCOMPARE(m_test->actionChars(), QString("custom_attr:Attribute"));
     m_test->clearActions();
 
     // with (non uppercase) undefined custom attributes
     m_test->setActiveWindowTitle("CustomAttr2");
+    m_test->triggerGlobalAutoType();
     m_autoType->performGlobalAutoType(m_dbList);
     QCOMPARE(m_test->actionChars(), QString(""));
     m_test->clearActions();
 
     // with mixedcase default attributes
     m_test->setActiveWindowTitle("CustomAttr3");
+    m_test->triggerGlobalAutoType();
     m_autoType->performGlobalAutoType(m_dbList);
     QCOMPARE(m_test->actionChars(), QString("custom_attr"));
     m_test->clearActions();
 
     // with resolve placeholders in window association title
     m_test->setActiveWindowTitle("AttrValueFirst");
+    m_test->triggerGlobalAutoType();
     m_autoType->performGlobalAutoType(m_dbList);
     QCOMPARE(m_test->actionChars(), QString("custom_attr_first"));
     m_test->clearActions();
 
     m_test->setActiveWindowTitle("lorem AttrValueFirstAndAttrValueSecond ipsum");
+    m_test->triggerGlobalAutoType();
     m_autoType->performGlobalAutoType(m_dbList);
     QCOMPARE(m_test->actionChars(), QString("custom_attr_first_and_second"));
     m_test->clearActions();
 
     m_test->setActiveWindowTitle("lorem AttrValueThird ipsum");
+    m_test->triggerGlobalAutoType();
     m_autoType->performGlobalAutoType(m_dbList);
     QCOMPARE(m_test->actionChars(), QString("custom_attr_third"));
     m_test->clearActions();