Fix some issues with temporary windows

Linux
1

Some plugin UIs will use "temporary windows", a quick example being a slider with a popup value.
This works on linux right now but causes issues with tiling WMs, it appears the WM is creating a window for that popup.

I fixed this just now, patch here:
https://github.com/DISTRHO/juce/commit/6783a776f0d479b8713551dff518642f692ff169.patch

(the patch includes 2 other small fixes I needed for some plugins)

 

2

Which window manager are you using? I tested this with Xmonad and can't reproduce it in the JuceDemo.

 

3

You can see this in any WM. (but the "error" happens in awesome and i3)

The most common widget to test are sub-submenus, like the ones in JuceDemoHost.
They will take the focus out of the parent window, because the sub-submenu is a window itself.

On tiling WMs this will cause the submenu to appear in a new screen area/section or even on a different screen...

 

The patch I posted fixes some other things too.
The fist part:

if (! comp->isOnDesktop())

Is because the juce code seems wrong. Why would it check if it was on desktop then add it again?
Whoever wrote the code probably mis-added the "!".

 

The last part:

popup->addToDesktop (ComponentPeer::windowIsTemporary);

Is needed to make a plugin I have work correctly.
Without the temporary flag juce will create a regular x11 window for it, which skips the fix this patch is all about.
 

I think there are other parts of the code that also need this extra flag (the submenus?),
but I'm waiting for this initial patch to be accepted before digging any deeper.

4

Bump.

Some of the changes here have not made it into juce yet.

The 1st and last part of patch will correct stuff in juce that is not specific to linux.
And the middle part (X11 specific) is useful so that we don't create a real window for popups.

5

Thanks, I'll take another look asap..

6

Thanks.

You're mis-reading the code about adding it to the desktop - 'comp' is a different object to 'this'. The original code is correct - your change would break all shadows for components that aren't on the desktop. 

Setting the window style to _NET_WM_WINDOW_TYPE_TOOLTIP for all temporary windows doesn't sound right to me.. This would also be applied to popup menus, etc, which surely is a less appropriate style for them than _NET_WM_WINDOW_TYPE_COMBO?

7

It is correct that _NET_WM_WINDOW_TYPE_TOOLTIP is not 100% correct for menu style windows, but I couldn't find how to detect that via juce. (if a window is supposed to be a popup, menu, tooltip, etc).

_NET_WM_WINDOW_TYPE_TOOLTIP is the most sane common option afaik.

8

I've added the windowIsTemporary flag now - that seems sensible. But I'm not prepared to change the hint to mark these windows as tooltips just because of problems in a tiling WM - I'm not confident that it wouldn't risk messing up popup menus on all the other more mainstream WMs.

9

ok, I understand.

now to resolve the merge conflicts... :/