Add file chooser on wl, x11 and gtk4 #606
Conversation
joantolo
changed the title
joantolo
force-pushed
the
add-file-chooser
branch
from
542d331 to
42caf3e
Compare
There was a problem hiding this comment.
@joantolo Thanks for working on this! I actually like the approach of using libportal to “offload” the responsibility of providing a suitable file dialog out of Cog, good idea.
There are a couple of things I commented around, mainly:
- Use
g_autoptr(...)wherever possible. - Declare variables right before first use (C11/C++ style).
- Make
libportalsuage optional. - Use the
data/check-stylescript to check that your code following the coding style.
With the above ironed out, I would be super happy to have this PR merged.
Picking the parent-private.h header from libportal.h I think is fine because the header itself has a comment stating that the license is different for that source file, it is unmodified from the original, and does not result in machine code added to Cog—only the definitions from it are needed at compile time. IIUC this is allowed as “combined work” by the LGPL, but IANAL so if anyone has a clearer view on this comments are welcome.
platform/common/meson.build
Outdated
| @@ -1,10 +1,12 @@ | |||
| cogplatformcommon_dependencies = [ | |||
| cogcore_dep, | |||
| dependency('epoxy'), | |||
| dependency('libportal'), | |||
There was a problem hiding this comment.
This adds a new depdendency unconditionally, and I am quite sure the ARM cross-build toolchains do not contain libportal (@psaavedra can confirm). I think it would be better to make the dependency optional, with a feature declared in meson_options.txt and the code built only if enabled. Then we can add the packages easily to the native CI builds once we updated them to use Ubuntu 22.04, because currently we are on 20.04 which does not have a libportal package.
There was a problem hiding this comment.
To use it I've set a config variable when libportal is found.
joantolo
force-pushed
the
add-file-chooser
branch
from
42caf3e to
ad29b77
Compare
XdpPortal allows to communicate with all Portal interfaces. In this case it is used the FileChooser portal to get a file chooser. It is used when "run-file-chooser" signal from WebKitWebView is emitted. It should get a XdpParent to keep the new dialog always on top of cog.
On libportal-gtk4 there is a gtk4 implementation of XdpParent which allows keeping the dialog always on top.
This is a copy of parent-private.h from libportal. And should be in sync to allow the use of XdpParent functionality from libportal. This is needed for the next commits to implement a different XdpParent, for the moment one for wayland and another one for x11.
The XdpParent needs to know the "parent_window" string to keep the dialog on top. On wayland this "parent_window" can be obtained using the xdg-foreign protocol. This protocol is used on this XdpParent implementation. First it must call export_toplevel method, and then register a callback to receive a handle which is the "parent_window". To store the required data for this XdpParent implementation it is used the private "data" pointer property as a xdp_parent_wl_data. XdpParent isn't responsible of freeing it, so that data is handled at cog-platform-wl.
This adds a new implementation of XdpParent. To get the "parent_window" is simpler, it's just the window_id which is the window property of xcb.
joantolo
force-pushed
the
add-file-chooser
branch
from
ad29b77 to
cf8d6d8
Compare
|
Should I modify the copy of |
I think it's not needed. Let me try to find a way of making the style checker ignore certain files; if not I can merge the PR manually. |
Use the file chooser provided from the desktop using the
FileChooserinterface of Portal.The only way I could find to use
XdpParentforwlandx11was to add the parent-header.-private.h from libportal.And then add an implementation as it is done for gtk.
On the wayland platform it was needed the addition of
xdg-foreigninterface which is needed byXdpParent. You can see the commits for a more detailed info.I'm open to suggestions.
This fixes the file chooser part of #594.