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,
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
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,
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,
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,
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,
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
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
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
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:
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
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
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
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
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
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 &&
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
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
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
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
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
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
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
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
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
25 matches
Mail list logo
