awesomeWM / awesome

awesome window manager
https://awesomewm.org/
GNU General Public License v2.0
6.39k stars 598 forks source link

claws mail doesn't obey tag assignment from awful.rules #236

Closed hoelzro closed 9 years ago

hoelzro commented 9 years ago

I have awesome configured so that claws-mail always starts on tag 3 on my right screen. When I upgraded to Awesome 3.5.6, I noticed that claws mail no longer obeyed my configuration, instead going to tag 1 on my left screen. Downgrading to 3.5.5 fixes the problem, and this behavior is present in master (bafa12c as of this writing)

I did some digging into the codebase, and it seems the responsible commit is f4a7b2d. I don't know if claws-mail is doing something silly, but I thought I would bring this to team awesome's attention. Is there anything I can try to help provide more information? I am also happy to help make the code changes necessary to fix this issue. My configuration is here, by the way: https://github.com/hoelzro/awesome-config

blueyed commented 9 years ago

It would be useful, if this could be easily reproduced. Is installing and starting claws mail enough?

Have you tried it with awesome's default config? (+ the rule)? Can you provide/paste the rule you are using, please?

hoelzro commented 9 years ago

@blueyed Here are the steps I took to reproduce the issue

Here is the rule I'm using with awful.rules:

    { rule = { class = "Claws-mail" },
      properties = { tag = tags[preferred_screen][3] } },
    -- preferred_screen is 1

Claws-mail seems to store its windows' positions and sizes in its rc file; I'm wondering if this is causing it to send requests to the X server that are interferring with Awesome's normal operations.

I have not tried this with a single monitor configuration yet.

blueyed commented 9 years ago

Thanks for the additional information!

Have you git-bisected it to https://github.com/awesomeWM/awesome/commit/f4a7b2d73a15adc03f705be689bf2713d1ac663e, i.e. are you sure that it is the commit breaking this for you?

hoelzro commented 9 years ago

@blueyed I used git-bisect to identify f4a7b2d as the problem, but that was without performing the steps above (using my awesome config and my regular claws-mail setup). I can try another bisect with technique outlined above later, if you'd like?

blueyed commented 9 years ago

It would be good to know, but then it's probably good enough for now. I wanted to just verify that you've used git-bisect.

hoelzro commented 9 years ago

I had some extra time tonight and I am hoping to update to 3.5.7 when it comes out, so I dug into this. My experience with the awesome codebase as well as X11 is limited, so please let me know if I've made any incorrect observations.

It seems to me that the logic here calculates where to place the parent of the actual window by taking the border into account (which is the change that f4a7b2d introduced). In my case, claws-mail asks to appear to 1920x29, and awesome calculates (due to a one pixel border) that claws-mail should be placed at 1919x19. My right monitor starts at 1920, so I'm guessing this is why claws-mail is being thrown onto the left monitor in spite of my configuration telling it to stay on the right.

claws-mail stores its preferred position in the clawsrc file; the position stored, however, is the position of the window-manager level decorations window, rather than the client window. If that weren't the case, claws-mail would store 1921 as the X coordinate and I wouldn't be experiencing any issues with window placement. It uses gdk_window_get_root_origin to get the WM-level window's position for storing in clawsrc, and it uses gtk_window_move on startup to move the main window into the position specified by clawsrc. The documentation for both of these functions seem to indicate that the coordinates involved are those of the WM-level decorations, so claws-mail wants to deal with decoration coordinates.

The ICCCM specification says that ConfigureWindow requests should be handled "irrespective of any reparenting that may have occurred". So that does that mean that claws-mail is wrong in thinking it should deal with decoration-level coordinates, or is the border compensation behavior introduced by f4a7b2d incorrect?

psychon commented 9 years ago

f4a7b2d says it fixes things with urxvt, so... "something must have been wrong before". However, your link to gtk_window_move made me wonder about window gravity. This change in awesome seems to assume StaticGravity and does indeed handle NorthWestGravity incorrectly. However, urxvt also uses NorthWestGravity, so I am confused on why this change fixes something there.

@hoelzro Can you tell us which gravity Claws Mails uses for its window? xwininfo has a field Window Gravity State and xprop WM_NORMAL_HINTS also supplies a window gravity. Are these both NorthWest?

hoelzro commented 9 years ago

I'm wondering if someone else has happened since f4a7b2d that would fix the urxvt problem; I just commented out the changes that commit introduced, rebuilt, restarted, and tried the urxvt test case from here, and I did not see the window move.

As far as claws-mail's gravity goes, it's NorthWestGravity according to xwininfo and NorthWest according to xprop.

hoelzro commented 9 years ago

For what it's worth, I am unable to reproduce the urxvt problem on v3.5.5.

blueyed commented 9 years ago

@hoelzro Thanks for investigating!

It might be interesting to see how master behaves regarding this issue. Have you tried it?

I've reverted f4a7b2d (which is f0ab2aeb on master), and could not reproduce the urxvt issue anymore, too (but also urxvt has changed since then).

The revert conflicted with ecddee44 - which you might want to try additionally in the 3.5 tree (in case you are using titlebars)..!

blueyed commented 9 years ago

To add to the confusion: https://github.com/awesomeWM/awesome/commit/ecddee44cb72c2211e188b97886f0a2ca6925ab5 (a recent commit from me to fix moving windows with titlebars) will re-trigger the urxvt issue (with `printf '\033]710;xft:monospace:size=10\033\')!?

hoelzro commented 9 years ago

@blueyed The revision I tried against was 66c4ff7, which was master as of last night. I take it that the urxvt issue happens with ecddee4 only if you're using titlebars? I'm not, so maybe that's why I didn't see it!

blueyed commented 9 years ago

@hoelzro Yes, ecddee4 breaks it when using titlebars, which I've only noticed myself now when looking closer at it - I usually do not titlebars myself. I've starting adding some tests for this - and got drawn into improving root.fake_input over it.. :/

hoelzro commented 9 years ago

@blueyed Thanks for putting some time into this! Is there anything I can do to help resolve this, @psychon?

psychon commented 9 years ago

Uhm. Could you guys try changing xwindow.c, function xwindow_configure to have ce.border_width = 0 instead of ce.border_width = border? I'm especially curious about the effect on HeidiSQL. Perhaps this is enough to revert ecddee44cb72c2211e188b97886f0a2ca6925 ?

blueyed commented 9 years ago

@psychon Did you mean that https://github.com/awesomeWM/awesome/commit/f0ab2aebebfaa391de3efdc9598e5fb23a52e483 could be reverted then (because ecddee4 is related to the titlebars)? Anyway, I've tried it both ways, but it triggers the issue in HeidiSQL still (either moving by border size or the height of the titlebar).

diff --git i/xwindow.c w/xwindow.c
index 55b27f6..a0f7d5f 100644
--- i/xwindow.c
+++ w/xwindow.c
@@ -92,7 +92,7 @@ xwindow_configure(xcb_window_t win, area_t geometry, int border)
     ce.y = geometry.y + border;
     ce.width = geometry.width;
     ce.height = geometry.height;
-    ce.border_width = border;
+    ce.border_width = 0;
     ce.above_sibling = XCB_NONE;
     ce.override_redirect = false;
     xcb_send_event(globalconf.connection, false, win,
psychon commented 9 years ago

Uhm. So we recently did some stuff with window gravities (#505). This apparently re-introduced the problem with urxvt and printf '\033]710;xft:monospace:size=10\033\'. @hoelzro You said in an earlier comment that claws mails uses NorthWest gravity. In this case #505 should fix this problem. Could you test this out?

hoelzro commented 9 years ago

@psychon Seems to work; thanks!

psychon commented 9 years ago

Ok, so this issue is solved and the urxvt one goes to #532.