Implement XDG Desktop Portal support#9727
Conversation
Signed-off-by: FifthTundraG <117035030+FifthTundraG@users.noreply.github.com>
Signed-off-by: FifthTundraG <117035030+FifthTundraG@users.noreply.github.com>
nilsding
left a comment
There was a problem hiding this comment.
Thanks for the PR! Looks promising so far :)
as for the AppImage, we build one as part of our CI for each pull request. if you would like to try that one I can start a CI run for this PR, provided the client builds just fine (and ideally after rebasing the changes on top of the latest master commit)
| //! if we are using portals this check will not work, that's why its usage in generalsettings.cpp is currently disabled. | ||
| //! as far as i can tell the portal does not let us do this check. how badly do we want it? the only two times its used is |
There was a problem hiding this comment.
hmm, I guess when portals are in use we have no other option than relying on the value of launchOnSystemStartup from the config file
perhaps we need to keep track of whether the background portal was used when setting this property
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
| //! please see Utility::hasLaunchOnStartup for why this is disabled | ||
| // if (enable == Utility::hasLaunchOnStartup(theme->appName())) { | ||
| // return; | ||
| // } |
There was a problem hiding this comment.
please do not leave commented out code and rather remove it 🙏
Co-authored-by: Matthieu Gallien <matthieu.gallien@nextcloud.com> Signed-off-by: FifthTundraG <117035030+FifthTundraG@users.noreply.github.com>
Signed-off-by: FifthTundraG <117035030+FifthTundraG@users.noreply.github.com>
… if available Signed-off-by: FifthTundraG <117035030+FifthTundraG@users.noreply.github.com>
Signed-off-by: FifthTundraG <117035030+FifthTundraG@users.noreply.github.com>
|
Requesting some assistance here: I am unsure of how to handle the As mentioned in my comment, the portal does not expose a way to determine if the application is set to launch on startup. @nilsding mentioned we could keep track of if the background portal was used, and if so make It's also worth noting that the changes in #9773 to generalsettings.cpp (this line) made it so the value of the checkbox is determined by the return value of |
Adds a framework for adding XDP portals and implements the Background portal for proper autostart.
Fixes flathub/com.nextcloud.desktopclient.nextcloud#18
Related: flathub/com.nextcloud.desktopclient.nextcloud#225
CC @mgallien
This is a draft. There are a few considerations that still have to be made:
org.chromium.Chromium.desktopinstead ofcom.nextcloud.desktopclient.nextcloud.desktop. It wasn't occurring on c21ff3d, which is before what seems to be a settings page redesign (which I did not notice until merging newer changes lol), so it may be related to that? This also isn't very consistent, it happened once and now it's not anymore.We need to figure out how we want portals to be configuredI think that portals should be the default, and if they fail then we fallback to legacy behavior.We could also use a buildflag (something likeUSE_PORTALS) and make it configurable that way.Utility::hasLaunchOnStartupdoes not work (and has not worked) in a sandbox. Its usage ingeneralsettings.cppwas temporarily disabled, as it was preventingXdgPortal::backgroundfrom being run when the "run on startup" toggle was unchecked, which meant the autostart file was never removed.Please keep in mind that I'm quite new to C++ and insanely long buildscripts (lol), so please have grace on any silly mistakes :)