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

Flatpak: Create Autostart File Via Background Portal #681

Merged
merged 6 commits into from
Jun 13, 2023

Conversation

Marukesu
Copy link
Contributor

with this calendar will call the background portal to create the daemon autostart on the first launch when sandboxed.

@danirabbit danirabbit requested a review from a team July 20, 2021 19:23
@danirabbit

This comment has been minimized.

@Marukesu

This comment has been minimized.

@marbetschar marbetschar mentioned this pull request Sep 6, 2021
2 tasks
@marbetschar
Copy link
Member

@Marukesu FYI I added a PR which unifies the daemon and regular app into the same binary. So we may have to rework this a little once this is merged: #702

@danirabbit
Copy link
Member

@Marukesu can you update this/resolve conflicts since there's now a single binary?

@Marukesu
Copy link
Contributor Author

Updated the PR, now this will show the dialog in two cases:

  • during launch with --background.
  • when closing the window.

Also, the daemon is always started now, so that we can show notifications when the application is open, even if we don't have permission to run in background.

@danirabbit
Copy link
Member

Is there a reason to only use the background portal when running in Flatpak? Can we use it all the time?

@Marukesu
Copy link
Contributor Author

Is there a reason to only use the background portal when running in Flatpak? Can we use it all the time?

The GTK portal backend uses the sandboxed app id to name the autostart file, but for host apps it is empty, so they create an unnamed .desktop that is replaced by others application on host that calls the portal (see flatpak/xdg-desktop-portal#650).

src/Application.vala Outdated Show resolved Hide resolved
src/MainWindow.vala Outdated Show resolved Hide resolved
Copy link
Collaborator

@jeremypw jeremypw left a comment

Choose a reason for hiding this comment

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

I can confirm that after removing the existing Calendar installation with AppCenter and renaming the existing autostart file and installing a flatpak built from this PR and logging out and in again the following:

  1. No daemon was running initially
  2. Running the flatpak with flatpak run launched the program but did not start the daemon
  3. Closing the window triggered a dialog requesting permission to run in background
  4. After giving permission the window closed but the daemon did not start
  5. After logging out and back in, the daemon did autostart.
  6. It was not possible to launch Calendar from the wingpanel indicator but launching from the dock and Applications Menu worked. (Is this an existing bug with flatpak/wingpanel?)

I would expect the daemon to start after giving it permission to do so and not have to log out and in.

@Marukesu
Copy link
Contributor Author

  1. Running the flatpak with flatpak run launched the program but did not start the daemon

I can't reproduce this here, starting the application should start the daemon, the only diference from master here is that starting the daemon is done before the --background check.

  1. After giving permission the window closed but the daemon did not start

The last commit should fix that, for some reason, chaining up base.delete_event () was causing an segfault.

  1. It was not possible to launch Calendar from the wingpanel indicator but launching from the dock and Applications Menu worked. (Is this an existing bug with flatpak/wingpanel?)

Looking at the code in the indicator, it tries to launch io.elementary.calendar --show-day, so i don't expect it to work with the sandboxed calendar.

@danirabbit
Copy link
Member

@Marukesu wanna update this again and remove the flatpak check since the background portal can work with host apps now? :)

@Marukesu Marukesu force-pushed the background-portal branch 3 times, most recently from 81c0b81 to 14d3807 Compare May 28, 2023 18:54
@Marukesu
Copy link
Contributor Author

@danirabbit this should be good to review again.

Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

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

Seems like it would simplify things quite a bit to use LibPortal like we did for Mail?

src/Application.vala Outdated Show resolved Hide resolved
Marukesu and others added 5 commits June 11, 2023 00:35
@@ -61,10 +63,17 @@ namespace Maya {
}

protected override void activate () {
new Calendar.TodayEventMonitor ();
Copy link
Member

Choose a reason for hiding this comment

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

Should this be done in activate? it looks like Mail does the equivalent in startup

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, Calendar doesn't have a startup method. This work since TodayEventMonitor is marked as a [SingleInstance] class and the construct method will only be called one time.

Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

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

Let's go 🚀 Thanks for working on this!

@danirabbit danirabbit merged commit a6951f8 into master Jun 13, 2023
2 checks passed
@danirabbit danirabbit deleted the background-portal branch June 13, 2023 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants