emgram769/lighthouse

Wrong WINDOW_TYPE

Closed this issue · 22 comments

So currently, when lighthouse starts up, it has the wrong WINDOW_TYPE

_NET_WM_WINDOW_TYPE(ATOM) = _NET_WM_WINDOW_TYPE_DOCK

Shouldn't it much rather be a popup type window? With my window manager (i3), lighthouse looks rather obsucre.

1424344106

Same weird look here, also i3 WM.

This could very well be the cause of Xmonad not focusing on lighthouse too.

I'd say it should be _NET_WM_WINDOW_TYPE_DIALOG, but I'm no expert with X. A quick change makes lighthouse look like this.

diff --git a/src/lighthouse.c b/src/lighthouse.c
index 33a37c8..c1f6ef8 100644
--- a/src/lighthouse.c
+++ b/src/lighthouse.c
@@ -1109,7 +1109,7 @@ int main(int argc, char **argv) {
     window_type_atom = atom_reply->atom;
     free(atom_reply);

-    atom_cookie = xcb_intern_atom(connection, 0, strlen("_NET_WM_WINDOW_TYPE_DOCK"), "_NET_WM_WINDOW_TYPE_DOCK");
+    atom_cookie = xcb_intern_atom(connection, 0, strlen("_NET_WM_WINDOW_TYPE_DIALOG"), "_NET_WM_WINDOW_TYPE_DIALOG");
     atom_reply = xcb_intern_atom_reply(connection, atom_cookie, NULL);
     if (atom_reply) {
       window_type_dock_atom = atom_reply->atom;

1424345340

I can confirm that this patch does put the window into focus on start in xmonad.

ah, it looks like it should definitely be dialogue. thanks to you both! I'll push the fix now

It changes focus but for some reason it does not work well on xmonad anymore. IT opens up an entire blank window for the dialogue.
screenshot from 2015-02-19 12 26 15

alright I've reverted it. looking at the documentation here http://standards.freedesktop.org/wm-spec/1.3/ar01s05.html and testing on my own machine, DOCK seems to work best. DIALOG works on my machine but gives it a border. I can look into it more in a couple hours

@ThomasDuPlessis Hmm that's interesting. lighthouse.

@emgram769 Is there a way to ensure that lighthouse gets focused despite what WM_CLASS the window has?

@jahkeup I believe WM_CLASS is used for setting specific handling of the program in the window manager, not something to give it the window any default properties. You should be able to use the WM_CLASS to override the default window settings in xmonad. If you mean override WM_WINDOW_TYPE, I'm not sure.

Under i3 _TYPE_DOCK windows are IIRC forced to stick to the top/bottom of the screen and to expand to full width. That's what causes the problem in the opening comment and under i3 in general.

Probably Xmonad - but I don't really know it, mind that - does not consider a _TYPE_DIALOG window special (i3 does and automatically floats it) and so draws it like any other _TYPE_NORMAL window.
At the same time bspwm does not consider _TYPE_DOCK special maybe?

From the EMWH description _TYPE_DIALOG seems the optimal WINDOW_TYPE for a "launcher". The borders could be removed using WM_CLASS filtering maybe?
I could work on a little patch to make the window type configurable between _TYPE_DOCK and _TYPE_DIALOG if that seems the best way to proceed.

That may have been xmonads fault, I set it to make lightouse a floating window and it still shows up like in the picture, however if I move it around alittle bit, it removes the extra white space and looks normal. I am not sure if its a matter of changing the lighthouse code or my xmonad config file.

I don't know lighthouse's code at all, but from a protocol perspective: if lighthouse doesn't want the window manager to interfere, it should set override_redirect on its window. Then the window type won't affect its placement and no borders will be drawn. However, lighthouse will also have to care for input focus itself since the window manager will no longer do so.

As for the window type in particular: note that a list of types can be provided for window managers that don't support all values. Ultimately, a window manager that chooses to make dialog windows tiled should imho be considered a spec violation / bug in the wm or desired behavior. Again, though, with override redirect this won't be an issue.

(On the other hand, lighthouse ignoring the size the window manager actually gives it should also be considered a bug. Once again, override redirect will solve this, though).

@emgram769 I think it'd be a whole lot better if you set the override redirect when actually creating the window, otherwise the window manager will first manage it and then let go of it, which could cause unwanted effects. Plus you're saving a roundtrip to X.

I'm not entirely sure how the override-redirect flag works, because when I or'd it with the xcb_create_window mask it failed with error 2 (xcb connection shutdown because of extension not supported)

Well, it should work to "or" it with the value mask of the request. Of course you need to also provide a value for it (which would be 1). See here for an example: https://github.com/i3/i3/blob/next/i3-input/main.c#L462

In fact I think your current commit is broken. You pass the same value list for changing the window attributes and for the value of the override redirect flag you end up passing the white_pixel value rather than "1".

One more note – you should set the override_redirect value to 0 in case lighthouse is run in dock mode.

Here's a patch for the current master that should fix this correctly:

diff --git a/src/lighthouse.c b/src/lighthouse.c
index 2d47c9b..1d79926 100644
--- a/src/lighthouse.c
+++ b/src/lighthouse.c
@@ -525,29 +525,23 @@ int main(int argc, char **argv) {

   /* Create a window. */
   xcb_window_t window = xcb_generate_id(connection);
-  uint32_t values[2];
+  uint32_t mask = XCB_CW_BACK_PIXEL | XCB_CW_OVERRIDE_REDIRECT | XCB_CW_EVENT_MASK;
+  uint32_t values[3];
   values[0] = screen->white_pixel;
-  values[1] = XCB_EVENT_MASK_EXPOSURE
+  values[1] = settings.dock_mode ? 0 : 1;
+  values[2] = XCB_EVENT_MASK_EXPOSURE
             | XCB_EVENT_MASK_KEY_PRESS
             | XCB_EVENT_MASK_KEY_RELEASE
             | XCB_EVENT_MASK_BUTTON_PRESS;
   xcb_void_cookie_t window_cookie = xcb_create_window_checked(connection,
     XCB_COPY_FROM_PARENT, window, screen->root, 0, 0, settings.width, settings.height, 0,
-    XCB_WINDOW_CLASS_INPUT_OUTPUT, screen->root_visual,
-    XCB_CW_BACK_PIXEL | XCB_CW_EVENT_MASK, values);
+    XCB_WINDOW_CLASS_INPUT_OUTPUT, screen->root_visual, mask, values);

   if (check_xcb_cookie(window_cookie, connection, "Failed to initialize window.")) {
     exit_code = 1;
     goto cleanup;
   }

-   /* Override-redirect to inform a window manager not to tamper with the window. */
-   window_cookie = xcb_change_window_attributes(connection, window, XCB_CW_OVERRIDE_REDIRECT, values);
-  if (check_xcb_cookie(window_cookie, connection, "Failed to override window redirect.")) {
-    exit_code = 1;
-    goto cleanup;
-  }
-
   /* Get the atoms to create a dock window type. */
   xcb_atom_t window_type_atom, window_type_dock_atom;
   xcb_intern_atom_cookie_t atom_cookie = xcb_intern_atom(connection, 0, strlen("_NET_WM_WINDOW_TYPE"), "_NET_WM_WINDOW_TYPE");

@thomacer / @emgram769 Just as a reminder, commit e65c28e didn't fix this bug, but instead introduced a new one.

@Airblader whoops sorry about this, I shouldn't have closed the issue. I added your patch in 81c08cc

I'll reopen this for the time being, until it is definitely fixed.

Well that patch I provided does fix it. ;)

sounds good :)