Review Request 124102: Replace other Notifications services when Plasma's notifications are enabled

[Martin Klapetek]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124102/
---

Review request for Plasma.


Repository: plasma-workspace


Description
---

We're getting lots of reports about notifications not being "closeable on 
click" or "not having any actions" or "not having Plasma theme". These all 
mostly come from users which have notify-osd package from Unity (ie. Ubuntu 
users installing plasma-desktop), but not only.

So this patch makes Plasma always be the Notification service provider if that 
option is enabled in the applet settings and/or if the applet is present 
somewhere (otherwise the dataengine is not loaded). On startup, it will get the 
PID of the current Notifications service, send SIGTERM to it and register its 
own service.


Diffs
-

  dataengines/notifications/notificationsengine.h 7810787 
  dataengines/notifications/notificationsengine.cpp c3bf373 

Diff: https://git.reviewboard.kde.org/r/124102/diff/


Testing
---

Having notify-osd running, plasmashell starts up, notify-osd is terminated, 
Plasma notifications appear.


Thanks,

Martin Klapetek

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 124102: Replace other Notifications services when Plasma's notifications are enabled

[Kai Uwe Broulik]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124102/#review81486
---


+1

*evil laughing up my sleeve*


dataengines/notifications/notificationsengine.cpp (line 63)


Given we require Qt 5.4, QTimer::singleShot(3000, this, 
&NotificationsEngine::registerDBusService);


- Kai Uwe Broulik


