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

portal-impl: Hard-code x-d-p-gtk as a last-resort fallback #1199

Merged
merged 2 commits into from
Nov 14, 2023

Conversation

smcv
Copy link
Collaborator

@smcv smcv commented Nov 14, 2023

x-d-p-gtk has historically been used as the portal implementation of last resort, and in particular, users of assemble-it-yourself desktop environments or otherwise-unsupported desktop environments have tended to rely on it for basic functionality, particularly now that its GNOME-specific parts have been removed.

We don't want to install a /usr/share/x-d-p/portals.conf that makes it the fallback (as proposed in xdg-desktop-portal#1192) because that would be higher-precedence than the fallback to the legacy UseIn mechanism. For example, x-d-p-wlr has UseIn=...;Wayfire;... which means that it will be used to provide screenshots and screencasts under Wayfire (even without fixing WayfireWM/wayfire#1995), but installing a /usr/share/x-d-p/portals.conf with only default=gtk would defeat that.

Instead, search for x-d-p-gtk as a lower precedence than the legacy UseIn mechanism. This means that (for example) users of Wayfire will get screenshots from x-d-p-wlr and file choosers from x-d-p-gtk unless configured otherwise, while users of environments with no more appropriate portal configuration at all (for example fvwm) will get x-d-p-gtk.

This change was previously a Debian-specific patch with a slightly different warning.

Resolves: #1102


Alternative to #1192 and the unfinished #1103.

src/portal-impl.c Show resolved Hide resolved
x-d-p-gtk has historically been used as the portal implementation of
last resort, and in particular, users of assemble-it-yourself desktop
environments or otherwise-unsupported desktop environments have tended
to rely on it for basic functionality, particularly now that its
GNOME-specific parts have been removed.

We don't want to install a /usr/share/x-d-p/portals.conf that makes it
the fallback (as proposed in xdg-desktop-portal#1192) because that would
be higher-precedence than the fallback to the legacy UseIn mechanism.
For example, x-d-p-wlr has UseIn=...;Wayfire;... which means that it
will be used to provide screenshots and screencasts under Wayfire (even
without fixing WayfireWM/wayfire#1995), but installing a
/usr/share/x-d-p/portals.conf with only default=gtk would defeat that.

Instead, search for x-d-p-gtk as a lower precedence than the legacy
UseIn mechanism. This means that (for example) users of Wayfire will
get screenshots from x-d-p-wlr and file choosers from x-d-p-gtk unless
configured otherwise, while users of environments with no more appropriate
portal configuration at all (for example fvwm) will get x-d-p-gtk.

This change was previously a Debian-specific patch with a slightly
different warning.

Resolves: flatpak#1102
Signed-off-by: Simon McVittie <[email protected]>
If we emit this same warning from more than one place, we'll still only
want to emit it once per run.

Signed-off-by: Simon McVittie <[email protected]>
@smcv
Copy link
Collaborator Author

smcv commented Nov 14, 2023

v2:

@GeorgesStavracas GeorgesStavracas added the config Issues with the portal configuration mechanism label Nov 14, 2023
@GeorgesStavracas GeorgesStavracas added this to the 1.20 milestone Nov 14, 2023
@GeorgesStavracas GeorgesStavracas added this pull request to the merge queue Nov 14, 2023
Merged via the queue into flatpak:main with commit a8693c6 Nov 14, 2023
4 checks passed
EvanDurfee added a commit to EvanDurfee/dotfiles that referenced this pull request Nov 21, 2023
Until a build of the xdg-desktop-portal including the commit from
flatpak/xdg-desktop-portal#1199 is available, a
portal config is necessary for desktop portals to work in flatpaks and
snaps.
@hfiguiere hfiguiere modified the milestones: 1.20, 1.18 Nov 23, 2023
@liskin
Copy link

liskin commented May 3, 2024

This only handles interfaces that use find_portal_implementation, but org.freedesktop.impl.portal.Settings is a special case that uses find_all_portal_implementations and that function won't add xdg-desktop-portal-gtk to the list unless it's explicitly requested in a portals.conf file. So in the absence of portals.conf, there will be no impl for org.freedesktop.impl.portal.Settings despite xdg-desktop-portal-gtk being a perfectly valid fallback impl for it.

The consequence is that users of these lightweight desktop environments aren't for example able to override org.freedesktop.appearance.color-scheme for libadwaita applications, as libadwaita uses xdg-desktop-portal to retrieve the preference.

Any chance the fallback can be extended to org.freedesktop.impl.portal.Settings?

liskin added a commit to liskin/dotfiles that referenced this pull request May 3, 2024
Without this, xdg-desktop-portal-gtk isn't used to provide
org.freedesktop.impl.portal.Settings, and therefore libadwaita apps
ignore gsettings/dconf (such as preferred color-scheme).

See flatpak/xdg-desktop-portal#1199,
flatpak/xdg-desktop-portal#1102,
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050981
@smcv
Copy link
Collaborator Author

smcv commented May 3, 2024

This only handles interfaces that use find_portal_implementation, but org.freedesktop.impl.portal.Settings is a special case that uses find_all_portal_implementations and that function won't add xdg-desktop-portal-gtk to the list unless it's explicitly requested in a portals.conf file. So in the absence of portals.conf, there will be no impl for org.freedesktop.impl.portal.Settings despite xdg-desktop-portal-gtk being a perfectly valid fallback impl for it.

Please open a separate issue rather than replying to a closed pull request.

Any chance the fallback can be extended to org.freedesktop.impl.portal.Settings?

If there is a reasonable way to describe the desired fallback, and it's implementable, then I think that would make sense. Perhaps you could open a pull request?

The consequence is that users of these lightweight desktop environments aren't for example able to override org.freedesktop.appearance.color-scheme for libadwaita applications

If either the desktop environment or an individual user wants to provide a portals.conf that expresses a preference, that is certainly something that they can do. For example, dwm as packaged in Debian

@smcv
Copy link
Collaborator Author

smcv commented May 3, 2024

If there is a reasonable way to describe the desired fallback

I think there are two things that could be considered reasonable fallbacks for find_all_portal_implementations(): "the fallback is a list containing only -gtk", or "the fallback is a list of every installed portal backend that implements Settings". The latter is what happened in 1.16.x and earlier, and is maybe a better answer for Settings than just -gtk (but if you're the one implementing this, it's up to you to choose something reasonable).

Please bear in mind, though, that these last-resort fallbacks are a last resort, and if you want full control, there are better ways to achieve that control (like having your minimal desktop environment announce a name for itself and select one or more suitable Settings backends, similar to what was done in the Debian packaging for dwm).

@liskin
Copy link

liskin commented May 4, 2024

Perhaps you could open a pull request?

I'll see what I can do (tomorrow-ish) 🙂

I think there are two things that could be considered reasonable fallbacks for find_all_portal_implementations(): "the fallback is a list containing only -gtk", or "the fallback is a list of every installed portal backend that implements Settings". The latter is what happened in 1.16.x and earlier, and is maybe a better answer for Settings than just -gtk (but if you're the one implementing this, it's up to you to choose something reasonable).

Oh, this is helpful. Gives me some thoughts to start with. I've also discovered a couple other related issues and pull requests discussing this, and I think the sentiment there was that "every installed portal backend that implements Settings" isn't desirable because x-d-p-gnome misbehaves when not running in an actual GNOME session. So I'm leaning towards making find_all_portal_implementations behave as close to find_portal_implementation as possible, as that seems the least suprising. That would mean "the fallback is a list containing only -gtk", and perhaps even to look for the UseIn stuff before the last resort fallback to -gtk.

Please bear in mind, though, that these last-resort fallbacks are a last resort, and if you want full control, there are better ways to achieve that control (like having your minimal desktop environment announce a name for itself and select one or more suitable Settings backends, similar to what was done in the Debian packaging for dwm).

As a user, I understand that and I obviously created a local portals.conf as soon as I discovered what the issue is. Still, I was very surprised to find that org.freedesktop.impl.portal.Settings doesn't have a fallback while everything else does. Surprises are bad.

As a maintainer of a lightweight window manager that happens to work (and be used) in multiple desktop environments, I'm not really in a position to ship a portals.conf, I can merely document that one is needed. But then, one is needed only because x-d-p doesn't consistently select a fallback impl for all interfaces. From this point of view, it seems better to fix this upstream rather than downstream—make x-d-p behave consistently, so users running a window manager outside of a desktop environment don't need to be surprised.

I mean, if the whole fallback mechanism wasn't there, I'd notice pretty quickly that I need a portals.conf. A half-functioning fallback, however, takes a while to diagnose… (I'm almost ashamed to admit how long it took me)
So if x-d-p upstream ended up accepting the fallback mechanism (I understand there was quite a discussion about this back then), there shouldn't really be opposition to making it work consistently—I hope. 🙂

Anyway, PR tomorrow-ish.

liskin added a commit to liskin/xdg-desktop-portal that referenced this pull request May 10, 2024
Since 1.18.2, we have a hardcoded fallback to xdg-desktop-portal-gtk as
a last resort if no other portal is defined by a configuration file.
This fallback mechanism, however, isn't implemented for the
org.freedesktop.impl.portal.Settings interface.

The consequence is that everything seems to work just fine, but user
preferences like color scheme are ignored in applications that use the
portal API to retrieve them (not just flatpak apps, also libadwaita
apps). That comes as a surprise to users; and since the portal stuff
otherwise works fine, it's not entirely straightforward to figure out
that configuring portal.conf would fix it.

I believe that if find_portal_implementation implements fallbacks, so
should find_all_portal_implementations, for consistency and to avoid
surprises.

The fallback logic this commit implements closely resembles the logic in
find_portal_implementation: we don't look for fallbacks if there is a
portal configured in portals.conf (or if portals.conf explicitly says
there should be none), and we only use the x-d-p-gtk as a last resort if
we haven't found any impl(s) using the legacy UseIn key.

(The above should be a self-contained description but there's some
additional thoughts at
flatpak#1199 (comment))

Fixes: d18c563 ("portal-impl: Hard-code x-d-p-gtk as a last-resort fallback")
Related: flatpak#1102
@liskin
Copy link

liskin commented May 10, 2024

Anyway, PR tomorrow-ish.

#1358

(apologies for adding another comment to a closed PR, this is the last one now that we have a PR 😃)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config Issues with the portal configuration mechanism
Projects
Status: Triaged
Status: Done
Development

Successfully merging this pull request may close these issues.

Mechanism to retain compatibility across x-d-p 1.16 -> 1.17
5 participants