D4799: Delay notifications until desktop session has loaded

2017-06-08 Thread Andrius Štikonas
stikonas added a comment. This introduces a regression https://bugs.kde.org/show_bug.cgi?id=380974 REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D4799 To: vpilo, #plasma_workspaces, #plasma, davidedmundson, mck182, broulik, dfaure, graesslin Cc: stikonas, pl

D4799: Delay notifications until desktop session has loaded

2017-03-22 Thread Valerio Pilo
This revision was automatically updated to reflect the committed changes. Closed by commit R289:2d40672b0c85: Do not remove queued notifications when the fd.o service starts. Also start the… (authored by vpilo). REPOSITORY R289 KNotifications CHANGES SINCE LAST UPDATE https://phabricator.kde

D4799: Delay notifications until desktop session has loaded

2017-03-22 Thread David Edmundson
davidedmundson accepted this revision. This revision is now accepted and ready to land. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D4799 To: vpilo, #plasma, #plasma_workspaces, davidedmundson, dfaure, broulik, graesslin, mck182 Cc: plasma-devel, davidedmundso

D4799: Delay notifications until desktop session has loaded

2017-03-22 Thread Valerio Pilo
vpilo added a comment. Ping [my excuses if it's not good practice to do so] REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D4799 To: vpilo, #plasma, #plasma_workspaces, davidedmundson, dfaure, broulik, graesslin, mck182 Cc: plasma-devel, davidedmundson,

D4799: Delay notifications until desktop session has loaded

2017-03-19 Thread Valerio Pilo
vpilo added a comment. I pushed https://phabricator.kde.org/D5012; this rev is now testable. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D4799 To: vpilo, #plasma, #plasma_workspaces, davidedmundson, dfaure, broulik, graesslin, mck182 Cc: plasma-devel, davi

D4799: Delay notifications until desktop session has loaded

2017-03-14 Thread Valerio Pilo
vpilo added reviewers: davidedmundson, dfaure, broulik, graesslin, mck182. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D4799 To: vpilo, #plasma, #plasma_workspaces, davidedmundson, dfaure, broulik, graesslin, mck182 Cc: plasma-devel, davidedmundson, dfaure, br

D4799: Delay notifications until desktop session has loaded

2017-03-10 Thread Valerio Pilo
vpilo updated this revision to Diff 12377. vpilo marked an inline comment as done. vpilo added reviewers: Plasma, Plasma: Workspaces. vpilo added a comment. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. New patchset. I moved the waiti

D4799: Delay notifications until desktop session has loaded

2017-03-10 Thread Valerio Pilo
vpilo added a dependent revision: D5012: Delay notifications until desktop session has loaded. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D4799 To: vpilo Cc: davidedmundson, dfaure, broulik, graesslin, mck182, #frameworks

D4799: Delay notifications until desktop session has loaded

2017-03-09 Thread Martin Klapetek
mck182 added a comment. Looks like this could work. The `KDE_FULL_SESSION` check is really old, I'm not sure if this has any implications. As David noted, the waiting thing should go to Plasma workspace, that's where it belongs. Also, it would be preferred if the implementation was p

D4799: Delay notifications until desktop session has loaded

2017-03-09 Thread David Edmundson
davidedmundson added a comment. Cool, but we should put the blocking code in plasma, not here. Otherwise we'd interefere with other platforms. Maybe in plasma-workspace/startkde REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D4799 To: vpilo Cc: davidedmunds

D4799: Delay notifications until desktop session has loaded

2017-03-08 Thread Valerio Pilo
vpilo updated this revision to Diff 12310. vpilo added a comment. New version, using the idea suggested by @davidedmundson . The KNotifications manager was clearing queued notifications also when the fd.o Notifications service initially registers; and while I get that it's done on unregi

D4799: Delay notifications until desktop session has loaded

2017-03-02 Thread Valerio Pilo
vpilo added a comment. View Revision In D4799#91246, @davidedmundson wrote: There is another solution that would work without any changes to KNotification. DBus has a solution to buffer messages and wait for a name to become available, it happens in DBus activation. If plasmashell was DBus activat

[Differential] [Commented On] D4799: Delay notifications until desktop session has loaded

2017-03-01 Thread David Edmundson
davidedmundson added a comment. There is another solution that would work without any changes to KNotification. DBus has a solution to buffer messages and wait for a name to become available, it happens in DBus activation. If plasmashell was DBus activated on org.freedesktop.Notificatio

[Differential] [Commented On] D4799: Delay notifications until desktop session has loaded

2017-02-28 Thread Valerio Pilo
vpilo added a comment. In https://phabricator.kde.org/D4799#91109, @dfaure wrote: > Yes interface->isServiceRegistered(dbusServiceName) is technically blocking, but it can't block for a long time, since it's only talking to the dbus daemon. The reply comes in rather quickly, unlike a blo

[Differential] [Commented On] D4799: Delay notifications until desktop session has loaded

2017-02-28 Thread David Faure
dfaure added a comment. Yes interface->isServiceRegistered(dbusServiceName) is technically blocking, but it can't block for a long time, since it's only talking to the dbus daemon. The reply comes in rather quickly, unlike a blocking call to another KDE process which could be busy or where t

[Differential] [Commented On] D4799: Delay notifications until desktop session has loaded

2017-02-28 Thread Valerio Pilo
vpilo added a comment. @mck182 I didn't notice before, but KNotifications is already making blocking calls on creation: src/notifybypopup.cpp@182: QDBusConnectionInterface *interface = QDBusConnection::sessionBus().interface(); d->dbusServiceExists = interface && inter

[Differential] [Commented On] D4799: Delay notifications until desktop session has loaded

2017-02-26 Thread Valerio Pilo
vpilo added a comment. In https://phabricator.kde.org/D4799#90169, @dfaure wrote: > About the code in kded that calls ksplash: that code is obsolete and currently only kept for compatibility reasons, see https://git.reviewboard.kde.org/r/129010/ > IOW you can ignore that code complete

[Differential] [Commented On] D4799: Delay notifications until desktop session has loaded

2017-02-26 Thread David Faure
dfaure added a comment. About the code in kded that calls ksplash: that code is obsolete and currently only kept for compatibility reasons, see https://git.reviewboard.kde.org/r/129010/ IOW you can ignore that code completely. REPOSITORY R289 KNotifications REVISION DETAIL https://pha

[Differential] [Commented On] D4799: Delay notifications until desktop session has loaded

2017-02-26 Thread Kai Uwe Broulik
broulik added a comment. > It is a bug in ksplash that the popups are visible at all. Fallback for KNotification is a KPassivePopup which is unredirect I think? Ksplash, in contrast to the splash screen, doesn't continuously re-raise itself and KWin doesn't do anything special to it eith

[Differential] [Commented On] D4799: Delay notifications until desktop session has loaded

2017-02-26 Thread Valerio Pilo
vpilo added a comment. @graesslin I disagree. Some popups might be useful, some are very useful; we should aim to never drop any. Just as an example, the popup with the Yakuake console toggle key is pretty fundamental to be reminded of, the first few times you start it up. It's all abou

[Differential] [Commented On] D4799: Delay notifications until desktop session has loaded

2017-02-25 Thread Martin Gräßlin
graesslin added a comment. I would say it is enough to just make plasma grab the interface early enough. It would result in the popups not show. We don't need to show those notifications. It is a bug in ksplash that the popups are visible at all. Another alternative could be to bind

[Differential] [Commented On] D4799: Delay notifications until desktop session has loaded

2017-02-25 Thread Martin Klapetek
mck182 added a comment. > 1. A quick check if KSplashQML is found in the processes list (afaics, there's no alternatives to ksplash) > 2. KNotifications could send an async call to KSplash with a very quick timeout and start deciding on its queued notifications if/after the answer arrives

[Differential] [Commented On] D4799: Delay notifications until desktop session has loaded

2017-02-25 Thread Valerio Pilo
vpilo added a comment. In https://phabricator.kde.org/D4799#89931, @mck182 wrote: > Thanks for the patch! I wanted to do exactly this a long > time ago. However this solution brings a burden to all > apps using KNotification in form of a blocking dbus call > which is further only

[Differential] [Commented On] D4799: Delay notifications until desktop session has loaded

2017-02-25 Thread Martin Klapetek
mck182 added a comment. Thanks for the patch! I wanted to do exactly this a long time ago. However this solution brings a burden to all apps using KNotification in form of a blocking dbus call which is further only relevant when used in Plasma. That's a no-no I'm afraid, we'd have to

[Differential] [Request, 35 lines] D4799: Delay notifications until desktop session has loaded

2017-02-25 Thread Valerio Pilo
vpilo created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY The KNotifications library is immediately available for programs to send out notifications from the beginning of a desktop session. But if a s