fix: create fake parent GtkWindow for file dialog

[tristan957]

Support for the XDG Desktop Portal first occurred in #19159, authored by me. Unfortunately there was a regression on the way that the file dialog was now presented. Normally, Chromium sets the "transient-for" property on the dialog using gtk::SetGtkTransientFor(). Unfortunately the work in the previous PR couldn't use this function because a GtkFileChooserNative is not a GtkWidget, which is required by the function, so the effects of window transiency were lost on platforms where GTK was >= 3.20.

BUT! The lack of parent GTK window for gtk_file_chooser_native_new() can be corrected by creating a fake GtkWindow. Given a GdkScreen and a type of GTK_WINDOW_TOPLEVEL, we are able to create GtkWindow that has an xid or Wayland handle of the Electron window. From here, we can pass the fake parent to the function. Instead of Chromium setting the transiency of the file chooser, the XDG Desktop Portal will now adequately be able to set window transiency.

Fixes https://github.com/electron/electron/issues/32857

Description of Change

Checklist

• [x] PR description included and stakeholders cc'd
• [x] npm test passes
• [x] tests are changed or added
• [x] relevant documentation is changed or added
• [x] PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: On Linux platforms with a GTK >= 3.20 using the XDG Desktop Portal, file dialogs will no longer appear behind their parent windows nor will they be movable as was the case prior to Electron v14.0.0.

[tristan957]

I need help testing this PR. CC @deepak1556

I keep running into this build issue:

[422/86805] ACTION //electron:electron_asar_bundle(//build/toolchain/linux:clang_x64)
FAILED: gen/electron/js2c/asar_bundle.js
python3 ../../../electron/build/npm-run.py --silent webpack -- --config /home/tristan957/Projects/electron/src/electron/build/webpack/webpack.config.asar.js --output-filename=asar_bundle.js --output-path=/home/tristan957/Projects/electron/src/electron/out/Testing/gen/electron/js2c --env.buildflags=/home/tristan957/Projects/electron/src/electron/out/Testing/gen/electron/buildflags/buildflags.h --env.mode=development
NPM script 'webpack' failed with code '1':
/home/tristan957/Projects/electron/src/electron/node_modules/loader-runner/lib/LoaderRunner.js:133
                if(isError) throw e;
                            ^

Error: error:0308010C:digital envelope routines::unsupported
    at new Hash (node:internal/crypto/hash:67:19)
    at Object.createHash (node:crypto:130:10)
    at module.exports (/home/tristan957/Projects/electron/src/electron/node_modules/webpack/lib/util/createHash.js:135:53)
    at NormalModule._initBuildHash (/home/tristan957/Projects/electron/src/electron/node_modules/webpack/lib/NormalModule.js:417:16)
    at handleParseError (/home/tristan957/Projects/electron/src/electron/node_modules/webpack/lib/NormalModule.js:471:10)
    at /home/tristan957/Projects/electron/src/electron/node_modules/webpack/lib/NormalModule.js:503:5
    at /home/tristan957/Projects/electron/src/electron/node_modules/webpack/lib/NormalModule.js:358:12
    at /home/tristan957/Projects/electron/src/electron/node_modules/loader-runner/lib/LoaderRunner.js:373:3
    at iterateNormalLoaders (/home/tristan957/Projects/electron/src/electron/node_modules/loader-runner/lib/LoaderRunner.js:214:10)
    at iterateNormalLoaders (/home/tristan957/Projects/electron/src/electron/node_modules/loader-runner/lib/LoaderRunner.js:221:10)
    at /home/tristan957/Projects/electron/src/electron/node_modules/loader-runner/lib/LoaderRunner.js:236:3
    at context.callback (/home/tristan957/Projects/electron/src/electron/node_modules/loader-runner/lib/LoaderRunner.js:111:13)
    at makeSourceMapAndFinish (/home/tristan957/Projects/electron/src/electron/node_modules/ts-loader/dist/index.js:59:5)
    at successLoader (/home/tristan957/Projects/electron/src/electron/node_modules/ts-loader/dist/index.js:41:5)
    at Object.loader (/home/tristan957/Projects/electron/src/electron/node_modules/ts-loader/dist/index.js:24:5)
    at LOADER_EXECUTION (/home/tristan957/Projects/electron/src/electron/node_modules/loader-runner/lib/LoaderRunner.js:119:14)
    at runSyncOrAsync (/home/tristan957/Projects/electron/src/electron/node_modules/loader-runner/lib/LoaderRunner.js:120:4)
    at iterateNormalLoaders (/home/tristan957/Projects/electron/src/electron/node_modules/loader-runner/lib/LoaderRunner.js:232:2)
    at Array.<anonymous> (/home/tristan957/Projects/electron/src/electron/node_modules/loader-runner/lib/LoaderRunner.js:205:4)
    at Storage.finished (/home/tristan957/Projects/electron/src/electron/node_modules/enhanced-resolve/lib/CachedInputFileSystem.js:43:16)
    at /home/tristan957/Projects/electron/src/electron/node_modules/enhanced-resolve/lib/CachedInputFileSystem.js:79:9
    at /home/tristan957/Projects/electron/src/electron/node_modules/graceful-fs/graceful-fs.js:90:16
    at FSReqCallback.readFileAfterClose [as oncomplete] (node:internal/fs/read_file_context:68:3) {
  opensslErrorStack: [ 'error:03000086:digital envelope routines::initialization error' ],
  library: 'digital envelope routines',
  reason: 'unsupported',
  code: 'ERR_OSSL_EVP_UNSUPPORTED'
}

so I couldn't test my changes. I tried disabling the SSL check, but that didn't work either. In order to verify that this commit does what it says it does, be on a platform which is using the XDG Desktop Portal with GTK >= 3.20. Create an electron application and then run a file dialog. If the file dialog is movable or appears behind the parent window, you have found the bug. Then checkout this branch. Repeat the steps. If and only if the parent no longer appears behind the parent window and is no longer movable, the bug has been fixed.

Presumably this is the check that Electron is failing on the xdg-desktop-portal-gtk side of things: https://github.com/flatpak/xdg-desktop-portal-gtk/blob/main/src/filechooser.c#L469.

[tristan957]

This PR should be backported for all electron versions still supported that are >=14.0.0

[tristan957]

Ping @zcbenz since you merged my last PR.

[deepak1556]

Thanks @tristan957 , I can test this PR locally tomorrow.

QQ: Does setting the fake parent as transient for the Electron window help with screen readers ? Or will that issue be not addressable with this fix ? Refs https://github.com/microsoft/vscode/issues/121811#issuecomment-1070926272

[codebytere]

@tristan957 this is happening becuase your local Node.js version is too new for Webpack - it's not an Electron issue. Try using v16 LTS, it should work.

See https://stackoverflow.com/questions/69394632/webpack-build-failing-with-err-ossl-evp-unsupported

[tristan957]

@codebytere thanks! I will now set my personal computer off to build electron!

I have a fix for the asan failure locally, I believe. Will shoot that off if I can confirm the commit.

Regarding the screen reader issue, maybe. One could assume that transiency has something to do with screen readers.

If someone could point me at an example program, which just launches a window, and then a file dialog on a button press or something, that would be great. Otherwise, I'll read some electron docs. Should be able to confirm the fix today.

[deepak1556]

The gist from https://github.com/electron/electron/issues/32857#issue-1130587899 can be used to test the fix.

[tristan957]

Another build failure 37000 artifacts later:

