lxqt / libqtxdg

Qt implementation of freedesktop.org xdg specs
https://lxqt.github.io
GNU Lesser General Public License v2.1
72 stars 35 forks source link

XdgDesktopFile::startDetached and XdgDesktopFile::actionActivate not "detaching" #245

Closed elviosak closed 3 years ago

elviosak commented 3 years ago

XdgDesktopFile::startDetached and XdgDesktopFile::actionActivate not "detaching". I noticed it in lxqt-panel menu and quicklaunch plugins, when killing the panel most programs opened with those plugins close with the panel (tested firefox, brave, chromium, nemo, vlc, looks like electron apps like vscodium do not close).

Expected Behavior

after opening, the programs should be independent of the program who launched it.

Current Behavior

all non-electron programs close

Possible Solution

looking at the code it looks like this https://github.com/lxqt/libqtxdg/blob/dbd0c513d30a161cbde6c33c9d4475e663612abb/src/qtxdg/xdgdesktopfile.cpp#L499 is false leading to the process not detaching, but i'm not sure what StartDetachTruly::instance does.

Steps to Reproduce (for bugs)

minimal test code with qtcreator, default new "Qt Widgets Application" without ui, hardcoded to firefox, change to something else if not available.

untitled.pro add: LIBS += -lQt5Xdg

mainwindow.cpp:

#include <QPushButton>
#include <QVBoxLayout>
#include <QWidget>

#include <qt5xdg/XdgDesktopFile>

MainWindow::MainWindow(QWidget *parent)
    : QMainWindow(parent)
{
    auto central = new QWidget;
    auto vbox = new QVBoxLayout(central);
    setCentralWidget(central);
    XdgDesktopFile d;
    d.load("/usr/share/applications/firefox.desktop");
    auto btn = new QPushButton(d.icon(), d.name(), this);
    vbox->addWidget(btn);
    connect(btn, &QPushButton::clicked, this, [=]{
        d.startDetached();
    });
    vbox->addSpacing(10);
    for (QString action: d.actions()){
        auto b = new QPushButton(d.actionIcon(action), d.actionName(action));
        connect(b, &QPushButton::clicked, this, [=]{
            d.actionActivate(action, QStringList());
        });
        vbox->addWidget(b);
    }
}

MainWindow::~MainWindow()
{
}

this will open a window with one or more buttons according to the desktop file used, open some of them then close this program.

Context

Although unlikely, a crash in lxqt-panel or other programs that use this lib could lead to a lot of lost work due to programs closing without warning.

System Information
tsujan commented 3 years ago

Not reproducible here. I start Firefox from Quick Launch and stop Panel's process from Session Settings; Firefox continues as usual. And the same for the main menu.

I also tested in a terminal and used Ctrl+C; no difference. Using Qps for killing or terminating Panel made no difference either. In all cases, Firefox continued as usual.

tsujan commented 3 years ago

@stefonarch, would you please test it too? I don't want to close this prematurely.

stefonarch commented 3 years ago

Just tested with Falkon from quicklaunch, it doesn't die when panel is stopped from session settings or killed.

tsujan commented 3 years ago

@slidinghotdog I found some time and also tested your code. Again, Firefox continued as usual when I closed MainWindow.

palinek commented 3 years ago
  1. Note the recent changes in libqtxdg (https://github.com/lxqt/libqtxdg/pull/244) & lxqt-session (https://github.com/lxqt/lxqt-session/pull/364) -> this is a feature
  2. It can be controlled by setting the env. var QTXDG_START_DETACH_TRULY
  3. "a crash in lxqt-panel or other programs that use this lib could lead to a lot of lost" -> no, as the TERM signal is sent to child processes only on clean application (lxqt-panel/runner/whatever) shutdown
tsujan commented 3 years ago

Oh, my installed lxqt-session is from a month ago. Should upgrade it to test everything. Thanks!

tsujan commented 3 years ago

Very nice! it works as intended.

I missed this feature in LXQt for a long time. Now, FeatherPad remembers cursor positions of its session files without needing to be closed before logout. And more...

luis-pereira commented 3 years ago

@slidinghotdog Which libqtxdg version are you using ?

elviosak commented 3 years ago

@luis-pereira version 3.6.0.13.gdbd0c51-1 from AUR, updated 1 or 2 days ago, edited OP

elviosak commented 3 years ago

@palinek this is what happens here, with xed qtxdg

tsujan commented 3 years ago

@palinek After giving it more thought, I think @slidinghotdog's concern is valid.

There are scenarios, where the user or coder may need to restart the panel frequently: making/editing an LXQt theme, adding a plugin, fixing a bug, etc.. He may have opened a browser, a terminal emulator, a text editor,... for his work by using the panel itself. Seeing them disappear can be very annoying.

Although I'm happy about the recent changes, I think they may be counterintuitive. I don't remember that removing/restarting a panel had caused killing unrelated apps like Firefox anywhere else. IMO, this feature will be useful only if it's limited to LXQt's logout/shutdown; otherwise, we'll receive lots of complaints.

EDIT: QTXDG_START_DETACH_TRULY is not the answer.

elviosak commented 3 years ago

Just tested on a fresh vm, and it looks like QProcess::terminate (with a test program just to start and terminate a process) as well as sending SIGTERM or SIGKILL with htop just closes the program without asking to save, tried with xed and featherpad, i thought programs would handle SIGTERM before closing.

tsujan commented 3 years ago

@slidinghotdog, It was supposed to be a feature for making apps exit cleanly at the end of an LXQt session (with QTXDG_START_DETACH_TRULY set to 0 , which is the default). However, it isn't limited to the session end and that's the problem -- at least, in my opinion.

tsujan commented 3 years ago

i thought programs would handle SIGTERM before closing

Not for showing a save prompt but for saving their configuration.

palinek commented 3 years ago

He may have opened a browser, a terminal emulator, a text editor,... for his work by using the panel itself. Seeing them disappear can be very annoying.

EDIT: QTXDG_START_DETACH_TRULY is not the answer.

And what should be the answer? The env. vars are IMO perfectly valid for configuring of low level libraries behavior. Note that the panel/runner developer is not any regular Joe and running panel/runner instance with some env. vars set is a matter of seconds.

tsujan commented 3 years ago

And what should be the answer?

@palinek, I don't know -- I even haven't found time to read its codes -- but the current behavior doesn't seem "natural" to me when I become an LXQt user, in contrast to a developer.

It isn't acceptable to me that, in the middle of a heavy work, my text editor suddenly disappears without a save prompt only because I've stooped lxqt-panel's process. That is acceptable when -- and only when -- I log out of LXQt.

elviosak commented 3 years ago

i thought programs would handle SIGTERM before closing

Not for showing a save prompt but for saving their configuration.

i was expecting something like the _NET_CLOSE_WINDOW but looks like there's no equivalent signal for that, and it looks like that isn't the purpose of those changes anyway.

It isn't acceptable to me that, in the middle of a heavy work, my text editor suddenly disappears without a save prompt only because I've stooped lxqt-panel's process. That is acceptable when -- and only when -- I log out of LXQt.

Could lxqt-panel forward the opening of programs to lxqt-session? So everything is bound to session instead of panel?

tsujan commented 3 years ago

@slidinghotdog Usually, apps do their last-second config saving and cleanup in reaction to signals like SIGTERM. I don't think anyone expects more. However, I've seen that, under KDE, if a (Qt?) text editor has unsaved documents, a prompt will be shown on logging out (I don't know if that's still the case because I don't use KDE). I don't think we can do that in LXQt but the current problem isn't related to it.

elviosak commented 3 years ago

However, I've seen that, under KDE, if a (Qt?) text editor has unsaved documents, a prompt will be shown on logging out

It still does, i tried it a few days ago, and qbittorrent always inhibits reboot/logout even if its option to do so is disabled, also happens on browsers playing media i think... Pretty annoying "feature" IMO.

Could lxqt-panel forward the opening of programs to lxqt-session? So everything is bound to session instead of panel?

Also i just remembered that people use lxqt-panel without the rest of lxqt so if this is doable it would need to account for that too.

stefonarch commented 3 years ago

Well, it does also for open libreoffice documents, that's not bad. In LXQt the intern recovery of Libroffice has to do it's work on next opening.

tsujan commented 3 years ago

OK, this is a bug -- or a regression:

I stop panel from LXQt Session Settings and the dialog of LXQt Session Settings disappears. How can the user know how to restore the panel? IMO, it should be fixed before 0.17.