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

Right, almost forgot: there are also 2 TODOs in the source, related to an accessibility string define that's present in Chromium but missing here + GetStringUTF16 crashing, does Electron..."trim" out accessibility strings from Chromium that it doesn't use?

One more note I forgot to add (geez I'm good at forgetting things to add to initial PR descriptions): there were a few values that had to be hardcoded from Adwaita's CSS, namely, the box-shadow and border-top-[left/right]-radius. For the latter in particular, my rationale is that both Yaru and Adwaita have a border-radius on the top of the window, thus omitting it would make the windows look out-of-place in what is quite likely the two most oft-used themes (Yaru is Ubuntu's default, Adwaita is the upstream GNOME default). On the other hand, this does mean that it still looks out of place in themes like Adapta that use sharp corners. I'm not sure if there's an ideal solution here, but I think this one is "good enough" for the moment (since there aren't any decorations right now).

On an unrelated note, it does seem that the Linux build failures are related to Goma issues, not this PR.

I think this is looking great!

I'm planning to build and test this locally but until I get around to do that I was wondering how is this going to affect compositors that support server-side decorations (e.g.: Plasma, sway, etc)?

Are they going to get duplicate decorations (one from the compositor and one from the client)?

I believe the server-side decoration protocol requires the client to essentially opt-in:

If compositor and client do not negotiate the use of a server-side decoration using this protocol, clients continue to self-decorate as they see fit.

From some stepping through the source code, it seems like this only occurs if remove_standard_frame is set to false. However, this PR ends up setting it to true unconditionally, thus as-is this should work "fine". That being said, losing the SSD isn't ideal, so I'll take a look into seeing if we can only enable CSD depending on whether or not a native frame is used.

@vially as of the latest commit, it should go back to using SSD on compositors that support it. I couldn't test with KDE, but I did test with Sway, where it appeared to work.

following along with excitement. thank you for taking on this complex task @refi64!

From some local testing, I believe this should be usable enough for general use. There are two outstanding open notes:

• The first Wayland patch isn't incredibly large, but it has already hit a merge conflict due to a rather unlucky renaming that impacted one of the few functions the patch uses. This would be something that would ideally be merged upstream, but I'm not sure if Chromium is generally open to accepting code it doesn't need yet?
• Related to the above, I was unable to test cross-window drag-and-drop, as it crashed an entirely unrelated section of the Wayland backend.

I believe these issues are not "blocking" per se because:

• The Wayland backend itself is still unstable and does hit upstream bugs from time to time.
• Right now, there are no window decorations on GNOME Wayland, so having something is still a sizable improvement.
• Note that the test failures seem to be entirely unrelated.

With this in mind, I'm now marking this PR as ready for review.

Thank you very much for that PR ❤️
I haven't though that someone would really make the effort to patch Electron just to support CSD on one single compositor under Wayland.
AFAIK all other compositors apart from Gnome support SSD under Wayland.

As a Gnome on Wayland user i would love to see this getting merged fast, because using Electron apps without any window decorations is "somewhat less than perfect".

Edit: Just out of interest: I'm not a desktop dev by any means, but did you need to introduce any GTK related dependencies to make this work?
IIRC this was the reason why the Mutter devs rejected SSD in the first place (they have not dependency on GTK yet and didn't want to introduce new ones).

@refi64 Please take a look at #29523, looks related.

I haven't though that someone would really make the effort to patch Electron just to support CSD on one single compositor

Well GNOME/Mutter isn't exactly obscure

AFAIK all other compositors apart from Gnome support SSD under Wayland.

Weston, the reference implementation, doesn't - even those that support zxdg_decoration may not always use SSD (if ever)

I'm not a desktop dev by any means, but did you need to introduce any GTK related dependencies to make this work?

Electron already depends on Gtk

IIRC this was the reason why the Mutter devs rejected SSD in the first place

Mutter already depends on Gtk

Mutter already depends on Gtk

Maybe I'm wrong about the other points, but at least this one could be right, as explained in this comment :https://gitlab.gnome.org/GNOME/mutter/-/issues/217#note_357576
And weston has no real userbase I would say ;)

However this is quite offtopic, so nevermind.

@nornagon does Chromium normally take patches for things not used in Chromium itself? I believe upstreaming it is probably doable otherwise, but I'd expect "we're not actively using this" to be a roadblock?

@refi64 It depends, but broadly speaking, no. It's worth trying anyway though, as sometimes you'll get a friendly reviewer and/or you can make a case for why it should be merged anyway (e.g. chromium might use it in the future; it sounds like there is a potential path there based on an argument that their window decorations should be better on wayland?)

If upstreaming this patch is a complete dead end, we should explore, in order of preference:

1. Can we implement this functionality without patching? e.g. can we subclass, or implement delegate methods, or fetch the underlying handles, etc. to avoid a patch?
2. Can we upstream a patch that refactors the code in an acceptable-to-upstream way that allows us to do (1)?
3. Can we make this patch smaller?

If we can't do any of the above, I am 👎 on this PR. Every patch we take on creates a significant maintenance burden. If landed as-is, this patch would be the largest Chromium patch we are carrying, by a factor of 2. A patch of this size is not worth the ongoing maintenance cost for what it buys, IMO.

Understood, I'll start taking a look into what it would take to upstream this. There are a few patches we're carrying in the Chromium Flatpak I should be upstreaming soon too, I guess this is extra incentive!

I'm happy to backport to beta (17.x) but I don't want to risk destabilizing 16.x and prior without further testing.

Thank you so much!
Is it possible to backport it to version 16 as well with enough testing?
At the end, it still needs a flag to be enabled 🙃

btw is --ozone-platform=wayland still necessary?

Great to see this merged! Just for some clarity though, what's going to be the policy for promoting it from behind a flag?

I am trying this out with Slack, which currently runs on electron 18.1.0 from what I can tell.
For me, Slack window does not take all space when maximised - it leaves quite significant margins all around (18-21 px). It is fine when tiling to the left or right.
This is on GNOME 42.2, Fedora 36. I've tried disabling dash-to-panel which I use.

I see the exact same behavior. I'm using Arch and am also on GNOME 42.

This issue #33161