-
Notifications
You must be signed in to change notification settings - Fork 456
Don't define browserWindowOptions type as splash if desktop environment is kde #707
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses a window focus issue on Linux systems running KDE Plasma desktop environment by conditionally excluding the splash window type when KDE is detected.
- Adds detection for KDE desktop environment using the
KDE_FULL_SESSIONenvironment variable - Modifies the window type assignment logic to avoid setting
type: 'splash'when running on KDE - Preserves the existing splash window behavior for other Linux desktop environments
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } | ||
|
|
||
| if (process.platform === 'linux') { | ||
| const isKde = process.env.KDE_FULL_SESSION === "true" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to KDE’s official documentation, the value of the environment variable KDE_FULL_SESSION may include version information in the future (for example, true-5.27). Therefore, simply checking for === "true" may fail in the future. A more robust approach is to check whether the variable exists and is not empty.
If you plan on using this variable to detect a running KDE session, check if the value is not empty instead of seeing if it equals true. The value might be changed in the future to include KDE version information.
https://userbase.kde.org/KDE_System_Administration/Environment_Variables#Automatically_Set_Variables
Or, a more general method that complies with the Freedesktop standard is to check the XDG_CURRENT_DESKTOP environment variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Single quotes are commonly used in this file, so maybe we should use 'true' to match the usual preference in .eslintrc and the style of the current file.
| const isKde = process.env.KDE_FULL_SESSION === "true" | |
| const isKde = process.env.KDE_FULL_SESSION === 'true' |
| } | ||
|
|
||
| if (process.platform === 'linux') { | ||
| const isKde = process.env.KDE_FULL_SESSION === "true" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would some notes here be helpful?
| const isKde = process.env.KDE_FULL_SESSION === "true" | |
| const isKde = process.env.KDE_FULL_SESSION === "true" | |
| // The 'splash' window type in Electron can cause input and focus issues on | |
| // the KDE Plasma desktop environment. By detecting KDE, we can avoid setting | |
| // this type and prevent the bug. | |
| // See issues: #661, #705 |
| const isKde = process.env.KDE_FULL_SESSION === "true" | |
| const isKde = process.env.KDE_FULL_SESSION === "true" | |
| // On KDE (Plasma), setting type='splash' causes windows to be unable to receive keyboard focus/input (#661, #705) | |
| // Therefore, 'splash' is disabled under KDE, while other Linux distributions still use it. |
Fixes main window not accepting input or focus when running on linux with kde plasma
Connected issues:
#705
#661