Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: enable WaylandWindowDecorations by default #39582

Merged
merged 1 commit into from Aug 24, 2023

Conversation

aiddya
Copy link
Contributor

@aiddya aiddya commented Aug 20, 2023

Description of Change

Reasons to enable the feature by default:

  • This feature flag only takes effect when the user opts into Wayland using one of the command line flags.
  • When using a compositor that supports XDG Decoration protocol (KDE and Sway), then this flag has no effect.
  • When using a compositor that doesn't support XDG Decoration protocol (GNOME and Weston), then this flag is necessary to have a functional window that can be moved and closed easily. This just adds more friction for users.
  • The feature was introduced behind a flag about 18 months ago and there aren't any open issues specific to this feature.

cc-ing reviewers of the original PR @nornagon @zcbenz @ckerr

Checklist

Release Notes

Notes: no-notes

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Aug 20, 2023
@VerteDinde VerteDinde added target/26-x-y PR should also be added to the "26-x-y" branch. target/27-x-y PR should also be added to the "27-x-y" branch. semver/patch backwards-compatible bug fixes semver/minor backwards-compatible functionality and removed semver/patch backwards-compatible bug fixes labels Aug 21, 2023
@VerteDinde
Copy link
Member

In theory, I'm fine with this change, but do we know what the state of upstream Chromium is regarding kWaylandWindowDecorations by default? If this is something that's also enabled in Chromium by default now, we may no longer even need this flag. (Note: I wouldn't hold the PR on removing it - I just think it would be helpful to understand how we're aligned/misaligned with Chromium if we do enable the flag by default)

@aiddya
Copy link
Contributor Author

aiddya commented Aug 21, 2023

The kWaylandWindowDecorations flag is not present in upstream, it's an electron-specific flag. I guess it would be helpful to provide some context here. X11 provides simple server-side decorations to all windows by default. This was sufficient for electron as it does not support any advanced window customization features like frameless windows or Window Controls Overlay on Linux. As a result, there was no implementation of client-side decorations on Linux.

Wayland requires clients to decorate themselves by default if they need decorations. It has a protocol to request server-side decorations called zxdg_decoration_manager_v1, but that request can be denied and the protocol is not supported on all compositors. In short, electron had to draw its own decorations on Wayland, which was implemented by #29618. Before merging, the PR changes were guarded by a feature flag, WaylandWindowDecorations.

Chromium always draws its own decorations, both on X11 and Wayland, unless the "Use system title bar and borders" option is turned on. This option cannot be turned on in Wayland compositors without support for zxdg_decoration_manager_v1. Enabling WaylandWindowDecorations by default should make electron behave similar to Chromium.

@VerteDinde
Copy link
Member

Thanks @aiddya, that's very helpful context 😄 In that case, this change looks good to me, thanks again for your contributions here!

Copy link
Contributor

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API LGTM

@itsananderson
Copy link
Contributor

API LGTM

@jkleinsc jkleinsc merged commit 33000c4 into electron:main Aug 24, 2023
22 checks passed
@release-clerk
Copy link

release-clerk bot commented Aug 24, 2023

No Release Notes

@trop
Copy link
Contributor

trop bot commented Aug 24, 2023

I have automatically backported this PR to "27-x-y", please check out #39644

@trop trop bot added in-flight/27-x-y and removed target/27-x-y PR should also be added to the "27-x-y" branch. labels Aug 24, 2023
@trop
Copy link
Contributor

trop bot commented Aug 24, 2023

I have automatically backported this PR to "26-x-y", please check out #39645

@trop trop bot added in-flight/26-x-y merged/27-x-y PR was merged to the "27-x-y" branch. merged/26-x-y PR was merged to the "26-x-y" branch. and removed target/26-x-y PR should also be added to the "26-x-y" branch. in-flight/27-x-y in-flight/26-x-y labels Aug 24, 2023
@aiddya aiddya deleted the enable-wayland-decorations branch September 29, 2023 01:59
csett86 added a commit to jitsi/jitsi-meet-electron that referenced this pull request Nov 4, 2023
Contains electron 26 and 27 updates, for details see

https://www.electronjs.org/blog/electron-26-0
https://www.electronjs.org/blog/electron-27-0

Mainly this should help with further bugfixes in webrtc as the contained
Chromium is implicitly upgraded from 114 to 118.

In wayland / pipewire terms we have only minor additions, eg:

WaylandWindowDecorations by default: electron/electron#39582
csett86 added a commit to jitsi/jitsi-meet-electron that referenced this pull request Nov 4, 2023
Contains electron 26 and 27 updates, for details see

https://www.electronjs.org/blog/electron-26-0
https://www.electronjs.org/blog/electron-27-0

Mainly this should help with further bugfixes in webrtc as the contained
Chromium is implicitly upgraded from 114 to 118.

In wayland / pipewire terms we have only minor additions, eg:

WaylandWindowDecorations by default: electron/electron#39582

which means we can remove the enable flag we had on this in main.js as its now
default.
csett86 added a commit to jitsi/jitsi-meet-electron that referenced this pull request Nov 11, 2023
Contains electron 26 and 27 updates, for details see

https://www.electronjs.org/blog/electron-26-0
https://www.electronjs.org/blog/electron-27-0

Contains a fix for Linux that lead to crashes when the graphics drivers changed
electron/electron#40467

Mainly this should help with further bugfixes in webrtc as the contained
Chromium is implicitly upgraded from 114 to 118.

In wayland / pipewire terms we have only minor additions, eg:

WaylandWindowDecorations by default: electron/electron#39582

which means we can remove the enable flag we had on this in main.js as its now
default.
csett86 added a commit to jitsi/jitsi-meet-electron that referenced this pull request Nov 11, 2023
Contains electron 26 and 27 updates, for details see

https://www.electronjs.org/blog/electron-26-0
https://www.electronjs.org/blog/electron-27-0

Contains a fix for Linux that lead to crashes when the graphics drivers changed
electron/electron#40467

Mainly this should help with further bugfixes in webrtc as the contained
Chromium is implicitly upgraded from 114 to 118.

In wayland / pipewire terms we have only minor additions, eg:

WaylandWindowDecorations by default: electron/electron#39582

which means we can remove the enable flag we had on this in main.js as its now
default.
MrHuangJser pushed a commit to MrHuangJser/electron that referenced this pull request Dec 11, 2023
csett86 added a commit to csett86/org.jitsi.jitsi-meet that referenced this pull request Jan 13, 2024
Since electron/electron#39582 WaylandWindowDecorations
are on by default, thus no longer the need to set this in the wrapper.

This was part of electron 28 and was backported to electron 27 and 26,
thus we can safely remove it from here as well.
xduugu added a commit to xduugu/org.signal.Signal that referenced this pull request Feb 13, 2024
Window decorations on wayland are enabled by default since electron v28 [1].

[1] electron/electron#39582
xduugu added a commit to xduugu/org.signal.Signal that referenced this pull request Feb 14, 2024
Window decorations on wayland are enabled by default since electron v28 [1].

[1] electron/electron#39582
xduugu added a commit to xduugu/org.signal.Signal that referenced this pull request Feb 26, 2024
Window decorations on wayland are enabled by default since electron v28 [1].

[1] electron/electron#39582
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-review/requested 🗳 merged/26-x-y PR was merged to the "26-x-y" branch. merged/27-x-y PR was merged to the "27-x-y" branch. new-pr 🌱 PR opened in the last 24 hours semver/minor backwards-compatible functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants