search

kotelnik / plasma-applet-active-window-control

Plasma 5 applet for controlling currently active window.
GNU General Public License v2.0
119 stars 18 forks source link

Causes Plasma Shell to crash #59

Closed nerdius closed 7 years ago

nerdius commented 7 years ago

The applet causes the entire Plasma Shell to crash when it's on the panel with the height of exactly 24 pixels (no more or less), the application menu is enabled, and a Qt/KDE application that has a menu is in focus.

Version: 1.7.1.

Reproducible: Always.

Steps to reproduce:

  1. Add a horizontal panel and add AWC widget on it.
  2. Open up the AWC settings, go to the Application Menu page and check the Enable application menu checkbox.
  3. Go to System Settings → Application Style → Widget Style → Fine Tuning. Under Menubar, click the Menubar Style drop-down list and select Application Menu widget option and click Apply.
  4. Open the Plasma panel settings and resize it so that it's height is exactly 24 pixels. (You might need to use a screenshot utility, an image editor for the measurement, and trail and error.)
  5. Open up a KDE/Qt application window that has a menu. (Note: Do not switch to an application that was open before you performed the step 3 instead; applications must be restarted in order for the setting to take effect.)

Actual results:

Expected results:

bkrith commented 7 years ago

Hi, you dont need all those steps to reproduce it!! :D I have a very thin panel and with embedded menu in ActiveWindowControl widget everything works perfect except some times crash (in very specific apps) only the menu and just stop working. But if I use the global menu widget I have plasma shell crash immediately (I believe reason is the thin panel and the problem appears after the last updates to me.. I know that because 2 weeks ago worked fine)! I think it's not issue of ActiveWindowControl but global menu functionality itself in plasma! (maybe I'm wrong..)

nerdius commented 7 years ago
  1. Yes, I think that you're wrong because I've tested the included Global Menu widget and it seems to be working just fine. I forgot to mention that in the issue.
  2. The crash doesn't normally occur; it only occurs when you follow the steps I've provided. I've checked it like 10 or 20 times and it only causes the crash when the panel height is 24 px and the application menu is enabled (like I described) in the report.
kupiqu commented 7 years ago

It might be related to issue #42, although in my case, last time I tried, it crashed latte panels but not plasma ones.

The particular setting 24 px in your case, 32 px in mine, may depend on screen size and/or font dpi and/or font size and/or the particular font being used... this may explain why a crash in one system is difficult to be reproduced in another...

nerdius commented 7 years ago

@kupiqu What does this webpage tell you? In my case it's 96 DPI, and 32 / 24 × 96 = 128. So, if the theory is right, yours should be 128 DPI.

kupiqu commented 7 years ago

Mmm, chromium, firefox and konqueror give me different numbers, you can better check it out in systemsettings -> fonts

In my case I force 144 dpi (96 by default).

However, I think it depends on fontsize too, give it a try, modify fontsize in systemsettings (you may need to logout or restart plasma) and then try to reproduce the crash at 24px, depending on the direction of the change, I think the crash may already happen for >24px, or you may need to go down 24px...

kupiqu commented 7 years ago

In fact the way I currently workaround the issue is by scaling down the font size within the active window control applet (x 0.95)

nerdius commented 7 years ago

I forced 144 DPI and restarted the shell. Everything became ginormous, and yeah, now it crashes on 32 px panel height. The default setting was 96 DPI in my case too.

In fact the way I currently workaround the issue is by scaling down the font size within the active window control applet (x 0.95)

I don't like that workaround — the text becomes smaller compared to that what's on the window title bars. My workaround is setting the panel height to 26 px instead of 24.

nerdius commented 7 years ago

I just updated to 1.7.2 and I can still replicate this.

kotelnik commented 7 years ago

Confirmed, replicated (96DPI fonts ~ 24px panel height). Possibly fixed in latest commit. Please try :).

kupiqu commented 7 years ago

It fixes the issue for me, which is nice. However, this is at the expense of a smaller font size in buttons as well as an offset between title (vertically centered) and buttons text (shifted up).

image

kupiqu commented 7 years ago

There is also the "menu button text size scale", which doesn't seem to modify anything in my system. Might it be because it is somehow at its limit size already?

kotelnik commented 7 years ago

It seems the shifted up text in buttons cannot be easily avoided other then reimplementing the ToolButtons with something different. I'll work on that. In the meantime try just commited code. It should fix the situation slightly. I've also fixed the "menu button text size scale", it should now work.

kupiqu commented 7 years ago

I see, looking forward for your new implementation of the toolbuttons.

The shift right now with last commit is still there, perhaps a bit smaller, but not so sure.

But I totally confirm that "menu button text size scale" now works great :)

nerdius commented 7 years ago

Confirmed. It seems to be fixed in the latest commits (3f0ad52 & 300558f).