xdg-desktop-portal
xdg-desktop-portal copied to clipboard
flatpak
portal-impl: fix config ordering
Quoting portals.conf(5):
Each key in the group contains a semi-colon separated list of portal backend implementation, to be searched for an implementation of the requested interface, in the same order as specified in the configuration file.
But this wasn't actually true. If the portals were set to z;y and z and y both implemented the same portal, y would be used.
Fixing this requires reworking how portals are selected — going through the config file and selecting the first available configured portal, rather than going through each known portal and checking whether it's allowed in the config file.
find_all_portal_implementations() is unchanged. The portal-first approach is fine for it, and it would be difficult for it to share as much code with find_portal_implementation() now that it's written to stop as soon as a configured portal is found.
Fixes: https://github.com/flatpak/xdg-desktop-portal/issues/1111
(I'm not sure to what extent https://github.com/flatpak/xdg-desktop-portal/commit/1394bb2817254eaac20648db7235b01bc8c69666 affected this — there's definitely some overlap, because I also fixed that bug. But given it's caused complicated merge conflicts, and it was merged instead of this despite my PR being open way longer, I want to feel confident that's not going to happen again before I put any more time in.)
(I'm not sure to what extent 1394bb2 affected this — there's definitely some overlap, because I also fixed that bug. But given it's caused complicated merge conflicts, and it was merged instead of this despite my PR being open way longer, I want to feel confident that's not going to happen again before I put any more time in.)
Hi @alyssais, I'm the author of 1394bb2. I was aware of this PR when I wrote that patch, and I tried it out see if the bug was already fixed (it didn't).
I think the only reason that my PR was merged faster than yours was that it was bumped up by several users from the Arch community.
I think this is a very important PR that I think should get merged since it has been the cause of bugs like this in the distribution that I contribute to. We currently use a very hacky workaround, which this PR would remove the need for.
@SoumyaRanjanPatnaik if you want to resolve the conflicts that'd be great! I had a go but I found it difficult because it's been so long I've forgotten a lot of the nuances I was trying to get right here.
@SoumyaRanjanPatnaik, if you can provide a separate PR that supersedes this one, using Co-authored-by to credit both yourself and @alyssais, that would probably be the way forward here.
@SoumyaRanjanPatnaik, if you can provide a separate PR that supersedes this one, using
Co-authored-byto credit both yourself and @alyssais, that would probably be the way forward here.
@smcv I've created a PR that supersedes this one.