On Juni 15, 2015, 2:30 nachm., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124102/
> ---
> 
> (Updated Juni 15, 2015, 2:30 nachm.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> We're getting lots of reports about notifications not being "closeable on 
> click" or "not having any actions" or "not having Plasma theme". These all 
> mostly come from users which have notify-osd package from Unity (ie. Ubuntu 
> users installing plasma-desktop), but not only.
> 
> So this patch makes Plasma always be the Notification service provider if 
> that option is enabled in the applet settings and/or if the applet is present 
> somewhere (otherwise the dataengine is not loaded). On startup, it will get 
> the PID of the current Notifications service, send SIGTERM to it and register 
> its own service.
> 
> 
> Diffs
> -
> 
>   dataengines/notifications/notificationsengine.h 7810787 
>   dataengines/notifications/notificationsengine.cpp c3bf373 
> 
> Diff: https://git.reviewboard.kde.org/r/124102/diff/
> 
> 
> Testing
> ---
> 
> Having notify-osd running, plasmashell starts up, notify-osd is terminated, 
> Plasma notifications appear.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 124102: Replace other Notifications services when Plasma's notifications are enabled

[Lukáš Tinkl]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124102/#review81489
---


A better option would be to call registerService() with 
QDBusConnectionInterface::ReplaceExistingService and 
DBusConnectionInterface::AllowReplacement

- Lukáš Tinkl


On Čer. 15, 2015, 4:30 odp., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124102/
> ---
> 
> (Updated Čer. 15, 2015, 4:30 odp.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> We're getting lots of reports about notifications not being "closeable on 
> click" or "not having any actions" or "not having Plasma theme". These all 
> mostly come from users which have notify-osd package from Unity (ie. Ubuntu 
> users installing plasma-desktop), but not only.
> 
> So this patch makes Plasma always be the Notification service provider if 
> that option is enabled in the applet settings and/or if the applet is present 
> somewhere (otherwise the dataengine is not loaded). On startup, it will get 
> the PID of the current Notifications service, send SIGTERM to it and register 
> its own service.
> 
> 
> Diffs
> -
> 
>   dataengines/notifications/notificationsengine.h 7810787 
>   dataengines/notifications/notificationsengine.cpp c3bf373 
> 
> Diff: https://git.reviewboard.kde.org/r/124102/diff/
> 
> 
> Testing
> ---
> 
> Having notify-osd running, plasmashell starts up, notify-osd is terminated, 
> Plasma notifications appear.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 124102: Replace other Notifications services when Plasma's notifications are enabled

[Martin Klapetek]

> On June 15, 2015, 9:29 p.m., Lukáš Tinkl wrote:
> > A better option would be to call registerService() with 
> > QDBusConnectionInterface::ReplaceExistingService and 
> > DBusConnectionInterface::AllowReplacement

It's the "attempt to replace it." part of it that I'm not entirely happy with. 
Plus we don't actually get to control all the servers and set 
"AllowReplacement" on them, so this might not be feasible in the end.


- Martin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124102/#review81489
---


On June 15, 2015, 4:30 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124102/
> ---
> 
> (Updated June 15, 2015, 4:30 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> We're getting lots of reports about notifications not being "closeable on 
> click" or "not having any actions" or "not having Plasma theme". These all 
> mostly come from users which have notify-osd package from Unity (ie. Ubuntu 
> users installing plasma-desktop), but not only.
> 
> So this patch makes Plasma always be the Notification service provider if 
> that option is enabled in the applet settings and/or if the applet is present 
> somewhere (otherwise the dataengine is not loaded). On startup, it will get 
> the PID of the current Notifications service, send SIGTERM to it and register 
> its own service.
> 
> 
> Diffs
> -
> 
>   dataengines/notifications/notificationsengine.h 7810787 
>   dataengines/notifications/notificationsengine.cpp c3bf373 
> 
> Diff: https://git.reviewboard.kde.org/r/124102/diff/
> 
> 
> Testing
> ---
> 
> Having notify-osd running, plasmashell starts up, notify-osd is terminated, 
> Plasma notifications appear.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 124102: Replace other Notifications services when Plasma's notifications are enabled

[Xuetian Weng]

> On June 15, 2015, 7:29 p.m., Lukáš Tinkl wrote:
> > A better option would be to call registerService() with 
> > QDBusConnectionInterface::ReplaceExistingService and 
> > DBusConnectionInterface::AllowReplacement
> 
> Martin Klapetek wrote:
> It's the "attempt to replace it." part of it that I'm not entirely happy 
> with. Plus we don't actually get to control all the servers and set 
> "AllowReplacement" on them, so this might not be feasible in the end.

That's why I hate dbus activation when it comes to multiple desktop 
environment. There's only one service of the specific service that can be 
dbus-activated by dbus.
And BTW notify-osd does not set the allow replacement flag (checked their 
source code).


- Xuetian


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124102/#review81489
---


On June 15, 2015, 2:30 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124102/
> ---
> 
> (Updated June 15, 2015, 2:30 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> We're getting lots of reports about notifications not being "closeable on 
> click" or "not having any actions" or "not having Plasma theme". These all 
> mostly come from users which have notify-osd package from Unity (ie. Ubuntu 
> users installing plasma-desktop), but not only.
> 
> So this patch makes Plasma always be the Notification service provider if 
> that option is enabled in the applet settings and/or if the applet is present 
> somewhere (otherwise the dataengine is not loaded). On startup, it will get 
> the PID of the current Notifications service, send SIGTERM to it and register 
> its own service.
> 
> 
> Diffs
> -
> 
>   dataengines/notifications/notificationsengine.h 7810787 
>   dataengines/notifications/notificationsengine.cpp c3bf373 
> 
> Diff: https://git.reviewboard.kde.org/r/124102/diff/
> 
> 
> Testing
> ---
> 
> Having notify-osd running, plasmashell starts up, notify-osd is terminated, 
> Plasma notifications appear.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 124102: Replace other Notifications services when Plasma's notifications are enabled

[Martin Klapetek]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124102/
---

(Updated June 22, 2015, 8:54 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Changes
---

Submitted with commit b533e7d13f7daac129d4f91b4cdd12d9362ff15b by Martin 
Klapetek to branch Plasma/5.3.


Repository: plasma-workspace


Description
---

We're getting lots of reports about notifications not being "closeable on 
click" or "not having any actions" or "not having Plasma theme". These all 
mostly come from users which have notify-osd package from Unity (ie. Ubuntu 
users installing plasma-desktop), but not only.

So this patch makes Plasma always be the Notification service provider if that 
option is enabled in the applet settings and/or if the applet is present 
somewhere (otherwise the dataengine is not loaded). On startup, it will get the 
PID of the current Notifications service, send SIGTERM to it and register its 
own service.


Diffs
-

  dataengines/notifications/notificationsengine.h 7810787 
  dataengines/notifications/notificationsengine.cpp c3bf373 

Diff: https://git.reviewboard.kde.org/r/124102/diff/


Testing
---

Having notify-osd running, plasmashell starts up, notify-osd is terminated, 
Plasma notifications appear.


Thanks,

Martin Klapetek

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel