[wip] feat: use GtkFileChooserNative for GTK File Dialog

[denisfalqueto]

Closes #2911.

GtkFileChooserNative has support for FileChooser XDG Portal.
That allows for native file open/save dialogs on GTK and QT/Plasma
environments.

Signed-off-by: Denis Falqueto denisf@trt3.jus.br

Checklist

• PR description included and stakeholders cc'd
• npm test passes
• PR title follows semantic commit guidelines

Release Notes

Notes: Use native file chooser dialog for opening and saving files.

[welcome]

💖 Thanks for opening this pull request! 💖

We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix.

Examples of commit messages with semantic prefixes:

• fix: don't overwrite prevent_default if default wasn't prevented
• feat: add app.isPackaged() method
• docs: app.isDefaultProtocolClient is now available on Linux

Things that will help get your PR across the finish line:

• Follow the JavaScript, C++, and Python coding style.
• Run npm run lint locally to catch formatting errors earlier.
• Document any user-facing changes you've made following the documentation styleguide.
• Include tests when adding/changing behavior.
• Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

[ckerr]

I'm a little unsure about the use case for this patch and would like to get some discussion on it before it's merged.

First, this is a first submitter patch and I'm always happy to see people improving Linux support, so I'm happy to see this patch.

But I'm not certain it's wise to require GTK+ 3.20 or higher, as that would break out-of-the-box support for Ubuntu 16.04 and older.

I'm also not sure what the use case for this patch is: Electron apps running on Windows or macOS likely won't be using ui/file_dialog_gtk.cc in the first place, and so wouldn't the native support be moot?

[ssokolow]

@ckerr GTK+ provides a mechanism for indirecting native dialogs through a desktop-provided out-of-process common dialog handler when GtkFileChooserNative is used.

There are two main reasons to do this:

1. The mechanism makes it possible for desktops which don't use GTK+ file choosers, like KDE and LXQt, to supply their native file choosers to Electron applications. (Currently, when running outside of Flatpak, you need to set the GTK_USE_PORTAL environment variable to opt into this behaviour.)
2. It provides transparent integration with Flatpak sandboxing, since the privileged out-of-process dialog host can poke a hole in the sandbox before returning the path to the Electron app. (There is also work being done toward having Snappy share the same portal infrastructure.)

[TingPing]

But I'm not certain it's wise to require GTK+ 3.20 or higher, as that would break out-of-the-box support for Ubuntu 16.04 and older.

Would just have to be a run time check, thankfully its only two symbols.

[ckerr]

@TingPing, I'd be OK with a runtime check. How would this be done?

GTK_CHECK_VERSION is a compile-time test, so that's not going to work.

I suppose we could use dlopen + dlsym, but I'm not sure what the implications are of doing that when we already have a normal initialized GTK+ session?

[TingPing]

I suppose we could use dlopen + dlsym, but I'm not sure what the implications are of doing that when we already have a normal initialized GTK+ session?

Yup. It is safe:

man dlopen

If the same shared object is loaded again with dlopen(), the same object handle is
returned. The dynamic linker maintains reference counts for object handles, so a dynami‐
cally loaded shared object is not deallocated until dlclose() has been called on it as
many times as dlopen() has succeeded on it. Any initialization returns (see below) are
called just once.

[TingPing]

@TingPing if you could look into the linux CI failures, we can get this PR moving!

The dialog_ type has to be changed to GtkNativeDialog and everything that references it has to be updated accordingly:

electron/atom/browser/ui/file_dialog_gtk.cc

Line 158 in 65bd2ff

GtkWidget* dialog_;

(but the conversation about dynamically loading the symbols should probably be handled also)

[TingPing]

@TingPing ping, is there any progress on this?

I didn't write this patch and I don't have the time or desire to compile Electron. I pointed out the (obvious) issue with this PR so I guess the author isn't interested.

[codebytere]

apologies, tagged the wrong person!

@denisfalqueto ping, is there any progress on this?

[codebytere]

Closing as there has been no activity on this PR or response from the author.

Happy to re-open should there be more activity!

[tristan957]

I am interested in picking this PR up. I am confused as to why we can't use https://developer.gnome.org/gtk3/stable/gtk3-Feature-Test-Macros.html#gtk-get-minor-version for checking if the minor version is >= 20. Can this not be checked at compile time?

Do I need to run dlopen, dlsym to check the minor version because it can't be checked at compile time? This is an easy PR once that implementation detail is ironed out.

cc: @TingPing @ckerr

[tristan957]

I am going to fork electron and get to work on this. I am going to assume that I need to dynamically load the .so for now.

[tristan957]

Please refer to PR #19159