Bug 346768

I have this problem too after my most recent update on Arch Linux a few hours ago, konsole  Version 15.04.0, Using KDE Frameworks 5.9.0.

I have the same issue, with the same configuration as Wolfram

Basically same as Wolfram, after upgrading from Kubuntu 14.10 to Kubuntu 15.04, konsole only starts with 1 tab, does not restore previously opened tabs.

Konsole Version 3.0.1
Using KDE Frameworks 5.9.0

*** Bug 347051 has been marked as a duplicate of this bug. ***

This is a repeat of the the same situation as with KDE3 -> KDE4 (bug #152761): konsole5 simply doesn't have any session management.

Cloned konsole git repo and started debugging the code.

* konsole does have session management code. It is the one I implemented  for KDE4 for bug #152761 (git commit ef3e43e11da8ea2777d70945a5111771481b24fd)
* the code is still working, an old KDE4 konsole session file copied from ~/.kde/share/session/konsole_* to ~/.config/session re-creates the windows & tabs correctly
* tested with "ksession -session <file>"
* tested with session login (make sure <file> is the name recorded in ~/.config/ksmserverrc for konsole)

So either the "session save" event never reaches konsole, or the session saving code fails. Will have to do some more debugging.

After some more debugging: I do not think the problem is in konsole, but somewhere deeper in the stack. When the user selects "Save Session" from the KMenu the save session code from konsole is called. But If I understand the debug output I added correctly, then the provided KConfig object is invalid and therefore will not be saved.

Example 1: konsole without -session

- start:
Session ID   "102262341b6148000143091725100000046460036"
Session Key  ""
-> there is no session information

- "Save Session" event:
SAVE Kconfig address   0xff1aa0
SAVE KConfig name      "session/konsole_102262341b6148000143091725100000046460036_1430917312_840058"
SAVE KConfig writable  NO
SAVE KConfig isDirty   YES
SAVE KConfig isDirty   NO

Corresponding information saved in ~/.config/ksmserverrc:
session,102262341b6148000143091725100000046460036_1430917312_840058

-> information looks correct, but KConfig isn't writable and not dirty. Therefore I assume that it isn't written to the disk after all.



Example 2: konsole -session 102262341b6148000143057827700000018470011_1430854163_934517

- start:
Session ID   "102262341b6148000143057827700000018470011"
Session Key  "1430854163_934517"
READ Kconfig address   0x259d270
READ KConfig name      "session/konsole_102262341b6148000143057827700000018470011_1430854163_934517"
READ KConfig writable  NO
READ KConfig isDirty   YES

-> there is session information, which is restored correctly

- after this session is asked to save itself???? (IMHO this shouldn't happen)
SAVE Kconfig address   0x259d270
SAVE KConfig name      "session/konsole_102262341b6148000143057827700000018470011_1430854163_934517"
SAVE KConfig writable  NO
SAVE KConfig isDirty   YES
SAVE KConfig isDirty   YES

-> this is the same KConfig object we read from

- "Save Session" event:
SAVE Kconfig address   0x259d270
SAVE KConfig name      "session/konsole_102262341b6148000143057827700000018470011_1430854163_934517"
SAVE KConfig writable  NO
SAVE KConfig isDirty   NO
SAVE KConfig isDirty   NO

-> it is still the same KConfig object. Again not writable

Corresponding information saved in ~/.config/ksmserverrc:
restartCommand10=src/konsole,-session,102262341b6148000143057827700000018470011_1430917601_970308

-> the session file that ksmserver expected to be written has a different name.


Conclusion: there is something severely wrong with "Saving Session", either for the whole KF5, or just for KF5kde4libs compatibility (which konsole is using).

for the record: stack backtrace when konsole "Save Session" code is reached:

#0  0x00007ffff7b01d8a in Konsole::SessionManager::saveSessions(KConfig*) (this=0x7ffff7db94c0 <(anonymous namespace)::Q_QGS_theSessionManager::innerFunction()::holder>, config=0x718b90)
    at /home/stefanb/git/konsole/src/SessionManager.cpp:272
#1  0x00007ffff7dd5391 in Konsole::MainWindow::saveGlobalProperties(KConfig*) (this=0x78c5e0, config=0x718b90)
    at /home/stefanb/git/konsole/src/MainWindow.cpp:610
#2  0x00007ffff644a3e1 in KMWSessionManager::saveState(QSessionManager&) ()
    at /lib64/libKF5XmlGui.so.5
#3  0x00007ffff649906d in KMWSessionManager::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) () at /lib64/libKF5XmlGui.so.5
#4  0x000000314bea7f66 in QMetaObject::activate(QObject*, int, int, void**) ()
    at /lib64/libQt5Core.so.5
#5  0x00007ffff2b77405 in QGuiApplication::saveStateRequest(QSessionManager&) () at /lib64/libQt5Gui.so.5
#6  0x00007ffff2b78e4b in QGuiApplicationPrivate::saveState() ()
    at /lib64/libQt5Gui.so.5
#7  0x00007fffe7db7f95 in sm_performSaveYourself(QXcbSessionManager*) ()
    at /usr/lib64/qt5/plugins/platforms/libqxcb.so
#8  0x00007fffe7db8234 in sm_saveYourselfCallback(_SmcConn*, void*, int, int, int, int) () at /usr/lib64/qt5/plugins/platforms/libqxcb.so

I guess the next points to investigate are
* KMWSessionManager::saveState(QSessionManager&) in libKF5XmlGui.so
* QGuiApplication::saveStateRequest(QSessionManager&) in libQt5Gui.so

Added some hack code to src/MainWindow.cpp:

 void MainWindow::saveGlobalProperties(KConfig* config)
 {
-    SessionManager::instance()->saveSessions(config);
+  //    SessionManager::instance()->saveSessions(config);
+  KConfig *new_config = config->copyTo("session/konsole_test");
+  SessionManager::instance()->saveSessions(new_config);
+  delete new_config;
 }

After "Save Session" a new file ~/.config/session/konsole_test is created with the correct content.

Conclusion: from konsole PoV Session Management is most likely implemented correctly. I'll have to look at other places for the problem.

FYI, I just noticed related bug #341930 , and reviewboard,
https://git.reviewboard.kde.org/r/123580/

(In reply to Rex Dieter from comment #10)

That bug is unrelated as far as I understand it

Created attachment 92457 [details]
hacky patch to fix session management handling in KMainWindow

kmxlgui/src/kmainwindow.cpp has:

bool KMWSessionManager::saveState(QSessionManager &)
{
    KConfig *config = KConfigGui::sessionConfig();
    ...
        KMainWindow::memberList().first()->saveGlobalProperties(config);

KConfigGui::sessionConfig() returns the session object of the application, i.e. the parameter to -session or a new config file name. Once that object is created it will *never* change.

This is wrong, because ksmserverrc stores the information that is in the QSessionManager.

The attached patch addresses:

- when KMWSessionManager::saveState() is called, create a new KConfig object based on the information from QSessionManager. (Hacky, duplicates code from KConfigGui for the file name)

- ignore the first save event after application has been started & restored with a valid -session parameter (hacky, we should find out why after application start a saveState event is generated)

- delete the old session file (MAJOR HACK, IMHO this should be in ksmserver)

With this patch in place:
- konsole saves the session correctly
- ksmserverrc records the correct config file name for konsole
- old session config files are removed

This bug should be moved to "frameworks-kxmlgui" product, but I don't have the rights for it

Re-assigned to frameworks-kxmlgui as requested, marking confirmed.

Created attachment 92500 [details]
kconfig: Add 2nd variant for KConfigGui::sessionConfig()

Created attachment 92501 [details]
kxmlgffff

Created attachment 92502 [details]
kxmlgui: Forward QSessionManager to KConfigGui

Cleaned up version of the initial patch with hacks removed.

Remaining problems:
- superfluous saveState signal is send to the application after startup. This will create an additional session file [KConfig unfortunately doesn't offer an interface to delete a configuration file]
- when user selects "Save Session" (and probably also when leaving the session with "restore previous session" setting on) the old obsolete session files are not deleted [responsibility of ksmserver??]

With these two patches merged session management will start working, but ~/.config/session will start to fill up with files.

Created attachment 92509 [details]
kconfig: Add 2nd variant for KConfigGui::sessionConfig()

When application receives a saveState signal it needs to get a KConfig object based on the QSessionManager information. Add a new variant that accepts the session manager object.

Created attachment 92510 [details]
kxmlgui: Forward QSessionManager to KConfigGui

Get a KConfig object that corresponds to the QSessionManager object that is passed in with the saveStateRequest signal.
    
To make sure that the data is stored to disk we have to call sync() on the object after the application has updated it. As the session manager always emits a saveStateRequest after starting the application we need to skip the sync() for the first request.

At least on X11 the session manager always emits a saveStateRequest after starting an application (also hinted at by the Qt documentation). Updated the kxmlgui patch to skip sync() for this initial request. Also call markAsClean() in kconfig before deleting the old KConfig object to avoid syncing it to the disk.

With these patches session management works (tested with konsole and kwrite) and session files are only generated when requested (e.g. "Save Session..."). Now the only problem left is that ksmserver doesn't delete the old obsolete session files after the new ones are written to disk.

@powers to be: how to proceed from here? Will you initiate a code review or is there something I should do?

Submitted patches for review:

https://git.reviewboard.kde.org/r/123705/
https://git.reviewboard.kde.org/r/123706/

The reason why old session files are not deleted is because no discardCommands are generated for them. I.e. in KDE4:

$ fgrep =rm, ~/.kde/share/config/ksmserverrc 
discardCommand1[$e]=rm,$HOME/.kde/share/config/session/kwin_101d71a91dc133000130736546600000024040000_1315318719_387676
discardCommand14[$e]=rm,$HOME/.kde/share/config/session/kmix_101d71a91dc133000130736547800000024040010_1315318719_305494
discardCommand15[$e]=rm,$HOME/.kde/share/config/session/konsole_101d71a91dc133000130738602700000127110028_1315318719_305520
discardCommand20[$e]=rm,$HOME/.kde/share/config/session/printer-applet_102262341b6148000131531845600000016760025_1315318719_305907

and in Plasma5:

$ fgrep =rm, ~/.config/ksmserverrc 
$

I'm guessing KConfigGui should add a discardCommand?

Created attachment 92511 [details]
kconfig: Set session manager discard command

If KConfig object provides a non-empty file path then set the session manager discard command to "rm <file path>". This makes sure that ksmserver removes old obsolete session files after storing a new
session.

Submitted 3rd patch for review:

https://git.reviewboard.kde.org/r/123707/

Hi,

This looks like a partial dup of this bug that I opened last night:

https://bugzilla.redhat.com/show_bug.cgi?id=1220060

xosview also doesn't remember ANYTHING about it's status which is supposed to be saved by the "save-session" process I think. It's unknown how many other apps have a similar problem.

George...

Git commit 9978cfd5ccd18509dd514b3a7ada8c158c209de1 by Rex Dieter, on behalf of Stefan Becker.
Committed on 11/05/2015 at 20:37.
Pushed by rdieter into branch 'master'.

Add KConfigGui::setSessionConfig()

When the application receives a saveState signal it needs to replace the
current KConfig object with a new one based on the QSessionManager
information. Add a new interface that accepts the new session id and key.
REVIEW: 123705

M  +32   -9    src/gui/kconfiggui.cpp
M  +15   -2    src/gui/kconfiggui.h

http://commits.kde.org/kconfig/9978cfd5ccd18509dd514b3a7ada8c158c209de1

Git commit fa86f6e4afd4a4f32d297f271d3daececc6c0ba4 by Rex Dieter, on behalf of Stefan Becker.
Committed on 11/05/2015 at 20:38.
Pushed by rdieter into branch 'master'.

Add session management for KMainWindow

- replace the KConfig object in KConfigGui with a new one using the
  session id and key from the QSessionManager object
- make sure that the data is stored after the application has update it
- if the KConfig object is represented by a local file then add a
  discard command to the session manager object
REVIEW: 123706

M  +16   -1    src/kmainwindow.cpp

http://commits.kde.org/kxmlgui/fa86f6e4afd4a4f32d297f271d3daececc6c0ba4

Just to confirm: F22 updated to kf5 5.10.0 and pulled the session management fixes in:

* kf5-kconfig-gui-5.10.0-3.fc22.x86_64
* kf5-kxmlgui-5.10.0-2.fc22.x86_64

KF5 based applications, like konsole or kwrite, now dump their session information to disk on "Save Session" or "Restore previous session", and the windows & tabs are restored after login.

This bug is not fixed.  My system has
        kf5-kxmlgui-5.10.0-2.fc22.x86_64
        kf5-kconfig-gui-5.10.0-3.fc22.x86_64
and is showing the same behavior (only one tab shows up after logging in again).

For what it's worth, the option to manually save sessions is also not working correctly.  Using that option, all of the previously open tabs appear after login, but all appear on the first desktop instead of the desktop they were on before logout.

The latter location issue is different (and more about kwin, tracked elsewhere)

(In reply to Rex Dieter from comment #30)
> The latter location issue is different (and more about kwin, tracked
> elsewhere)

My apologies.  I can't reproduce what I reported.  Today, when I have multiple windows open, they all reappear after logging back in.  The window locations are not preserved, but that is a separate issue, as you pointed out.

Could this issue/bug be re-opened? I am running Qt-5.5.0, plasma-5.4.1, frameworks-5.14.0, and applications-15.08.1 and konsole windows are essentially never restored (perhaps once out of 20 logins). The konsole session file (~/.config/session/konsole_*) is always written, but they are never deleted (so the session directory fills with konsole_* files), and ~/.config/ksmserverrc has no konsole entries.

It appears that frameworks-5.14.0 includes the above patches (in a somewhat modified form), and I have patched qtbase-opensource-src-5.5.0 with both QTBUG-46310 and QTBUG-45484 patches.

I can confirm, that this bug still is not fixed. I'm using Kubuntu 15.04 on my home-pc with continuous backport ppa. I'm getting kde-updates every day. The actual state is, that BUG 341930 seems to be resolved, but with konsole it's like a random generator as Jon Stanley reported. Sometimes it works, with all tabs restored, but the window at the wrong position (after a long waiting time), sometimes no konsole at all. In that case is's gone forever and must be rebuild manually.
If this isn't fixed in next Kubuntu-release that will come in October, I will stick to 14.10 ( Plasma 4) on my office-machines, forever loosing the chance to upgrade.

Wolfram,

To add to the confusion. I'm on a Fedora 22 x86_64 system. I think I had a duplicate bug to this one, certainly the behavior sounds VERY similar. Shortly after my report, the problem seems to have gone away and I have not seen it since then.

I DO see similar problems though. It's like an update comes along and you install it but the "bad" or "incorrect" config entries live on and on. For a while I was seeing several other apps appearing on startup even though I had ONLY  xosview and 3 konsoles active AND "task bar autohide" could NOT be turned off (argh). The extras were from some sound(?) app control panel and several instances of xosview. I installed and configured a Fedora 23 system with it's KDE config... all worked well. I saved my old config and merged the Fedora 23 KDE configs from .kde and .gtk'*'  to my Fedora 22 configs... Most of the problems went away.

I still have the old KDE configs in case anyone is interested.

George...

Hi George,

to eliminate problems caused by old config entries, I installed a fresh Kubuntu 15.04 virtual machine and to get the newest kde-code, installed continuous integration ppa on it.
The result is, that bug 341930 seems to be fixed for many applications, except konsole. If it is restored, the tabs are restored, but not the last position and size of the window. Sometimes it is'n restored at all!
What is so difficult with konsole ?

Wolfram...

I just installed a new VMware-machine from Kubuntu 15.10 beta 2 image.
Same problems with Konsole here, still not fixed !

(In reply to Wolfram from comment #36)
> I just installed a new VMware-machine from Kubuntu 15.10 beta 2 image.
> Same problems with Konsole here, still not fixed !

Yust have to add, that the problem has changed in the way, that now the Tabs exist (if the window is reopened) but position, size and  virtual desktop are not restored correctly. Sometimes the window isn't restored at all. In KDE 4 this is working as it should.

After working with the Final Kubuntu 15.10 for more than one week, I can report, that the session-problems still are there. The tabs are stored correctly, if the Konsole-window appears, but this rarely happens.
So I suppose this bug-report can be closed, and the session-problems are related to bug 341930.
As I understand, the problems are caused by a bug in Qt5 and we have to wait for a solution.

Created attachment 95380 [details]
Add commitData() slot to KMainWindow

Created attachment 95381 [details]
Handle non-Shutdown/Checkpoint Qt saveStateRequest() signals

The fixes in https://bugs.kde.org/show_bug.cgi?id=341930 are in Qt-5.5.0+, and for me, have no effect on the session management problem.  However, I believe I have a fix for this using the three patches:
(1) https://bugs.kde.org/attachment.cgi?id=95380
(2) https://bugs.kde.org/attachment.cgi?id=95381
(3) https://bugreports.qt.io/browse/QTBUG-49241,https://codereview.qt-project.org/#/c/140115/

I'd be great if anyone able to test these could try them, and perhaps expedite the Qt patch.
thanks to all.

(In reply to John Stanley from comment #41)
> The fixes in https://bugs.kde.org/show_bug.cgi?id=341930 are in Qt-5.5.0+,
> and for me, have no effect on the session management problem.  However, I
> believe I have a fix for this using the three patches:
> (1) https://bugs.kde.org/attachment.cgi?id=95380
> (2) https://bugs.kde.org/attachment.cgi?id=95381
> (3)
> https://bugreports.qt.io/browse/QTBUG-49241,https://codereview.qt-project.
> org/#/c/140115/
> 
> I'd be great if anyone able to test these could try them, and perhaps
> expedite the Qt patch.
> thanks to all.

I would like to test it. Can you give me a short hint how to accomplish. Do I have to install the whole KDE-development environment ?

By the way: your third attachment gives the error message "Issue Does Not Exist".

Sorry about the the links in Comment 41, (3); there are two links there, separated by a comma:
  https://bugreports.qt.io/browse/QTBUG-49241
  https://codereview.qt-project.org/#/c/140115/

As far as building, you need to rebuild three packages:
  qtbase-5.5.1 ( http://download.qt.io/official_releases/qt/5.5/5.5.1/submodules/qtbase-opensource-src-5.5.1.tar.xz )
  kxmlgui-5.15.0 ( http://download.kde.org/stable/frameworks/5.15/kxmlgui-5.15.0.tar.xz )
  plasma-workspace-5.4.2 ( http://download.kde.org/stable/plasma/5.4.2/plasma-workspace-5.4.2.tar.xz )
Assuming you already have these installed, the build order is immaterial, otherwise build in the order shown.

(In reply to Wolfram from comment #42)
> (In reply to John Stanley from comment #41)
> > The fixes in https://bugs.kde.org/show_bug.cgi?id=341930 are in Qt-5.5.0+,
> > and for me, have no effect on the session management problem.  However, I
> > believe I have a fix for this using the three patches:
> > (1) https://bugs.kde.org/attachment.cgi?id=95380
> > (2) https://bugs.kde.org/attachment.cgi?id=95381
> > (3)
> > https://bugreports.qt.io/browse/QTBUG-49241,https://codereview.qt-project.
> > org/#/c/140115/
> > 
> > I'd be great if anyone able to test these could try them, and perhaps
> > expedite the Qt patch.
> > thanks to all.
> 
> I would like to test it. Can you give me a short hint how to accomplish. Do
> I have to install the whole KDE-development environment ?
>
Yes, more or less. I build all packages for systems, and I keep build trees around for the time-consuming (and issue-laden) builds. I also build only a subset of the Qt-split packages. Kxmlgui is a quick rebuild, and rebuilding from the plasma-workspace and qtbase build trees takes only a few minutes. If you have the KF5/Qt5 development packages installed, then you just have to rebuild qtbase, kxmlgui, and plasma-workspace, restoring the previous versions afterwards if needed.

> By the way: your third attachment gives the error message "Issue Does Not
> Exist".

Thank you John,
I was too blind to see the two links. I will start testing next weekend (hopefully, never built kde before).

(In reply to John Stanley from comment #43)
> Sorry about the the links in Comment 41, (3); there are two links there,
> separated by a comma:
>   https://bugreports.qt.io/browse/QTBUG-49241
>   https://codereview.qt-project.org/#/c/140115/
> 
> As far as building, you need to rebuild three packages:
>   qtbase-5.5.1 (
> http://download.qt.io/official_releases/qt/5.5/5.5.1/submodules/qtbase-
> opensource-src-5.5.1.tar.xz )
>   kxmlgui-5.15.0 (
> http://download.kde.org/stable/frameworks/5.15/kxmlgui-5.15.0.tar.xz )
>   plasma-workspace-5.4.2 (
> http://download.kde.org/stable/plasma/5.4.2/plasma-workspace-5.4.2.tar.xz )
> Assuming you already have these installed, the build order is immaterial,
> otherwise build in the order shown.

are the patches included in the tarballs ?

(In reply to Wolfram from comment #46)
> (In reply to John Stanley from comment #43)
> > Sorry about the the links in Comment 41, (3); there are two links there,
> > separated by a comma:
> >   https://bugreports.qt.io/browse/QTBUG-49241
> >   https://codereview.qt-project.org/#/c/140115/
> > 
> > As far as building, you need to rebuild three packages:
> >   qtbase-5.5.1 (
> > http://download.qt.io/official_releases/qt/5.5/5.5.1/submodules/qtbase-
> > opensource-src-5.5.1.tar.xz )
> >   kxmlgui-5.15.0 (
> > http://download.kde.org/stable/frameworks/5.15/kxmlgui-5.15.0.tar.xz )
> >   plasma-workspace-5.4.2 (
> > http://download.kde.org/stable/plasma/5.4.2/plasma-workspace-5.4.2.tar.xz )
> > Assuming you already have these installed, the build order is immaterial,
> > otherwise build in the order shown.
> 
> are the patches included in the tarballs ?

The 3 patches, three separate files, one for each package/tarball, are given in Comment 44; they have to be applied before building/rebuilding each package.

Here the result of my struggle with the build-process (I followed this description: <https://community.kde.org/Frameworks/Building>):

- managed to build qt5
- managed to build 51 of 254 kde-modules
- error message in all unsuccessful builds : "Could not find a package configuration file provided by"

I'm shortly before giving up.

FYI, I also hit this bug on upgrade to Fedora 23 -- and after trying F23 Gnome, Cinnamon, etc, decided to switch to openSUSE (LEAP 4.21 + updates), with KDE, but konsole still fails to restore across sessions. :-(

I tried installing gnome-terminal, but that also fails to save-and-restore.

I then installed xfce4-terminal which *does* save-and-restore perfectly well, afaict, -- even though I'm running it on a KDE desktop.

(In reply to John Stanley from comment #47)
> (In reply to Wolfram from comment #46)
> > (In reply to John Stanley from comment #43)
> > > Sorry about the the links in Comment 41, (3); there are two links there,
> > > separated by a comma:
> > >   https://bugreports.qt.io/browse/QTBUG-49241
> > >   https://codereview.qt-project.org/#/c/140115/
> > > 
> > > As far as building, you need to rebuild three packages:
> > >   qtbase-5.5.1 (
> > > http://download.qt.io/official_releases/qt/5.5/5.5.1/submodules/qtbase-
> > > opensource-src-5.5.1.tar.xz )
> > >   kxmlgui-5.15.0 (
> > > http://download.kde.org/stable/frameworks/5.15/kxmlgui-5.15.0.tar.xz )
> > >   plasma-workspace-5.4.2 (
> > > http://download.kde.org/stable/plasma/5.4.2/plasma-workspace-5.4.2.tar.xz )
> > > Assuming you already have these installed, the build order is immaterial,
> > > otherwise build in the order shown.
> > 
> > are the patches included in the tarballs ?
> 
> The 3 patches, three separate files, one for each package/tarball, are given
> in Comment 44; they have to be applied before building/rebuilding each
> package.

Is your patch implemented in KDE by now ?

Because I was not able to compile the environment until now (see Comment 48), I just installed Kubuntu 15.10 in a virtual machine with 'kubuntu-ppa/staging-plasma' activated. There they use the newest plasma version 5.5.3. As result I can say, that the the session management problems still exist in that version.

(In reply to Wolfram from comment #50)
> (In reply to John Stanley from comment #47)
> > (In reply to Wolfram from comment #46)
> > > (In reply to John Stanley from comment #43)
> > > > Sorry about the the links in Comment 41, (3); there are two links there,
> > > > separated by a comma:
> > > >   https://bugreports.qt.io/browse/QTBUG-49241
> > > >   https://codereview.qt-project.org/#/c/140115/
> > > > 
> > > > As far as building, you need to rebuild three packages:
> > > >   qtbase-5.5.1 (
> > > > http://download.qt.io/official_releases/qt/5.5/5.5.1/submodules/qtbase-
> > > > opensource-src-5.5.1.tar.xz )
> > > >   kxmlgui-5.15.0 (
> > > > http://download.kde.org/stable/frameworks/5.15/kxmlgui-5.15.0.tar.xz )
> > > >   plasma-workspace-5.4.2 (
> > > > http://download.kde.org/stable/plasma/5.4.2/plasma-workspace-5.4.2.tar.xz )
> > > > Assuming you already have these installed, the build order is immaterial,
> > > > otherwise build in the order shown.
> > > 
> > > are the patches included in the tarballs ?
> > 
> > The 3 patches, three separate files, one for each package/tarball, are given
> > in Comment 44; they have to be applied before building/rebuilding each
> > package.
> 
> Is your patch implemented in KDE by now ?
> 
> Because I was not able to compile the environment until now (see Comment
> 48), I just installed Kubuntu 15.10 in a virtual machine with
> 'kubuntu-ppa/staging-plasma' activated. There they use the newest plasma
> version 5.5.3. As result I can say, that the the session management problems
> still exist in that version.

Sorry, for the delay. I've pretty much given up on both KDE and Qt regarding this, no movement at all, no one appears to care -- so, no,  the patches are not in the latest KDE (5.5.3/5.18.0). It'd be nice, at least, if someone would explain why there's no interest..

Hi,
John/everyone,

I have been thinking about this situation.

I have multiple Fedora systems (F22/23/24). Generally they seem to be working ok now... more or less, including the Konsole and tabs problem. 

As I visualize it, the developers add config items, remove config items, change config items as they develop. Anything they don't hit directly would tend to remain. After a while, the config(s) would tend to have stuff that's no longer needed, wanted, or changed, or doesn't work any more as a result. I would like to suggest some form of migration of config(s) be performed by the developers in the install scripts (think rpmsave) to help eliminate the unused "stuff". For example, my fedora 22 system with KDE, starts the apps that I have running at the time of the save but it also starts things that ARE NOT REFERENCED by the save of configuration process AND/OR are NOT even running... so they remain. I don't know enough of the config process to isolate where the extra apps/config items are located. I don't want to remove everything and re-config.

There's a kde-testing repo for Fedora systems but whoever builds/maintains needs to rebuild the repo index or other files AFTER update so the repodata can be syncronized. I wrote a bug on this...

Have you tried removing .config and .kde/share/apps/ ... Like what would happen for a brand new user?

Good luck,

George...

(In reply to George R. Goffe from comment #52)
> Hi,
> John/everyone,
> 
> I have been thinking about this situation.
> 
> I have multiple Fedora systems (F22/23/24). Generally they seem to be
> working ok now... more or less, including the Konsole and tabs problem. 
> 
> As I visualize it, the developers add config items, remove config items,
> change config items as they develop. Anything they don't hit directly would
> tend to remain. After a while, the config(s) would tend to have stuff that's
> no longer needed, wanted, or changed, or doesn't work any more as a result.
> I would like to suggest some form of migration of config(s) be performed by
> the developers in the install scripts (think rpmsave) to help eliminate the
> unused "stuff". For example, my fedora 22 system with KDE, starts the apps
> that I have running at the time of the save but it also starts things that
> ARE NOT REFERENCED by the save of configuration process AND/OR are NOT even
> running... so they remain. I don't know enough of the config process to
> isolate where the extra apps/config items are located. I don't want to
> remove everything and re-config.
> 
> There's a kde-testing repo for Fedora systems but whoever builds/maintains
> needs to rebuild the repo index or other files AFTER update so the repodata
> can be syncronized. I wrote a bug on this...
> 
> Have you tried removing .config and .kde/share/apps/ ... Like what would
> happen for a brand new user?
> 
> Good luck,
> 
> George...
Actually, during testing the patches I posted I removed .config/session + .config/ksmserverrc frequently. More generally, after several frameworks and/or plasma5 updates I had graphics/opengl artifacts and removing .config entirely fixed the problems. For the latest update (plasma-5.5.3/frameworks-5.18.0), no issues whatsoever, so an improvement.

I'm curious, what version of konsole do have installed?

(In reply to John Stanley from comment #53)
> (In reply to George R. Goffe from comment #52)
> > Hi,
> > John/everyone,
> > 
> > I have been thinking about this situation.
> > 
> > I have multiple Fedora systems (F22/23/24). Generally they seem to be
> > working ok now... more or less, including the Konsole and tabs problem. 
> > 
> > As I visualize it, the developers add config items, remove config items,
> > change config items as they develop. Anything they don't hit directly would
> > tend to remain. After a while, the config(s) would tend to have stuff that's
> > no longer needed, wanted, or changed, or doesn't work any more as a result.
> > I would like to suggest some form of migration of config(s) be performed by
> > the developers in the install scripts (think rpmsave) to help eliminate the
> > unused "stuff". For example, my fedora 22 system with KDE, starts the apps
> > that I have running at the time of the save but it also starts things that
> > ARE NOT REFERENCED by the save of configuration process AND/OR are NOT even
> > running... so they remain. I don't know enough of the config process to
> > isolate where the extra apps/config items are located. I don't want to
> > remove everything and re-config.
> > 
> > There's a kde-testing repo for Fedora systems but whoever builds/maintains
> > needs to rebuild the repo index or other files AFTER update so the repodata
> > can be syncronized. I wrote a bug on this...
> > 
> > Have you tried removing .config and .kde/share/apps/ ... Like what would
> > happen for a brand new user?
> > 
> > Good luck,
> > 
> > George...
> Actually, during testing the patches I posted I removed .config/session +
> .config/ksmserverrc frequently. More generally, after several frameworks
> and/or plasma5 updates I had graphics/opengl artifacts and removing .config
> entirely fixed the problems. For the latest update
> (plasma-5.5.3/frameworks-5.18.0), no issues whatsoever, so an improvement.
> 
> I'm curious, what version of konsole do have installed?

Since yesterday I have plasma-5.5.3/frameworks-5.18.0 running on my Kubuntu 15.10 wokplace-system via backports-ppa. The session-problems still exist, so I tried your suggestion regarding .config in the following way: Logged out, renamed .config to .config_save in a konsole-session and logged in again. The result was, that I was not able to get my two screens running in kde anymore. Luckily I had saved my old .config and could switch back. Then I tried it with a new user. Here the two screens worked, but the session problem still exist.

Maybe my environment is kind of strange: Two screens, folder-view on one screen, desktop-view on the other, four virtual desktops with different sessions running in each of them. But all this worked on in kde4 with no problems !

Since the forced change to kde5 in kubuntu (I promise to stick to LTS-release next time), my daily workaround is this: starting a new konsole window (and the tabs i need) if missing, if the positions of the desktop icons are messed up, copying  ~/.config/plasma-org.kde.plasma.desktop-appletsrc.save to ~/.config/plasma-org.kde.plasma.desktop-appletsrc, then logging off and back in. If the konsole window appears, the tabs exist (this seems to be fixed), if not, I have to start a new konsole window again. Sometimes I also have to spread the the sessions back to the virtual desktops where they belong. It is frustrating !

(In reply to John Stanley from comment #51)
> (In reply to Wolfram from comment #50)
> > (In reply to John Stanley from comment #47)
> > > (In reply to Wolfram from comment #46)
> > > > (In reply to John Stanley from comment #43)
> > > > > Sorry about the the links in Comment 41, (3); there are two links there,
> > > > > separated by a comma:
> > > > >   https://bugreports.qt.io/browse/QTBUG-49241
> > > > >   https://codereview.qt-project.org/#/c/140115/
> > > > > 
> > > > > As far as building, you need to rebuild three packages:
> > > > >   qtbase-5.5.1 (
> > > > > http://download.qt.io/official_releases/qt/5.5/5.5.1/submodules/qtbase-
> > > > > opensource-src-5.5.1.tar.xz )
> > > > >   kxmlgui-5.15.0 (
> > > > > http://download.kde.org/stable/frameworks/5.15/kxmlgui-5.15.0.tar.xz )
> > > > >   plasma-workspace-5.4.2 (
> > > > > http://download.kde.org/stable/plasma/5.4.2/plasma-workspace-5.4.2.tar.xz )
> > > > > Assuming you already have these installed, the build order is immaterial,
> > > > > otherwise build in the order shown.
> > > > 
> > > > are the patches included in the tarballs ?
> > > 
> > > The 3 patches, three separate files, one for each package/tarball, are given
> > > in Comment 44; they have to be applied before building/rebuilding each
> > > package.
> > 
> > Is your patch implemented in KDE by now ?
> > 
> > Because I was not able to compile the environment until now (see Comment
> > 48), I just installed Kubuntu 15.10 in a virtual machine with
> > 'kubuntu-ppa/staging-plasma' activated. There they use the newest plasma
> > version 5.5.3. As result I can say, that the the session management problems
> > still exist in that version.
> 
> Sorry, for the delay. I've pretty much given up on both KDE and Qt regarding
> this, no movement at all, no one appears to care -- so, no,  the patches are
> not in the latest KDE (5.5.3/5.18.0). It'd be nice, at least, if someone
> would explain why there's no interest..

I think it's partially due to some bugs were closed as duplicate of bug #341930 which continued as bug #354724, there've happened to be several discussions recently, with somewhat contradictory approach with which I personally disagree. I tend to fully support you review request to Qt, set +1 there, and you KDE patches look good as well, it's a shame I just came accross these reports, just after you posted a link in Gerrit. I've got all the KDE stuff compiled from sources, so it's no problem to try and test you patches which I plan to do right away and give them some time of every day work.

Just one question: have you posted a review request with these patches on KDE Reviewboard? I'm not sure if I have enough rights to vote there in case patch testing is OK, but still there's better chance to have your patches merged with the review request there.

Anyway, thank you for your efforts and apparently right fixes of this whole problem.

I've just applied and quite intensively tested two patches John attached.

At first for patches: I've actually looked through them before applying, and there're a couple of questions:

1. I've modified kxmlgui patch a little — at first I've replaces close event stuff for calling queryClose() method. Following XSM protocol one is not allowed to close windows on the saving state as a) this is not necessary shutdown that save-yourself is triggered , it is explicitly stated in the documentation, and b) shutdown, if any, could normally be cancelled and session would continue, and closing some windows would just spoil this user's session.

Another thing I removed is restore() method which is obviously the same as before, but always returns false. Is there any reason for it?

2. KSMServer patch is what I first have tried not to apply at all. Except the first edit, which seems to be right as it's just follows the comment, other stuff seems to be redundant. Could you please explain you ideas behind these changes? Anyway, everything works even without it (see below).

As for testing, I've tested the following use cases:

1. Plain saving session. It's generally works even without any patches (except Qt patch https://codereview.qt-project.org/#/c/140115 I've applied much earlier). Works fine.

2. Saving session with several "modified" clients — in my case it was KWrite with unsaved document and Konsole with "vim" running in one of the tabs. On logging out I was gently and sequencially asked for both KWrite and Konsole. Saved nothing and logged out. Works fine.

3. Triggering session save with several "modified" clients and cancel logging out in one of them. In my case they were KWrite again with an unsaved document and Kate with an unsaved document. Here I tried two sub-clauses:

    a) at first, say "don't save" to KWrite and "Cancel" to Kate. Session continued successfully, no data is lost in both programs. Just KWin moved both of them to "Any virtual desktop" — it's apparently the default behaviour, appropriate for now. So works fine;

    b) finally, say "save" to KWrite and "don't save" to Kate. Session finished and then restored successfully except Kate crashed, which is apparently separate bug in Kate, which I will address, too. In terms of SM works fine.

As a conclusion, I think, this patch can be candidate for merge right now without even waiting for Qt patch — there's no harm in saving documents beforehand, but, of course, without Qt changes of this report: https://codereview.qt-project.org/#/c/140115 applications won't be restarted.

I'm attaching the patch with my modification. If there's no objection, John, I'd suggest you as original author to post it to https://reviewboard.kde.org so that we can have it reviewed and potentially merged as soon as possible.

Created attachment 96997 [details]
KXmlGui patch with my modifications

- sending close event was replaced with calling queryClose() as the earlier violates XSMP (http://www.x.org/releases/X11R7.6/doc/libSM/xsmp.html)
- changes in restore() method were removed

(In reply to Alexey Chernov from comment #55)
> (In reply to John Stanley from comment #51)
> > (In reply to Wolfram from comment #50)
> > > (In reply to John Stanley from comment #47)
> > > > (In reply to Wolfram from comment #46)
> > > > > (In reply to John Stanley from comment #43)
> > > > > > Sorry about the the links in Comment 41, (3); there are two links there,
> > > > > > separated by a comma:
> > > > > >   https://bugreports.qt.io/browse/QTBUG-49241
> > > > > >   https://codereview.qt-project.org/#/c/140115/
> > > > > > 
> > > > > > As far as building, you need to rebuild three packages:
> > > > > >   qtbase-5.5.1 (
> > > > > > http://download.qt.io/official_releases/qt/5.5/5.5.1/submodules/qtbase-
> > > > > > opensource-src-5.5.1.tar.xz )
> > > > > >   kxmlgui-5.15.0 (
> > > > > > http://download.kde.org/stable/frameworks/5.15/kxmlgui-5.15.0.tar.xz )
> > > > > >   plasma-workspace-5.4.2 (
> > > > > > http://download.kde.org/stable/plasma/5.4.2/plasma-workspace-5.4.2.tar.xz )
> > > > > > Assuming you already have these installed, the build order is immaterial,
> > > > > > otherwise build in the order shown.
> > > > > 
> > > > > are the patches included in the tarballs ?
> > > > 
> > > > The 3 patches, three separate files, one for each package/tarball, are given
> > > > in Comment 44; they have to be applied before building/rebuilding each
> > > > package.
> > > 
> > > Is your patch implemented in KDE by now ?
> > > 
> > > Because I was not able to compile the environment until now (see Comment
> > > 48), I just installed Kubuntu 15.10 in a virtual machine with
> > > 'kubuntu-ppa/staging-plasma' activated. There they use the newest plasma
> > > version 5.5.3. As result I can say, that the the session management problems
> > > still exist in that version.
> > 
> > Sorry, for the delay. I've pretty much given up on both KDE and Qt regarding
> > this, no movement at all, no one appears to care -- so, no,  the patches are
> > not in the latest KDE (5.5.3/5.18.0). It'd be nice, at least, if someone
> > would explain why there's no interest..
> 
> I think it's partially due to some bugs were closed as duplicate of bug
> #341930 which continued as bug #354724, there've happened to be several
> discussions recently, with somewhat contradictory approach with which I
> personally disagree. I tend to fully support you review request to Qt, set
> +1 there, and you KDE patches look good as well, it's a shame I just came
> accross these reports, just after you posted a link in Gerrit. I've got all
> the KDE stuff compiled from sources, so it's no problem to try and test you
> patches which I plan to do right away and give them some time of every day
> work.
> 
> Just one question: have you posted a review request with these patches on
> KDE Reviewboard? I'm not sure if I have enough rights to vote there in case
> patch testing is OK, but still there's better chance to have your patches
> merged with the review request there.
> 
First, thanks so much for looking at this issue.
No I haven't posted to the  KDE Reviewboard ; give me a day or two max to orient myself, and I'll do so.
 
> Anyway, thank you for your efforts and apparently right fixes of this whole
> problem.

(In reply to John Stanley from comment #58)
> First, thanks so much for looking at this issue.
> No I haven't posted to the  KDE Reviewboard ; give me a day or two max to
> orient myself, and I'll do so.

Yes, no problem. Thank you.

(In reply to Alexey Chernov from comment #56)
> I've just applied and quite intensively tested two patches John attached.
> 
> At first for patches: I've actually looked through them before applying, and
> there're a couple of questions:
> 
> 1. I've modified kxmlgui patch a little — at first I've replaces close event
> stuff for calling queryClose() method. Following XSM protocol one is not
> allowed to close windows on the saving state as a) this is not necessary
> shutdown that save-yourself is triggered , it is explicitly stated in the
> documentation, and b) shutdown, if any, could normally be cancelled and
> session would continue, and closing some windows would just spoil this
> user's session.
Agreed. But, the close event here is actually "fake", no windows are actually closed. In tracing queryClose(), it seemed appropriate to hook KMainWindow::closeEvent() in KMainWindow::commitData() to pick up the "auto save" functionality in KMainWindow::closeEvent(). KMainWindow::closeEvent() potentially does some KMainWindow-specific auto saving, and then simply calls queryClose().
Anyway, I'm happy to go with your call on this, but have a look at KMainWindow::closeEvent() if you haven't already.
> 
> Another thing I removed is restore() method which is obviously the same as
> before, but always returns false. Is there any reason for it?
>
The  KMainWindow::restore() currently always returns false, which is not useful for apps that choose to iterate over windows for multi-window restorations (using a false return code terminate iteration). My change is to return true on successful restoration, and false otherwise.

> 2. KSMServer patch is what I first have tried not to apply at all. Except
> the first edit, which seems to be right as it's just follows the comment,
> other stuff seems to be redundant. Could you please explain you ideas behind
> these changes? Anyway, everything works even without it (see below).
> 
Yup, the first edit is the only important one. The others, well, I was tinkering a bit, and thinking of doing more, but its a bit time-consuming; I should have removed them before posting. The final two edits are merely code restructuring, no functional changes, should've removed 'em. The remaining edits, associated with "Phase 2 for non-WM clients", are an attempt to allow non-WM apps which want to do their shutdown processing in Phase 2 to do so before the WM is shutdown (also in Phase 2). I don't specifically know of any such apps, but, in principle there could be. 

> As for testing, I've tested the following use cases:
> 
> 1. Plain saving session. It's generally works even without any patches
> (except Qt patch https://codereview.qt-project.org/#/c/140115 I've applied
> much earlier). Works fine.
> 
> 2. Saving session with several "modified" clients — in my case it was KWrite
> with unsaved document and Konsole with "vim" running in one of the tabs. On
> logging out I was gently and sequencially asked for both KWrite and Konsole.
> Saved nothing and logged out. Works fine.
> 
> 3. Triggering session save with several "modified" clients and cancel
> logging out in one of them. In my case they were KWrite again with an
> unsaved document and Kate with an unsaved document. Here I tried two
> sub-clauses:
> 
>     a) at first, say "don't save" to KWrite and "Cancel" to Kate. Session
> continued successfully, no data is lost in both programs. Just KWin moved
> both of them to "Any virtual desktop" — it's apparently the default
> behaviour, appropriate for now. So works fine;
> 
>     b) finally, say "save" to KWrite and "don't save" to Kate. Session
> finished and then restored successfully except Kate crashed, which is
> apparently separate bug in Kate, which I will address, too. In terms of SM
> works fine.
> 
> As a conclusion, I think, this patch can be candidate for merge right now
> without even waiting for Qt patch — there's no harm in saving documents
> beforehand, but, of course, without Qt changes of this report:
> https://codereview.qt-project.org/#/c/140115 applications won't be restarted.
> 
> I'm attaching the patch with my modification. If there's no objection, John,
> I'd suggest you as original author to post it to https://reviewboard.kde.org
> so that we can have it reviewed and potentially merged as soon as possible.

Thanks again for the help and interest and I'll get on the https://reviewboard.kde.org post asap.

(In reply to John Stanley from comment #60)
> (In reply to Alexey Chernov from comment #56)
> > I've just applied and quite intensively tested two patches John attached.
> > 
> > At first for patches: I've actually looked through them before applying, and
> > there're a couple of questions:
> > 
> > 1. I've modified kxmlgui patch a little — at first I've replaces close event
> > stuff for calling queryClose() method. Following XSM protocol one is not
> > allowed to close windows on the saving state as a) this is not necessary
> > shutdown that save-yourself is triggered , it is explicitly stated in the
> > documentation, and b) shutdown, if any, could normally be cancelled and
> > session would continue, and closing some windows would just spoil this
> > user's session.
> Agreed. But, the close event here is actually "fake", no windows are
> actually closed. In tracing queryClose(), it seemed appropriate to hook
> KMainWindow::closeEvent() in KMainWindow::commitData() to pick up the "auto
> save" functionality in KMainWindow::closeEvent(). KMainWindow::closeEvent()
> potentially does some KMainWindow-specific auto saving, and then simply
> calls queryClose().
> Anyway, I'm happy to go with your call on this, but have a look at
> KMainWindow::closeEvent() if you haven't already.

Yeah, I've looked at it just briefly applying the patch and now have looked closer. As far as I can see, it still can close window as soon as queryClose() returns true (as it accepts the event). But you're right that there's some code of auto-saving, yes.

Another problem arises in that basically the client should ask session manager a permission to interact with the user to get the monopoly attention, but it's not so, as queryClose doesn't have any clue of session manager, it just shows the dialog. But still such things are minor, we need to bring back session restore first.

> > Another thing I removed is restore() method which is obviously the same as
> > before, but always returns false. Is there any reason for it?
> >
> The  KMainWindow::restore() currently always returns false, which is not
> useful for apps that choose to iterate over windows for multi-window
> restorations (using a false return code terminate iteration). My change is
> to return true on successful restoration, and false otherwise.

Oh, yes, didn't notice, thanks for pointing out.

> > 2. KSMServer patch is what I first have tried not to apply at all. Except
> > the first edit, which seems to be right as it's just follows the comment,
> > other stuff seems to be redundant. Could you please explain you ideas behind
> > these changes? Anyway, everything works even without it (see below).
> > 
> Yup, the first edit is the only important one. The others, well, I was
> tinkering a bit, and thinking of doing more, but its a bit time-consuming; I
> should have removed them before posting. The final two edits are merely code
> restructuring, no functional changes, should've removed 'em. The remaining
> edits, associated with "Phase 2 for non-WM clients", are an attempt to allow
> non-WM apps which want to do their shutdown processing in Phase 2 to do so
> before the WM is shutdown (also in Phase 2). I don't specifically know of
> any such apps, but, in principle there could be. 

OK.

> > As for testing, I've tested the following use cases:
> > 
> > 1. Plain saving session. It's generally works even without any patches
> > (except Qt patch https://codereview.qt-project.org/#/c/140115 I've applied
> > much earlier). Works fine.
> > 
> > 2. Saving session with several "modified" clients — in my case it was KWrite
> > with unsaved document and Konsole with "vim" running in one of the tabs. On
> > logging out I was gently and sequencially asked for both KWrite and Konsole.
> > Saved nothing and logged out. Works fine.
> > 
> > 3. Triggering session save with several "modified" clients and cancel
> > logging out in one of them. In my case they were KWrite again with an
> > unsaved document and Kate with an unsaved document. Here I tried two
> > sub-clauses:
> > 
> >     a) at first, say "don't save" to KWrite and "Cancel" to Kate. Session
> > continued successfully, no data is lost in both programs. Just KWin moved
> > both of them to "Any virtual desktop" — it's apparently the default
> > behaviour, appropriate for now. So works fine;
> > 
> >     b) finally, say "save" to KWrite and "don't save" to Kate. Session
> > finished and then restored successfully except Kate crashed, which is
> > apparently separate bug in Kate, which I will address, too. In terms of SM
> > works fine.
> > 
> > As a conclusion, I think, this patch can be candidate for merge right now
> > without even waiting for Qt patch — there's no harm in saving documents
> > beforehand, but, of course, without Qt changes of this report:
> > https://codereview.qt-project.org/#/c/140115 applications won't be restarted.
> > 
> > I'm attaching the patch with my modification. If there's no objection, John,
> > I'd suggest you as original author to post it to https://reviewboard.kde.org
> > so that we can have it reviewed and potentially merged as soon as possible.
> 
> Thanks again for the help and interest and I'll get on the
> https://reviewboard.kde.org post asap.

Yes, thank you.

(In reply to Alexey Chernov from comment #61)
> (In reply to John Stanley from comment #60)
> > (In reply to Alexey Chernov from comment #56)
> > > I've just applied and quite intensively tested two patches John attached.
> > > 
> > > At first for patches: I've actually looked through them before applying, and
> > > there're a couple of questions:
> > > 
> > > 1. I've modified kxmlgui patch a little — at first I've replaces close event
> > > stuff for calling queryClose() method. Following XSM protocol one is not
> > > allowed to close windows on the saving state as a) this is not necessary
> > > shutdown that save-yourself is triggered , it is explicitly stated in the
> > > documentation, and b) shutdown, if any, could normally be cancelled and
> > > session would continue, and closing some windows would just spoil this
> > > user's session.
> > Agreed. But, the close event here is actually "fake", no windows are
> > actually closed. In tracing queryClose(), it seemed appropriate to hook
> > KMainWindow::closeEvent() in KMainWindow::commitData() to pick up the "auto
> > save" functionality in KMainWindow::closeEvent(). KMainWindow::closeEvent()
> > potentially does some KMainWindow-specific auto saving, and then simply
> > calls queryClose().
> > Anyway, I'm happy to go with your call on this, but have a look at
> > KMainWindow::closeEvent() if you haven't already.
> 
> Yeah, I've looked at it just briefly applying the patch and now have looked
> closer. As far as I can see, it still can close window as soon as
> queryClose() returns true (as it accepts the event). But you're right that
> there's some code of auto-saving, yes.
> 
Yea, but, in this case we are instigating the CloseEvent (not Qt via close()/quit() for example), and so as I understand it, no actual close will be triggered by event acceptance here. That is, I thought that an actual close is done by Qt in, e.g., the close() method after Qt does a sendEvent() and checks for acceptance Here, we only use sendEvent() to decide whether or not to cancel the session 

> Another problem arises in that basically the client should ask session
> manager a permission to interact with the user to get the monopoly
> attention, but it's not so, as queryClose doesn't have any clue of session
> manager, it just shows the dialog. But still such things are minor, we need
> to bring back session restore first.
> 
> > > Another thing I removed is restore() method which is obviously the same as
> > > before, but always returns false. Is there any reason for it?
> > >
> > The  KMainWindow::restore() currently always returns false, which is not
> > useful for apps that choose to iterate over windows for multi-window
> > restorations (using a false return code terminate iteration). My change is
> > to return true on successful restoration, and false otherwise.
> 
> Oh, yes, didn't notice, thanks for pointing out.
> 
> > > 2. KSMServer patch is what I first have tried not to apply at all. Except
> > > the first edit, which seems to be right as it's just follows the comment,
> > > other stuff seems to be redundant. Could you please explain you ideas behind
> > > these changes? Anyway, everything works even without it (see below).
> > > 
> > Yup, the first edit is the only important one. The others, well, I was
> > tinkering a bit, and thinking of doing more, but its a bit time-consuming; I
> > should have removed them before posting. The final two edits are merely code
> > restructuring, no functional changes, should've removed 'em. The remaining
> > edits, associated with "Phase 2 for non-WM clients", are an attempt to allow
> > non-WM apps which want to do their shutdown processing in Phase 2 to do so
> > before the WM is shutdown (also in Phase 2). I don't specifically know of
> > any such apps, but, in principle there could be. 
> 
> OK.
> 
> > > As for testing, I've tested the following use cases:
> > > 
> > > 1. Plain saving session. It's generally works even without any patches
> > > (except Qt patch https://codereview.qt-project.org/#/c/140115 I've applied
> > > much earlier). Works fine.
> > > 
> > > 2. Saving session with several "modified" clients — in my case it was KWrite
> > > with unsaved document and Konsole with "vim" running in one of the tabs. On
> > > logging out I was gently and sequencially asked for both KWrite and Konsole.
> > > Saved nothing and logged out. Works fine.
> > > 
> > > 3. Triggering session save with several "modified" clients and cancel
> > > logging out in one of them. In my case they were KWrite again with an
> > > unsaved document and Kate with an unsaved document. Here I tried two
> > > sub-clauses:
> > > 
> > >     a) at first, say "don't save" to KWrite and "Cancel" to Kate. Session
> > > continued successfully, no data is lost in both programs. Just KWin moved
> > > both of them to "Any virtual desktop" — it's apparently the default
> > > behaviour, appropriate for now. So works fine;
> > > 
> > >     b) finally, say "save" to KWrite and "don't save" to Kate. Session
> > > finished and then restored successfully except Kate crashed, which is
> > > apparently separate bug in Kate, which I will address, too. In terms of SM
> > > works fine.
> > > 
> > > As a conclusion, I think, this patch can be candidate for merge right now
> > > without even waiting for Qt patch — there's no harm in saving documents
> > > beforehand, but, of course, without Qt changes of this report:
> > > https://codereview.qt-project.org/#/c/140115 applications won't be restarted.
> > > 
> > > I'm attaching the patch with my modification. If there's no objection, John,
> > > I'd suggest you as original author to post it to https://reviewboard.kde.org
> > > so that we can have it reviewed and potentially merged as soon as possible.
> > 
> > Thanks again for the help and interest and I'll get on the
> > https://reviewboard.kde.org post asap.
> 
> Yes, thank you.

(In reply to John Stanley from comment #62)
> (In reply to Alexey Chernov from comment #61)
> > (In reply to John Stanley from comment #60)
> > > (In reply to Alexey Chernov from comment #56)
> > > > I've just applied and quite intensively tested two patches John attached.
> > > > 
> > > > At first for patches: I've actually looked through them before applying, and
> > > > there're a couple of questions:
> > > > 
> > > > 1. I've modified kxmlgui patch a little — at first I've replaces close event
> > > > stuff for calling queryClose() method. Following XSM protocol one is not
> > > > allowed to close windows on the saving state as a) this is not necessary
> > > > shutdown that save-yourself is triggered , it is explicitly stated in the
> > > > documentation, and b) shutdown, if any, could normally be cancelled and
> > > > session would continue, and closing some windows would just spoil this
> > > > user's session.
> > > Agreed. But, the close event here is actually "fake", no windows are
> > > actually closed. In tracing queryClose(), it seemed appropriate to hook
> > > KMainWindow::closeEvent() in KMainWindow::commitData() to pick up the "auto
> > > save" functionality in KMainWindow::closeEvent(). KMainWindow::closeEvent()
> > > potentially does some KMainWindow-specific auto saving, and then simply
> > > calls queryClose().
> > > Anyway, I'm happy to go with your call on this, but have a look at
> > > KMainWindow::closeEvent() if you haven't already.
> > 
> > Yeah, I've looked at it just briefly applying the patch and now have looked
> > closer. As far as I can see, it still can close window as soon as
> > queryClose() returns true (as it accepts the event). But you're right that
> > there's some code of auto-saving, yes.
> > 
> Yea, but, in this case we are instigating the CloseEvent (not Qt via
> close()/quit() for example), and so as I understand it, no actual close will
> be triggered by event acceptance here. That is, I thought that an actual
> close is done by Qt in, e.g., the close() method after Qt does a sendEvent()
> and checks for acceptance Here, we only use sendEvent() to decide whether or
> not to cancel the session

Yes, you're probably right then, since we send close event by ourselves. I think, it's quite easy to test this as well — just left one client untouched and another one with modifications before logging out, cancel the shutdown in the latter and see what happens. I believe, you have tried the initial version of the patch, did you happen to follow this scenario?

> > Another problem arises in that basically the client should ask session
> > manager a permission to interact with the user to get the monopoly
> > attention, but it's not so, as queryClose doesn't have any clue of session
> > manager, it just shows the dialog. But still such things are minor, we need
> > to bring back session restore first.
> > 
> > > > Another thing I removed is restore() method which is obviously the same as
> > > > before, but always returns false. Is there any reason for it?
> > > >
> > > The  KMainWindow::restore() currently always returns false, which is not
> > > useful for apps that choose to iterate over windows for multi-window
> > > restorations (using a false return code terminate iteration). My change is
> > > to return true on successful restoration, and false otherwise.
> > 
> > Oh, yes, didn't notice, thanks for pointing out.
> > 
> > > > 2. KSMServer patch is what I first have tried not to apply at all. Except
> > > > the first edit, which seems to be right as it's just follows the comment,
> > > > other stuff seems to be redundant. Could you please explain you ideas behind
> > > > these changes? Anyway, everything works even without it (see below).
> > > > 
> > > Yup, the first edit is the only important one. The others, well, I was
> > > tinkering a bit, and thinking of doing more, but its a bit time-consuming; I
> > > should have removed them before posting. The final two edits are merely code
> > > restructuring, no functional changes, should've removed 'em. The remaining
> > > edits, associated with "Phase 2 for non-WM clients", are an attempt to allow
> > > non-WM apps which want to do their shutdown processing in Phase 2 to do so
> > > before the WM is shutdown (also in Phase 2). I don't specifically know of
> > > any such apps, but, in principle there could be. 
> > 
> > OK.
> > 
> > > > As for testing, I've tested the following use cases:
> > > > 
> > > > 1. Plain saving session. It's generally works even without any patches
> > > > (except Qt patch https://codereview.qt-project.org/#/c/140115 I've applied
> > > > much earlier). Works fine.
> > > > 
> > > > 2. Saving session with several "modified" clients — in my case it was KWrite
> > > > with unsaved document and Konsole with "vim" running in one of the tabs. On
> > > > logging out I was gently and sequencially asked for both KWrite and Konsole.
> > > > Saved nothing and logged out. Works fine.
> > > > 
> > > > 3. Triggering session save with several "modified" clients and cancel
> > > > logging out in one of them. In my case they were KWrite again with an
> > > > unsaved document and Kate with an unsaved document. Here I tried two
> > > > sub-clauses:
> > > > 
> > > >     a) at first, say "don't save" to KWrite and "Cancel" to Kate. Session
> > > > continued successfully, no data is lost in both programs. Just KWin moved
> > > > both of them to "Any virtual desktop" — it's apparently the default
> > > > behaviour, appropriate for now. So works fine;
> > > > 
> > > >     b) finally, say "save" to KWrite and "don't save" to Kate. Session
> > > > finished and then restored successfully except Kate crashed, which is
> > > > apparently separate bug in Kate, which I will address, too. In terms of SM
> > > > works fine.
> > > > 
> > > > As a conclusion, I think, this patch can be candidate for merge right now
> > > > without even waiting for Qt patch — there's no harm in saving documents
> > > > beforehand, but, of course, without Qt changes of this report:
> > > > https://codereview.qt-project.org/#/c/140115 applications won't be restarted.
> > > > 
> > > > I'm attaching the patch with my modification. If there's no objection, John,
> > > > I'd suggest you as original author to post it to https://reviewboard.kde.org
> > > > so that we can have it reviewed and potentially merged as soon as possible.
> > > 
> > > Thanks again for the help and interest and I'll get on the
> > > https://reviewboard.kde.org post asap.
> > 
> > Yes, thank you.

(In reply to Alexey Chernov from comment #63)
> (In reply to John Stanley from comment #62)
> > (In reply to Alexey Chernov from comment #61)
> > > (In reply to John Stanley from comment #60)
> > > > (In reply to Alexey Chernov from comment #56)
> > > > > I've just applied and quite intensively tested two patches John attached.
> > > > > 
> > > > > At first for patches: I've actually looked through them before applying, and
> > > > > there're a couple of questions:
> > > > > 
> > > > > 1. I've modified kxmlgui patch a little — at first I've replaces close event
> > > > > stuff for calling queryClose() method. Following XSM protocol one is not
> > > > > allowed to close windows on the saving state as a) this is not necessary
> > > > > shutdown that save-yourself is triggered , it is explicitly stated in the
> > > > > documentation, and b) shutdown, if any, could normally be cancelled and
> > > > > session would continue, and closing some windows would just spoil this
> > > > > user's session.
> > > > Agreed. But, the close event here is actually "fake", no windows are
> > > > actually closed. In tracing queryClose(), it seemed appropriate to hook
> > > > KMainWindow::closeEvent() in KMainWindow::commitData() to pick up the "auto
> > > > save" functionality in KMainWindow::closeEvent(). KMainWindow::closeEvent()
> > > > potentially does some KMainWindow-specific auto saving, and then simply
> > > > calls queryClose().
> > > > Anyway, I'm happy to go with your call on this, but have a look at
> > > > KMainWindow::closeEvent() if you haven't already.
> > > 
> > > Yeah, I've looked at it just briefly applying the patch and now have looked
> > > closer. As far as I can see, it still can close window as soon as
> > > queryClose() returns true (as it accepts the event). But you're right that
> > > there's some code of auto-saving, yes.
> > > 
> > Yea, but, in this case we are instigating the CloseEvent (not Qt via
> > close()/quit() for example), and so as I understand it, no actual close will
> > be triggered by event acceptance here. That is, I thought that an actual
> > close is done by Qt in, e.g., the close() method after Qt does a sendEvent()
> > and checks for acceptance Here, we only use sendEvent() to decide whether or
> > not to cancel the session
> 
> Yes, you're probably right then, since we send close event by ourselves. I
> think, it's quite easy to test this as well — just left one client untouched
> and another one with modifications before logging out, cancel the shutdown
> in the latter and see what happens. I believe, you have tried the initial
> version of the patch, did you happen to follow this scenario?

I did do a quick 'n dirty check with konsole (had to patch konsole's MainWindow::queryClose() to allow session cancellation though): open several apps plus konsole, launch vi in konsole, attempt to logout, refuse, and it works as expected -- session is canceled and windows remain open,
 
> 
> > > Another problem arises in that basically the client should ask session
> > > manager a permission to interact with the user to get the monopoly
> > > attention, but it's not so, as queryClose doesn't have any clue of session
> > > manager, it just shows the dialog. But still such things are minor, we need
> > > to bring back session restore first.
> > > 
> > > > > Another thing I removed is restore() method which is obviously the same as
> > > > > before, but always returns false. Is there any reason for it?
> > > > >
> > > > The  KMainWindow::restore() currently always returns false, which is not
> > > > useful for apps that choose to iterate over windows for multi-window
> > > > restorations (using a false return code terminate iteration). My change is
> > > > to return true on successful restoration, and false otherwise.
> > > 
> > > Oh, yes, didn't notice, thanks for pointing out.
> > > 
> > > > > 2. KSMServer patch is what I first have tried not to apply at all. Except
> > > > > the first edit, which seems to be right as it's just follows the comment,
> > > > > other stuff seems to be redundant. Could you please explain you ideas behind
> > > > > these changes? Anyway, everything works even without it (see below).
> > > > > 
> > > > Yup, the first edit is the only important one. The others, well, I was
> > > > tinkering a bit, and thinking of doing more, but its a bit time-consuming; I
> > > > should have removed them before posting. The final two edits are merely code
> > > > restructuring, no functional changes, should've removed 'em. The remaining
> > > > edits, associated with "Phase 2 for non-WM clients", are an attempt to allow
> > > > non-WM apps which want to do their shutdown processing in Phase 2 to do so
> > > > before the WM is shutdown (also in Phase 2). I don't specifically know of
> > > > any such apps, but, in principle there could be. 
> > > 
> > > OK.
> > > 
> > > > > As for testing, I've tested the following use cases:
> > > > > 
> > > > > 1. Plain saving session. It's generally works even without any patches
> > > > > (except Qt patch https://codereview.qt-project.org/#/c/140115 I've applied
> > > > > much earlier). Works fine.
> > > > > 
> > > > > 2. Saving session with several "modified" clients — in my case it was KWrite
> > > > > with unsaved document and Konsole with "vim" running in one of the tabs. On
> > > > > logging out I was gently and sequencially asked for both KWrite and Konsole.
> > > > > Saved nothing and logged out. Works fine.
> > > > > 
> > > > > 3. Triggering session save with several "modified" clients and cancel
> > > > > logging out in one of them. In my case they were KWrite again with an
> > > > > unsaved document and Kate with an unsaved document. Here I tried two
> > > > > sub-clauses:
> > > > > 
> > > > >     a) at first, say "don't save" to KWrite and "Cancel" to Kate. Session
> > > > > continued successfully, no data is lost in both programs. Just KWin moved
> > > > > both of them to "Any virtual desktop" — it's apparently the default
> > > > > behaviour, appropriate for now. So works fine;
> > > > > 
> > > > >     b) finally, say "save" to KWrite and "don't save" to Kate. Session
> > > > > finished and then restored successfully except Kate crashed, which is
> > > > > apparently separate bug in Kate, which I will address, too. In terms of SM
> > > > > works fine.
> > > > > 
> > > > > As a conclusion, I think, this patch can be candidate for merge right now
> > > > > without even waiting for Qt patch — there's no harm in saving documents
> > > > > beforehand, but, of course, without Qt changes of this report:
> > > > > https://codereview.qt-project.org/#/c/140115 applications won't be restarted.
> > > > > 
> > > > > I'm attaching the patch with my modification. If there's no objection, John,
> > > > > I'd suggest you as original author to post it to https://reviewboard.kde.org
> > > > > so that we can have it reviewed and potentially merged as soon as possible.
> > > > 
> > > > Thanks again for the help and interest and I'll get on the
> > > > https://reviewboard.kde.org post asap.
> > > 
> > > Yes, thank you.

Created attachment 97078 [details]
Updated plasma-workspace SM patch

Is the following correct:
- Things are mostly fixed now
- The ksmserver patch is still needed to make aborted shutdown/logout work correctly?
If so, I'm planning to have a look at the ksmserver patch (no promises). The logic in ksmserver does look dubious to me, on the other hand it has worked alright for a long time so one needs to be very careful.

Since yesterday I upgraded to Kubuntu 16.04 and nothing has improved. It's even worse, because now hibernation, my only workaround, does'n work anymore.
That should be no surprise, because they stick to qt 5.5 and as I could read here, the problems are fixed in only within qt 5.6. So, because not able to build my own KDE, I'm depending on the backport team. But even on the staging branch, no one seems to be working on it actually.

An attempt with KDE-Neon in a virtual-machine gave this result: Konsole-settings are totally broken. Not even the appearance is restored after restart.

Are there too few people like us having these problems ?

Kubuntu 16.04 does not have Qt 5.6, so session management is not expected to work right in it.

(In reply to Andreas Hartmetz from comment #68)
> Kubuntu 16.04 does not have Qt 5.6, so session management is not expected to
> work right in it.

But KDE-Neon has, and it does not work either.

I've seen session management in Fedora 23 testing get better when qt5.6 was first introduced and indeed had a few session restores which were 100% complete and repeatable, but then it's gotten worse again.  However, I haven't seen the issue mentioned in this ticket (konsole not saving tabs or preferences) in quite some time.

Hi,

Please accept my apologies for this out of bandwidth response.

Maybe "we" should abandon all the distributions that use 5.5 and go  with Fedora Rawhide? I'm sure that would get someone's attention. I use Fedora 23 and Rawhide (24) and don't seem to have these problems.

George...

In KDE Neon it seems to work now.

konsole session saving does *not* work in fedora 23 (tested with a brand new ID).  haven't tried 24 beta yet.  but 23 is definitely (for me) unusable because of this.

(In reply to Mark T. Kennedy from comment #73)
> konsole session saving does *not* work in fedora 23 (tested with a brand new
> ID).  haven't tried 24 beta yet.  but 23 is definitely (for me) unusable
> because of this.

It works if you change the Desktop Session settings to "Restore manually saved session" and manually save the session before exiting the session. It doesn't work if you use "Restore previous session" (which is supposed to automatically save and restore the session).

(In reply to Jonathan Wakely from comment #74)
> (In reply to Mark T. Kennedy from comment #73)
> > konsole session saving does *not* work in fedora 23 (tested with a brand new
> > ID).  haven't tried 24 beta yet.  but 23 is definitely (for me) unusable
> > because of this.
> 
> It works if you change the Desktop Session settings to "Restore manually
> saved session" and manually save the session before exiting the session. It
> doesn't work if you use "Restore previous session" (which is supposed to
> automatically save and restore the session).

Certainly not for me. All my consoles are gone, and I have never have anything but "Restore manually saved session".

(In reply to cacho96 from comment #75)
> (In reply to Jonathan Wakely from comment #74)
> > (In reply to Mark T. Kennedy from comment #73)
> > > konsole session saving does *not* work in fedora 23 (tested with a brand new
> > > ID).  haven't tried 24 beta yet.  but 23 is definitely (for me) unusable
> > > because of this.
> > 
> > It works if you change the Desktop Session settings to "Restore manually
> > saved session" and manually save the session before exiting the session. It
> > doesn't work if you use "Restore previous session" (which is supposed to
> > automatically save and restore the session).
> 
> Certainly not for me. All my consoles are gone, and I have never have
> anything but "Restore manually saved session".

BTW, this is not a konsole problem. I use 5 rxvt's. My rxvt's are not shown (yet present in the session file), but when I open rxvt's with my shortcut, one by one appear again with placement and sizes as they should. Very strange.

I upgraded to Kubuntu 16.10 yesterday. As i can say, now all is working as it did before upgrading to 15.04, one and a half year ago.
From my opinion this thread can be closed.

closing as requested, thanks.

For the record, konsole had a separate session restore bug (bug 367447) which has also been fixed.