From 8c9530e3ecff1e8c92702788c362e9fb1f99e23e Mon Sep 17 00:00:00 2001 From: Toni Spets Date: Sat, 27 Mar 2021 13:44:08 +0200 Subject: [PATCH] Auto-Type: Allow actions to fail and be retried AutoTypeActions are required to return a result object with information if they can be retried or not. An error string is also provided to show a user friendly message why said action did not succeed if even after retries it keeps failing. This is a prerequisite for waiting for modifier keys to be released on X11. --- src/autotype/AutoType.cpp | 24 +++++++- src/autotype/AutoTypeAction.cpp | 16 +++--- src/autotype/AutoTypeAction.h | 70 +++++++++++++++++++++--- src/autotype/mac/AutoTypeMac.cpp | 9 ++- src/autotype/mac/AutoTypeMac.h | 6 +- src/autotype/test/AutoTypeTest.cpp | 9 ++- src/autotype/test/AutoTypeTest.h | 6 +- src/autotype/windows/AutoTypeWindows.cpp | 9 ++- src/autotype/windows/AutoTypeWindows.h | 6 +- src/autotype/xcb/AutoTypeXCB.cpp | 31 +++++++---- src/autotype/xcb/AutoTypeXCB.h | 8 +-- 11 files changed, 143 insertions(+), 51 deletions(-) diff --git a/src/autotype/AutoType.cpp b/src/autotype/AutoType.cpp index f226f59fb..a8d1ed091 100644 --- a/src/autotype/AutoType.cpp +++ b/src/autotype/AutoType.cpp @@ -294,7 +294,8 @@ void AutoType::executeAutoTypeActions(const Entry* entry, QWidget* hideWindow, c window = m_plugin->activeWindow(); } - Tools::wait(qMax(100, config()->get(Config::AutoTypeStartDelay).toInt())); + int delay = qMax(100, config()->get(Config::AutoTypeStartDelay).toInt()); + Tools::wait(delay); for (const auto& action : asConst(actions)) { if (m_plugin->activeWindow() != window) { @@ -304,8 +305,25 @@ void AutoType::executeAutoTypeActions(const Entry* entry, QWidget* hideWindow, c return; } - action->exec(m_executor); - QCoreApplication::processEvents(QEventLoop::AllEvents, 10); + constexpr int max_retries = 5; + for (int i = 1; i <= max_retries; i++) { + auto result = action->exec(m_executor); + + QCoreApplication::processEvents(QEventLoop::AllEvents, 10); + + if (result.isOk()) { + break; + } + + if (!result.canRetry() || i == max_retries) { + MessageBox::critical(getMainWindow(), tr("Auto-Type Error"), result.errorString()); + emit autotypeRejected(); + m_inAutoType.unlock(); + return; + } + + Tools::wait(delay); + } } m_windowForGlobal = 0; diff --git a/src/autotype/AutoTypeAction.cpp b/src/autotype/AutoTypeAction.cpp index 587dee0f5..d43ff7002 100644 --- a/src/autotype/AutoTypeAction.cpp +++ b/src/autotype/AutoTypeAction.cpp @@ -32,9 +32,9 @@ AutoTypeKey::AutoTypeKey(const QChar& character, Qt::KeyboardModifiers modifiers { } -void AutoTypeKey::exec(AutoTypeExecutor* executor) const +AutoTypeAction::Result AutoTypeKey::exec(AutoTypeExecutor* executor) const { - executor->execType(this); + return executor->execType(this); } AutoTypeDelay::AutoTypeDelay(int delayMs, bool setExecDelay) @@ -43,7 +43,7 @@ AutoTypeDelay::AutoTypeDelay(int delayMs, bool setExecDelay) { } -void AutoTypeDelay::exec(AutoTypeExecutor* executor) const +AutoTypeAction::Result AutoTypeDelay::exec(AutoTypeExecutor* executor) const { if (setExecDelay) { // Change the delay between actions @@ -52,14 +52,16 @@ void AutoTypeDelay::exec(AutoTypeExecutor* executor) const // Pause execution Tools::wait(delayMs); } + + return AutoTypeAction::Result::Ok(); } -void AutoTypeClearField::exec(AutoTypeExecutor* executor) const +AutoTypeAction::Result AutoTypeClearField::exec(AutoTypeExecutor* executor) const { - executor->execClearField(this); + return executor->execClearField(this); } -void AutoTypeBegin::exec(AutoTypeExecutor* executor) const +AutoTypeAction::Result AutoTypeBegin::exec(AutoTypeExecutor* executor) const { - executor->execBegin(this); + return executor->execBegin(this); } diff --git a/src/autotype/AutoTypeAction.h b/src/autotype/AutoTypeAction.h index cf5ce6570..cc291fb5f 100644 --- a/src/autotype/AutoTypeAction.h +++ b/src/autotype/AutoTypeAction.h @@ -28,8 +28,61 @@ class AutoTypeExecutor; class KEEPASSXC_EXPORT AutoTypeAction { public: + class Result + { + public: + Result() + : m_isOk(false) + , m_canRetry(false) + , m_error(QString()) + { + } + + static Result Ok() + { + return Result(true, false, QString()); + } + + static Result Retry(const QString& error) + { + return Result(false, true, error); + } + + static Result Failed(const QString& error) + { + return Result(false, false, error); + } + + bool isOk() const + { + return m_isOk; + } + + bool canRetry() const + { + return m_canRetry; + } + + const QString& errorString() const + { + return m_error; + } + + private: + bool m_isOk; + bool m_canRetry; + QString m_error; + + Result(bool isOk, bool canRetry, const QString& error) + : m_isOk(isOk) + , m_canRetry(canRetry) + , m_error(error) + { + } + }; + AutoTypeAction() = default; - virtual void exec(AutoTypeExecutor* executor) const = 0; + virtual Result exec(AutoTypeExecutor* executor) const = 0; virtual ~AutoTypeAction() = default; }; @@ -38,7 +91,7 @@ class KEEPASSXC_EXPORT AutoTypeKey : public AutoTypeAction public: explicit AutoTypeKey(const QChar& character, Qt::KeyboardModifiers modifiers = Qt::NoModifier); explicit AutoTypeKey(Qt::Key key, Qt::KeyboardModifiers modifiers = Qt::NoModifier); - void exec(AutoTypeExecutor* executor) const override; + Result exec(AutoTypeExecutor* executor) const override; const QChar character; const Qt::Key key = Qt::Key_unknown; @@ -49,7 +102,7 @@ class KEEPASSXC_EXPORT AutoTypeDelay : public AutoTypeAction { public: explicit AutoTypeDelay(int delayMs, bool setExecDelay = false); - void exec(AutoTypeExecutor* executor) const override; + Result exec(AutoTypeExecutor* executor) const override; const int delayMs; const bool setExecDelay; @@ -58,24 +111,25 @@ public: class KEEPASSXC_EXPORT AutoTypeClearField : public AutoTypeAction { public: - void exec(AutoTypeExecutor* executor) const override; + Result exec(AutoTypeExecutor* executor) const override; }; class KEEPASSXC_EXPORT AutoTypeBegin : public AutoTypeAction { public: - void exec(AutoTypeExecutor* executor) const override; + Result exec(AutoTypeExecutor* executor) const override; }; class KEEPASSXC_EXPORT AutoTypeExecutor { public: virtual ~AutoTypeExecutor() = default; - virtual void execBegin(const AutoTypeBegin* action) = 0; - virtual void execType(const AutoTypeKey* action) = 0; - virtual void execClearField(const AutoTypeClearField* action) = 0; + virtual AutoTypeAction::Result execBegin(const AutoTypeBegin* action) = 0; + virtual AutoTypeAction::Result execType(const AutoTypeKey* action) = 0; + virtual AutoTypeAction::Result execClearField(const AutoTypeClearField* action) = 0; int execDelayMs = 25; + QString error; }; #endif // KEEPASSX_AUTOTYPEACTION_H diff --git a/src/autotype/mac/AutoTypeMac.cpp b/src/autotype/mac/AutoTypeMac.cpp index a8f31f6c6..dcb783bb9 100644 --- a/src/autotype/mac/AutoTypeMac.cpp +++ b/src/autotype/mac/AutoTypeMac.cpp @@ -215,12 +215,13 @@ AutoTypeExecutorMac::AutoTypeExecutorMac(AutoTypePlatformMac* platform) { } -void AutoTypeExecutorMac::execBegin(const AutoTypeBegin* action) +AutoTypeAction::Result AutoTypeExecutorMac::execBegin(const AutoTypeBegin* action) { Q_UNUSED(action); + return AutoTypeAction::Result::Ok(); } -void AutoTypeExecutorMac::execType(const AutoTypeKey* action) +AutoTypeAction::Result AutoTypeExecutorMac::execType(const AutoTypeKey* action) { if (action->modifiers & Qt::ShiftModifier) { m_platform->sendKey(Qt::Key_Shift, true); @@ -251,12 +252,14 @@ void AutoTypeExecutorMac::execType(const AutoTypeKey* action) } Tools::sleep(execDelayMs); + return AutoTypeAction::Result::Ok(); } -void AutoTypeExecutorMac::execClearField(const AutoTypeClearField* action) +AutoTypeAction::Result AutoTypeExecutorMac::execClearField(const AutoTypeClearField* action) { Q_UNUSED(action); execType(new AutoTypeKey(Qt::Key_Up, Qt::ControlModifier)); execType(new AutoTypeKey(Qt::Key_Down, Qt::ControlModifier | Qt::ShiftModifier)); execType(new AutoTypeKey(Qt::Key_Backspace)); + return AutoTypeAction::Result::Ok(); } diff --git a/src/autotype/mac/AutoTypeMac.h b/src/autotype/mac/AutoTypeMac.h index 46e303c95..008ff9aa1 100644 --- a/src/autotype/mac/AutoTypeMac.h +++ b/src/autotype/mac/AutoTypeMac.h @@ -57,9 +57,9 @@ class AutoTypeExecutorMac : public AutoTypeExecutor public: explicit AutoTypeExecutorMac(AutoTypePlatformMac* platform); - void execBegin(const AutoTypeBegin* action) override; - void execType(const AutoTypeKey* action) override; - void execClearField(const AutoTypeClearField* action) override; + AutoTypeAction::Result execBegin(const AutoTypeBegin* action) override; + AutoTypeAction::Result execType(const AutoTypeKey* action) override; + AutoTypeAction::Result execClearField(const AutoTypeClearField* action) override; private: AutoTypePlatformMac* const m_platform; diff --git a/src/autotype/test/AutoTypeTest.cpp b/src/autotype/test/AutoTypeTest.cpp index 98fc42e05..69c71a53c 100644 --- a/src/autotype/test/AutoTypeTest.cpp +++ b/src/autotype/test/AutoTypeTest.cpp @@ -102,17 +102,20 @@ AutoTypeExecutorTest::AutoTypeExecutorTest(AutoTypePlatformTest* platform) { } -void AutoTypeExecutorTest::execBegin(const AutoTypeBegin* action) +AutoTypeAction::Result AutoTypeExecutorTest::execBegin(const AutoTypeBegin* action) { Q_UNUSED(action); + return AutoTypeAction::Result::Ok(); } -void AutoTypeExecutorTest::execType(const AutoTypeKey* action) +AutoTypeAction::Result AutoTypeExecutorTest::execType(const AutoTypeKey* action) { m_platform->addAction(action); + return AutoTypeAction::Result::Ok(); } -void AutoTypeExecutorTest::execClearField(const AutoTypeClearField* action) +AutoTypeAction::Result AutoTypeExecutorTest::execClearField(const AutoTypeClearField* action) { Q_UNUSED(action); + return AutoTypeAction::Result::Ok(); } diff --git a/src/autotype/test/AutoTypeTest.h b/src/autotype/test/AutoTypeTest.h index 6ec41c416..d61c9f6b5 100644 --- a/src/autotype/test/AutoTypeTest.h +++ b/src/autotype/test/AutoTypeTest.h @@ -64,9 +64,9 @@ class AutoTypeExecutorTest : public AutoTypeExecutor public: explicit AutoTypeExecutorTest(AutoTypePlatformTest* platform); - void execBegin(const AutoTypeBegin* action) override; - void execType(const AutoTypeKey* action) override; - void execClearField(const AutoTypeClearField* action) override; + AutoTypeAction::Result execBegin(const AutoTypeBegin* action) override; + AutoTypeAction::Result execType(const AutoTypeKey* action) override; + AutoTypeAction::Result execClearField(const AutoTypeClearField* action) override; private: AutoTypePlatformTest* const m_platform; diff --git a/src/autotype/windows/AutoTypeWindows.cpp b/src/autotype/windows/AutoTypeWindows.cpp index 66aaeb4ad..f391dd015 100644 --- a/src/autotype/windows/AutoTypeWindows.cpp +++ b/src/autotype/windows/AutoTypeWindows.cpp @@ -226,12 +226,13 @@ AutoTypeExecutorWin::AutoTypeExecutorWin(AutoTypePlatformWin* platform) { } -void AutoTypeExecutorWin::execBegin(const AutoTypeBegin* action) +AutoTypeAction::Result AutoTypeExecutorWin::execBegin(const AutoTypeBegin* action) { Q_UNUSED(action); + return AutoTypeAction::Result::Ok(); } -void AutoTypeExecutorWin::execType(const AutoTypeKey* action) +AutoTypeAction::Result AutoTypeExecutorWin::execType(const AutoTypeKey* action) { if (action->modifiers & Qt::ShiftModifier) { m_platform->sendKey(Qt::Key_Shift, true); @@ -262,12 +263,14 @@ void AutoTypeExecutorWin::execType(const AutoTypeKey* action) } Tools::sleep(execDelayMs); + return AutoTypeAction::Result::Ok(); } -void AutoTypeExecutorWin::execClearField(const AutoTypeClearField* action) +AutoTypeAction::Result AutoTypeExecutorWin::execClearField(const AutoTypeClearField* action) { Q_UNUSED(action); execType(new AutoTypeKey(Qt::Key_Home, Qt::ControlModifier)); execType(new AutoTypeKey(Qt::Key_End, Qt::ControlModifier | Qt::ShiftModifier)); execType(new AutoTypeKey(Qt::Key_Backspace)); + return AutoTypeAction::Result::Ok(); } diff --git a/src/autotype/windows/AutoTypeWindows.h b/src/autotype/windows/AutoTypeWindows.h index 1c1df10dd..883677b57 100644 --- a/src/autotype/windows/AutoTypeWindows.h +++ b/src/autotype/windows/AutoTypeWindows.h @@ -54,9 +54,9 @@ class AutoTypeExecutorWin : public AutoTypeExecutor public: explicit AutoTypeExecutorWin(AutoTypePlatformWin* platform); - void execBegin(const AutoTypeBegin* action) override; - void execType(const AutoTypeKey* action) override; - void execClearField(const AutoTypeClearField* action) override; + AutoTypeAction::Result execBegin(const AutoTypeBegin* action) override; + AutoTypeAction::Result execType(const AutoTypeKey* action) override; + AutoTypeAction::Result execClearField(const AutoTypeClearField* action) override; private: AutoTypePlatformWin* const m_platform; diff --git a/src/autotype/xcb/AutoTypeXCB.cpp b/src/autotype/xcb/AutoTypeXCB.cpp index 88e26cf3a..0255d5da3 100644 --- a/src/autotype/xcb/AutoTypeXCB.cpp +++ b/src/autotype/xcb/AutoTypeXCB.cpp @@ -395,11 +395,10 @@ bool AutoTypePlatformX11::GetKeycode(KeySym keysym, int* keycode, int* group, un * window to simulate keyboard. If modifiers (shift, control, etc) * are set ON, many events will be sent. */ -void AutoTypePlatformX11::sendKey(KeySym keysym, unsigned int modifiers) +AutoTypeAction::Result AutoTypePlatformX11::sendKey(KeySym keysym, unsigned int modifiers) { if (keysym == NoSymbol) { - qWarning("No such key: keysym=0x%lX", keysym); - return; + return AutoTypeAction::Result::Failed(tr("Trying to send invalid keysym.")); } int keycode; @@ -417,8 +416,8 @@ void AutoTypePlatformX11::sendKey(KeySym keysym, unsigned int modifiers) /* determine keycode, group and mask for the given keysym */ if (!GetKeycode(keysym, &keycode, &group, &wanted_mask)) { - qWarning("Unable to get valid keycode for key: keysym=0x%lX", keysym); - return; + return AutoTypeAction::Result::Failed(tr("Unable to get valid keycode for key: ") + + QString(XKeysymToString(keysym))); } wanted_mask |= modifiers; @@ -498,6 +497,8 @@ void AutoTypePlatformX11::sendKey(KeySym keysym, unsigned int modifiers) XkbLockGroup(m_dpy, XkbUseCoreKbd, group_active); XFlush(m_dpy); } + + return AutoTypeAction::Result::Ok(); } int AutoTypePlatformX11::MyErrorHandler(Display* my_dpy, XErrorEvent* event) @@ -517,29 +518,37 @@ AutoTypeExecutorX11::AutoTypeExecutorX11(AutoTypePlatformX11* platform) { } -void AutoTypeExecutorX11::execBegin(const AutoTypeBegin* action) +AutoTypeAction::Result AutoTypeExecutorX11::execBegin(const AutoTypeBegin* action) { Q_UNUSED(action); m_platform->updateKeymap(); + return AutoTypeAction::Result::Ok(); } -void AutoTypeExecutorX11::execType(const AutoTypeKey* action) +AutoTypeAction::Result AutoTypeExecutorX11::execType(const AutoTypeKey* action) { + AutoTypeAction::Result result; + if (action->key != Qt::Key_unknown) { - m_platform->sendKey(qtToNativeKeyCode(action->key), qtToNativeModifiers(action->modifiers)); + result = m_platform->sendKey(qtToNativeKeyCode(action->key), qtToNativeModifiers(action->modifiers)); } else { - m_platform->sendKey(qcharToNativeKeyCode(action->character), qtToNativeModifiers(action->modifiers)); + result = m_platform->sendKey(qcharToNativeKeyCode(action->character), qtToNativeModifiers(action->modifiers)); } - Tools::sleep(execDelayMs); + if (result.isOk()) { + Tools::sleep(execDelayMs); + } + + return result; } -void AutoTypeExecutorX11::execClearField(const AutoTypeClearField* action) +AutoTypeAction::Result AutoTypeExecutorX11::execClearField(const AutoTypeClearField* action) { Q_UNUSED(action); execType(new AutoTypeKey(Qt::Key_Home, Qt::ControlModifier)); execType(new AutoTypeKey(Qt::Key_End, Qt::ControlModifier | Qt::ShiftModifier)); execType(new AutoTypeKey(Qt::Key_Backspace)); + return AutoTypeAction::Result::Ok(); } bool AutoTypePlatformX11::raiseWindow(WId window) diff --git a/src/autotype/xcb/AutoTypeXCB.h b/src/autotype/xcb/AutoTypeXCB.h index 1a70a3faa..24c514290 100644 --- a/src/autotype/xcb/AutoTypeXCB.h +++ b/src/autotype/xcb/AutoTypeXCB.h @@ -56,7 +56,7 @@ public: AutoTypeExecutor* createExecutor() override; void updateKeymap(); - void sendKey(KeySym keysym, unsigned int modifiers = 0); + AutoTypeAction::Result sendKey(KeySym keysym, unsigned int modifiers = 0); private: QString windowTitle(Window window, bool useBlacklist); @@ -104,9 +104,9 @@ class AutoTypeExecutorX11 : public AutoTypeExecutor public: explicit AutoTypeExecutorX11(AutoTypePlatformX11* platform); - void execBegin(const AutoTypeBegin* action) override; - void execType(const AutoTypeKey* action) override; - void execClearField(const AutoTypeClearField* action) override; + AutoTypeAction::Result execBegin(const AutoTypeBegin* action) override; + AutoTypeAction::Result execType(const AutoTypeKey* action) override; + AutoTypeAction::Result execClearField(const AutoTypeClearField* action) override; private: AutoTypePlatformX11* const m_platform;