fix: calling of X11 functions when running under Wayland #33355

p2004a · 2022-03-20T19:53:43Z

Description of Change

TLDR: I primarily try to address the #33325 and #32436 issues with this PR.

In #30814 there was fe40088 committed that removed all if (!features::IsUsingOzonePlatform()) { blocks. Those blocks were guarding calls to X11 functions under wayland and were added as part of #26022. Calling X11 functions under wayland can cause the application to crash (as visible in #33325 and #32436).

This PR:

Replaces USE_X11 with USE_OZONE blocks as ozone is the only build of chrome nowadays (Ref: [Bug]: segfault on startup at gpu_init.cc under native Wayland #32436 (comment)) and only call X11 code only when running under X11. I believe the functionality hidden on non-x11 platform is not critical and this is overall net positive to current state.
Replaces WindowStateWatcher with ozone native implementation in ElectronDesktopWindowTreeHostLinux: it is always instantiated under ozone, but the wayland decorations remain guarded by the WaylandWindowDecorations feature flag (As was added in fix: Add support for Wayland window decorations #29618).

This makes the maximize, enter-full-screen, etc. events of BrowserWindow work under wayland.
Eliminates the X11 calls in file and message dialogs. Frankly I'm not sure if call to gtk_window_present is needed at all: under my system it's noop, but maybe there are DEs where it matters, so keeping it.

Checklist

PR description included and stakeholders cc'd
npm test passes: kind of ~~I can't make the tests even start running [1]~~
PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

notes: Fixed multiple issues when running under Wayland caused by calling X11 functions.

[1]: clean e init --bootstrap testing followed by e test I failing for me with Running "ninja -j 200 node:headers" in /home/p2004a/Workspace/electron/e/src/out/Testing ninja: error: unknown target 'node:headers', did you mean 'node_headers_1'?, I can't make it work :(, I was testing functions with my own draft app. I hope CI run as part of this PR will handle this omission.

welcome · 2022-03-20T19:53:45Z

💖 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.

codebytere · 2022-03-21T13:50:12Z

This also addresses #31382 cc @deepak1556

dsanders11

Added some comments. I haven't tested this, just adding some suggested changes.

[1]: clean e init --bootstrap testing followed by e test I failing for me with Running "ninja -j 200 node:headers" in /home/p2004a/Workspace/electron/e/src/out/Testing ninja: error: unknown target 'node:headers', did you mean 'node_headers_1'?, I can't make it work :(, I was testing functions with my own draft app. I hope CI run as part of this PR will handle this omission.

Thanks for mentioning that! Fixing it over in electron/build-tools#351.

shell/browser/ui/file_dialog_gtk.cc

shell/browser/ui/message_box_gtk.cc

shell/browser/native_window_views.cc

shell/browser/ui/electron_desktop_window_tree_host_linux.h

p2004a · 2022-03-22T21:26:10Z

I've rebased the branch because there was a merge conflict.

Thank you @dsanders11 for taking a look and fixing build tools!

I've run the tests and I've got exactly the same result on the origin:main and on my p2004a:wayland-fixes branches so I don't think I've broken anything ;). It doesn't mean that tests pass though, 10 tests don't, and those are the same tests on both main and my branch: https://gist.github.com/p2004a/480d7248de118c86c204c785bd05a9fd. I don't plan on debugging those here.

shell/browser/native_window_views.cc

shell/browser/ui/file_dialog_gtk.cc

shell/browser/native_window_views.cc

deepak1556

Thanks for working on this, really appreciate it!

Reviews are based on first pass on the PR,

Can you also remove the USE_X11 define from

electron/BUILD.gn

Line 562 in 8ad1470

"USE_X11=1",
Can you apply the changes from https://chromium-review.googlesource.com/c/chromium/src/+/2289772/ and https://chromium-review.googlesource.com/c/chromium/src/+/2317185 to GlobalMenuBarX11 which better aligns with upstream. Should end up in a state similar to https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/ui/BUILD.gn;l=3873-3882

shell/browser/native_window_views.cc

shell/browser/ui/file_dialog_gtk.cc

p2004a · 2022-03-23T21:00:54Z

Can you also remove the USE_X11 define from

Done, I also had to cleanup usage in shell/browser/ui/gtk/menu_util.cc

Can you apply the changes from upstream to GlobalMenuBarX11

I would prefer to not do it as part of this PR, there are many more places where X11 code needs to dropped and replaced with proper ozone abstractions.

deepak1556

LGTM with minor change.

shell/browser/ui/electron_desktop_window_tree_host_linux.cc

deepak1556

@p2004a can you rebase to resolve the conflicts, thanks!

trop · 2022-03-29T10:50:58Z

@deepak1556 has manually backported this PR to "18-x-y", please check out #33498

Backports #33355

trop · 2022-03-29T11:32:49Z

@deepak1556 has manually backported this PR to "17-x-y", please check out #33499

…3498) Backports #33355 Co-authored-by: Marek Rusinowski <marekrusinowski@gmail.com>

Backports #33355

prusnak · 2022-04-01T11:21:08Z

ISTM this still has to be backported to 16.x.x

deepak1556 · 2022-04-01T11:35:22Z

The PR has not been backported to 16-x-y yet, I didn't spend time on it because 16-x-y also requires the following #32603 for the runtime to work on wayland. If anyone is interested to work on both these backports, feel free to!

) * fix: don't call X11 functions in file dialog and message box * refactor: remove unused GtkUiPlatform declaration * fix: set gtk darktheme only when running under X11 * fix: replace X11 window state watcher with implementation using ozone * fix: make sure global menu barr is used only when supported * fix: don't call X11 function in native window views under wayland * style: fix lint issues * fix: use GtkUiPlatform::ShowGtkWindow instead of gtk_window_present directly * refactor: extract CreateGlobalMenuBar into separate function * refactor: move checking for WaylandWindowDecorations inside class * fix: check if we run under X11 only in ozone build * refactor: drop including unused ui/base/ui_base_features.h header * fix: modify ui_gtk_public_header.patch to also export gtk_ui.h * fix: refactor guarding of X11 calls - Introduce patch exposing new electron_can_call_x11 property - Replace defined(USE_OZONE) with BUILDFLAG(OZONE_PLATFORM_X11) flags * fix: remove the last remaining usage of USE_X11 * fix: usage of BUILDFLAG(OZONE_PLATFORM_X11) not building on non ozone * fix: call UpdateWindowState from OnBoundsChanged only under X11

electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Mar 20, 2022

p2004a changed the title ~~fix: Fix calling of X11 functions when running under Wayland~~ fix: fix calling of X11 functions when running under Wayland Mar 20, 2022

miniak changed the title ~~fix: fix calling of X11 functions when running under Wayland~~ fix: calling of X11 functions when running under Wayland Mar 20, 2022

codebytere requested review from dsanders11, ckerr and deepak1556 March 21, 2022 13:50

dsanders11 mentioned this pull request Mar 22, 2022

fix: properly handle target argument in e build electron/build-tools#351

Merged

dsanders11 reviewed Mar 22, 2022

View reviewed changes

p2004a force-pushed the wayland-fixes branch from 4c01845 to 8c94ba6 Compare March 22, 2022 20:46

p2004a requested a review from dsanders11 March 22, 2022 20:49

zcbenz added semver/patch backwards-compatible bug fixes target/16-x-y labels Mar 22, 2022

electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Mar 22, 2022

zcbenz reviewed Mar 23, 2022

View reviewed changes

shell/browser/native_window_views.cc Outdated Show resolved Hide resolved

shell/browser/ui/file_dialog_gtk.cc Show resolved Hide resolved

shell/browser/native_window_views.cc Show resolved Hide resolved

deepak1556 reviewed Mar 23, 2022

View reviewed changes

shell/browser/native_window_views.cc Outdated Show resolved Hide resolved

shell/browser/ui/file_dialog_gtk.cc Outdated Show resolved Hide resolved

p2004a requested review from a team as code owners March 23, 2022 19:37

p2004a requested review from zcbenz and deepak1556 March 23, 2022 21:01

deepak1556 reviewed Mar 24, 2022

View reviewed changes

shell/browser/ui/electron_desktop_window_tree_host_linux.cc Outdated Show resolved Hide resolved

p2004a requested a review from deepak1556 March 24, 2022 19:34

deepak1556 approved these changes Mar 25, 2022

View reviewed changes

p2004a added 2 commits March 25, 2022 08:38

fix: don't call X11 functions in file dialog and message box

eb565f7

refactor: remove unused GtkUiPlatform declaration

08cb0bc

trop bot added in-flight/18-x-y and removed needs-manual-bp/18-x-y labels Mar 29, 2022

