Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clubhouse: Request background permission #1218

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

danigm
Copy link
Contributor

@danigm danigm commented Nov 25, 2020

Copy link

@dylanmccall dylanmccall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This crossed my inbox and I was just doing stuff with this API in a side project, so I figured I'd jump in :)

@@ -3082,6 +3083,7 @@ def __init__(self):
super().__init__(application_id=CLUBHOUSE_NAME,
inactivity_timeout=self._INACTIVITY_TIMEOUT,
resource_base_path='/com/hack_computer/Clubhouse')
Portal.request_background()
Copy link

@dylanmccall dylanmccall Nov 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That RequestBackground d-bus method should (at least in the GNOME implementation) trigger a system-modal dialog to ask for permission. Otherwise it assumes no. Are you sure that's happening here?

I find that the portal only shows a dialog if the application has registered itself correctly and has a window visible. For an application I made, I found that the best way to do this is in response to a user action (flipping a switch).

However, for my case I realized the application also needed to check periodically to see if it was still allowed (or not allowed) to run in the background. I did that by connecting to the main window's window_state_event and running RequestBackground after a timeout when the window was focused.

Copy link

@dylanmccall dylanmccall Nov 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I decided I wanted more specifics because it's bothering me for multiple reasons :b Poking at this with Bustle I see…

Screenshot from 2020-11-25 13-27-41

So, the background request fails (asynchronously) because org.freedesktop.impl.portal.Access.AccessDialog fails, saying "Only the focused app is allowed to show a system access dialog". It immediately emits the following Response signal: (uint32 1, {'background': <false>, 'autostart': <false>}).

Some background on that behaviour: flatpak/xdg-desktop-portal#42

options = {}
if message:
options['reason'] = GLib.Variant('s', message)
proxy.RequestBackground('(sa{sv})', '', options)
Copy link

@dylanmccall dylanmccall Nov 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is another error case here which might be worth handling by watching the returned Request object. If the user denies the request to run in the background, the portal will never ask again. The user needs to do flatpak permission-reset com.hack_computer.Clubhouse or poke around in GNOME Control Center to fix it. (Or there might be a way to reset your permissions inside a flatpak? I can't say I actually looked very hard when I did it).

Here's some code where I did that in GNOME Break Timer:

https://gitlab.gnome.org/GNOME/gnome-break-timer/-/blob/master/src/settings/MainWindow.vala#L278-311

https://gitlab.gnome.org/GNOME/gnome-break-timer/-/blob/master/src/settings/BreakManager.vala#L117-210

https://gitlab.gnome.org/GNOME/gnome-break-timer/-/blob/master/src/settings/MainWindow.vala#L278-311

For Clubhouse this might not be too much of a problem since it's just asking for the Background permission to remove pesky notifications, although it is rather unfortunate to have only one chance to do it right when we aren't handling the error. (It makes it really fiddly to test, for one thing).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this long and detailed description of the service, I'll take a second look to try to improve the integration. I've been doing some testing with a fedora 33 virtual machine and it looks like that just with the request, the inactivity dialog dissapears, but maybe that's just a implementation issue and this can fail in other distributions, so I'll try to implement the whole flow.

Copy link

@dylanmccall dylanmccall Nov 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect what might be happening there is it gets denied permission, rather than simply not granted. (Which feels like a bug in the portal, come to think of it. I reported it over here: flatpak/xdg-desktop-portal#551).

So then it is technically not allowed to run in the background, although I think what happens from there is undefined.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've detected that if the user allows, the app will run in background without any problem, and if the user declines, the app will be stopped when the window disappears, after a few seconds. But there's no notification, if the app request the background permission, the system do that automatically without asking again or notify to the user.

@danigm danigm force-pushed the request-background branch from 1973103 to 51dc53e Compare November 26, 2020 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants