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.

Sign up for GitHub

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

Jump to bottom

feat: add support for ELECTRON_OZONE_PLATFORM_HINT env var #39792

Merged
merged 2 commits into from Sep 20, 2023
Merged

feat: add support for ELECTRON_OZONE_PLATFORM_HINT env var #39792

merged 2 commits into from Sep 20, 2023
+18 −2

Conversation

vially
Copy link
Contributor

@vially vially commented Sep 10, 2023

Description of Change

Note

This is a resubmit of #35614, since #35630 is still blocked on upstream/pending further discussion.

Add support for the ELECTRON_OZONE_PLATFORM_HINT environment variable in order to make it easier for users to enable Wayland support for all Electron apps without having to manually patch application .desktop files (or other similar workarounds #30897).

The accepted values are the same as upstream, with the special auto value being recommended for most users (especially for those switching between X11 and Wayland environments).

When both the --ozone-platform-hint command line flag and the ELECTRON_OZONE_PLATFORM_HINT environment variable are specified, the command line flag has higher priority.

Fixes #30897

Checklist

Release Notes

Notes: Added support for ELECTRON_OZONE_PLATFORM_HINT environment variable on Linux.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Sep 10, 2023
deepak1556
deepak1556 reviewed Sep 11, 2023
View reviewed changes
Copy link
Member

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

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

LGTM, pending CI pass

@deepak1556
Copy link
Member

/cc @jkleinsc @codebytere on the CI failure.

@codebytere
Copy link
Member

@vially can you please rebase?

ckerr
ckerr reviewed Sep 11, 2023
View reviewed changes
Copy link
Member

@ckerr ckerr left a comment

Choose a reason for hiding this comment

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

Had a couple of nitpicks but the idea LGTM. Thanks for the patch!

shell/browser/electron_browser_main_parts_linux.cc Outdated Show resolved Hide resolved
shell/browser/electron_browser_main_parts_linux.cc Outdated Show resolved Hide resolved
@vially
Copy link
Contributor Author

vially commented Sep 11, 2023

@vially can you please rebase?

@codebytere I've rebased and addressed and @ckerr's comments.

@ckerr
Copy link
Member

ckerr commented Sep 12, 2023

CI is failing with these errors, unrelated to the code in the PR:

Forked prs not allowed to run on OSS projects

Is this something new? We've had lots of PRs submitted from outside contributors, e.g. this one from a couple weeks ago #39597

@codebytere
Copy link
Member

codebytere commented Sep 13, 2023 •
edited

@ckerr it is an underlying infrastructure issue - it’s nothing to do with forked PRs or an actual setting despite what it says (we of course have that enabled)

it just needs to be rerun

jkleinsc
jkleinsc requested changes Sep 14, 2023
View reviewed changes
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.

This should be documented in https://github.com/electron/electron/blob/main/docs/api/environment-variables.md

@jkleinsc jkleinsc added the semver/minor backwards-compatible functionality label Sep 14, 2023
@vially
Copy link
Contributor Author

vially commented Sep 14, 2023

This should be documented in https://github.com/electron/electron/blob/main/docs/api/environment-variables.md

Fixed in latest commit.

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Sep 17, 2023
@ckerr ckerr requested a review from jkleinsc September 18, 2023 17:02
jkleinsc
jkleinsc reviewed Sep 18, 2023
View reviewed changes
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

@jkleinsc jkleinsc merged commit 58fd882 into electron:main Sep 20, 2023
14 checks passed
@release-clerk
Copy link

release-clerk bot commented Sep 20, 2023

Release Notes Persisted

Added support for ELECTRON_OZONE_PLATFORM_HINT environment variable on Linux.

@vially vially deleted the add-ozone-hint-env-var branch September 20, 2023 21:57
This was referenced Sep 25, 2023
Open
Closed
@ckerr ckerr changed the title fix: add support for ELECTRON_OZONE_PLATFORM_HINT env var feat: add support for ELECTRON_OZONE_PLATFORM_HINT env var Nov 28, 2023
MrHuangJser pushed a commit to MrHuangJser/electron that referenced this pull request Dec 11, 2023
@vially @jkleinsc
fix: add support for ELECTRON_OZONE_PLATFORM_HINT env var (electron#3…
ef7cf9c 
…9792)

Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
@C0rn3j C0rn3j mentioned this pull request Feb 8, 2024
Closed
xduugu added a commit to xduugu/org.signal.Signal that referenced this pull request Feb 13, 2024
@xduugu
Simplify logic to enable wayland ozone backend
79b16a2 
This uses the new environment variable `ELECTRON_OZONE_PLATFORM_HINT` [1],
which is available since electron v28 [2].

[1] https://www.electronjs.org/docs/latest/api/environment-variables#electron_ozone_platform_hint-linux
[2] electron/electron#39792
xduugu added a commit to xduugu/org.signal.Signal that referenced this pull request Feb 14, 2024
@xduugu
Simplify logic to enable wayland ozone backend
ebf5834 
This uses the new environment variable `ELECTRON_OZONE_PLATFORM_HINT` [1],
which is available since electron v28 [2].

[1] https://www.electronjs.org/docs/latest/api/environment-variables#electron_ozone_platform_hint-linux
[2] electron/electron#39792
xduugu added a commit to xduugu/org.signal.Signal that referenced this pull request Feb 14, 2024
@xduugu
Simplify logic to enable wayland ozone backend
633da98 
This uses the new environment variable `ELECTRON_OZONE_PLATFORM_HINT` [1],
which is available since electron v28 [2].

[1] https://www.electronjs.org/docs/latest/api/environment-variables#electron_ozone_platform_hint-linux
[2] electron/electron#39792
xduugu added a commit to xduugu/org.signal.Signal that referenced this pull request Feb 26, 2024
@xduugu
Simplify logic to enable wayland ozone backend
1a3ab7b 
This uses the new environment variable `ELECTRON_OZONE_PLATFORM_HINT` [1],
which is available since electron v28 [2].

[1] https://www.electronjs.org/docs/latest/api/environment-variables#electron_ozone_platform_hint-linux
[2] electron/electron#39792
@QuarticCat
Copy link

Hi, can you also add an environment variable for --enable-wayland-ime? This is extremely helpful for people using an input method. I'm not familiar with the electron codebase and cannot find where to modify it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkleinsc jkleinsc jkleinsc approved these changes

@ckerr ckerr ckerr approved these changes

@deepak1556 deepak1556 deepak1556 approved these changes

Assignees
No one assigned
Labels
api-review/requested 🗳 no-backport semver/minor backwards-compatible functionality
Projects
None yet
Milestone
No milestone
6 participants
@vially @deepak1556 @codebytere @ckerr @QuarticCat @jkleinsc