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: Use fallback portals for Settings as well #1358

Merged
merged 2 commits into from
Oct 30, 2024

Conversation

liskin
Copy link
Contributor

@liskin liskin commented 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 #1199 (comment))

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

@tommie
Copy link

tommie commented Aug 13, 2024

I like this. I just spent an hour or two figuring out why GNOME Software Center refused to go into dark mode under XFCE: https://tommie.github.io/a/2024/08/xfce-gnome-dark-mode

Turns out it was because Settings requires the portals.conf (Xubuntu 24.04; XDP 1.14.4) and libadwaita seems to not fall back on gsettings once it's tried XDP.

However, I'm curious why the UseIn is deprecated. Is there a replacement? Assuming one machine supports multiple desktop environments for users to choose from, it seems like a global configuration isn't expressive enough. The workaround is a user-specific configuration. Forcing users to create a config file feels like a regression.

@smcv
Copy link
Collaborator

smcv commented Aug 14, 2024

However, I'm curious why the UseIn is deprecated. Is there a replacement? Assuming one machine supports multiple desktop environments for users to choose from, it seems like a global configuration isn't expressive enough.

Agreed, and that's why there's a per-desktop-environment configuration (which supersedes UseIn), documented in https://flatpak.github.io/xdg-desktop-portal/docs/portals.conf.html. XFCE provides an xfce-portals.conf since https://gitlab.xfce.org/xfce/xfce4-session/-/issues/181; if that hasn't made it into Xubuntu, please report that to your distro vendor.

@tommie
Copy link

tommie commented Aug 14, 2024

xfce-portals.conf

Oh, sorry, I missed the part where it said that the prefix is matched against the desktop name. So that's the replacement.

Thanks for the link!

@tommie
Copy link

tommie commented Aug 14, 2024

Apologies for the tangential noise on this PR. Just to close this off:

Ubuntu 24.04 is unlucky to have xfce4-session 4.18.3, while the new config file is in 4.18.4. 4.18.4 is in Debian testing.

I also found your https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050802.

So I filed https://bugs.launchpad.net/ubuntu/+source/xfce4-session/+bug/2076999.

src/portal-impl.c Outdated Show resolved Hide resolved
src/portal-impl.c Outdated Show resolved Hide resolved
src/portal-impl.c Outdated Show resolved Hide resolved
src/portal-impl.c Outdated Show resolved Hide resolved
@swick
Copy link
Contributor

swick commented Oct 4, 2024

The change seems fine to me but I'd like @ebassi opinion on it.

@liskin
Copy link
Contributor Author

liskin commented Oct 5, 2024

Rebased on top on today's main and applied the suggestions.

Copy link
Member

@GeorgesStavracas GeorgesStavracas left a comment

Choose a reason for hiding this comment

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

Superficial comments only, but I also would like to hear what @ebassi thinks of it

src/portal-impl.c Outdated Show resolved Hide resolved
src/portal-impl.c Outdated Show resolved Hide resolved
src/portal-impl.c Outdated Show resolved Hide resolved
src/portal-impl.c Outdated Show resolved Hide resolved
@swick
Copy link
Contributor

swick commented Oct 30, 2024

LGTM and ebassi approves

@GeorgesStavracas GeorgesStavracas added this to the 1.20 milestone Oct 30, 2024
@GeorgesStavracas GeorgesStavracas added the config Issues with the portal configuration mechanism label Oct 30, 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
@GeorgesStavracas GeorgesStavracas enabled auto-merge (rebase) October 30, 2024 12:34
@GeorgesStavracas GeorgesStavracas merged commit 4f928ee into flatpak:main Oct 30, 2024
5 checks passed
@liskin liskin deleted the settings-fallback branch November 1, 2024 08:57
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
Development

Successfully merging this pull request may close these issues.

6 participants