[30/54204] ACTION //ui/webui/resources/cr_elements/cr_checkbox:closure_compile_module(//build/toolchain/linux:clang_x64)
FAILED: gen/ui/webui/resources/cr_elements/cr_checkbox/closure_compile_module.js
python3 ../../../third_party/closure_compiler/js_binary.py --compiler ../../../third_party/closure_compiler/compiler/compiler.jar --output gen/ui/webui/resources/cr_elements/cr_checkbox/closure_compile_module.js --deps gen/ui/webui/resources/cr_elements/cr_checkbox/cr_checkbox.m.js_library --sources --checks-only --flags jscomp_error=accessControls jscomp_error=checkTypes jscomp_error=checkVars jscomp_error=constantProperty jscomp_error=deprecated jscomp_error=externsValidation jscomp_error=globalThis jscomp_error=invalidCasts jscomp_error=misplacedTypeAnnotation jscomp_error=missingProperties jscomp_error=missingReturn jscomp_error=nonStandardJsDocs jscomp_error=suspiciousCode jscomp_error=undefinedNames jscomp_error=undefinedVars jscomp_error=unknownDefines jscomp_error=uselessCode jscomp_error=visibility compilation_level=SIMPLE_OPTIMIZATIONS generate_exports=false extra_annotation_name=attribute extra_annotation_name=demo extra_annotation_name=element language_in=ECMASCRIPT_2017 language_out=ECMASCRIPT5_STRICT jscomp_off=duplicate js_module_root=../../ui/webui/resources/ js_module_root=gen/ui/webui/resources/ module_resolution=BROWSER_WITH_TRANSFORMED_PREFIXES browser_resolver_prefix_replacements=\"chrome://resources/=./\" browser_resolver_prefix_replacements=\"//resources/=./\" browser_resolver_prefix_replacements=\"../polymer/polymer_bundled.min.js=../polymer/polymer_bundled.js\" browser_resolver_prefix_replacements=\"chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js=../../third_party/polymer/v3_0/components-chromium/polymer/polymer_bundled.js\" browser_resolver_prefix_replacements=\"//resources/polymer/v3_0/polymer/polymer_bundled.min.js=../../third_party/polymer/v3_0/components-chromium/polymer/polymer_bundled.js\" browser_resolver_prefix_replacements=\"chrome://resources/polymer/v3_0/=../../third_party/polymer/v3_0/components-chromium/\" browser_resolver_prefix_replacements=\"//resources/polymer/v3_0/=../../third_party/polymer/v3_0/components-chromium/\" hide_warnings_for=externs.zip hide_warnings_for=third_party/polymer/v3_0/components-chromium/ polymer_version=2 --externs ../../../third_party/closure_compiler/externs/chrome.js
../../../third_party/closure_compiler/compiler/compiler.jar --jscomp_error=accessControls --jscomp_error=checkTypes --jscomp_error=checkVars --jscomp_error=constantProperty --jscomp_error=deprecated --jscomp_error=externsValidation --jscomp_error=globalThis --jscomp_error=invalidCasts --jscomp_error=misplacedTypeAnnotation --jscomp_error=missingProperties --jscomp_error=missingReturn --jscomp_error=nonStandardJsDocs --jscomp_error=suspiciousCode --jscomp_error=undefinedNames --jscomp_error=undefinedVars --jscomp_error=unknownDefines --jscomp_error=uselessCode --jscomp_error=visibility --compilation_level=SIMPLE_OPTIMIZATIONS --generate_exports=false --extra_annotation_name=attribute --extra_annotation_name=demo --extra_annotation_name=element --language_in=ECMASCRIPT_2017 --language_out=ECMASCRIPT5_STRICT --jscomp_off=duplicate --js_module_root=../../ui/webui/resources/ --js_module_root=gen/ui/webui/resources/ --module_resolution=BROWSER_WITH_TRANSFORMED_PREFIXES --browser_resolver_prefix_replacements="chrome://resources/=./" --browser_resolver_prefix_replacements="//resources/=./" --browser_resolver_prefix_replacements="../polymer/polymer_bundled.min.js=../polymer/polymer_bundled.js" --browser_resolver_prefix_replacements="chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js=../../third_party/polymer/v3_0/components-chromium/polymer/polymer_bundled.js" --browser_resolver_prefix_replacements="//resources/polymer/v3_0/polymer/polymer_bundled.min.js=../../third_party/polymer/v3_0/components-chromium/polymer/polymer_bundled.js" --browser_resolver_prefix_replacements="chrome://resources/polymer/v3_0/=../../third_party/polymer/v3_0/components-chromium/" --browser_resolver_prefix_replacements="//resources/polymer/v3_0/=../../third_party/polymer/v3_0/components-chromium/" --hide_warnings_for=externs.zip --hide_warnings_for=third_party/polymer/v3_0/components-chromium/ --polymer_version=2 --externs=../../../third_party/polymer/v3_0/components-chromium/polymer/externs/closure-types.js --externs=../../../third_party/polymer/v3_0/components-chromium/polymer/externs/polymer-dom-api-externs.js --externs=../../../third_party/polymer/v3_0/components-chromium/polymer/externs/polymer-externs.js --externs=../../../third_party/polymer/v3_0/components-chromium/polymer/externs/shadycss-externs.js --externs=../../../third_party/polymer/v3_0/components-chromium/polymer/externs/webcomponents-externs.js --externs=../../../third_party/closure_compiler/externs/chrome.js --js_output_file gen/ui/webui/resources/cr_elements/cr_checkbox/closure_compile_module.js --js ../../../third_party/polymer/v3_0/components-chromium/polymer/polymer_bundled.js ../../../third_party/polymer/v3_0/components-chromium/iron-behaviors/iron-control-state.js ../../../third_party/polymer/v3_0/components-chromium/iron-a11y-keys-behavior/iron-a11y-keys-behavior.js ../../../third_party/polymer/v3_0/components-chromium/iron-behaviors/iron-button-state.js ../../../third_party/polymer/v3_0/components-chromium/paper-ripple/paper-ripple.js ../../../third_party/polymer/v3_0/components-chromium/paper-behaviors/paper-ripple-behavior.js gen/ui/webui/resources/cr_elements/cr_checkbox/cr_checkbox.m.js --checks-only
gen/ui/webui/resources/cr_elements/cr_checkbox/cr_checkbox.m.js:175:4: ERROR - [JSC_POLYMER_UNQUALIFIED_BEHAVIOR] Behaviors must be global names or qualified names that are declared as object literals or array literals of other valid Behaviors.
  175|     PaperRippleBehavior,

