Merge pull request #1954 from Scrumplex/fix/open-paths-immediately

launcher/DesktopServices.cpp

@@ -37,140 +37,33 @@
 #include <QDesktopServices>
 #include <QDir>
 #include <QProcess>
-
-/**
- * This shouldn't exist, but until QTBUG-9328 and other unreported bugs are fixed, it needs to be a thing.
- */
-#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
-
-#include <errno.h>
-#include <sys/types.h>
-#include <sys/wait.h>
-#include <unistd.h>
-
-template <typename T>
-bool IndirectOpen(T callable, qint64* pid_forked = nullptr)
-{
-    auto pid = fork();
-    if (pid_forked) {
-        if (pid > 0)
-            *pid_forked = pid;
-        else
-            *pid_forked = 0;
-    }
-    if (pid == -1) {
-        qWarning() << "IndirectOpen failed to fork: " << errno;
-        return false;
-    }
-    // child - do the stuff
-    if (pid == 0) {
-        // unset all this garbage so it doesn't get passed to the child process
-        qunsetenv("LD_PRELOAD");
-        qunsetenv("LD_LIBRARY_PATH");
-        qunsetenv("LD_DEBUG");
-        qunsetenv("QT_PLUGIN_PATH");
-        qunsetenv("QT_FONTPATH");
-
-        // open the URL
-        auto status = callable();
-
-        // detach from the parent process group.
-        setsid();
-
-        // die. now. do not clean up anything, it would just hang forever.
-        _exit(status ? 0 : 1);
-    } else {
-        // parent - assume it worked.
-        int status;
-        while (waitpid(pid, &status, 0)) {
-            if (WIFEXITED(status)) {
-                return WEXITSTATUS(status) == 0;
-            }
-            if (WIFSIGNALED(status)) {
-                return false;
-            }
-        }
-        return true;
-    }
-}
-#endif
+#include "FileSystem.h"
 
 namespace DesktopServices {
-bool openDirectory(const QString& path, [[maybe_unused]] bool ensureExists)
+bool openPath(const QFileInfo& path, bool ensureFolderPathExists)
 {
-    qDebug() << "Opening directory" << path;
-    QDir parentPath;
-    QDir dir(path);
-    if (ensureExists && !dir.exists()) {
-        parentPath.mkpath(dir.absolutePath());
+    qDebug() << "Opening path" << path;
+    if (ensureFolderPathExists) {
+        FS::ensureFolderPathExists(path);
     }
-    auto f = [&]() { return QDesktopServices::openUrl(QUrl::fromLocalFile(dir.absolutePath())); };
-#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
-    if (!isSandbox()) {
-        return IndirectOpen(f);
-    }
-#endif
-    return f();
+    return openUrl(QUrl::fromLocalFile(QFileInfo(path).absolutePath()));
 }
 
-bool openFile(const QString& path)
+bool openPath(const QString& path, bool ensureFolderPathExists)
 {
-    qDebug() << "Opening file" << path;
-    auto f = [&]() { return QDesktopServices::openUrl(QUrl::fromLocalFile(path)); };
-#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
-    if (!isSandbox()) {
-        return IndirectOpen(f);
-    } else {
-        return f();
-    }
-#else
-    return f();
-#endif
-}
-
-bool openFile(const QString& application, const QString& path, const QString& workingDirectory, qint64* pid)
-{
-    qDebug() << "Opening file" << path << "using" << application;
-#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
-    // FIXME: the pid here is fake. So if something depends on it, it will likely misbehave
-    if (!isSandbox()) {
-        return IndirectOpen([&]() { return QProcess::startDetached(application, QStringList() << path, workingDirectory); }, pid);
-    } else {
-        return QProcess::startDetached(application, QStringList() << path, workingDirectory, pid);
-    }
-#else
-    return QProcess::startDetached(application, QStringList() << path, workingDirectory, pid);
-#endif
+    return openPath(QFileInfo(path), ensureFolderPathExists);
 }
 
 bool run(const QString& application, const QStringList& args, const QString& workingDirectory, qint64* pid)
 {
     qDebug() << "Running" << application << "with args" << args.join(' ');
-#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
-    if (!isSandbox()) {
-        // FIXME: the pid here is fake. So if something depends on it, it will likely misbehave
-        return IndirectOpen([&]() { return QProcess::startDetached(application, args, workingDirectory); }, pid);
-    } else {
     return QProcess::startDetached(application, args, workingDirectory, pid);
 }
-#else
-    return QProcess::startDetached(application, args, workingDirectory, pid);
-#endif
-}
 
 bool openUrl(const QUrl& url)
 {
     qDebug() << "Opening URL" << url.toString();
-    auto f = [&]() { return QDesktopServices::openUrl(url); };
-#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
-    if (!isSandbox()) {
-        return IndirectOpen(f);
-    } else {
-        return f();
-    }
-#else
-    return f();
-#endif
+    return QDesktopServices::openUrl(url);
 }
 
 bool isFlatpak()
@@ -191,9 +84,4 @@ bool isSnap()
 #endif
 }
 
-bool isSandbox()
-{
-    return isSnap() || isFlatpak();
-}
-
 }  // namespace DesktopServices

