-
Notifications
You must be signed in to change notification settings - Fork 38
Work around Electron's broken Wayland detection #357
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
Conversation
🚧 Test build enqueued. |
🚧 Started test build. |
I've verified the fix works locally, but maybe some of the other variable checks for Wayland can be removed / altered / refined. I didn't dare touch them 🙏 |
✅ Test build succeeded. To test this build, install it from the testing repository:
|
hi @aksestok , what's the plan? are you merging this PR or rebasing the electron version to this one? electron/electron#48298 |
I'm not a maintainer, so I can't merge anything unfortunately. The upstream Electron fix needs to be bumped by Slack themselves. |
Maybe @AKoskovich can take a look? 🙏 |
Can verify this in a bit, traveling at the moment |
Even with this, I don't see a GUI at all (window opens and has correct title etc, just no contents). On sway. Just going to stick with 734c1fac156edb431cfec1f49dfd38088b2c394640973de7ed4f4d4ea1593d6c for now... |
This build does seem to work for rhel10/gnome 47 |
It seems like text-input-v3 is enabled out-of-the box for chromium 140 as well ( https://chromium-review.googlesource.com/c/chromium/src/+/6441739 ), so said flag could also be removed. EDIT: preliminary testing seems to suggest the |
@aksestok Sorry for the delay, just got back. Is there a reason to add XDG_SESSION_TYPE to the check for Wayland? If the Wayland socket is not enabled then the app will fail to launch if the host has a Wayland session, and that's the default state so this will break the majority of installs.
We could enable the wayland socket by default, but I'm still waiting on Electron to fix some regressions detailed in some other issues here still. FYI removed the change to the check and now the app launches with the wayland socket enabled and disabled, and it uses wayland when it is enabled. Note that it seems like Electron has a new wayland regression, on Fedora 42 w/Wayland, I cannot drag the window around. It always pops up the window decoration menu regardless of left or right click. |
No worries @AKoskovich 😊
No particular reason other than that's what was used in flathub/org.signal.Signal#965 (and it's the variable that Electron now is supposed to use to determine if it should launch on Wayland). I'll revert the change since it seems redundant with the current checks already in place.
@czhang03 I haven't tested this nor have any opinion on removal. Happy to make the change in the PR if a collaborator finds it usefeul / necessary before merging. |
🚧 Test build enqueued. |
🚧 Started test build. |
✅ Test build succeeded. To test this build, install it from the testing repository:
|
It will basically remove the need for a wayland check once slack updated its electron. If you push a version, I can help test to confirm input method still works. |
Inspired by flathub/org.signal.Signal#965.
Fixes #356.