flatpak / xdg-desktop-portal

Desktop integration portal
https://flatpak.github.io/xdg-desktop-portal/
GNU Lesser General Public License v2.1
568 stars 187 forks source link

portals.conf: The wrong portals are picked up #1417

Closed A6GibKm closed 1 day ago

A6GibKm commented 1 month ago

Operating System

Fedora

XDG Desktop Portal version

1.18

XDG Desktop Portal version (Other)

No response

Desktop Environment

GNOME

Desktop Environment (Other)

No response

Expected Behavior

The order in which portals implementations set in gnome-portal.conf should be followed.

If the config says, for example,

[preferred]
default=gnome;gtk;
org.freedesktop.impl.portal.Secret=oo7-portal;gnome-keyring;

oo7-portal should be selected.

Current Behavior

The order in which portals implementations set in gnome-portal.conf is not followed. In the example above gnome-keyring is selected.

Steps to Reproduce

In a GNOME based distro, setup a dummy portal:

$ cat /usr/share/xdg-desktop-portal/portals/oo7-portal.portal 
[portal]
DBusName=org.freedesktop.impl.portal.desktop.oo7
Interfaces=org.freedesktop.impl.portal.Secret;

and all the required files (dbus activation file, binary, etc)

Setup a config file ~/.config/gnome-portals.conf with contents:

[preferred]
default=gnome;gtk;
org.freedesktop.impl.portal.Secret=oo7-portal;gnome-keyring;

and start xdg-desktop-portal -r -v. It will print

...
XDP: Found 'gnome-keyring' in configuration for org.freedesktop.impl.portal.Secret
XDP: Using gnome-keyring.portal for org.freedesktop.impl.portal.Secret (config)

The fn

// src/portal-impl.c:238
static gint
sort_impl_by_use_in_and_name (gconstpointer a,
                              gconstpointer b)
{
  const PortalImplementation *pa = a;
  const PortalImplementation *pb = b;
  const char **desktops;
  int i;

  desktops = get_current_lowercase_desktops ();

  for (i = 0; desktops[i] != NULL; i++)
    {
      gboolean use_a = pa->use_in != NULL
                     ? g_strv_case_contains ((const char **)pa->use_in, desktops[i])
                     : FALSE;
      gboolean use_b = pb->use_in != NULL
                     ? g_strv_case_contains ((const char **)pb->use_in, desktops[i])
                     : FALSE;

      if (use_a != use_b)
        return use_b - use_a;
      else if (use_a)
        break;
      else
        continue;
    }

  return strcmp (pa->source, pb->source);
}

Will always pickup gnome-keyring because:

  1. The deprecated key UseIn=gnome is not set on the new portal
  2. oo7-portal goes after gnome-keyring alphabetically
  3. Even if the two above are fixed, gnome-keyring is added to the list before oo7-portal

As per man portals.conf:

org.freedesktop.impl.portal.* (string)

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.

alphabetical order should not take preference over the position in the config file.

I tested removing gnome-keyring from the config file and that works fine.

Anything else we should know?

Here is the actual portal impl https://github.com/bilelmoussaoui/oo7/pull/101

smcv commented 1 month ago

Did you label the expected behaviour and the current (actual) behaviour in your issue report the wrong way round? The text of the report doesn't seem to match the headings.

smcv commented 1 month ago

If my guess is correct and you've labelled expected behaviour and current behaviour the wrong way round, then this is probably the same issue that https://github.com/flatpak/xdg-desktop-portal/pull/1378 aims to solve.

A6GibKm commented 1 month ago

I edited it, yes it is the same issue. However I am not sure if the point about UseIn was raised previously (portals with it will always go first).

GeorgesStavracas commented 1 day ago

Fixed by https://github.com/flatpak/xdg-desktop-portal/pull/1378