[Touch-packages] [Bug 1446865]
I'm actually aware of the problem with session management since last summer, now I've upgraded my stuff to have more KF5-based applications and suprisingly found it still just doesn't work. So I've dived deeper into it this time, reading all the discussion here and last part of bug #341930, both Andreas's change proposals and the thread in the mailing- list. I likely agree with comment #18 of Andreas and in my point of view it is the following: 1. As a whole it's a massive regression since KDE4 which affects all Qt5 applications, most of which behave correctly as a session clients. Even server parts of both KWin and KSMServer now behave correctly, thanks to Thomas's fixes, I think. But as a whole it just doesn't work at all. 2. This problem is caused by some bug in processing session management messages in Qt, which earlier wasn't a big pain and could be avoided, but due to significant changes in the whole interaction process, in the API etc. now it can't be avoided and lead to (1). 3. There's initial change (https://codereview.qt- project.org/#/c/142232/) by Andreas, which perfectly fixes the problem with any observable problems. It also fixes a fault in the session management protocol implementation for at least two OSs, which is good for Qt itself. 4. There could be potentially affected client applications which: a) were already been ported to/written for Qt5; b) process some valuable data which shouldn't be lost; c) would like to use session management to prevent loss of unsaved data; d) still don't care to follow session management protocol correctly and just exploit old hacks and errors in its implementation, which exist historically, but now is moved to a new place. Unfortunately, this term is a little objectless since it wasn't mentioned any real-life application like this. 5. I completely don't like the proposed way to preserve the compatibility with (4) and make the use case of broken session management client implementation legal and default, but also try to allow proper-written apps to still survive somehow, adding some strange workarounds to Qt as closing all the windows, but not too much, or API properties to enable proper processing of SM messages. To sum it all up, I've applied the patch (3) and have all the session management things back again without any other changes to KDE or whatever, it's already released versions (KF-5.18.0, Plasma-5.5.3, applications-15.12.1). I'll also test Windows behaviour with some toy application. Unless any problems arise, I see no reason why this tiny and simple (and right) fix isn't applied and merged. -- You received this bug notification because you are a member of Ubuntu Touch seeded packages, which is subscribed to qtbase-opensource-src in Ubuntu. https://bugs.launchpad.net/bugs/1446865 Title: KDE5/Qt5 does not support session restoration Status in KDE Base Workspace: Confirmed Status in Qt: New Status in plasma-workspace package in Ubuntu: Confirmed Status in qtbase-opensource-src package in Ubuntu: Confirmed Bug description: KDE5/Qt5 does not support proper session restoration. ProblemType: Bug DistroRelease: Ubuntu 15.10 To manage notifications about this bug go to: https://bugs.launchpad.net/kdebase-workspace/+bug/1446865/+subscriptions -- Mailing list: https://launchpad.net/~touch-packages Post to : touch-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~touch-packages More help : https://help.launchpad.net/ListHelp
[Touch-packages] [Bug 1446865]
(In reply to Thomas Lübking from comment #26) > (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. Rather interesting indicator. Why don't you apply it to Qt5 or KF5 then? What a selective vision :) > > > 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... Hmmm... so session management "doesn't seem very important", but there're applications which expect a) to be closed gently, and also b) to have an opportunity to interact to the user the very special way, so that the rest of the world is waiting for them and doesn't logout, but it's surely not session management. Nice. Following this way we have, I think, thousands of apps which don't use X, kernel etc. and other not so important stuff. > 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. The matter is just that if you like the fruitful results of some service or protocol, you need to follow the rules of it. If you violate them and currently it just works, it's natural that anyone can change something internally and you are going to fail. Rather atomic thing. My proposal is just to have library code of Qt following the proper interaction process, which is expected by anyone who haven't read this discussion and just wants to support session management in the application. Nothing against any workarounds in KSMServer, KF wrappers or anywhere else downstream. > > 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. There's no one accepted fix of Qt to fix anything against. There's a way to fix applications to interact with session manager properly though and add some fixes and workarounds to make it work somehow, at least with any local Qt patch. According to the comments of https://codereview.qt-project.org/#/c/146566/, that's something what Andreas is doing. > > 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. Again. Please try to reread it: https://www.kde.org/code-of-conduct/. Hopefully, it's English would impress you more. > > 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 b
[Touch-packages] [Bug 1446865]
(In reply to Thomas Lübking from comment #22) > > 5. I completely don't like the proposed way to preserve the compatibility > > with (4) and make > > the use case of broken session management client implementation legal and > > default, but > > also try to allow proper-written apps to still survive somehow, adding some > > strange > > workarounds to Qt as closing all the windows, but not too much, or API > > properties to enable > > proper processing of SM messages. > > No ofense, but what you "like" is completely irrelevant. Comments like this clearly don't help the discussion or solving the problem, especially when you start your reply with them. I won't answer you the same style, but given that it's not the first one from you, my earnest request to you is to please respect each other and avoid such comments in future. In case of any questions feel free to consult https://www.kde.org/code-of-conduct/. Thank you. > You propose to intentionally break clients by library changes in some minor > update Never mentioned minor update or particular version. Please don't distort. > to teach developers to do right No intention to do it, but any specs probably means something like that. > but while you might aim their face, you're gonna hit the users (and probably yours) Users were already hit when the significant part of functionality important for someone's every day use case is broken. I just can't get why it's OK to break everything for one part of users and ultimately save broken implementation to preserve some ephemeral compatibility for another. That's probably the biggest question for me in this thread. Maybe I'm wrong and those who use sessions are somewhat less important than users that sometimes save their data on quitting? It's worth mentioning then, and I'll immediately give up. > We had that (I kindly remind of the qDeleteAll fix ...) and it cooked up > hell. Still better than a couple of API methods like "enableSpecifiedBehaviour()" or deleting and trying to catch SIGSEGV, right?) > commitDataRequest hardly shows up in lxr.kde.org, what means it's probably > not used at all and aboutToQuit (which isn't used but could come to rescue) > isn't used too much either. > > The BY FAR! omnipresent pattern is to listen to queryClose() which is > called/emitted on -guess what- close events from KMainWindow. > And that's for pretty much sure why the (wrong) behavior in QSessionManager > exists. If it wasn't done before for some reason, it's better to just fix the applications, especially given that you don't need any changes in Qt to have just the same functionality with the new approach. If it's still too much to change while porting to Qt5/KF5, I really wonder what porting is. Once again: we all could already apply the fix of Andreas and be busy fixing the necessary applications rather than keep discussing here. > Is this behavior correct? No. > Does this matter? NO! > > It's ok to spam a #warning that this behavior is shit and deprecate and kill > it for Qt6 On the Qt6 release you would say that everyone already rely on the workaround there was in Qt5 etc. etc. That's an endless story. By the way, do you really think it's so much major change that it can't be changed before Qt6? Seriously, with no API change and with just removing unexpected actions? > and we might even bail out (aka "fix") KMainWindow applications > NOW by invoking queryClose() on QGuiApplicationPrivate::commitData() but > regardless, we MUST assume this to be a global default pattern that > applications (also beyond KDE) rely on (also because it's absolutely natural > to intercept closing to save data and not think of closing on session end > could be something entirely different - actually the illegal behavior > happens to be the most sane one...) I just kindly remind your description of current Plasma 5 and it's application state: https://bugs.kde.org/show_bug.cgi?id=341930#c30. It was written months ago, but nothing changed too dramatically from then. Even if the proper fix could break some apps, they all are *already in* transition process, Wayland is just around the corner with another transition process, so now it's the perfect time to fix something to make it finally working properly rather than make life easier for now and have this pain for years again and again. > > Now, *actually* closing windows to test interaction on session end is of > course just as wrong - if the user cancels the logout by such incident, we > should not have closed random other windows before (letting alone that it > causes this but) - therefore I frankly do not understand what's so > complicated about just faking a close event to serve the present "save your > stuff" pattern in a majority in clients without causing the destructive > close itself which may not only be a bit premature, but also triggers this > bug. > > It's the least invasive solution that does not require everyone to signal > "yes, i can sessionmanagement" (what's not
[Touch-packages] [Bug 1446865]
(In reply to Andreas Hartmetz from comment #29) > We cannot change Qt in a way that breaks existing applications. Qt5 has not > exactly just been released, and commercial customers value stability very > much. Some of them even pay for Qt licenses, which is good for all Qt users, > so really, we should not make things worse for them. The same way commercial customers or applications would be affected with API changes. I think, this issue (and fix) more concerns the environment than the application itself. -- You received this bug notification because you are a member of Ubuntu Touch seeded packages, which is subscribed to qtbase-opensource-src in Ubuntu. https://bugs.launchpad.net/bugs/1446865 Title: KDE5/Qt5 does not support session restoration Status in KDE Base Workspace: Confirmed Status in Qt: New Status in plasma-workspace package in Ubuntu: Confirmed Status in qtbase-opensource-src package in Ubuntu: Confirmed Bug description: KDE5/Qt5 does not support proper session restoration. ProblemType: Bug DistroRelease: Ubuntu 15.10 To manage notifications about this bug go to: https://bugs.launchpad.net/kdebase-workspace/+bug/1446865/+subscriptions -- Mailing list: https://launchpad.net/~touch-packages Post to : touch-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~touch-packages More help : https://help.launchpad.net/ListHelp
[Touch-packages] [Bug 1446865]
(In reply to Thomas Lübking from comment #24) > (In reply to Alexey Chernov from comment #23) > > > Comments like this clearly don't help > Seriously, you asked for breaking clients because that's what you'd "like" > to do - what did you expect to hear? That's simply not an acceptable stance. No one here presents any absolutely true point, otherwise there were no discussion. I just wrote my point of view and emphasized that it's just mine and not some ultimate truth. > > 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. As to your question, I'd prefer properly test the patch, including success scenario for the default not-aware-of-session- management application, and release it as soon as possible. > > Users were already hit when the significant part of functionality important > > for someone's every day use case is broken. > Let's be honest: session restorage is apparently relevant for only a > minority of users. According to what? Your assumption? It's not too evident for me, sorry. > 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? Don't you contradict with yourself in these two points? Anyway, it's very subjective and I wouldn't argue on what's more important. I agree that data loss is the worst thing which could happen. I just think it doesn't mean it should result in some messy API or library code when someone's relying on the undocumented side-effects. Just because it will surely lead to more bugs and more data loss in the future. It's just the 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. > Also there's absolutely NO reason why we should not care about both - except > that you'd "like" to break client code and risk data loss for some reason > that completely escapes me. 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. > > Still better than a couple of API methods like "enableSpecifiedBehaviour()" > > I fully agree on that proposal to be of little help - it will be mostly > ignored or used w/o accounting the implications. Ok. > > Once again: we all could already apply the fix of Andreas and be busy fixing > > the necessary applications rather than keep discussing here. > > It does NOT only affect KDE applications, there're hundreds of Qt > applications which might have adopted this pattern - or simply don't care > about session management itfp. > Also the proper order is to fix and roll out clients, *then* remove the > deprecated upstream code. That's why "=> Qt6" for this approach. 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. Since we have what we have, there's no other way than to start fixing it on both sides. I think nobody is against if it would be synchronized. > > On the Qt6 release you would say that everyone already rely on the > > workaround there was in Qt5 etc. etc. > > No. Because you would tell people during Qt5 don't do this and don't rely on > it because it's not gonna work with Qt6, so that when things are ported to > Qt6, client code has to be fixed. Oh, you're too optimistic here. Why it's still not fixed during porting on Qt5? Only because it just works. It won't be fixed until it's broken or would be planned to fix as we discussed above. > Breaking it now and depending client behavior on whether it's linked against > Qt 5.6 or Qt 5.7 is plain wrong and begging for trouble. That's again due to your assumption that session management is of lower priority. I'm pretty sure there would be packages that would require just most recent Qt version, and it would be acceptable. What's wrong in relying on changes in recent Qt release and informing the maintainer of it with more strict requirement? There're backports if someone is interested in special cases. > > I just kindly re
