(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 gonna happen) and we don't risk
> loosing the users data (or breaking the ability to cancel the logout)

In a couple of years nobody, including you, would remember what on earth are these close events coming when SM server asked to save state, what's the proper processing of it and how it's connected with the documentation and specs.

What is especially sad, is that nobody would understand the reason of preserving some stability in actually unstable environment, still being ported to the new framework. That's like having ugly workarounds now which were added to preserve stability of Plasma in KDE 4.0. It's nonsense.

To sum it up, I think, the most valueable thing here is that we seem to generally agree on what the proper fix is (especially with your comment #8). The only arguable point is whether it's safe to have it fixed now or should another (possible API-changing) workaround should be added instead.