Chromium starts maximized #1599

Closed
psychon opened this Issue Feb 25, 2017 · 13 comments

Comments

Projects
None yet
6 participants
Owner

psychon commented Feb 25, 2017

No time right now to investigate, so I'll just report this:

Output of awesome --version:

awesome 9999 (Harder, Better, Faster, Stronger)
 • Compiled against Lua 5.2.4 (running with Lua 5.2)
 • D-Bus support: ✔
 • execinfo support: ✔
 • xcb-randr version: 1.5
 • LGI version: 0.9.1

Perhaps some actually useful "version" information:

$ ls -l /usr/bin/awesome
-rwxr-xr-x 1 root root 2263216 Feb 18 15:13 /usr/bin/awesome

How to reproduce the issue:

  • Start chromium

Actual result:

Chromium opens maximized. This means xprop says _NET_WM_STATE(ATOM) = _NET_WM_STATE_MAXIMIZED_VERT, _NET_WM_STATE_MAXIMIZED_HORZ and the tasklist entry looks like this:
foo

Oh and:

$ sleep 1 ; awesome-client 'local c = client.focus ; return c.maximized, c.maximized_vertical, c.maximized_horizontal'                                                                                                     
   boolean false
   boolean true
   boolean true

Expected result:

Chromium does not start maximized.
No idea why it previously did not do this.

Owner

psychon commented Feb 25, 2017

First observations: I'm bad at not trying to debug things myself.

Second observations: Chromium maximizes itself. The maximisation is coming from ewmh_process_state_atom() handling a request to add the maximized properties.

However, since I am quite sure that this did not happen before, I did a git bisect between v4.0 and my current master. The result is:

3b1599bd990cc6f9d73f232faca580ce79b7cf53 is the first bad commit
commit 3b1599bd990cc6f9d73f232faca580ce79b7cf53
Author: Emmanuel Lepage Vallee <elv1313@gmail.com>
Date:   Tue Jan 24 04:19:45 2017 -0500

    maximize: Stop doing it in 2 operations.
    
    Before 4.0, maximizing could only be done in 2 operations.
    
    4.0 add an unified way, but kept doing 2 operations. The old
    Lua EWMH code to serialize the 2 operations was dropped when
    the codepath was simplified and replaced by a generic version
    in awful.placement. However this version never implemented
    combining multiple mementos into 1.
    
    This commit unify the maximize C code, drop the ugly macro
    template and actually fixes a couple more issues that were
    caused because request::geometry was sent twice.

:040000 040000 82f918e86fd8e059da4f491ffce0c5d15b302f01 75f2ec8bf8fec85812efddff6abfc4ed639bbd5a M	lib
:040000 040000 4f14258b723741991e764a46127df8b671983aa3 ccaf2df1ff3205fd521f1e434cf1093071cb3de3 M	objects
Owner

blueyed commented Feb 25, 2017

This is #1559.

My local workaround for this is currently to have a callback in the default/always rule:

callback = function(c)
  c.maximized, c.maximized_vertical, c.maximized_horizontal = false, false, false
end,
Owner

psychon commented Feb 25, 2017

Sigh. You are right. Chromium remembers that it was maximized and my keybindings just toggle between "maximized" and "not maximized" without touching the vertical/horizontal.
(Actually: Chromium tries to maximize itself, but only sets the other two properties, not maximized)

Can we change this logic please? Having separate maximized, maximized_vertical and maximized_horizontal states like this is confusing. Toggling maximized without it having any effect is even more confusing.
What was the problem again with defining c.maximized = c.maximized_vertical and c.maximized_horizontal? At least with this definition I wouldn't be toggling a boolean which had no effect.

Owner

blueyed commented Feb 25, 2017

@psychon
I agree that this is messy currently, but can only see that different states are useful.
It just needs to be fixed somehow.

Toggling maximized without it having any effect is even more confusing.

Yeah, it only changes the indicators in the tasklist as of now.

What about maximized = false unsetting max_h and max_v if both are set?
And when using maximized = true again it would only set maximized, not max_h/max_v?

(When I last looked into it, I got distracted into #1571.)

I think we need more input from @Elv13 here after all.. #1559's title/description seemed to miss the point about this issue initially.

Member

actionless commented Feb 25, 2017

i think the best way here could be to reset vert and hor maximization when maximize is getting unset

Owner

blueyed commented Feb 25, 2017

ACK, although that would still not fix the issue when managing a new client.

Member

actionless commented Feb 25, 2017

nut it not sounds like an issue -- it just was maximized before (and also it seems like when running it for the first time after installing it starts maximized as well, at least it was in some versions)

Owner

blueyed commented Feb 25, 2017

@actionless
I failed to parse your first sentence - "nut"?

IIRC it was not about being maximized before with KMyMoney, but also / more related to the layout being used before - i.e. on a maximized layout it would set/request max_v/max_h during startup, although it was just floating or something like that.

Member

actionless commented Feb 25, 2017

*but :-)

Owner

Elv13 commented Feb 25, 2017

diff --git a/objects/client.c b/objects/client.c
index 7683aa4c4..5a738da4e 100644
--- a/objects/client.c
+++ b/objects/client.c
@@ -1515,6 +1515,16 @@ client_manage(xcb_window_t w, xcb_get_geometry_reply_t *wgeom, xcb_get_window_at
 
     luaA_class_emit_signal(L, &client_class, "list", 0);
 
+    /* As `client_set_maximized_common` wont be called, make sure to cleanup
+     * the maximization states.
+     */
+    if (c->maximized_vertical && c->maximized_horizontal)
+        c->maximized = true;
+
+    /* Remove partial maximization at startup because it's confusing. */
+    c->maximized_vertical  = false;
+    c->maximized_horizontal = false;
+
     /* client is still on top of the stack; emit signal */
     luaA_object_emit_signal(L, -1, "manage", 0);
     /* pop client */

Untested. There is code client_set_maximized_common to prevent this, but apparently it isn't called, so maybe this will be enough. It also remove the partial maximization at startup as suggested by @psychon in the original PR that introduced the regression.

Edit:
I am not sure the remove part is enough. It wont set the X props. Something like if (c->maximized_vertical ^ c->maximized_horizontal) do_it() might be necessary.

Contributor

0x041E commented Apr 3, 2017

Doubleclick chromium's own titlebar it does it's own maximizing/unmaximizing which will fix it

DanielAW commented Apr 3, 2017

The behaviour described in this issue happens for me in every application I try to open (with 4.1, 4.0 was fine). Only xterm seems to work. roxterm for example does not work.
The aformentioned workaround works though. (I'm running a skeleton config)

Elv13 added a commit to Elv13/awesome-1 that referenced this issue Apr 23, 2017

client: Handle maximize differently at startup
This way max_h+max_h wont happen by accident. This isn't pretty
but it will get the job done.

Fix #1599 #1559
Close #1715 #1712

Elv13 added a commit to Elv13/awesome-1 that referenced this issue Apr 24, 2017

client: Handle maximize differently at startup
This way max_h+max_h wont happen by accident. This isn't pretty
but it will get the job done.

Fix #1599 #1559
Close #1715 #1712

Elv13 added a commit to Elv13/awesome-1 that referenced this issue May 14, 2017

client: Handle maximize differently at startup
This way max_h+max_h wont happen by accident. This isn't pretty
but it will get the job done.

Fix #1599 #1559
Close #1715 #1712

Elv13 added a commit to Elv13/awesome-1 that referenced this issue May 22, 2017

client: Handle maximize differently at startup
This way max_h+max_h wont happen by accident. This isn't pretty
but it will get the job done.

Fix #1599 #1559
Close #1715 #1712
Owner

blueyed commented May 23, 2017

#1748 (comment) is about fixing this, in case you want to try it.

@blueyed blueyed closed this in 951386e May 23, 2017

petoju added a commit to petoju/awesome that referenced this issue Sep 10, 2017

client: Handle maximize differently at startup
This way max_h+max_h wont happen by accident. This isn't pretty
but it will get the job done.

Fix #1599 #1559
Close #1715 #1712
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment