Bismuth-Forge / bismuth

KDE Plasma add-on, that tiles your windows automatically and lets you manage them via keyboard, similarly to i3, Sway or dwm.
https://bismuth-forge.github.io/bismuth/
Other
2.41k stars 93 forks source link

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

Open jkcdarunday opened 1 year ago

jkcdarunday commented 1 year ago

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):

https://github.com/Bismuth-Forge/bismuth/assets/4564810/2baaaaa9-3902-4bc4-b28c-c1f6514ea2c5

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

https://github.com/Bismuth-Forge/bismuth/assets/4564810/7ccfb2f5-3cb6-40f6-bcf7-85a666b84cc8

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 commented 1 year ago

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

Vistaus commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Thanks! Great work! That's helped

farline99 commented 1 year ago

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