From 9464aa46b8195da6837db22883a8e79d3acff3d1 Mon Sep 17 00:00:00 2001 From: Nick Bolton Date: Mon, 12 Aug 2024 16:39:18 +0100 Subject: [PATCH] Show message box explaining why settings are read-only (#7437) * Swap assert for warning log lines * Flush on IPC write * Flush on cleanup instead of write * Record core started setting * Show server first start message * Show message when active scope is read-only * Show read only message on change * Show read only on window show * Try to improve main window size policy * Revert addition of resizer * Remove redundant file path fiddling * Remove dead code and fixed missing const * Print path and use queued connection * Improve read-only message on Windows * Only show toggle warning when dialog visible * Update ChangeLog * Fixed include --- ChangeLog | 1 + src/gui/src/MainWindow.cpp | 29 ++-- src/gui/src/MainWindow.h | 2 +- src/gui/src/MainWindowBase.ui | 136 +++++++++++------- src/gui/src/main.cpp | 2 - src/lib/gui/byte_utils.h | 2 +- src/lib/gui/config/AppConfig.cpp | 4 +- src/lib/gui/config/AppConfig.h | 5 +- src/lib/gui/config/ConfigScopes.h | 2 +- src/lib/gui/config/IAppConfig.h | 9 +- src/lib/gui/config/IConfigScopes.h | 1 + src/lib/gui/core/CoreProcess.cpp | 33 ++++- src/lib/gui/core/CoreProcess.h | 6 +- src/lib/gui/dialogs/SettingsDialog.cpp | 25 ++++ src/lib/gui/dialogs/SettingsDialog.h | 13 +- src/lib/gui/ipc/QDataStreamProxy.h | 5 +- src/lib/gui/ipc/QIpcClient.cpp | 16 ++- src/lib/gui/ipc/QIpcClient.h | 2 +- src/lib/gui/messages.cpp | 20 ++- src/lib/gui/messages.h | 6 +- src/lib/gui/proxy/QSettingsProxy.cpp | 32 +++-- src/lib/gui/proxy/QSettingsProxy.h | 2 +- src/lib/synergy/Screen.cpp | 24 +++- src/test/shared/gui/mocks/AppConfigMock.h | 5 +- .../unittests/gui/config/AppConfigTests.cpp | 1 + 25 files changed, 277 insertions(+), 106 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9f196f0eb..f522058d2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -79,6 +79,7 @@ Enhancements: - #7434 Show dark logo in dark mode and improve .env loader - #7435 Add reset settings menu action and env var - #7436 Introduced new env vars for testing +- #7437 Show message box explaining why settings are read-only # 1.14.6 diff --git a/src/gui/src/MainWindow.cpp b/src/gui/src/MainWindow.cpp index 7b3348ed3..c80da4ed4 100644 --- a/src/gui/src/MainWindow.cpp +++ b/src/gui/src/MainWindow.cpp @@ -799,7 +799,7 @@ void MainWindow::closeEvent(QCloseEvent *event) { qDebug("window should hide to tray"); } -void MainWindow::showFirstRunMessage() { +void MainWindow::showFirstConnectedMessage() { if (m_AppConfig.startedBefore()) { return; } @@ -808,7 +808,7 @@ void MainWindow::showFirstRunMessage() { m_ConfigScopes.save(); const auto isServer = m_CoreProcess.mode() == CoreMode::Server; - messages::showFirstRunMessage( + messages::showFirstConnectedMessage( this, m_AppConfig.closeToTray(), m_AppConfig.enableService(), isServer); } @@ -887,10 +887,14 @@ void MainWindow::updateStatus() { } void MainWindow::onCoreProcessStateChanged(CoreProcessState state) { - qDebug("core process state changed: %d", static_cast(state)); - updateStatus(); + if (state == CoreProcessState::Started) { + qDebug("recording that core has started"); + m_AppConfig.setStartedBefore(true); + m_ConfigScopes.save(); + } + if (state == CoreProcessState::Started || state == CoreProcessState::Starting) { disconnect( @@ -933,7 +937,7 @@ void MainWindow::onCoreConnectionStateChanged(CoreConnectionState state) { if (state != CoreConnectionState::Connected) { secureSocket(false); } else if (isVisible()) { - showFirstRunMessage(); + showFirstConnectedMessage(); showDevThanksMessage(); } } @@ -1127,12 +1131,21 @@ void MainWindow::enableServer(bool enable) { m_AppConfig.setServerGroupChecked(enable); m_pRadioGroupServer->setChecked(enable); m_pWidgetServer->setEnabled(enable); - m_pWidgetServerInverse->setVisible(m_AppConfig.invertConnection()); + m_pWidgetServerInput->setVisible(m_AppConfig.invertConnection()); if (enable) { m_pButtonToggleStart->setEnabled(true); m_pActionStartCore->setEnabled(true); m_CoreProcess.setMode(CoreProcess::Mode::Server); + + // The server can run without any clients configured, and this is actually + // what you'll want to do the first time since you'll be prompted when an + // unrecognized client tries to connect. + if (!m_AppConfig.startedBefore()) { + qDebug("auto-starting core server for first time"); + m_CoreProcess.start(); + messages::showFirstServerStartMessage(this); + } } } @@ -1140,8 +1153,8 @@ void MainWindow::enableClient(bool enable) { qDebug(enable ? "client enabled" : "client disabled"); m_AppConfig.setClientGroupChecked(enable); m_pRadioGroupClient->setChecked(enable); - m_pWidgetClient->setEnabled(enable); - m_pWidgetClient->setVisible(!m_AppConfig.invertConnection()); + m_pWidgetClientInput->setEnabled(enable); + m_pWidgetClientInput->setVisible(!m_AppConfig.invertConnection()); if (enable) { m_pButtonToggleStart->setEnabled(true); diff --git a/src/gui/src/MainWindow.h b/src/gui/src/MainWindow.h index 7fb2426e2..56eebabc1 100644 --- a/src/gui/src/MainWindow.h +++ b/src/gui/src/MainWindow.h @@ -176,7 +176,7 @@ private: void setupControls(); void resizeEvent(QResizeEvent *event) override; void moveEvent(QMoveEvent *event) override; - void showFirstRunMessage(); + void showFirstConnectedMessage(); void showDevThanksMessage(); QString productName() const; void updateStatus(); diff --git a/src/gui/src/MainWindowBase.ui b/src/gui/src/MainWindowBase.ui index 23d000550..d736b082e 100644 --- a/src/gui/src/MainWindowBase.ui +++ b/src/gui/src/MainWindowBase.ui @@ -110,29 +110,49 @@ 15 - - - - 0 + + + + + 0 + 0 + - - - - Use this computer's keyboard and mouse - - - - - - - (make this computer the server) - - - 20 - - - - + + + 0 + + + 0 + + + 0 + + + 0 + + + 0 + + + + + Use this computer's keyboard and mouse + + + + + + + (make this computer the server) + + + 20 + + + + + @@ -153,7 +173,7 @@ 0 - + 15 @@ -231,7 +251,7 @@ 20 - 10 + 1 @@ -278,32 +298,52 @@ 15 - - - - 0 + + + + + 0 + 0 + - - - - Use another computer’s mouse and keyboard - - - - - - - (make this computer the client) - - - 20 - - - - + + + 0 + + + 0 + + + 0 + + + 0 + + + 0 + + + + + Use another computer’s mouse and keyboard + + + + + + + (make this computer the client) + + + 20 + + + + + - + 15 @@ -378,7 +418,7 @@ 20 - 10 + 1 diff --git a/src/gui/src/main.cpp b/src/gui/src/main.cpp index 370825c04..5efc53baa 100644 --- a/src/gui/src/main.cpp +++ b/src/gui/src/main.cpp @@ -50,8 +50,6 @@ public: static void msleep(unsigned long msecs) { QThread::msleep(msecs); } }; -QString getSystemSettingPath(); - #if defined(Q_OS_MAC) bool checkMacAssistiveDevices(); #endif diff --git a/src/lib/gui/byte_utils.h b/src/lib/gui/byte_utils.h index 9d9034da3..941abfc74 100644 --- a/src/lib/gui/byte_utils.h +++ b/src/lib/gui/byte_utils.h @@ -20,7 +20,7 @@ #include #include #include -#include +#include namespace synergy::gui { diff --git a/src/lib/gui/config/AppConfig.cpp b/src/lib/gui/config/AppConfig.cpp index 869f329ab..62cdf13e4 100644 --- a/src/lib/gui/config/AppConfig.cpp +++ b/src/lib/gui/config/AppConfig.cpp @@ -394,6 +394,8 @@ void AppConfig::loadScope(ConfigScopes::Scope scope) { m_Scopes.setActiveScope(scope); + qDebug("active scope file path: %s", qPrintable(m_Scopes.activeFilePath())); + // only signal ready if there is at least one setting in the required scope. // this prevents the current settings from being set back to default. if (m_Scopes.scopeContains( @@ -443,7 +445,7 @@ void AppConfig::persistLogDir() const { // Begin getters /////////////////////////////////////////////////////////////////////////////// -IConfigScopes &AppConfig::scopes() { return m_Scopes; } +IConfigScopes &AppConfig::scopes() const { return m_Scopes; } bool AppConfig::activationHasRun() const { return m_ActivationHasRun; } diff --git a/src/lib/gui/config/AppConfig.h b/src/lib/gui/config/AppConfig.h index 5577ec924..3e3346375 100644 --- a/src/lib/gui/config/AppConfig.h +++ b/src/lib/gui/config/AppConfig.h @@ -126,7 +126,6 @@ public: IConfigScopes &scopes, std::shared_ptr deps = std::make_shared()); - IConfigScopes &scopes(); void determineScope(); /** @@ -139,6 +138,7 @@ public: // Getters (overrides) // + IConfigScopes &scopes() const override; ProcessMode processMode() const override; ElevateMode elevateMode() const override; bool tlsEnabled() const override; @@ -189,6 +189,7 @@ public: // // Setters (overrides) // + void setScreenName(const QString &s) override; void setPort(int i) override; void setNetworkInterface(const QString &s) override; @@ -211,9 +212,9 @@ public: // Setters (new methods) // + void setStartedBefore(bool b); void setActivationHasRun(bool value); void setWizardHasRun(); - void setStartedBefore(bool b); void setSerialKey(const QString &serialKey); void clearSerialKey(); void setLicenseNextCheck(unsigned long long); diff --git a/src/lib/gui/config/ConfigScopes.h b/src/lib/gui/config/ConfigScopes.h index ac8ad8758..7ff670a6e 100644 --- a/src/lib/gui/config/ConfigScopes.h +++ b/src/lib/gui/config/ConfigScopes.h @@ -59,7 +59,7 @@ public: Scope activeScope() const override; QSettingsProxy &activeSettings() override; const QSettingsProxy &activeSettings() const override; - QString activeFilePath() const; + QString activeFilePath() const override; signals: void ready(); diff --git a/src/lib/gui/config/IAppConfig.h b/src/lib/gui/config/IAppConfig.h index ccedba6ac..c973fa1ed 100644 --- a/src/lib/gui/config/IAppConfig.h +++ b/src/lib/gui/config/IAppConfig.h @@ -19,6 +19,8 @@ #include "ElevateMode.h" +#include "gui/config/IConfigScopes.h" + #include namespace synergy::gui { @@ -26,13 +28,16 @@ namespace synergy::gui { enum class ProcessMode { kService, kDesktop }; class IAppConfig { + using IConfigScopes = synergy::gui::IConfigScopes; + public: virtual ~IAppConfig() = default; // - // Setters + // Getters // + virtual IConfigScopes &scopes() const = 0; virtual QString tlsCertPath() const = 0; virtual int tlsKeyLength() const = 0; virtual bool tlsEnabled() const = 0; @@ -64,7 +69,7 @@ public: virtual bool clientGroupChecked() const = 0; // - // Getters + // Setters // virtual void setLoadFromSystemScope(bool loadFromSystemScope) = 0; diff --git a/src/lib/gui/config/IConfigScopes.h b/src/lib/gui/config/IConfigScopes.h index af56a5773..e52c3911b 100644 --- a/src/lib/gui/config/IConfigScopes.h +++ b/src/lib/gui/config/IConfigScopes.h @@ -38,6 +38,7 @@ public: virtual bool isActiveScopeWritable() const = 0; virtual QSettingsProxy &activeSettings() = 0; virtual const QSettingsProxy &activeSettings() const = 0; + virtual QString activeFilePath() const = 0; /** * @brief Signals to listeners that the settings that they should read. diff --git a/src/lib/gui/core/CoreProcess.cpp b/src/lib/gui/core/CoreProcess.cpp index 88ec105d1..bb5e7f1ee 100644 --- a/src/lib/gui/core/CoreProcess.cpp +++ b/src/lib/gui/core/CoreProcess.cpp @@ -35,6 +35,7 @@ #include #include #include +#include using namespace synergy::license; using namespace synergy::gui::license; @@ -59,7 +60,25 @@ QString processModeToString(ProcessMode mode) { return "service"; default: qFatal("invalid process mode"); - return ""; + abort(); + } +} + +QString processStateToString(CoreProcess::ProcessState state) { + using enum CoreProcess::ProcessState; + + switch (state) { + case Starting: + return "starting"; + case Started: + return "started"; + case Stopping: + return "stopping"; + case Stopped: + return "stopped"; + default: + qFatal("invalid process state"); + abort(); } } @@ -139,8 +158,8 @@ QString CoreProcess::Deps::getProfileRoot() const { // CoreProcess::CoreProcess( - IAppConfig &appConfig, IServerConfig &serverConfig, const ILicense &license, - std::shared_ptr deps) + const IAppConfig &appConfig, const IServerConfig &serverConfig, + const ILicense &license, std::shared_ptr deps) : m_appConfig(appConfig), m_serverConfig(serverConfig), m_license(license), @@ -175,7 +194,9 @@ void CoreProcess::onIpcClientServiceReady() { qDebug("service ready, continuing core process stop"); stop(); } else { - qCritical("service ready, but process state is not starting or stopping"); + // This may happen when the IPC connection fails and then reconnects. + qWarning( + "ignoring service ready, process state is not starting or stopping"); } } @@ -644,6 +665,10 @@ void CoreProcess::setProcessState(ProcessState state) { return; } + qDebug( + "core process state changed: %s -> %s", // + qPrintable(processStateToString(m_processState)), + qPrintable(processStateToString(state))); m_processState = state; emit processStateChanged(state); } diff --git a/src/lib/gui/core/CoreProcess.h b/src/lib/gui/core/CoreProcess.h index 6e18b7279..46f7c4cc4 100644 --- a/src/lib/gui/core/CoreProcess.h +++ b/src/lib/gui/core/CoreProcess.h @@ -59,7 +59,7 @@ public: enum class ConnectionState { Disconnected, Connecting, Connected, Listening }; explicit CoreProcess( - IAppConfig &appConfig, IServerConfig &serverConfig, + const IAppConfig &appConfig, const IServerConfig &serverConfig, const ILicense &license, std::shared_ptr deps = std::make_shared()); @@ -119,8 +119,8 @@ private: void checkOSXNotification(const QString &line); #endif - IAppConfig &m_appConfig; - IServerConfig &m_serverConfig; + const IAppConfig &m_appConfig; + const IServerConfig &m_serverConfig; const ILicense &m_license; std::shared_ptr m_pDeps; QString m_address; diff --git a/src/lib/gui/dialogs/SettingsDialog.cpp b/src/lib/gui/dialogs/SettingsDialog.cpp index a3b71f730..4867dc4dd 100644 --- a/src/lib/gui/dialogs/SettingsDialog.cpp +++ b/src/lib/gui/dialogs/SettingsDialog.cpp @@ -21,6 +21,7 @@ #include "UpgradeDialog.h" #include "gui/core/CoreProcess.h" #include "gui/license/license_config.h" +#include "gui/messages.h" #include "gui/tls/TlsCertificate.h" #include "gui/tls/TlsUtility.h" #include "gui/validators/ScreenNameValidator.h" @@ -61,6 +62,15 @@ SettingsDialog::SettingsDialog( m_pScreenNameError = new validators::ValidationError(this); m_pLineEditScreenName->setValidator(new validators::ScreenNameValidator( m_pLineEditScreenName, m_pScreenNameError, &serverConfig.screens())); + + connect( + this, &SettingsDialog::shown, this, + [this] { + if (!m_appConfig.isActiveScopeWritable()) { + showReadOnlyMessage(); + } + }, + Qt::QueuedConnection); } // @@ -107,6 +117,10 @@ void SettingsDialog::on_m_pRadioSystemScope_toggled(bool checked) { m_appConfig.setLoadFromSystemScope(checked); loadFromConfig(); updateControls(); + + if (isVisible() && !m_appConfig.isActiveScopeWritable()) { + showReadOnlyMessage(); + } } void SettingsDialog::on_m_pPushButtonTlsCertPath_clicked() { @@ -147,6 +161,16 @@ void SettingsDialog::on_m_pCheckBoxServiceEnabled_toggled(bool) { // End of auto-connect slots // +void SettingsDialog::showEvent(QShowEvent *event) { + QDialog::showEvent(event); + emit shown(); +} + +void SettingsDialog::showReadOnlyMessage() { + const auto activeScopeFilename = m_appConfig.scopes().activeFilePath(); + messages::showReadOnlySettings(this, activeScopeFilename); +} + void SettingsDialog::accept() { if (!m_pLineEditScreenName->hasAcceptableInput()) { QMessageBox::warning( @@ -228,6 +252,7 @@ void SettingsDialog::updateTlsControls() { const auto tlsEnabled = m_tlsUtility.isAvailableAndEnabled(); const auto writable = m_appConfig.isActiveScopeWritable(); + m_pCheckBoxEnableTls->setEnabled(writable); m_pCheckBoxEnableTls->setChecked(writable && tlsEnabled); m_pLineEditTlsCertPath->setText(m_appConfig.tlsCertPath()); diff --git a/src/lib/gui/dialogs/SettingsDialog.h b/src/lib/gui/dialogs/SettingsDialog.h index b536ee6e6..23e090d8e 100644 --- a/src/lib/gui/dialogs/SettingsDialog.h +++ b/src/lib/gui/dialogs/SettingsDialog.h @@ -39,10 +39,14 @@ class SettingsDialog : public QDialog, public Ui::SettingsDialogBase { Q_OBJECT public: + void extracted(); SettingsDialog( QWidget *parent, IAppConfig &appConfig, const IServerConfig &serverConfig, const License &license, const CoreProcess &coreProcess); +signals: + void shown(); + private slots: void on_m_pCheckBoxEnableTls_clicked(bool checked); void on_m_pCheckBoxLogToFile_stateChanged(int); @@ -56,6 +60,11 @@ private slots: private: void accept() override; void reject() override; + void showEvent(QShowEvent *event) override; + bool isClientMode() const; + void updateTlsControls(); + void updateTlsControlsEnabled(); + void showReadOnlyMessage(); /// @brief Load all settings. void loadFromConfig(); @@ -69,10 +78,6 @@ private: /// @brief Enables controls when they should be. void updateControls(); - bool isClientMode() const; - void updateTlsControls(); - void updateTlsControlsEnabled(); - [[no_unique_address]] CoreTool m_coreTool; validators::ValidationError *m_pScreenNameError; diff --git a/src/lib/gui/ipc/QDataStreamProxy.h b/src/lib/gui/ipc/QDataStreamProxy.h index d6af903a5..25a4b4258 100644 --- a/src/lib/gui/ipc/QDataStreamProxy.h +++ b/src/lib/gui/ipc/QDataStreamProxy.h @@ -23,9 +23,8 @@ class QDataStreamProxy { public: explicit QDataStreamProxy() = default; - explicit QDataStreamProxy(QTcpSocket *socket) { - m_Stream = std::make_unique(socket); - } + explicit QDataStreamProxy(QTcpSocket *socket) + : m_Stream(std::make_unique(socket)) {} virtual ~QDataStreamProxy() = default; virtual qint64 writeRawData(const char *data, int len) { diff --git a/src/lib/gui/ipc/QIpcClient.cpp b/src/lib/gui/ipc/QIpcClient.cpp index 2ce1285f7..c7665aa31 100644 --- a/src/lib/gui/ipc/QIpcClient.cpp +++ b/src/lib/gui/ipc/QIpcClient.cpp @@ -79,14 +79,17 @@ void QIpcClient::connectToHost() { } void QIpcClient::disconnectFromHost() { - m_isConnecting = false; - qInfo("disconnected from background service"); m_pReader->stop(); + m_pSocket->flush(); m_pSocket->close(); + + m_isConnecting = false; m_isConnected = false; + + qInfo("disconnected from background service"); } -void QIpcClient::onSocketError(QAbstractSocket::SocketError socketError) const { +void QIpcClient::onSocketError(QAbstractSocket::SocketError socketError) { QString text; switch (socketError) { case 0: @@ -101,6 +104,7 @@ void QIpcClient::onSocketError(QAbstractSocket::SocketError socketError) const { } qWarning("ipc connection error, %s", qUtf8Printable(text)); + m_isConnected = false; QTimer::singleShot(kRetryInterval, this, &QIpcClient::onRetryConnect); } @@ -154,6 +158,12 @@ void QIpcClient::sendCommand( void QIpcClient::onIpcReaderHelloBack() { qDebug("ipc hello back received"); + + if (m_isConnected) { + qWarning("ipc already connected, ignoring hello back"); + return; + } + m_isConnected = true; serviceReady(); } diff --git a/src/lib/gui/ipc/QIpcClient.h b/src/lib/gui/ipc/QIpcClient.h index cfd0f8c3a..d8b5bb6be 100644 --- a/src/lib/gui/ipc/QIpcClient.h +++ b/src/lib/gui/ipc/QIpcClient.h @@ -48,7 +48,7 @@ private slots: void onRetryConnect(); void onSocketConnected() const; void onIpcReaderHelloBack(); - void onSocketError(QAbstractSocket::SocketError error) const; + void onSocketError(QAbstractSocket::SocketError error); void onIpcReaderRead(const QString &text); private: diff --git a/src/lib/gui/messages.cpp b/src/lib/gui/messages.cpp index b03e18e8d..4ad6ac960 100644 --- a/src/lib/gui/messages.cpp +++ b/src/lib/gui/messages.cpp @@ -149,7 +149,16 @@ void showCloseReminder(QWidget *parent) { QMessageBox::information(parent, "Notification area icon", message); } -void showFirstRunMessage( +void showFirstServerStartMessage(QWidget *parent) { + QMessageBox::information( + parent, "Server is running", + "

Great, the server is now running.

" + "

Now you can connect your other computers to this server. " + "You should see a prompt here on the server when a new client tries to " + "connect.

"); +} + +void showFirstConnectedMessage( QWidget *parent, bool closeToTray, bool enableService, bool isServer) { auto message = QString("

Synergy is now connected!

"); @@ -265,4 +274,13 @@ bool showClearSettings(QWidget *parent) { return message.clickedButton() == clear; } +void showReadOnlySettings(QWidget *parent, const QString &systemSettingsPath) { + QString nativePath = QDir::toNativeSeparators(systemSettingsPath); + QMessageBox::information( + parent, "Read-only settings", + QString("

Settings are read-only because you only have read access " + "to the file:

%1

") + .arg(nativePath)); +} + } // namespace synergy::gui::messages diff --git a/src/lib/gui/messages.h b/src/lib/gui/messages.h index 18a82be2a..054ebb405 100644 --- a/src/lib/gui/messages.h +++ b/src/lib/gui/messages.h @@ -33,7 +33,9 @@ void messageHandler( void raiseCriticalDialog(); -void showFirstRunMessage( +void showFirstServerStartMessage(QWidget *parent); + +void showFirstConnectedMessage( QWidget *parent, bool closeToTray, bool enableService, bool isServer); void showCloseReminder(QWidget *parent); @@ -48,4 +50,6 @@ showNewClientPrompt(QWidget *parent, const QString &clientName); bool showClearSettings(QWidget *parent); +void showReadOnlySettings(QWidget *parent, const QString &systemSettingsPath); + } // namespace synergy::gui::messages diff --git a/src/lib/gui/proxy/QSettingsProxy.cpp b/src/lib/gui/proxy/QSettingsProxy.cpp index b25df356b..f580f2f99 100644 --- a/src/lib/gui/proxy/QSettingsProxy.cpp +++ b/src/lib/gui/proxy/QSettingsProxy.cpp @@ -29,26 +29,29 @@ namespace synergy::gui::proxy { const auto kLegacyOrgDomain = "http-symless-com"; - -const auto kSystemConfigFilename = "SystemConfig.ini"; +const auto kLegacySystemConfigFilename = "SystemConfig.ini"; #if defined(Q_OS_UNIX) -const auto kUnixSystemConfigPath = "/usr/local/etc/symless/"; +const auto kUnixSystemConfigPath = "/usr/local/etc/"; #endif // // Free functions // -QString getSystemSettingPath() { - const QString settingFilename(kSystemConfigFilename); +/** + * @brief The base dir for the system settings file. + * + * Important: Qt will append the org name as a dir, and the app name as the + * settings filename, i.e.: `{base-dir}/Synergy/Synergy.ini` + */ +QString getSystemSettingsBaseDir() { #if defined(Q_OS_WIN) - return QCoreApplication::applicationDirPath() + QDir::separator(); -#elif defined(Q_OS_MAC) - // it would be nice to use /Library dir, but qt has no elevate system. - return kUnixSystemConfigPath + settingFilename; -#elif defined(Q_OS_LINUX) - // qt already adds application and filename to the end of the path on linux. + return QCoreApplication::applicationDirPath(); +#elif defined(Q_OS_UNIX) + // Qt already adds application and filename to the end of the path. + // On macOS, it would be nice to use /Library dir, but qt has no elevate + // system. return kUnixSystemConfigPath; #else #error "unsupported platform" @@ -62,7 +65,8 @@ void migrateLegacySystemSettings(QSettings &settings) { } QSettings::setPath( - QSettings::IniFormat, QSettings::SystemScope, kSystemConfigFilename); + QSettings::IniFormat, QSettings::SystemScope, + kLegacySystemConfigFilename); QSettings oldSystemSettings( QSettings::IniFormat, QSettings::SystemScope, QCoreApplication::organizationName(), @@ -75,7 +79,7 @@ void migrateLegacySystemSettings(QSettings &settings) { } QSettings::setPath( - QSettings::IniFormat, QSettings::SystemScope, getSystemSettingPath()); + QSettings::IniFormat, QSettings::SystemScope, getSystemSettingsBaseDir()); } void migrateLegacyUserSettings(QSettings &newSettings) { @@ -151,7 +155,7 @@ void QSettingsProxy::loadSystem() { QSettings::setPath( QSettings::Format::IniFormat, QSettings::Scope::SystemScope, - getSystemSettingPath()); + getSystemSettingsBaseDir()); m_pSettings = std::make_unique( QSettings::Format::IniFormat, QSettings::Scope::SystemScope, orgName, diff --git a/src/lib/gui/proxy/QSettingsProxy.h b/src/lib/gui/proxy/QSettingsProxy.h index fb54bb1fe..bd965cc84 100644 --- a/src/lib/gui/proxy/QSettingsProxy.h +++ b/src/lib/gui/proxy/QSettingsProxy.h @@ -21,7 +21,7 @@ namespace synergy::gui::proxy { -QString getSystemSettingPath(); +QString getSystemSettingBaseDir(); class QSettingsProxy { public: diff --git a/src/lib/synergy/Screen.cpp b/src/lib/synergy/Screen.cpp index 027306d32..55c0fef14 100644 --- a/src/lib/synergy/Screen.cpp +++ b/src/lib/synergy/Screen.cpp @@ -57,9 +57,27 @@ Screen::~Screen() { } assert(!m_enabled); - // TODO: why assert this? it appears to be false when an elevated dialog - // appears on windows and the process is killed. - assert(m_entered == m_isPrimary); + // Originally there was an assert here added before 2009 (history not in + // tact). This condition seems to occur on a Windows client when the process + // is shut down to make way for a new elevated process (e.g. at login screen). + // The reason why this assert was originally added is unclear, and was causing + // pain when using debug builds; you lose control of the client when it's at + // the login screen. Therefore it has been converted to a warning so that we + // can still see when it happens but it won't cause the process to pause. This + // also gives us the added benefit of seeing when it happens in production. + // Perhaps it indicates that the cursor is still being controlled on the + // client while it's shutting down? i.e. the screen is entered and is not the + // server, or the screen is not entered and is the server. + if (m_entered == m_isPrimary) { + LOG( + (CLOG_DEBUG "current screen: entered=%s, primary=%s", // + m_entered ? "yes" : "no", m_isPrimary ? "yes" : "no")); + if (m_isPrimary) { + LOG((CLOG_WARN "current primary screen is not entered on shutdown")); + } else { + LOG((CLOG_WARN "current secondary screen is entered on shutdown")); + } + } delete m_screen; LOG((CLOG_DEBUG "closed display")); diff --git a/src/test/shared/gui/mocks/AppConfigMock.h b/src/test/shared/gui/mocks/AppConfigMock.h index c687c377d..c7e3ec8b3 100644 --- a/src/test/shared/gui/mocks/AppConfigMock.h +++ b/src/test/shared/gui/mocks/AppConfigMock.h @@ -39,9 +39,10 @@ public: } // - // Setters + // Getters // + MOCK_METHOD(synergy::gui::IConfigScopes &, scopes, (), (const, override)); MOCK_METHOD(QString, tlsCertPath, (), (const, override)); MOCK_METHOD(int, tlsKeyLength, (), (const, override)); MOCK_METHOD(bool, tlsEnabled, (), (const, override)); @@ -73,7 +74,7 @@ public: MOCK_METHOD(bool, clientGroupChecked, (), (const, override)); // - // Getters + // Setters // MOCK_METHOD( diff --git a/src/test/unittests/gui/config/AppConfigTests.cpp b/src/test/unittests/gui/config/AppConfigTests.cpp index 3d335a636..e9bed0f05 100644 --- a/src/test/unittests/gui/config/AppConfigTests.cpp +++ b/src/test/unittests/gui/config/AppConfigTests.cpp @@ -49,6 +49,7 @@ public: MOCK_METHOD(const QSettingsProxy &, activeSettings, (), (const, override)); MOCK_METHOD(QSettingsProxy &, activeSettings, (), (override)); MOCK_METHOD(void, save, (bool), (override)); + MOCK_METHOD(QString, activeFilePath, (), (const, override)); }; struct DepsMock : public AppConfig::Deps {