(In reply to Alexey Chernov from comment #25) > According to what? According to "This is not fixed in years and each and every session management code was ported as "#if 0"" If there was some relevant interest, it would be fixed long time, since it's really not that hard. > > Loosing your data is however relevant for everyone. And the latter is the by > > far more severe issue. Restarting applications is merely an annoyance, > > loosing your work is truely expensive. > Hey, how could session management be "apparently relevant for only a > minority of users", but fixes in its behaviour be crucial for a lot of them? [...] > Fully agree here, but we should confirm that nobody said in the beggining > that upstream changes were about to break session management for KF5 > applications. It was just broken. Errr... what? Session management (in terms of "please restore the desktop as I left it") doesn't seem very important, but if you click "logout" and *booom*, gone is your work of the last two hours because the application had no chance (or, well, listened to the wrong events) to ask you to save it, that's pretty important... We're apparently talking past each other. There're two steps: a) logout, clients ask to save your stuff. That works (because of the close event) b) login, clients should restart. That's broken (because the close event is not just an event, but the window "illegally" being withdrawn during logout) You propse to fix (b) by breaking (a) and I'm trying to convince you that this is a really bad idea. > bugs which should be fixed either in library and its clients. It's better to > fix them when no one really relies on the stability too much. It looks like > this time is now for KF5-based application and environment. Yes, we should fix KMainWindow now (if faking close events is finally not considered a permanent behavior despite the majority of clients will probably do that in return to the data commit request - with a fair share actually just calling close() ...) but that has absolutely no implications on whether it's ok to easily break away from established (even though maybe wrong) behavior. > No, that's just postponing and messing up the whole problem. If, as you > stated, almost no one implemented easy and pretty simple interaction > appeared in Qt5, even less would care of possible bugs and corner cases of > the workaround, more complex protocol with close event you propose. There > would be just another argument that it's just too messy, not to mention > already existing argument that no one uses session management. Sorry, but I really cannot read any sense into this paragraph. Please try to rephrase it. The above isn't English grammar at all. > It won't be fixed until it's broken So you demand to jeopardize userdata because otherwise code won't be fixed. Sorry, but there's no way you're ever gonna convince me in this. Any solution that builds upon "jeopardize userdata" is not a solution at all. It's malicious. > I'm pretty sure there would be packages that would require just > most recent Qt version, and it would be acceptable. And jeopardize userdata. What exactly should this help in this case? > What's wrong in relying on changes in recent Qt release That you are not. "Recent Qt releases" start to jeopardize userdata because (no way) all Qt client code has been fixed, ie. distros must not use Qt 5.7 until all client code releases picked up the change. > In this way all the bugfixes should be postponed till Qt6 If a bugfix is *very* likely gonna break more things more badly than it fixes and there is absolutely no need to take this risk? Yes, avoid that fix and come up with a more robust interim solution. > By the way, are you completely sure your changes won't break any clients? > What if someone relies exactly on the current code? Err, on the close event? Well, that's exactly what I suggest to preserve. You're not gonna see more from a ::close() call in client code (it's no virtual and not invoked as slot) > In the opposite, close event or aboutToClose do mean closing > and quitting, but don't imply any data interactions (as it's too late) Errr, no. Please read the Qt API docs. You simply ::ignore() the close event and nothing is gonna be closed or changed. That's why it's massively intercepted to ask users to save stuff - what's also explicitly suggested in the API docs. And that's why somebody considered it a good idea to explicitly close windows on session end, what it's just not. That "somebody" actually wanted to trigger and check the result of the event, but not really close windows. And that's what I'm proposing to deal with the situation. The justification for QGuiApplication::commitDataRequest() though is, that a QGuiApplication doesn't necessarily have any windows at all and still wants to save stuff on logout (and of course before the SIGTERM) ===================== Completely untested patch (not even a build attempt) for Qt 5.6: ===================== diff --git a/src/gui/kernel/qguiapplication.cpp b/src/gui/kernel/qguiapplication.cpp index 7d469dd..3b09ddd 100644 --- a/src/gui/kernel/qguiapplication.cpp +++ b/src/gui/kernel/qguiapplication.cpp @@ -3232,8 +3232,31 @@ void QGuiApplicationPrivate::commitData() Q_Q(QGuiApplication); is_saving_session = true; emit q->commitDataRequest(*session_manager); - if (session_manager->allowsInteraction() && !tryCloseAllWindows()) - session_manager->cancel(); + if (session_manager->allowsInteraction()) { + // trigger close event handling in all windows + // this allows them to clean up and if they refuse + // to close, we cancel the logout + // NOTICE: do NOT actually close windows - this behavior is + // explicitly wrong and would prevent the window from being + // restored with the session + QCloseEvent closeEvent; + QWindowList list = QGuiApplication::topLevelWindows(); + QWindowList processedWindows; + for (int i = 0; i < list.size(); ++i) { + QWindow *w = list.at(i); + if (!processedWindows.contains(w)) { + sendEvent(w, &closeEvent); + if (!ce.isAccepted()) { + session_manager->cancel(); + break; // logout canceled, no further close hinting + } + processedWindows.append(w); + list = QGuiApplication::topLevelWindows(); + i = -1; + } + } + } + } is_saving_session = false; } ====================== --- off topic > > > Never mentioned minor update or particular version. Please don't distort. > > So you meant to schedule this for Qt6? > No. I just stated that I didn't mention any particular version, no other > implications. https://en.wikipedia.org/wiki/Software_versioning major.minor.patch|revision|release|build So either you point Qt6 or a minor release. --- off topic on off topic > No, it's not off-topic Yes it is. KDE's problems lie in different areas (predominantly QtQuick, to be precise) - broken session management isn't a showstopper at all in lights of "desktop completely freezes", "plasmashell constantly runs at 100% CPU" and "everything crashes all the time with obscure backtraces" bugreports.