launcher/DesktopServices.h

@@ -3,31 +3,30 @@
 #include <QString>
 #include <QUrl>
 
+class QFileInfo;
+
 /**
  * This wraps around QDesktopServices and adds workarounds where needed
  * Use this instead of QDesktopServices!
  */
 namespace DesktopServices {
 /**
- * Open a file in whatever application is applicable
+ * Open a path in whatever application is applicable.
+ * @param ensureFolderPathExists Make sure the path exists
  */
-bool openFile(const QString& path);
+bool openPath(const QFileInfo& path, bool ensureFolderPathExists = false);
 
 /**
- * Open a file in the specified application
+ * Open a path in whatever application is applicable.
+ * @param ensureFolderPathExists Make sure the path exists
  */
-bool openFile(const QString& application, const QString& path, const QString& workingDirectory = QString(), qint64* pid = 0);
+bool openPath(const QString& path, bool ensureFolderPathExists = false);
 
 /**
  * Run an application
  */
 bool run(const QString& application, const QStringList& args, const QString& workingDirectory = QString(), qint64* pid = 0);
 
-/**
- * Open a directory
- */
-bool openDirectory(const QString& path, bool ensureExists = false);
-
 /**
  * Open the URL, most likely in a browser. Maybe.
  */
@@ -42,9 +41,4 @@ bool isFlatpak();
  * Determine whether the launcher is running in a Snap environment
  */
 bool isSnap();
-
-/**
- * Determine whether the launcher is running in a sandboxed (Flatpak or Snap) environment
- */
-bool isSandbox();
 }  // namespace DesktopServices

launcher/FileSystem.cpp

@@ -272,15 +272,19 @@ bool ensureFilePathExists(QString filenamepath)
     return success;
 }
 
-bool ensureFolderPathExists(QString foldernamepath)
+bool ensureFolderPathExists(const QFileInfo folderPath)
 {
-    QFileInfo a(foldernamepath);
     QDir dir;
-    QString ensuredPath = a.filePath();
+    QString ensuredPath = folderPath.filePath();
     bool success = dir.mkpath(ensuredPath);
     return success;
 }
 
+bool ensureFolderPathExists(const QString folderPathName)
+{
+    return ensureFolderPathExists(QFileInfo(folderPathName));
+}
+
 bool copyFileAttributes(QString src, QString dst)
 {
 #ifdef Q_OS_WIN32

launcher/FileSystem.h

@@ -91,7 +91,13 @@ bool ensureFilePathExists(QString filenamepath);
  * Creates all the folders in a path for the specified path
  * last segment of the path is treated as a folder name and is created!
  */
-bool ensureFolderPathExists(QString filenamepath);
+bool ensureFolderPathExists(const QFileInfo folderPath);
+
+/**
+ * Creates all the folders in a path for the specified path
+ * last segment of the path is treated as a folder name and is created!
+ */
+bool ensureFolderPathExists(const QString folderPathName);
 
 /**
  * @brief Copies a directory and it's contents from src to dest

launcher/ui/MainWindow.cpp

@@ -1197,43 +1197,43 @@ void MainWindow::undoTrashInstance()
 
 void MainWindow::on_actionViewLauncherRootFolder_triggered()
 {
-    DesktopServices::openDirectory(".");
+    DesktopServices::openPath(".");
 }
 
 void MainWindow::on_actionViewInstanceFolder_triggered()
 {
     QString str = APPLICATION->settings()->get("InstanceDir").toString();
-    DesktopServices::openDirectory(str);
+    DesktopServices::openPath(str);
 }
 
 void MainWindow::on_actionViewCentralModsFolder_triggered()
 {
-    DesktopServices::openDirectory(APPLICATION->settings()->get("CentralModsDir").toString(), true);
+    DesktopServices::openPath(APPLICATION->settings()->get("CentralModsDir").toString(), true);
 }
 
 void MainWindow::on_actionViewIconThemeFolder_triggered()
 {
-    DesktopServices::openDirectory(APPLICATION->themeManager()->getIconThemesFolder().path(), true);
+    DesktopServices::openPath(APPLICATION->themeManager()->getIconThemesFolder().path(), true);
 }
 
 void MainWindow::on_actionViewWidgetThemeFolder_triggered()
 {
-    DesktopServices::openDirectory(APPLICATION->themeManager()->getApplicationThemesFolder().path(), true);
+    DesktopServices::openPath(APPLICATION->themeManager()->getApplicationThemesFolder().path(), true);
 }
 
 void MainWindow::on_actionViewCatPackFolder_triggered()
 {
-    DesktopServices::openDirectory(APPLICATION->themeManager()->getCatPacksFolder().path(), true);
+    DesktopServices::openPath(APPLICATION->themeManager()->getCatPacksFolder().path(), true);
 }
 
 void MainWindow::on_actionViewIconsFolder_triggered()
 {
-    DesktopServices::openDirectory(APPLICATION->icons()->getDirectory(), true);
+    DesktopServices::openPath(APPLICATION->icons()->getDirectory(), true);
 }
 
 void MainWindow::on_actionViewLogsFolder_triggered()
 {
-    DesktopServices::openDirectory("logs", true);
+    DesktopServices::openPath("logs", true);
 }
 
 void MainWindow::refreshInstances()
@@ -1452,7 +1452,7 @@ void MainWindow::on_actionViewSelectedInstFolder_triggered()
 {
     if (m_selectedInstance) {
         QString str = m_selectedInstance->instanceRoot();
-        DesktopServices::openDirectory(QDir(str).absolutePath());
+        DesktopServices::openPath(QFileInfo(str));
     }
 }
 

launcher/ui/dialogs/IconPickerDialog.cpp

@@ -159,5 +159,5 @@ IconPickerDialog::~IconPickerDialog()
 
 void IconPickerDialog::openFolder()
 {
-    DesktopServices::openDirectory(APPLICATION->icons()->getDirectory(), true);
+    DesktopServices::openPath(APPLICATION->icons()->getDirectory(), true);
 }

launcher/ui/pages/instance/ExternalResourcesPage.cpp

@@ -290,12 +290,12 @@ void ExternalResourcesPage::disableItem()
 
 void ExternalResourcesPage::viewConfigs()
 {
-    DesktopServices::openDirectory(m_instance->instanceConfigFolder(), true);
+    DesktopServices::openPath(m_instance->instanceConfigFolder(), true);
 }
 
 void ExternalResourcesPage::viewFolder()
 {
-    DesktopServices::openDirectory(m_model->dir().absolutePath(), true);
+    DesktopServices::openPath(m_model->dir().absolutePath(), true);
 }
 
 bool ExternalResourcesPage::current(const QModelIndex& current, const QModelIndex& previous)

launcher/ui/pages/instance/ScreenshotsPage.cpp

@@ -324,8 +324,7 @@ void ScreenshotsPage::onItemActivated(QModelIndex index)
     if (!index.isValid())
         return;
     auto info = m_model->fileInfo(index);
-    QString fileName = info.absoluteFilePath();
-    DesktopServices::openFile(info.absoluteFilePath());
+    DesktopServices::openPath(info);
 }
 
 void ScreenshotsPage::onCurrentSelectionChanged(const QItemSelection& selected)
@@ -352,7 +351,7 @@ void ScreenshotsPage::onCurrentSelectionChanged(const QItemSelection& selected)
 
 void ScreenshotsPage::on_actionView_Folder_triggered()
 {
-    DesktopServices::openDirectory(m_folder, true);
+    DesktopServices::openPath(m_folder, true);
 }
 
 void ScreenshotsPage::on_actionUpload_triggered()

launcher/ui/pages/instance/VersionPage.cpp

@@ -447,12 +447,12 @@ void VersionPage::on_actionAdd_Empty_triggered()
 
 void VersionPage::on_actionLibrariesFolder_triggered()
 {
-    DesktopServices::openDirectory(m_inst->getLocalLibraryPath(), true);
+    DesktopServices::openPath(m_inst->getLocalLibraryPath(), true);
 }
 
 void VersionPage::on_actionMinecraftFolder_triggered()
 {
-    DesktopServices::openDirectory(m_inst->gameRoot(), true);
+    DesktopServices::openPath(m_inst->gameRoot(), true);
 }
 
 void VersionPage::versionCurrent(const QModelIndex& current, [[maybe_unused]] const QModelIndex& previous)

launcher/ui/pages/instance/WorldListPage.cpp

@@ -207,7 +207,7 @@ void WorldListPage::on_actionRemove_triggered()
 
 void WorldListPage::on_actionView_Folder_triggered()
 {
-    DesktopServices::openDirectory(m_worlds->dir().absolutePath(), true);
+    DesktopServices::openPath(m_worlds->dir().absolutePath(), true);
 }
 
 void WorldListPage::on_actionDatapacks_triggered()
@@ -223,7 +223,7 @@ void WorldListPage::on_actionDatapacks_triggered()
 
     auto fullPath = m_worlds->data(index, WorldList::FolderRole).toString();
 
-    DesktopServices::openDirectory(FS::PathCombine(fullPath, "datapacks"), true);
+    DesktopServices::openPath(FS::PathCombine(fullPath, "datapacks"), true);
 }
 
 void WorldListPage::on_actionReset_Icon_triggered()

launcher/ui/widgets/ThemeCustomizationWidget.cpp

@@ -34,11 +34,11 @@ ThemeCustomizationWidget::ThemeCustomizationWidget(QWidget* parent) : QWidget(pa
     connect(ui->backgroundCatComboBox, QOverload<int>::of(&QComboBox::currentIndexChanged), this, &ThemeCustomizationWidget::applyCatTheme);
 
     connect(ui->iconsFolder, &QPushButton::clicked, this,
-            [] { DesktopServices::openDirectory(APPLICATION->themeManager()->getIconThemesFolder().path()); });
+            [] { DesktopServices::openPath(APPLICATION->themeManager()->getIconThemesFolder().path()); });
     connect(ui->widgetStyleFolder, &QPushButton::clicked, this,
-            [] { DesktopServices::openDirectory(APPLICATION->themeManager()->getApplicationThemesFolder().path()); });
+            [] { DesktopServices::openPath(APPLICATION->themeManager()->getApplicationThemesFolder().path()); });
     connect(ui->catPackFolder, &QPushButton::clicked, this,
-            [] { DesktopServices::openDirectory(APPLICATION->themeManager()->getCatPacksFolder().path()); });
+            [] { DesktopServices::openPath(APPLICATION->themeManager()->getCatPacksFolder().path()); });
 }
 
 ThemeCustomizationWidget::~ThemeCustomizationWidget()