D25106: Also allow invoking session restoration logic when apps are manually launched

[Nathaniel Graham]

ngraham closed this revision.
ngraham added a comment.


  Phew!

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D25106

To: ngraham, davidedmundson, #frameworks, dfaure, vkrause
Cc: winterz, lbeltrame, mlaurent, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, ngraham, bruns

D25106: Also allow invoking session restoration logic when apps are manually launched

[Allen Winter]

winterz added a comment.


  yay!

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D25106

To: ngraham, davidedmundson, #frameworks, dfaure, vkrause
Cc: winterz, lbeltrame, mlaurent, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, ngraham, bruns

D25106: Also allow invoking session restoration logic when apps are manually launched

[Laurent Montel]

mlaurent added a comment.


  I updated kconfig and now it's ok. I can't see the "2 kontact" bug.
  Thanks

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D25106

To: ngraham, davidedmundson, #frameworks, dfaure, vkrause
Cc: lbeltrame, mlaurent, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D25106: Also allow invoking session restoration logic when apps are manually launched

[Nathaniel Graham]

ngraham added a comment.


  Oops, sorry. D25219  was just reviewed 
and has landed now; can you folks verify whether the problem in Kontact is 
fixed now?

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D25106

To: ngraham, davidedmundson, #frameworks, dfaure, vkrause
Cc: lbeltrame, mlaurent, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D25106: Also allow invoking session restoration logic when apps are manually launched

[David Edmundson]

davidedmundson added a comment.


  This patch was supposed to be partnered with a change to 
KConfigGui::sessionConfig() to keep behaviour the same.
  
  It seems that is not merged.

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D25106

To: ngraham, davidedmundson, #frameworks, dfaure, vkrause
Cc: lbeltrame, mlaurent, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D25106: Also allow invoking session restoration logic when apps are manually launched

[Luca Beltrame]

lbeltrame added a comment.


  Found also by openQA: two kontact windows get opened.

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D25106

To: ngraham, davidedmundson, #frameworks, dfaure, vkrause
Cc: lbeltrame, mlaurent, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D25106: Also allow invoking session restoration logic when apps are manually launched

[Volker Krause]

vkrause added a comment.


  This indeed seems to have negative impact on Kontact, while other people get 
duplicates it doesn't start at all for me anymore. With this patch reverted 
it's back to normal.

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D25106

To: ngraham, davidedmundson, #frameworks, dfaure, vkrause
Cc: mlaurent, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25106: Also allow invoking session restoration logic when apps are manually launched

[Laurent Montel]

mlaurent added reviewers: dfaure, vkrause.

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D25106

To: ngraham, davidedmundson, #frameworks, dfaure, vkrause
Cc: mlaurent, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25106: Also allow invoking session restoration logic when apps are manually launched

[Laurent Montel]

mlaurent reopened this revision.
mlaurent added a comment.
This revision is now accepted and ready to land.


  This patch creates a bug in kontact as it creates 2 kontacts now.
  
  Could you reevaluate this patch please ?
  
  Regards

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D25106

To: ngraham, davidedmundson, #frameworks
Cc: mlaurent, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25106: Also allow invoking session restoration logic when apps are manually launched

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R263:46e5b32ed929: Also allow invoking session restoration 
logic when apps are manually launched (authored by ngraham).

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25106?vs=69157=70055

REVISION DETAIL
  https://phabricator.kde.org/D25106

AFFECTED FILES
  src/kmainwindow.cpp

To: ngraham, davidedmundson, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25106: Also allow invoking session restoration logic when apps are manually launched

[Nathaniel Graham]

ngraham added a comment.


  Yep, there are no changes in behavior or regressions when testing with 
Okular, Kate, System Settings and Discover.

REPOSITORY
  R263 KXmlGui

BRANCH
  allow-restoring-session-manually (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D25106

To: ngraham, davidedmundson, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25106: Also allow invoking session restoration logic when apps are manually launched

[David Edmundson]

davidedmundson accepted this revision.
davidedmundson added a comment.
This revision is now accepted and ready to land.


  > Normal session restoration behavior after a reboot is unchanged
  
  Please make sure you try with a few apps that aren't just dolphin

REPOSITORY
  R263 KXmlGui

BRANCH
  allow-restoring-session-manually (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D25106

To: ngraham, davidedmundson, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25106: Also allow invoking session restoration logic when apps are manually launched

[Nathaniel Graham]

ngraham added a comment.


  Can I get a review on this?

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D25106

To: ngraham, davidedmundson, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25106: Also allow invoking session restoration logic when apps are manually launched

[Nathaniel Graham]

ngraham added a comment.


  Ping.

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D25106

To: ngraham, davidedmundson, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25106: Also allow invoking session restoration logic when apps are manually launched

[Nathaniel Graham]

ngraham added a comment.


  Done, see D25219 

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D25106

To: ngraham, davidedmundson, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25106: Also allow invoking session restoration logic when apps are manually launched

[David Edmundson]

davidedmundson added a comment.


  > KConfigGui::sessionConfig();
  
  This method behaves weirdly and not what I expected from reading this code.
  
  If no-one has explicitly called KConfigGui::sessionConfig it creates a new 
one using qApp->sessionConfig() regardless of whether we're session restored or 
not. If we're not sessionRestored it'll effectively leak a bogus config with a 
broken ID name.
  
  I think  to accept the patch we would need to also change
  
  kconfiggui.cpp:42 to be
  
  - if (!hasSessionConfig()) {
  
  + if (!hasSessionConfig() && qApp->isSessionRestored()) {

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D25106

To: ngraham, davidedmundson, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25106: Also allow invoking session restoration logic when apps are manually launched

[Nathaniel Graham]

ngraham added a comment.


  Ping.

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D25106

To: ngraham, davidedmundson, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25106: Also allow invoking session restoration logic when apps are manually launched

[Nathaniel Graham]

ngraham added a dependent revision: D11382: Add an option to show tabs from 
last time when Dolphin starts.

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D25106

To: ngraham, davidedmundson, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25106: Also allow invoking session restoration logic when apps are manually launched

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: davidedmundson, Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  Right now session restoration logic only ever gets invoked when apps are 
started
  automatically after rebooting, because of the check for 
`qApp->isSessionRestored()`.
  This limits the feature to only working for that use case, since 
`qApp->isSessionRestored()`
  is set by Qt and we can't override it in the app itself to signal that we 
want the session
  restoration behavior for other reasons. As a result, we can't use it to 
implement the
  behavior of restoring the condition of app when it's manually launched, which 
has been
  requested for several of our apps.
  
  This patch removes the check for `qApp->isSessionRestored()`, which opens the 
door to
  implementing the requested features in our apps.
  
  CCBUG: 397463
  CCBUG: 413564

TEST PLAN
  D11382  now works
  
  Normal session restoration behavior after a reboot is unchanged
  
  Apps with session restoration behavior but without a user-facing option to 
invoke it
  don't restore session at inappropriate times.

REPOSITORY
  R263 KXmlGui

BRANCH
  allow-restoring-session-manually (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D25106

AFFECTED FILES
  src/kmainwindow.cpp

To: ngraham, davidedmundson, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns