After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 778851 - Support the notification portal
Support the notification portal
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: Notifications
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Marek Kašík
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-02-17 17:12 UTC by Matthias Clasen
Modified: 2017-03-02 21:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
notifications: Forward notification settings to portal (10.05 KB, patch)
2017-02-22 21:05 UTC, Matthias Clasen
none Details | Review
notifications: Forward notification settings to portal (10.05 KB, patch)
2017-02-22 21:08 UTC, Matthias Clasen
none Details | Review
notifications: Forward notification settings to portal (10.00 KB, patch)
2017-02-26 15:47 UTC, Matthias Clasen
committed Details | Review

Description Matthias Clasen 2017-02-17 17:12:11 UTC
xdg-desktop-portal has a portal API to allow sandboxed applications to use notifications. The permissions for this are stored in the flatpak permission store. It would be great if the notification panel store its permissions there too (or at least forward the permissions), so we can use the existing UI to control the permissions for sandboxed apps.
Comment 1 Matthias Clasen 2017-02-22 21:05:45 UTC
Created attachment 346501 [details] [review]
notifications: Forward notification settings to portal

The flatpak notification portal reads permissions out
of the permission store, so forward the notification
permissions there in order to prevent sandboxed
applications from sending notifications.

The difference is a bit cosmetic, since the shell would
not show the notification anyway in this case, but it
is nicer to just cut off the calls and not let them
through the portal in the first place.
Comment 2 Matthias Clasen 2017-02-22 21:08:15 UTC
Created attachment 346502 [details] [review]
notifications: Forward notification settings to portal

The flatpak notification portal reads permissions out
of the permission store, so forward the notification
permissions there in order to prevent sandboxed
applications from sending notifications.

The difference is a bit cosmetic, since the shell would
not show the notification anyway in this case, but it
is nicer to just cut off the calls and not let them
through the portal in the first place.
Comment 3 Marek Kašík 2017-02-23 16:10:00 UTC
Is there a flatpak package which uses this and which I can use to test the patch?
Comment 4 Matthias Clasen 2017-02-23 16:13:54 UTC
Any flatpak'ed application that use the GApplication api for notifications will use this. 

As an example, my recipes application was using it. Currently, it does not do system notifications, but that will come back.

I do have a portal test application here:

https://github.com/matthiasclasen/portal-test/

No ready-made flatpak for this, but I think there's a build script that creates one. I should look at hosting a flatpak for it somewhere.
Comment 5 Matthias Clasen 2017-02-23 16:16:00 UTC
And, just to be clear: you won't see a difference in behavior, really. The difference is that without this patch, gnome-shell will receive the notification calls from sandboxed apps, and decide not to show the notification based on the gsettings. With this patch, the portal will decide to reject the notification calls based on the stored permissions, and gnome-shell will never see the calls.
Comment 6 Matthias Clasen 2017-02-23 16:16:49 UTC
Oh, and to just very that the patch does in fact update the permission store as expected, you can use this little tool:

https://github.com/matthiasclasen/permission-viewer
Comment 7 Marek Kašík 2017-02-24 18:33:53 UTC
Review of attachment 346502 [details] [review]:

Thank you for the patch. I've tested it on my system and the only issue I've seen was when the "notifications" table or "notification" ID does not exist. It then shows warning to the user. I think that we could just create the table or the ID by calling the "Set" method. The only problem here is content of the "data" associated with the ID. Maybe we could just use empty string. Tell me if I'm wrong and the "notifications" table or the "notification" ID should be created elsewhere please.

Another idea (for another patch) is that we should combine the PermissionStore and GSettings informations and show "Off" for an application if any of those 2 sources disables notification for the application.

Btw, I've got notification from the org.gnome.PortalTest app only for every second click on the "Notify me!" button, is it intentional?
Comment 8 Matthias Clasen 2017-02-24 18:54:26 UTC
thanks for the feedback. I'll have to investigate how the tables are supposed to come into existence. And I'll check out your portal-test issue too.
Comment 9 Matthias Clasen 2017-02-26 15:47:16 UTC
Created attachment 346761 [details] [review]
notifications: Forward notification settings to portal

The flatpak notification portal reads permissions out
of the permission store, so forward the notification
permissions there in order to prevent sandboxed
applications from sending notifications.

The difference is a bit cosmetic, since the shell would
not show the notification anyway in this case, but it
is nicer to just cut off the calls and not let them
through the portal in the first place.
Comment 10 Matthias Clasen 2017-02-26 15:48:27 UTC
I changed the patch to quietly ignore a read error and create the table.

I can't reproduce the 'every second click' problem you mentioned.
Comment 11 Marek Kašík 2017-02-27 11:01:27 UTC
Review of attachment 346761 [details] [review]:

Thank you for the change. The patch looks good to me.
Comment 12 Marek Kašík 2017-02-27 15:04:24 UTC
(In reply to Matthias Clasen from comment #10)
> I can't reproduce the 'every second click' problem you mentioned.

I've just tried it again and it seems that I just have to click on the notification and it shows the next notification as expected so don't worry about it please.
Comment 13 Matthias Clasen 2017-03-02 21:41:01 UTC
Attachment 346761 [details] pushed as 8b9be45 - notifications: Forward notification settings to portal