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

fix: Use optional chaining to fix error when windowId is undefined in Wayland #490

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jkcdarunday
Copy link

@jkcdarunday jkcdarunday commented Jul 13, 2023

Summary

On KWin Wayland (5.27.6), I noticed that bismuth is throwing the ff. error when tiling Firefox causing tiling to break so I investigated and was able to pinpoint the error. It turns out windowId is undefined for some apps so I changed it to optional chaining which fixed the problems I've been having with tiling.

org.kde.bismuth: arrangeScreen/finished,[object Object]
org.kde.bismuth: Oops! TypeError: Cannot read property 'toString' of undefined.
org.kde.bismuth: Client added: KWin::XwaylandWindow(0x556f7f219900)

Here's how it worked before this change (the other windows don't resize immediately, eventually they stop tiling altogether):

bismuth-broken.mp4

Here's how it worked after this change (the other windows resize immediately):

bismuth-working.mp4

Breaking Changes

Should not break anything.

UI Changes

None

Test Plan

  1. On KDE Plasma Wayland, open Firefox
  2. Open three new windows
  3. Close two of the three windows that you just opened
    a. The remaining windows should resize correctly
    b. Previously, these windows don't resize as one of the handlers throws the above error and fails to process the event

Related Issues

Potentially closes #473 or at least fixes one problem related to wayland.

@dobladez
Copy link

This change indeed fixes (or works around?) issue #473 for me. Thanks!

@Vistaus
Copy link

Vistaus commented Jul 20, 2023

I just installed Bismuth from your branch on openSUSE Tumbleweed, but it makes the screen very glitchy and half-tiling doesn't work (only quarter tiling). And some apps don't open at all.

@jkcdarunday
Copy link
Author

That's odd, everything works perfectly on mine and I don't think the change would prevent any window that already opens from opening. Probably a different problem.

@endeavour
Copy link

Thanks, this is great! Please merge this and get it into the Arch repos so I don't have to keep manually building things.

@owl-from-hogvarts
Copy link

Thanks! Great work! That's helped

@farline99
Copy link

farline99 commented Aug 17, 2023

Thank you so much! ❤️ How much a single symbol can do for humanity 😁 I built the RPM package for Fedora 38 from the official spec file, here you go, maybe someone will need it (the process of building from source code is a pain for a normal human, don't want others to suffer).

bismuth_unpack_it.zip

kbloom added a commit to kbloom/bismuth that referenced this pull request Dec 18, 2023
This is a reimplementation of Bismuth-Forge#490
My esbuild and tsc on Debian Testing right now aren't smart enough to
convert the ?. optional chaining into something that Qt 5.15 can
understand.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Broken on KDE 5.27 beta
6 participants