deepak1556 added a commit that referenced this pull request Mar 29, 2022

fix: calling of X11 functions when running under Wayland

3c973f1

Backports #33355

deepak1556 mentioned this pull request Mar 29, 2022

fix: calling of X11 functions when running under Wayland #33499

Merged

trop bot added in-flight/17-x-y and removed needs-manual-bp/17-x-y labels Mar 29, 2022

proletarius101 mentioned this pull request Mar 29, 2022

Add native Wayland support such that it doesn't need to be blurry on HiDPI devices standardnotes/desktop#707

Open

2 tasks

codebytere pushed a commit that referenced this pull request Mar 29, 2022

fix: calling of X11 functions when running under Wayland (#33355) (#3…

5c3d6aa

…3498) Backports #33355 Co-authored-by: Marek Rusinowski <marekrusinowski@gmail.com>

trop bot added merged/18-x-y and removed in-flight/18-x-y labels Mar 29, 2022

jkleinsc pushed a commit that referenced this pull request Mar 29, 2022

fix: calling of X11 functions when running under Wayland (#33499)

db0ad4f

Backports #33355

trop bot added merged/17-x-y and removed in-flight/17-x-y labels Mar 29, 2022

Xelphos mentioned this pull request Mar 30, 2022

Vscode does not launch in wayland session microsoft/vscode#146349

Closed

teutat3s mentioned this pull request Mar 31, 2022

Update electron to 17.3.1, important upstream wayland fixes signalapp/Signal-Desktop#5847

Closed

7 tasks

ckerr mentioned this pull request Mar 31, 2022

Remove usage of USE_X11 #31382

Closed

5 tasks

prusnak mentioned this pull request Apr 1, 2022

chore: cherry-pick dd4c3ddadbb9 from chromium #32603

Closed

chrhasse mentioned this pull request Apr 2, 2022

[Bug]: app_id not set when running Electron with native Wayland #33578

Closed

3 tasks

multun mentioned this pull request Apr 6, 2022

Can't launch in wayland now. VSCodium/vscodium#1035

Closed

2 tasks

p2004a mentioned this pull request May 10, 2022

Update electron from 17.1.2 to 17.4.3 gajop/spring-launcher#108

Merged

commit-lint bot mentioned this pull request May 18, 2022

[pull] main from electron:main Blaquez-home/electron#3

Open

DoodlesEpic mentioned this pull request Jul 7, 2022

Black Screen GNOME 41.6 and 42.0 Wayland bitwarden/clients#2661

Closed

DoodlesEpic mentioned this pull request Jul 18, 2022

Logseq blank screen Flatpak logseq/logseq#5642

Closed

crissi-tuxedo mentioned this pull request Mar 11, 2024

[Gentoo] Electron UI appears blank in Wayland tuxedocomputers/tuxedo-control-center#294

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: calling of X11 functions when running under Wayland #33355

fix: calling of X11 functions when running under Wayland #33355

p2004a commented Mar 20, 2022 •

edited

welcome bot commented Mar 20, 2022

codebytere commented Mar 21, 2022 •

edited

dsanders11 left a comment

p2004a commented Mar 22, 2022

deepak1556 left a comment

p2004a commented Mar 23, 2022

deepak1556 left a comment

deepak1556 left a comment

trop bot commented Mar 29, 2022

trop bot commented Mar 29, 2022

prusnak commented Apr 1, 2022

deepak1556 commented Apr 1, 2022

fix: calling of X11 functions when running under Wayland #33355

fix: calling of X11 functions when running under Wayland #33355

Conversation

p2004a commented Mar 20, 2022 • edited

Description of Change

Checklist

Release Notes

welcome bot commented Mar 20, 2022

codebytere commented Mar 21, 2022 • edited

dsanders11 left a comment

Choose a reason for hiding this comment

p2004a commented Mar 22, 2022

deepak1556 left a comment

Choose a reason for hiding this comment

p2004a commented Mar 23, 2022

deepak1556 left a comment

Choose a reason for hiding this comment

deepak1556 left a comment

Choose a reason for hiding this comment

trop bot commented Mar 29, 2022

trop bot commented Mar 29, 2022

prusnak commented Apr 1, 2022

deepak1556 commented Apr 1, 2022

p2004a commented Mar 20, 2022 •

edited

codebytere commented Mar 21, 2022 •

edited