Something else I am missing?

[tristan957]

Is it possible to call gtk_widget_set_visible() without actually presenting a window? Would need to read more source code.

[tristan957]

Nope, not possible since gtk_widget_show() is called when gtk_widget_set_visible() is called.

[tristan957]

So from my perspective it seems the only solution is to resort to DBus...ugh

[tristan957]

Seems like libportal supports the File Chooser portal, would it be possible for electron to depend on libportal on Linux?

https://github.com/flatpak/libportal/blob/main/libportal/filechooser.h

[tristan957]

I guess not since Electron presumably still supports Ubuntu 18.04

[zcbenz]

Chromium does have an implementation of file chooser portal which can be copied: https://source.chromium.org/chromium/chromium/src/+/main:ui/shell_dialogs/select_file_dialog_linux_portal.cc

but I wonder if we should just change the code to use ui::SelectFileDialog directly.

[tristan957]

I'll take a look at this next week. Thanks for the tip.

but I wonder if we should just change the code to use ui::SelectFileDialog directly.

Do you mean just #include <ui/shell_dialogs/select_file_dialog_linux_portal.h>?

Is there any reason Electron can't just use the file dialogs that Chromium provides? Never quite understood why Electron has their own implementations.

[zcbenz]

Do you mean just #include <ui/shell_dialogs/select_file_dialog_linux_portal.h>?

Chromium has code to choose native dialog for different desktops, we should not use select_file_dialog_linux_portal.h directly. https://source.chromium.org/chromium/chromium/src/+/main:ui/shell_dialogs/shell_dialog_linux.cc;l=87;drc=01512dd6b2d0887b00ae8345c833d8c56d106cb4;bpv=0;bpt=1

Is there any reason Electron can't just use the file dialogs that Chromium provides? Never quite understood why Electron has their own implementations.

Chromium's dialog code was lacking features we need, but it was years ago and things have changed a lot.

[tristan957]

It would be interesting to re-evaluate this. Reducing the amount of duplicate effort on this front would be great.

[codebytere]

@tristan957 you're right and we have a bit - https://github.com/electron/electron/pull/30663 is one example

i can definitely try to look into this - we do need our own implementations at least for the dialog apis still since we expose more than Chromium, but there's absolutely some cleanup to be done imo

[tristan957]

@codebytere what functionality does Electron need on top of what Chromium offers?

[zcbenz]

I'm closing this PR since this approach can not fix the issue.