From 5a05e3e83d00c596d54d5d51d6904a24c4ecfb84 Mon Sep 17 00:00:00 2001 From: Alexey Rochev Date: Sun, 8 Mar 2026 01:28:21 +0300 Subject: [PATCH] Simplify activation token handling and fix activation of dialogs on Wayland We were setting XDG_ACTIVATION_TOKEN env variable after calling show() and then calling activateWindow(), which conflicts with Qt requesting activation token by on show(). Set it before show() so Qt uses it and we don't need to call activateWindow() afterwards. --- .../screens/addtorrent/addtorrentdialog.cpp | 4 +- .../screens/addtorrent/addtorrenthelpers.cpp | 4 +- src/ui/screens/addtorrent/addtorrenthelpers.h | 3 +- src/ui/screens/mainwindow/mainwindow.cpp | 111 ++++++------------ 4 files changed, 41 insertions(+), 81 deletions(-) diff --git a/src/ui/screens/addtorrent/addtorrentdialog.cpp b/src/ui/screens/addtorrent/addtorrentdialog.cpp index 60e58fc2..9fcaee0b 100644 --- a/src/ui/screens/addtorrent/addtorrentdialog.cpp +++ b/src/ui/screens/addtorrent/addtorrentdialog.cpp @@ -420,7 +420,7 @@ namespace tremotesf { info().log("Parsed, result = {}", magnetLink); auto* const torrent = mRpc->torrentByHash(magnetLink.infoHashV1); if (torrent) { - askForMergingTrackers(torrent, std::move(magnetLink.trackers), parentWidget()); + createAskForMergingTrackersDialog(torrent, std::move(magnetLink.trackers), parentWidget())->show(); return true; } } catch (const std::runtime_error& e) { @@ -438,7 +438,7 @@ namespace tremotesf { auto& [infoHashV1, trackers] = *mTorrentFileInfoHashAndTrackers; auto* const torrent = mRpc->torrentByHash(infoHashV1); if (torrent) { - askForMergingTrackers(torrent, std::move(trackers), parentWidget()); + createAskForMergingTrackersDialog(torrent, std::move(trackers), parentWidget())->show(); return true; } return false; diff --git a/src/ui/screens/addtorrent/addtorrenthelpers.cpp b/src/ui/screens/addtorrent/addtorrenthelpers.cpp index 7de5a053..bcaba716 100644 --- a/src/ui/screens/addtorrent/addtorrenthelpers.cpp +++ b/src/ui/screens/addtorrent/addtorrenthelpers.cpp @@ -42,7 +42,8 @@ namespace tremotesf { }; } - QDialog* askForMergingTrackers(Torrent* torrent, std::vector> trackers, QWidget* parent) { + QDialog* + createAskForMergingTrackersDialog(Torrent* torrent, std::vector> trackers, QWidget* parent) { auto* const settings = Settings::instance(); QMessageBox* messageBox{}; if (settings->get_askForMergingTrackersWhenAddingExistingTorrent()) { @@ -95,7 +96,6 @@ namespace tremotesf { } messageBox->setAttribute(Qt::WA_DeleteOnClose); messageBox->setModal(false); - messageBox->show(); return messageBox; } diff --git a/src/ui/screens/addtorrent/addtorrenthelpers.h b/src/ui/screens/addtorrent/addtorrenthelpers.h index d5ee25de..e315c440 100644 --- a/src/ui/screens/addtorrent/addtorrenthelpers.h +++ b/src/ui/screens/addtorrent/addtorrenthelpers.h @@ -27,7 +27,8 @@ namespace tremotesf { AddTorrentParameters getAddTorrentParameters(Rpc* rpc); AddTorrentParameters getInitialAddTorrentParameters(Rpc* rpc); - QDialog* askForMergingTrackers(Torrent* torrent, std::vector> trackers, QWidget* parent); + QDialog* + createAskForMergingTrackersDialog(Torrent* torrent, std::vector> trackers, QWidget* parent); QString bencodeErrorString(bencode::Error::Type type); } diff --git a/src/ui/screens/mainwindow/mainwindow.cpp b/src/ui/screens/mainwindow/mainwindow.cpp index 0828111a..5bb3aa82 100644 --- a/src/ui/screens/mainwindow/mainwindow.cpp +++ b/src/ui/screens/mainwindow/mainwindow.cpp @@ -171,50 +171,32 @@ namespace tremotesf { window->raise(); } -#ifdef TREMOTESF_UNIX_FREEDESKTOP - void activeWindowOnX11(QWidget* window, const std::optional& startupNotificationId) { - if (startupNotificationId.has_value()) { - info().log("Removing startup notification with id {}", *startupNotificationId); - KStartupInfo::setNewStartupId(window->windowHandle(), *startupNotificationId); - KStartupInfo::appStarted(*startupNotificationId); - } - window->activateWindow(); - } - - void activeWindowOnWayland( - [[maybe_unused]] QWidget* window, [[maybe_unused]] const std::optional& xdgActivationToken - ) { - if (xdgActivationToken.has_value()) { - info().log("Activating window with token {}", *xdgActivationToken); - // Qt gets new token from XDG_ACTIVATION_TOKEN environment variable - // It we be read and unset in QWidget::activateWindow() call below - qputenv(xdgActivationTokenEnvVariable, *xdgActivationToken); - } - window->activateWindow(); - } -#endif - - void activateWindowCompat( - QWidget* window, [[maybe_unused]] const std::optional& windowActivationToken = {} + void consumeActivationToken( + [[maybe_unused]] QWidget* window, [[maybe_unused]] std::optional& windowActivationToken ) { - info().log("Activating window {}", *window); #ifdef TREMOTESF_UNIX_FREEDESKTOP + if (!windowActivationToken.has_value()) { + return; + } switch (KWindowSystem::platform()) { case KWindowSystem::Platform::X11: debug().log("Windowing system is X11"); - activeWindowOnX11(window, windowActivationToken); + info().log("Removing startup notification with id {}", *windowActivationToken); + KStartupInfo::setNewStartupId(window->windowHandle(), *windowActivationToken); + KStartupInfo::appStarted(*windowActivationToken); break; case KWindowSystem::Platform::Wayland: debug().log("Windowing system is Wayland"); - activeWindowOnWayland(window, windowActivationToken); + info().log("Activating window with token {}", *windowActivationToken); + // Qt gets new token from XDG_ACTIVATION_TOKEN environment variable + // It wll be read and unset in QWidget::activateWindow() or QWidget::show() + qputenv(xdgActivationTokenEnvVariable, *windowActivationToken); break; default: warning().log("Unknown windowing system"); - window->activateWindow(); break; } -#else - window->activateWindow(); + windowActivationToken.reset(); #endif } } @@ -1024,7 +1006,7 @@ namespace tremotesf { const auto existingDialog = mTorrentPropertiesDialogs.find(hashString); if (existingDialog != mTorrentPropertiesDialogs.end()) { unminimizeAndRaiseWindow(existingDialog->second); - activateWindowCompat(existingDialog->second); + existingDialog->second->activateWindow(); } else { auto dialog = new TorrentPropertiesDialog(torrent, mViewModel.rpc(), mWindow); dialog->setAttribute(Qt::WA_DeleteOnClose); @@ -1176,7 +1158,7 @@ namespace tremotesf { auto existingDialog = mWindow->findChild({}, Qt::FindDirectChildrenOnly); if (existingDialog) { unminimizeAndRaiseWindow(existingDialog); - activateWindowCompat(existingDialog); + existingDialog->activateWindow(); } else { auto dialog = createDialog(); dialog->setAttribute(Qt::WA_DeleteOnClose); @@ -1493,33 +1475,21 @@ namespace tremotesf { } else { info().log("NSApp is not hidden, activating main window"); unminimizeAndRaiseWindow(mWindow); - activateWindowCompat(mWindow); + mWindow->activateWindow(); } #else if (!mWindow->isHidden()) { info().log("Main window is not hidden, activating it"); unminimizeAndRaiseWindow(mWindow); - activateWindowCompat(mWindow, windowActivationToken); + consumeActivationToken(mWindow, windowActivationToken); + mWindow->activateWindow(); return; } -# ifdef TREMOTESF_UNIX_FREEDESKTOP - // With Wayland we need to set XDG_ACTIVATION_TOKEN environment variable before show() - // so that Qt handles activation automatically - if (windowActivationToken.has_value() && KWindowSystem::isPlatformWayland()) { - info().log("Showing window with token {}", *windowActivationToken); - // Qt gets new token from XDG_ACTIVATION_TOKEN environment variable - // It we be read and unset in QWidget::show() call below - qputenv(xdgActivationTokenEnvVariable, *windowActivationToken); - windowActivationToken.reset(); - } -# endif + consumeActivationToken(mWindow, windowActivationToken); info().log("Showing window {}", *mWindow); mWindow->show(); unminimizeAndRaiseWindow(mWindow); - if (windowActivationToken.has_value()) { - activateWindowCompat(mWindow, windowActivationToken); - } for (const auto& window : mOtherWindowsHiddenByUs) { if (window) { info().log("Showing window {}", *window); @@ -1602,38 +1572,32 @@ namespace tremotesf { void showAddTorrentFileDialogs(const QStringList& files, std::optional windowActivationToken) { const bool setParent = Settings::instance()->get_showMainWindowWhenAddingTorrent(); for (const QString& filePath : files) { - auto* const dialog = showAddTorrentFileDialog(filePath, setParent); - if (windowActivationToken.has_value()) { - activateWindowCompat(dialog, windowActivationToken); - // Can use token only once - windowActivationToken.reset(); - } + showAddTorrentFileDialog(filePath, setParent, windowActivationToken); } } - QDialog* showAddTorrentFileDialog(const QString& filePath, bool setParent) { + void showAddTorrentFileDialog( + const QString& filePath, bool setParent, std::optional& windowActivationToken + ) { auto* const dialog = new AddTorrentDialog( mViewModel.rpc(), AddTorrentDialog::FileParams{filePath}, setParent ? mWindow : nullptr ); dialog->setAttribute(Qt::WA_DeleteOnClose); + consumeActivationToken(dialog, windowActivationToken); dialog->show(); - return dialog; } - void - showAddTorrentLinksDialog(const QStringList& urls, const std::optional& windowActivationToken) { + void showAddTorrentLinksDialog(const QStringList& urls, std::optional& windowActivationToken) { auto* const dialog = new AddTorrentDialog( mViewModel.rpc(), AddTorrentDialog::UrlParams{urls}, Settings::instance()->get_showMainWindowWhenAddingTorrent() ? mWindow : nullptr ); dialog->setAttribute(Qt::WA_DeleteOnClose); + consumeActivationToken(dialog, windowActivationToken); dialog->show(); - if (windowActivationToken.has_value()) { - activateWindowCompat(dialog, windowActivationToken); - } } void askForMergingTrackers( @@ -1642,13 +1606,13 @@ namespace tremotesf { ) { const bool setParent = Settings::instance()->get_showMainWindowWhenAddingTorrent(); for (auto& [torrent, trackers] : existingTorrents) { - auto* const dialog = - tremotesf::askForMergingTrackers(torrent, std::move(trackers), setParent ? mWindow : nullptr); - if (windowActivationToken.has_value()) { - activateWindowCompat(dialog, windowActivationToken); - // Can use token only once - windowActivationToken.reset(); - } + auto* const dialog = tremotesf::createAskForMergingTrackersDialog( + torrent, + std::move(trackers), + setParent ? mWindow : nullptr + ); + consumeActivationToken(dialog, windowActivationToken); + dialog->show(); } } @@ -1663,7 +1627,6 @@ namespace tremotesf { dialog->setAttribute(Qt::WA_DeleteOnClose, true); dialog->setModal(false); dialog->show(); - activateWindowCompat(dialog); }; QObject::connect(mViewModel.rpc(), &Rpc::torrentAddDuplicate, this, [=] { @@ -1681,9 +1644,7 @@ namespace tremotesf { }); } - void showDelayedTorrentAddDialog( - const QStringList& torrents, const std::optional& windowActivationToken - ) { + void showDelayedTorrentAddDialog(const QStringList& torrents, std::optional windowActivationToken) { debug().log("MainWindow: showing delayed torrent add dialog"); const auto dialog = new QMessageBox( QMessageBox::Information, @@ -1702,10 +1663,8 @@ namespace tremotesf { detailedText += '\n'; } dialog->setDetailedText(detailedText); + consumeActivationToken(dialog, windowActivationToken); dialog->show(); - if (windowActivationToken.has_value()) { - activateWindowCompat(dialog, windowActivationToken); - } QObject::connect(mViewModel.rpc(), &Rpc::connectedChanged, dialog, [=, this] { if (mViewModel.rpc()->isConnected()) dialog->close(); });